Skip to content

Commit

Permalink
Use Strorage generic instead of Context for Delta and `Accessor…
Browse files Browse the repository at this point in the history
…yDelta` (#913)

* Start moving from Context to Storage generic

* Move AccessoryDelta to Storage generic

* Spell checking

* Remove `Send + Sync` bound, as they are not needed in current context

* Missing space...
  • Loading branch information
citizen-stig authored Sep 22, 2023
1 parent 93b5191 commit 052a3a3
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 37 deletions.
2 changes: 1 addition & 1 deletion module-system/sov-modules-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub trait Spec {
type Address: RollupAddress + BorshSerialize + BorshDeserialize;

/// Authenticated state storage used by the rollup. Typically some variant of a merkle-patricia trie.
type Storage: Storage + Clone + Send + Sync;
type Storage: Storage + Send + Sync;

/// The public key used for digital signatures
#[cfg(feature = "native")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ where

/// Deletes a key-value pair from the map.
///
/// This is equivalent to [`AccessoryStateMap::remove`], but doesn't deserialize and
/// return the value beforing deletion.
/// This is equivalent to [`AccessoryStateMap::remove`],
/// but doesn't deserialize and return the value before deletion.
pub fn delete<Q, C>(&self, key: &Q, working_set: &mut AccessoryWorkingSet<C>)
where
Codec::KeyCodec: EncodeKeyLike<Q, K>,
Expand All @@ -197,7 +197,7 @@ where
/// Generates an arbitrary [`AccessoryStateMap`] instance.
///
/// See the [`arbitrary`] crate for more information.
pub fn arbitrary_workset<C>(
pub fn arbitrary_working_set<C>(
u: &mut arbitrary::Unstructured<'a>,
working_set: &mut AccessoryWorkingSet<C>,
) -> arbitrary::Result<Self>
Expand Down
4 changes: 2 additions & 2 deletions module-system/sov-modules-api/src/state/containers/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ where
/// Deletes a key-value pair from the map.
///
/// This is equivalent to [`StateMap::remove`], but doesn't deserialize and
/// return the value beforing deletion.
/// return the value before deletion.
pub fn delete<Q, C>(&self, key: &Q, working_set: &mut WorkingSet<C>)
where
Codec: StateCodec,
Expand All @@ -219,7 +219,7 @@ where
/// Returns an arbitrary [`StateMap`] instance.
///
/// See the [`arbitrary`] crate for more information.
pub fn arbitrary_workset<C>(
pub fn arbitrary_working_set<C>(
u: &mut arbitrary::Unstructured<'a>,
working_set: &mut WorkingSet<C>,
) -> arbitrary::Result<Self>
Expand Down
52 changes: 22 additions & 30 deletions module-system/sov-modules-api/src/state/scratchpad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,48 +11,40 @@ use crate::{Context, Spec};

/// A working set accumulates reads and writes on top of the underlying DB,
/// automating witness creation.
pub struct Delta<C: Context> {
inner: <C as Spec>::Storage,
witness: <<C as Spec>::Storage as Storage>::Witness,
pub struct Delta<S: Storage> {
inner: S,
witness: S::Witness,
cache: StorageInternalCache,
}

impl<C: Context> Delta<C> {
fn new(inner: <C as Spec>::Storage) -> Self {
impl<S: Storage> Delta<S> {
fn new(inner: S) -> Self {
Self::with_witness(inner, Default::default())
}

fn with_witness(
inner: <C as Spec>::Storage,
witness: <<C as Spec>::Storage as Storage>::Witness,
) -> Self {
fn with_witness(inner: S, witness: S::Witness) -> Self {
Self {
inner,
witness,
cache: Default::default(),
}
}

fn freeze(
&mut self,
) -> (
OrderedReadsAndWrites,
<<C as Spec>::Storage as Storage>::Witness,
) {
fn freeze(&mut self) -> (OrderedReadsAndWrites, S::Witness) {
let cache = std::mem::take(&mut self.cache);
let witness = std::mem::take(&mut self.witness);

(cache.into(), witness)
}
}

impl<C: Context> Debug for Delta<C> {
impl<S: Storage> Debug for Delta<S> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Delta").finish()
}
}

impl<C: Context> StateReaderAndWriter for Delta<C> {
impl<S: Storage> StateReaderAndWriter for Delta<S> {
fn get(&mut self, key: &StorageKey) -> Option<StorageValue> {
self.cache.get_or_fetch(key, &self.inner, &self.witness)
}
Expand All @@ -74,8 +66,8 @@ type RevertableWrites = HashMap<CacheKey, Option<CacheValue>>;
/// 1. With [`WorkingSet::checkpoint`].
/// 2. With [`WorkingSet::revert`].
pub struct StateCheckpoint<C: Context> {
delta: Delta<C>,
accessory_delta: AccessoryDelta<C>,
delta: Delta<C::Storage>,
accessory_delta: AccessoryDelta<C::Storage>,
}

impl<C: Context> StateCheckpoint<C> {
Expand Down Expand Up @@ -139,16 +131,16 @@ impl<C: Context> StateCheckpoint<C> {
}
}

struct AccessoryDelta<C: Context> {
struct AccessoryDelta<S: Storage> {
// This inner storage is never accessed inside the zkVM because reads are
// not allowed, so it can result as dead code.
#[allow(dead_code)]
storage: <C as Spec>::Storage,
storage: S,
writes: RevertableWrites,
}

impl<C: Context> AccessoryDelta<C> {
fn new(storage: <C as Spec>::Storage) -> Self {
impl<S: Storage> AccessoryDelta<S> {
fn new(storage: S) -> Self {
Self {
storage,
writes: Default::default(),
Expand All @@ -167,7 +159,7 @@ impl<C: Context> AccessoryDelta<C> {
}
}

impl<C: Context> StateReaderAndWriter for AccessoryDelta<C> {
impl<S: Storage> StateReaderAndWriter for AccessoryDelta<S> {
fn get(&mut self, key: &StorageKey) -> Option<StorageValue> {
let cache_key = key.to_cache_key();
if let Some(value) = self.writes.get(&cache_key) {
Expand All @@ -191,8 +183,8 @@ impl<C: Context> StateReaderAndWriter for AccessoryDelta<C> {
/// 1. By using the checkpoint() method, where all the changes are added to the underlying StateCheckpoint.
/// 2. By using the revert method, where the most recent changes are reverted and the previous `StateCheckpoint` is returned.
pub struct WorkingSet<C: Context> {
delta: RevertableWriter<Delta<C>>,
accessory_delta: RevertableWriter<AccessoryDelta<C>>,
delta: RevertableWriter<Delta<C::Storage>>,
accessory_delta: RevertableWriter<AccessoryDelta<C::Storage>>,
events: Vec<Event>,
}

Expand Down Expand Up @@ -276,10 +268,6 @@ where
}

impl<T: StateReaderAndWriter> StateReaderAndWriter for RevertableWriter<T> {
fn delete(&mut self, key: &StorageKey) {
self.writes.insert(key.to_cache_key(), None);
}

fn get(&mut self, key: &StorageKey) -> Option<StorageValue> {
if let Some(value) = self.writes.get(&key.to_cache_key()) {
value.as_ref().cloned().map(Into::into)
Expand All @@ -292,6 +280,10 @@ impl<T: StateReaderAndWriter> StateReaderAndWriter for RevertableWriter<T> {
self.writes
.insert(key.to_cache_key(), Some(value.into_cache_value()));
}

fn delete(&mut self, key: &StorageKey) {
self.writes.insert(key.to_cache_key(), None);
}
}

impl<C: Context> WorkingSet<C> {
Expand Down
2 changes: 1 addition & 1 deletion module-system/sov-state/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub use crate::witness::{ArrayWitness, Witness};

/// A prefix prepended to each key before insertion and retrieval from the storage.
///
/// When interacing with state containers, you will usually use the same working set instance to
/// When interacting with state containers, you will usually use the same working set instance to
/// access them, as required by the module API. This also means that you might get key collisions,
/// so it becomes necessary to prepend a prefix to each key.
#[derive(
Expand Down

0 comments on commit 052a3a3

Please sign in to comment.