Merge branch 'tomas/refactor-storage-prefix-iter' (#335)

* tomas/refactor-storage-prefix-iter:
  changelog: add #335
  wasm checksums update
  test/vm_host_env: refactor prefix iter tests with new `iter_prefix` fn
  storage_api: build a nicer `iter_prefix` function on top of StorageRead

.changelog/unreleased/improvements/335-refactor-storage-prefix-iter.md

@@ -0,0 +1,3 @@
+- Added a simpler prefix iterator API that returns `std::iter::Iterator` with
+  the storage keys parsed and a variant that also decodes stored values with
+  Borsh ([#335](https://github.com/anoma/namada/pull/335))

shared/src/ledger/storage_api/mod.rs

@@ -96,3 +96,69 @@ pub trait StorageWrite {
     /// Delete a value at the given key from storage.
     fn delete(&mut self, key: &storage::Key) -> Result<()>;
 }
+
+/// Iterate items matching the given prefix.
+pub fn iter_prefix_bytes<'a>(
+    storage: &'a impl StorageRead<'a>,
+    prefix: &crate::types::storage::Key,
+) -> Result<impl Iterator<Item = Result<(storage::Key, Vec<u8>)>> + 'a> {
+    let iter = storage.iter_prefix(prefix)?;
+    let iter = itertools::unfold(iter, |iter| {
+        match storage.iter_next(iter) {
+            Ok(Some((key, val))) => {
+                let key = match storage::Key::parse(key).into_storage_result() {
+                    Ok(key) => key,
+                    Err(err) => {
+                        // Propagate key encoding errors into Iterator's Item
+                        return Some(Err(err));
+                    }
+                };
+                Some(Ok((key, val)))
+            }
+            Ok(None) => None,
+            Err(err) => {
+                // Propagate `iter_next` errors into Iterator's Item
+                Some(Err(err))
+            }
+        }
+    });
+    Ok(iter)
+}
+
+/// Iterate Borsh encoded items matching the given prefix.
+pub fn iter_prefix<'a, T>(
+    storage: &'a impl StorageRead<'a>,
+    prefix: &crate::types::storage::Key,
+) -> Result<impl Iterator<Item = Result<(storage::Key, T)>> + 'a>
+where
+    T: BorshDeserialize,
+{
+    let iter = storage.iter_prefix(prefix)?;
+    let iter = itertools::unfold(iter, |iter| {
+        match storage.iter_next(iter) {
+            Ok(Some((key, val))) => {
+                let key = match storage::Key::parse(key).into_storage_result() {
+                    Ok(key) => key,
+                    Err(err) => {
+                        // Propagate key encoding errors into Iterator's Item
+                        return Some(Err(err));
+                    }
+                };
+                let val = match T::try_from_slice(&val).into_storage_result() {
+                    Ok(val) => val,
+                    Err(err) => {
+                        // Propagate val encoding errors into Iterator's Item
+                        return Some(Err(err));
+                    }
+                };
+                Some(Ok((key, val)))
+            }
+            Ok(None) => None,
+            Err(err) => {
+                // Propagate `iter_next` errors into Iterator's Item
+                Some(Err(err))
+            }
+        }
+    });
+    Ok(iter)
+}

tests/src/vm_host_env/mod.rs

@@ -148,9 +148,11 @@ mod tests {
         tx_host_env::init();
 
         let empty_key = storage::Key::parse("empty").unwrap();
-        let mut iter = tx::ctx().iter_prefix(&empty_key).unwrap();
+        let mut iter =
+            namada_tx_prelude::iter_prefix_bytes(tx::ctx(), &empty_key)
+                .unwrap();
         assert!(
-            tx::ctx().iter_next(&mut iter).unwrap().is_none(),
+            iter.next().is_none(),
             "Trying to iter a prefix that doesn't have any matching keys \
              should yield an empty iterator."
         );
@@ -167,15 +169,12 @@ mod tests {
         });
 
         // Then try to iterate over their prefix
-        let iter = tx::ctx().iter_prefix(&prefix).unwrap();
-        let iter = itertools::unfold(iter, |iter| {
-            if let Ok(Some((key, value))) = tx::ctx().iter_next(iter) {
-                let decoded_value = i32::try_from_slice(&value[..]).unwrap();
-                return Some((key, decoded_value));
-            }
-            None
+        let iter = namada_tx_prelude::iter_prefix(tx::ctx(), &prefix)
+            .unwrap()
+            .map(|item| item.unwrap());
+        let expected = (0..10).map(|i| {
+            (storage::Key::parse(format!("{}/{}", prefix, i)).unwrap(), i)
         });
-        let expected = (0..10).map(|i| (format!("{}/{}", prefix, i), i));
         itertools::assert_equal(iter.sorted(), expected.sorted());
     }
 
@@ -380,29 +379,25 @@ mod tests {
             tx::ctx().write(&new_key, 11_i32).unwrap();
         });
 
-        let iter_pre = vp::CTX.iter_prefix(&prefix).unwrap();
-        let iter_pre = itertools::unfold(iter_pre, |iter| {
-            if let Ok(Some((key, value))) = vp::CTX.iter_pre_next(iter) {
-                if let Ok(decoded_value) = i32::try_from_slice(&value[..]) {
-                    return Some((key, decoded_value));
-                }
-            }
-            None
+        let ctx_pre = vp::CTX.pre();
+        let iter_pre = namada_vp_prelude::iter_prefix(&ctx_pre, &prefix)
+            .unwrap()
+            .map(|item| item.unwrap());
+        let expected_pre = (0..10).map(|i| {
+            (storage::Key::parse(format!("{}/{}", prefix, i)).unwrap(), i)
         });
-        let expected_pre = (0..10).map(|i| (format!("{}/{}", prefix, i), i));
         itertools::assert_equal(iter_pre.sorted(), expected_pre.sorted());
 
-        let iter_post = vp::CTX.iter_prefix(&prefix).unwrap();
-        let iter_post = itertools::unfold(iter_post, |iter| {
-            if let Ok(Some((key, value))) = vp::CTX.iter_post_next(iter) {
-                let decoded_value = i32::try_from_slice(&value[..]).unwrap();
-                return Some((key, decoded_value));
-            }
-            None
-        });
+        let ctx_post = vp::CTX.post();
+        let iter_post = namada_vp_prelude::iter_prefix(&ctx_post, &prefix)
+            .unwrap()
+            .map(|item| item.unwrap());
         let expected_post = (0..10).map(|i| {
             let val = if i == 5 { 100 } else { i };
-            (format!("{}/{}", prefix, i), val)
+            (
+                storage::Key::parse(format!("{}/{}", prefix, i)).unwrap(),
+                val,
+            )
         });
         itertools::assert_equal(iter_post.sorted(), expected_post.sorted());
     }

tx_prelude/src/lib.rs

@@ -23,7 +23,9 @@ pub use namada::ledger::governance::storage as gov_storage;
 pub use namada::ledger::parameters::storage as parameters_storage;
 pub use namada::ledger::storage::types::encode;
 use namada::ledger::storage_api;
-pub use namada::ledger::storage_api::{StorageRead, StorageWrite};
+pub use namada::ledger::storage_api::{
+    iter_prefix, iter_prefix_bytes, StorageRead, StorageWrite,
+};
 pub use namada::ledger::treasury::storage as treasury_storage;
 pub use namada::ledger::tx_env::TxEnv;
 pub use namada::proto::{Signed, SignedTxData};

vp_prelude/src/lib.rs

@@ -22,7 +22,9 @@ use std::marker::PhantomData;
 pub use borsh::{BorshDeserialize, BorshSerialize};
 pub use error::*;
 pub use namada::ledger::governance::storage as gov_storage;
-pub use namada::ledger::storage_api::{self, StorageRead};
+pub use namada::ledger::storage_api::{
+    self, iter_prefix, iter_prefix_bytes, StorageRead,
+};
 pub use namada::ledger::vp_env::VpEnv;
 pub use namada::ledger::{parameters, pos as proof_of_stake};
 pub use namada::proto::{Signed, SignedTxData};
@@ -334,7 +336,7 @@ impl StorageRead<'_> for CtxPreStorageRead<'_> {
         prefix: &storage::Key,
     ) -> Result<Self::PrefixIter, storage_api::Error> {
         // Note that this is the same as `CtxPostStorageRead`
-        iter_prefix(prefix)
+        iter_prefix_impl(prefix)
     }
 
     fn iter_next(
@@ -411,7 +413,7 @@ impl StorageRead<'_> for CtxPostStorageRead<'_> {
         prefix: &storage::Key,
     ) -> Result<Self::PrefixIter, storage_api::Error> {
         // Note that this is the same as `CtxPreStorageRead`
-        iter_prefix(prefix)
+        iter_prefix_impl(prefix)
     }
 
     fn iter_next(
@@ -442,7 +444,7 @@ impl StorageRead<'_> for CtxPostStorageRead<'_> {
     }
 }
 
-fn iter_prefix(
+fn iter_prefix_impl(
     prefix: &storage::Key,
 ) -> Result<KeyValIterator<(String, Vec<u8>)>, storage_api::Error> {
     let prefix = prefix.to_string();

wasm/checksums.json

@@ -1,19 +1,19 @@
 {
-    "tx_bond.wasm": "tx_bond.37dda241d2041c7534a3e65199979af8de1c3128927048ebe5a69586ff7fe7a0.wasm",
-    "tx_from_intent.wasm": "tx_from_intent.b74d3c5b49c79ae3237b83fa896e04dc839194b5a3f74f5099b03663d06c81c8.wasm",
-    "tx_ibc.wasm": "tx_ibc.2642651c00baf6f8a9f078ac049cf6ba4fcd98d2a9ca8d4ec81d9d54b6d9ca8a.wasm",
-    "tx_init_account.wasm": "tx_init_account.4761be38a6fc5c731a310993d897f569f71701791105bbaac64070b3bab82905.wasm",
-    "tx_init_nft.wasm": "tx_init_nft.d3989943620ffd568800c44f47dbd8db00080fb5fc18e566df22e7a41d754c7a.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.b9ae52416d25ed51b312b0dccb893a3de2feb1e1d1f73306d0ca7941f295904b.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.3b0f95a2225510fc17cb28d971ac020767ecd5f1ebf6a9c2652f8338ed9ae2b0.wasm",
-    "tx_mint_nft.wasm": "tx_mint_nft.e53a544f131500db263abb4172039a1bdae035493b662782a7d4b0fbb77a228e.wasm",
-    "tx_transfer.wasm": "tx_transfer.3f092cc2b18c1031e0be662dca8a414de5d043082408f5989bb14b2aea5f5b3f.wasm",
-    "tx_unbond.wasm": "tx_unbond.4d937a43fc32a9e849abf801988b73298c96c652a4a72b887c9e7936488206cd.wasm",
+    "tx_bond.wasm": "tx_bond.ee44010745c50f798d9c838b0121e5713ec4dc2ecfae093c3f3b332ed8af85d2.wasm",
+    "tx_from_intent.wasm": "tx_from_intent.578f429e933f5a6530628a2fa9c1d52d78fb652fe2acd796cb956fddf842acaa.wasm",
+    "tx_ibc.wasm": "tx_ibc.0b61d351e12fe8afc13198fcfa40a206904d027388707b947825255233c3af04.wasm",
+    "tx_init_account.wasm": "tx_init_account.dfb26afc4434f3687b13238d9cfed93f03ee7ac21e6ec7755851ef9dafea8f08.wasm",
+    "tx_init_nft.wasm": "tx_init_nft.7382e9515ec538ef94fbc60a369cb2c793306ee7cef54a28858425395bf4eca4.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.7d745b1e8dba5d4a7a0e1a8b8fae93ee5af946dda71462af9038bc201058edf2.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.8ab24cab601f1c976f2ce0f84d8f4e5de39fbebf7e4239c0c941668780d6b86b.wasm",
+    "tx_mint_nft.wasm": "tx_mint_nft.e3dc0937b49f0924ee73d452c492cc6f53bfe8d629635195496b0b49c08d6615.wasm",
+    "tx_transfer.wasm": "tx_transfer.34c106e507d103089a4247225e87d966df20b13781a340303a3ccff50c1bc9e2.wasm",
+    "tx_unbond.wasm": "tx_unbond.81ed86fd48ef0b71f3494bf704b51b9eea7488aacd02313b69cb5512521af3ab.wasm",
     "tx_update_vp.wasm": "tx_update_vp.f32dd6d4225d426d60bf82cda4e6d6432f869a201b65ff135a9fd589679a9f0a.wasm",
     "tx_vote_proposal.wasm": "tx_vote_proposal.829b383d2db6b077b5a6112cacb75af63cc152bf2fb66e2df45f42d55c251b41.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.846c26a11c331652c80ed42f1b90caa90346b5e93bbb6761e5cae8f948f73851.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.9fc2aca6243a167ba11689410c0373afe1cb7ecbdc9891af1c01bd7052849ea1.wasm",
     "vp_nft.wasm": "vp_nft.ded411ba44c11b9fc6a3626c6858c760b0fae6d3cdef4664ec5ce0a938149edc.wasm",
-    "vp_testnet_faucet.wasm": "vp_testnet_faucet.1ba83fcbf94b596c12313f405103baa6c10ef64f1c013134730c1926aed6503f.wasm",
+    "vp_testnet_faucet.wasm": "vp_testnet_faucet.4a13e20a3d263d74d7e325ab86a874bbcd60d4562f23d8a483919676f48d80aa.wasm",
     "vp_token.wasm": "vp_token.9799a1d3e276c19b58ad6e78f0f1c6f8d4b17da72ff7fee582658d33bbd67583.wasm",
-    "vp_user.wasm": "vp_user.eb57fe7018a20f8f7cb71b9515b17023ec745ad4cefb5648eb9b1d5584ab093b.wasm"
+    "vp_user.wasm": "vp_user.aadcf526d1cbe2c469a6a8eaacac13601400557ab3e87382ded16c44e8cc59be.wasm"
 }