From f9fe8cb9365624532e9a88b0d62024c2016202f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 27 Mar 2023 08:59:18 +0100 Subject: [PATCH 01/10] tets/core/storage: add a test for order of addresses in storage keys --- core/src/types/storage.rs | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index 7a0a394a86..12e6169fd6 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -1142,6 +1142,7 @@ mod tests { use proptest::prelude::*; use super::*; + use crate::types::address::testing::arb_address; proptest! { /// Tests that any key that doesn't contain reserved prefixes is valid. @@ -1299,6 +1300,48 @@ mod tests { assert_eq!(epochs.get_epoch(BlockHeight(550)), Some(Epoch(7))); assert_eq!(epochs.get_epoch(BlockHeight(600)), Some(Epoch(8))); } + + proptest! { + /// Ensure that addresses in storage keys preserve the order of the + /// addresses. + #[test] + fn test_address_in_storage_key_order( + addr1 in arb_address(), + addr2 in arb_address(), + ) { + test_address_in_storage_key_order_aux(addr1, addr2) + } + } + + fn test_address_in_storage_key_order_aux(addr1: Address, addr2: Address) { + println!("addr1 {addr1}"); + println!("addr2 {addr2}"); + let expected_order = addr1.cmp(&addr2); + + // Turn the addresses into strings + let str1 = addr1.to_string(); + let str2 = addr2.to_string(); + println!("addr1 str {str1}"); + println!("addr1 str {str2}"); + let order = str1.cmp(&str2); + assert_eq!(order, expected_order); + + // Turn the addresses into storage keys + let key1 = Key::from(addr1.to_db_key()); + let key2 = Key::from(addr2.to_db_key()); + println!("addr1 key {key1}"); + println!("addr2 key {key2}"); + let order = key1.cmp(&key2); + assert_eq!(order, expected_order); + + // Turn the addresses into raw storage keys (formatted to strings) + let raw1 = addr1.raw(); + let raw2 = addr2.raw(); + println!("addr 1 raw {raw1}"); + println!("addr 2 raw {raw2}"); + let order = raw1.cmp(&raw2); + assert_eq!(order, expected_order); + } } /// Helpers for testing with storage types. From 89fa77fc1c2cb4c76021b028e58b74f4308472e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Tue, 28 Mar 2023 09:17:13 +0100 Subject: [PATCH 02/10] core/types/address: order addresses by their string (bech32m) format --- core/src/types/address.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/core/src/types/address.rs b/core/src/types/address.rs index ae96842f8c..9afa52e2d2 100644 --- a/core/src/types/address.rs +++ b/core/src/types/address.rs @@ -100,15 +100,7 @@ pub type Result = std::result::Result; /// An account's address #[derive( - Clone, - BorshSerialize, - BorshDeserialize, - BorshSchema, - PartialEq, - Eq, - PartialOrd, - Ord, - Hash, + Clone, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq, Eq, Hash, )] pub enum Address { /// An established address is generated on-chain @@ -119,6 +111,21 @@ pub enum Address { Internal(InternalAddress), } +// We're using the string format of addresses (bech32m) for ordering to ensure +// that addresses as strings, storage keys and storage keys as strings preserve +// the order. +impl PartialOrd for Address { + fn partial_cmp(&self, other: &Self) -> Option { + self.encode().partial_cmp(&other.encode()) + } +} + +impl Ord for Address { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.encode().cmp(&other.encode()) + } +} + impl Address { /// Encode an address with Bech32m encoding pub fn encode(&self) -> String { From 1d8b900453e6f7753edb15d24375cfaa4c11384a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 28 Mar 2023 08:56:02 +0000 Subject: [PATCH 03/10] [ci] wasm checksums update --- wasm/checksums.json | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index 9e821b3575..a336d03400 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,20 +1,20 @@ { - "tx_bond.wasm": "tx_bond.f7f4169dbd708a4776fa6f19c32c259402a7b7c34b9c1ac34029788c27f125fa.wasm", - "tx_change_validator_commission.wasm": "tx_change_validator_commission.49143933426925d549739dd06890f1c4709a27eca101596e34d5e0dbec1bd4c5.wasm", - "tx_ibc.wasm": "tx_ibc.e8081fbfb59dbdb42a2f66e89056fff3058bd7c3111e131b8ff84452752d94f5.wasm", - "tx_init_account.wasm": "tx_init_account.9ad3971335452cc7eada0dc35fb1e6310a05e8e2367e3067c0af8e21dad5705b.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.d0419c9cd9de905c92a0b9839ab94cdc3daf19b050375798f55503150ddc2bfa.wasm", - "tx_init_validator.wasm": "tx_init_validator.ef991c1019164b5d2432a3fba01e4b116825b024cc0dc3bcecdd1555ae7c6579.wasm", - "tx_reveal_pk.wasm": "tx_reveal_pk.b0509274d06dfe22988f03dd4c42e0a70bc40e21983f9670a603c057784dbd8f.wasm", - "tx_transfer.wasm": "tx_transfer.deae9d3955f0761331c21f253f4dc9b1bc93fe268a53049f9eb41d969848c62d.wasm", - "tx_unbond.wasm": "tx_unbond.8c63c856db5b608b44179abf29ec8996005d5a5e5467b11b1afad09b9052e69a.wasm", - "tx_update_vp.wasm": "tx_update_vp.287a8dde719b278b10c63384adbeacf40906b40e173b3ab0d0fac659e323951a.wasm", - "tx_vote_proposal.wasm": "tx_vote_proposal.f86a66ce7c96be2ed4ee6b8d1fa0186264749ef88ec22575bf2c31ca82341c3e.wasm", - "tx_withdraw.wasm": "tx_withdraw.c9d451ccf7561db4a1113fa5c4c9d8266f185030c3ceb57e0204707de1489792.wasm", - "vp_implicit.wasm": "vp_implicit.03f75fbf6a2a4b343ba0d5691d381e643af1e592ed6dcc7f65b5554caf2f52df.wasm", - "vp_masp.wasm": "vp_masp.013f6e673ad10fcf233f1cda940fea7042c2ba4ac79b30c4fd9dc6cfa60a6080.wasm", - "vp_testnet_faucet.wasm": "vp_testnet_faucet.a9bbcb7fbc484fe703e0bf18661701302bf2cc9425f4e8bcc8becc8ecc58a51c.wasm", - "vp_token.wasm": "vp_token.ac90ced308b618c6d991939a60735a1679e773bb5e76dd03a4dc4c9d56180ddd.wasm", - "vp_user.wasm": "vp_user.cf363aaf2fd13faa79501d8c8c251bafe22992b9989035b2c2edaf3edbe629fe.wasm", - "vp_validator.wasm": "vp_validator.4f7b0efb2f742b4b605738fe48ede23f57623ff083f23dc18c5bf8c5e26294cd.wasm" + "tx_bond.wasm": "tx_bond.3fbd9f21a7fb1ea3ad9e7e35ceccf0b3cec6f3bfcc192b762e6f02dac36e6dea.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.a7788841a2a89b81d4e31ee058eabf3a2042642a73297d59b2bbb348ee0f3e99.wasm", + "tx_ibc.wasm": "tx_ibc.9057055a2ca07ea9c3b0b5d587bc4d7d138c3197a843151b7064c747881770be.wasm", + "tx_init_account.wasm": "tx_init_account.05fba94092683984fe52dd0c7dd4d66a9b62a448ca3fe7281bfa711457e19759.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.2d952f3068e708fb0f2d5085bee0622c11ac303f73712d15782e4a91e3bc268e.wasm", + "tx_init_validator.wasm": "tx_init_validator.56af521501e22bfe2a8b65c31e6a7424bb6868da81e8f583229aa30c2d23404d.wasm", + "tx_reveal_pk.wasm": "tx_reveal_pk.93ac324e81450fb3ce3852113a17c92328f826319ac979f5d53660c87544013d.wasm", + "tx_transfer.wasm": "tx_transfer.cc04b814dfaa9bc0a4f9e5340820173e1a94fed5cdf762d31e6fbfd23ec1f58a.wasm", + "tx_unbond.wasm": "tx_unbond.be25010b518fa098c68319064e1e7492de2953cf8e2d52537ff07150fd8cb580.wasm", + "tx_update_vp.wasm": "tx_update_vp.88c649a97a6d718b8c33e6440ed8b4c3b91aa7072cfc60362f7e490f1909134d.wasm", + "tx_vote_proposal.wasm": "tx_vote_proposal.b123b51369102fad83f0b73dfaa65cd6419111dcd090d5342c46a1f3eead3c54.wasm", + "tx_withdraw.wasm": "tx_withdraw.3a4fb67ee3118a550d5648266100ea6f40af5a7495ffacd2b98b1a15ad3d88c2.wasm", + "vp_implicit.wasm": "vp_implicit.7033fb123369a7cce109ce0670a1dde0d09c72909999ce1012dafedcd593ad39.wasm", + "vp_masp.wasm": "vp_masp.3f1deb1034615e33b2d8e68bb507da9416e6cc48b8c076aa259d4a4896e4bfeb.wasm", + "vp_testnet_faucet.wasm": "vp_testnet_faucet.dabc8dbf1c3db77788ca9aa07e2c83979d1fc8ecb4a8be4c94f62c86820eaff2.wasm", + "vp_token.wasm": "vp_token.72d27dfcd98dcaaf62aff854d5655fb4f6f1d1cf0a9a107416591ebf004d146a.wasm", + "vp_user.wasm": "vp_user.8398cef9eb4e6c0fa058a75a13e8e42d1edfabd92bd819a69cda63c79620a097.wasm", + "vp_validator.wasm": "vp_validator.7d8573cdb92189e608b91151b7c735ce8ba425ed67a8bf3d53264e1be2c9bccb.wasm" } \ No newline at end of file From eeec6022cfb42e5b0e53e1f7596fe6ea5da7f6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Tue, 28 Mar 2023 09:57:48 +0100 Subject: [PATCH 04/10] changelog: add #1256 --- .../unreleased/bug-fixes/1256-fix-addr-storage-key-ord.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/1256-fix-addr-storage-key-ord.md diff --git a/.changelog/unreleased/bug-fixes/1256-fix-addr-storage-key-ord.md b/.changelog/unreleased/bug-fixes/1256-fix-addr-storage-key-ord.md new file mode 100644 index 0000000000..64271bba36 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1256-fix-addr-storage-key-ord.md @@ -0,0 +1,3 @@ +- Addresses are now being ordered by their string format (bech32m) + to ensure that their order is preserved inside raw storage keys. + ([#1256](https://github.com/anoma/namada/pull/1256)) \ No newline at end of file From b59cb00d35a5c2e2da63ddc90f33b7ed3994dddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 10 Mar 2023 10:04:54 +0000 Subject: [PATCH 05/10] core/lazy_map: test and fix for address sub-key --- .../storage_api/collections/lazy_map.rs | 190 +++++++++++++++--- 1 file changed, 162 insertions(+), 28 deletions(-) diff --git a/core/src/ledger/storage_api/collections/lazy_map.rs b/core/src/ledger/storage_api/collections/lazy_map.rs index 80072f2486..496e828ee9 100644 --- a/core/src/ledger/storage_api/collections/lazy_map.rs +++ b/core/src/ledger/storage_api/collections/lazy_map.rs @@ -40,7 +40,7 @@ pub struct LazyMap { pub type NestedMap = LazyMap; /// Possible sub-keys of a [`LazyMap`] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum SubKey { /// Data sub-key, further sub-keyed by its literal map key Data(K), @@ -81,7 +81,7 @@ pub enum NestedAction { } /// Possible sub-keys of a nested [`LazyMap`] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum NestedSubKey { /// Data sub-key Data { @@ -141,27 +141,37 @@ where Some(Some(suffix)) => suffix, }; + // A helper to validate the 2nd key segment + let validate_sub_key = |raw_sub_key| { + if let Ok(key_in_kv) = storage::KeySeg::parse(raw_sub_key) { + let nested = self.at(&key_in_kv).is_valid_sub_key(key)?; + match nested { + Some(nested_sub_key) => Ok(Some(NestedSubKey::Data { + key: key_in_kv, + nested_sub_key, + })), + None => { + Err(ValidationError::InvalidNestedSubKey(key.clone())) + .into_storage_result() + } + } + } else { + Err(ValidationError::InvalidSubKey(key.clone())) + .into_storage_result() + } + }; + // Match the suffix against expected sub-keys match &suffix.segments[..2] { [DbKeySeg::StringSeg(sub_a), DbKeySeg::StringSeg(sub_b)] if sub_a == DATA_SUBKEY => { - if let Ok(key_in_kv) = storage::KeySeg::parse(sub_b.clone()) { - let nested = self.at(&key_in_kv).is_valid_sub_key(key)?; - match nested { - Some(nested_sub_key) => Ok(Some(NestedSubKey::Data { - key: key_in_kv, - nested_sub_key, - })), - None => Err(ValidationError::InvalidNestedSubKey( - key.clone(), - )) - .into_storage_result(), - } - } else { - Err(ValidationError::InvalidSubKey(key.clone())) - .into_storage_result() - } + validate_sub_key(sub_b.clone()) + } + [DbKeySeg::StringSeg(sub_a), DbKeySeg::AddressSeg(sub_b)] + if sub_a == DATA_SUBKEY => + { + validate_sub_key(sub_b.raw()) } _ => Err(ValidationError::InvalidSubKey(key.clone())) .into_storage_result(), @@ -266,17 +276,27 @@ where Some(Some(suffix)) => suffix, }; + // A helper to validate the 2nd key segment + let validate_sub_key = |raw_sub_key| { + if let Ok(key_in_kv) = storage::KeySeg::parse(raw_sub_key) { + Ok(Some(SubKey::Data(key_in_kv))) + } else { + Err(ValidationError::InvalidSubKey(key.clone())) + .into_storage_result() + } + }; + // Match the suffix against expected sub-keys match &suffix.segments[..] { [DbKeySeg::StringSeg(sub_a), DbKeySeg::StringSeg(sub_b)] if sub_a == DATA_SUBKEY => { - if let Ok(key_in_kv) = storage::KeySeg::parse(sub_b.clone()) { - Ok(Some(SubKey::Data(key_in_kv))) - } else { - Err(ValidationError::InvalidSubKey(key.clone())) - .into_storage_result() - } + validate_sub_key(sub_b.clone()) + } + [DbKeySeg::StringSeg(sub_a), DbKeySeg::AddressSeg(sub_b)] + if sub_a == DATA_SUBKEY => + { + validate_sub_key(sub_b.raw()) } _ => Err(ValidationError::InvalidSubKey(key.clone())) .into_storage_result(), @@ -523,6 +543,7 @@ where mod test { use super::*; use crate::ledger::storage::testing::TestWlStorage; + use crate::types::address::{self, Address}; #[test] fn test_lazy_map_basics() -> storage_api::Result<()> { @@ -533,7 +554,7 @@ mod test { // The map should be empty at first assert!(lazy_map.is_empty(&storage)?); - assert!(lazy_map.len(&storage)? == 0); + assert_eq!(lazy_map.len(&storage)?, 0); assert!(!lazy_map.contains(&storage, &0)?); assert!(!lazy_map.contains(&storage, &1)?); assert!(lazy_map.iter(&storage)?.next().is_none()); @@ -552,7 +573,7 @@ mod test { assert!(!lazy_map.contains(&storage, &0)?); assert!(lazy_map.contains(&storage, &key)?); assert!(!lazy_map.is_empty(&storage)?); - assert!(lazy_map.len(&storage)? == 2); + assert_eq!(lazy_map.len(&storage)?, 2); let mut map_it = lazy_map.iter(&storage)?; assert_eq!(map_it.next().unwrap()?, (key, val.clone())); assert_eq!(map_it.next().unwrap()?, (key2, val2.clone())); @@ -566,7 +587,7 @@ mod test { let removed = lazy_map.remove(&mut storage, &key)?.unwrap(); assert_eq!(removed, val); assert!(!lazy_map.is_empty(&storage)?); - assert!(lazy_map.len(&storage)? == 1); + assert_eq!(lazy_map.len(&storage)?, 1); assert!(!lazy_map.contains(&storage, &0)?); assert!(!lazy_map.contains(&storage, &1)?); assert!(!lazy_map.contains(&storage, &123)?); @@ -579,7 +600,120 @@ mod test { let removed = lazy_map.remove(&mut storage, &key2)?.unwrap(); assert_eq!(removed, val2); assert!(lazy_map.is_empty(&storage)?); - assert!(lazy_map.len(&storage)? == 0); + assert_eq!(lazy_map.len(&storage)?, 0); + + let storage_key = lazy_map.get_data_key(&key); + assert_eq!( + lazy_map.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(key)) + ); + + let storage_key2 = lazy_map.get_data_key(&key2); + assert_eq!( + lazy_map.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(key2)) + ); + + Ok(()) + } + + #[test] + fn test_lazy_map_with_addr_key() -> storage_api::Result<()> { + let mut storage = TestWlStorage::default(); + + let key = storage::Key::parse("test").unwrap(); + let lazy_map = LazyMap::::open(key); + + // Insert a new value and check that it's added + let (key, val) = ( + address::testing::established_address_1(), + "Test".to_string(), + ); + lazy_map.insert(&mut storage, key.clone(), val.clone())?; + + assert_eq!(lazy_map.len(&storage)?, 1); + let mut map_it = lazy_map.iter(&storage)?; + assert_eq!(map_it.next().unwrap()?, (key.clone(), val.clone())); + drop(map_it); + + let (key2, val2) = ( + address::testing::established_address_2(), + "Test2".to_string(), + ); + lazy_map.insert(&mut storage, key2.clone(), val2.clone())?; + + assert_eq!(lazy_map.len(&storage)?, 2); + let mut map_it = lazy_map.iter(&storage)?; + assert!(key < key2, "sanity check - this influences the iter order"); + assert_eq!(map_it.next().unwrap()?, (key.clone(), val)); + assert_eq!(map_it.next().unwrap()?, (key2.clone(), val2)); + assert!(map_it.next().is_none()); + drop(map_it); + + let storage_key = lazy_map.get_data_key(&key); + assert_eq!( + lazy_map.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(key)) + ); + + let storage_key2 = lazy_map.get_data_key(&key2); + assert_eq!( + lazy_map.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(key2)) + ); + + Ok(()) + } + + #[test] + fn test_nested_lazy_map_with_addr_key() -> storage_api::Result<()> { + let mut storage = TestWlStorage::default(); + + let key = storage::Key::parse("test").unwrap(); + let lazy_map = NestedMap::>::open(key); + + // Insert a new value and check that it's added + let (key, sub_key, val) = ( + address::testing::established_address_1(), + 1_u64, + "Test".to_string(), + ); + lazy_map + .at(&key) + .insert(&mut storage, sub_key, val.clone())?; + + assert_eq!(lazy_map.at(&key).len(&storage)?, 1); + let mut map_it = lazy_map.iter(&storage)?; + let expected_key = NestedSubKey::Data { + key: key.clone(), + nested_sub_key: SubKey::Data(sub_key), + }; + assert_eq!( + map_it.next().unwrap()?, + (expected_key.clone(), val.clone()) + ); + drop(map_it); + + let (key2, sub_key2, val2) = ( + address::testing::established_address_2(), + 2_u64, + "Test2".to_string(), + ); + lazy_map + .at(&key2) + .insert(&mut storage, sub_key2, val2.clone())?; + + assert_eq!(lazy_map.at(&key2).len(&storage)?, 1); + let mut map_it = lazy_map.iter(&storage)?; + assert!(key < key2, "sanity check - this influences the iter order"); + let expected_key2 = NestedSubKey::Data { + key: key2, + nested_sub_key: SubKey::Data(sub_key2), + }; + assert_eq!(map_it.next().unwrap()?, (expected_key, val)); + assert_eq!(map_it.next().unwrap()?, (expected_key2, val2)); + assert!(map_it.next().is_none()); + drop(map_it); Ok(()) } From f7cbdef0891f38d2522da7832ea961a13b2c2ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 13 Mar 2023 07:24:48 +0000 Subject: [PATCH 06/10] core/lazy_set: test and fix for address sub-key --- .../storage_api/collections/lazy_set.rs | 77 ++++++++++++++++--- 1 file changed, 68 insertions(+), 9 deletions(-) diff --git a/core/src/ledger/storage_api/collections/lazy_set.rs b/core/src/ledger/storage_api/collections/lazy_set.rs index 47b271a34e..8379b541c1 100644 --- a/core/src/ledger/storage_api/collections/lazy_set.rs +++ b/core/src/ledger/storage_api/collections/lazy_set.rs @@ -28,7 +28,7 @@ pub struct LazySet { } /// Possible sub-keys of a [`LazySet`] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum SubKey { /// Literal set key Data(K), @@ -88,16 +88,20 @@ where Some(Some(suffix)) => suffix, }; + // A helper to validate the 2nd key segment + let validate_sub_key = |raw_sub_key| { + if let Ok(key) = storage::KeySeg::parse(raw_sub_key) { + Ok(Some(SubKey::Data(key))) + } else { + Err(ValidationError::InvalidSubKey(key.clone())) + .into_storage_result() + } + }; + // Match the suffix against expected sub-keys match &suffix.segments[..] { - [DbKeySeg::StringSeg(sub)] => { - if let Ok(key) = storage::KeySeg::parse(sub.clone()) { - Ok(Some(SubKey::Data(key))) - } else { - Err(ValidationError::InvalidSubKey(key.clone())) - .into_storage_result() - } - } + [DbKeySeg::StringSeg(sub)] => validate_sub_key(sub.clone()), + [DbKeySeg::AddressSeg(sub)] => validate_sub_key(sub.raw()), _ => Err(ValidationError::InvalidSubKey(key.clone())) .into_storage_result(), } @@ -265,6 +269,7 @@ where mod test { use super::*; use crate::ledger::storage::testing::TestWlStorage; + use crate::types::address::{self, Address}; #[test] fn test_lazy_set_basics() -> storage_api::Result<()> { @@ -331,6 +336,60 @@ mod test { assert!(lazy_set.try_insert(&mut storage, key).is_ok()); assert!(lazy_set.try_insert(&mut storage, key).is_err()); + let storage_key = lazy_set.get_key(&key); + assert_eq!( + lazy_set.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(key)) + ); + + let storage_key2 = lazy_set.get_key(&key2); + assert_eq!( + lazy_set.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(key2)) + ); + + Ok(()) + } + + #[test] + fn test_lazy_set_with_addr_key() -> storage_api::Result<()> { + let mut storage = TestWlStorage::default(); + + let key = storage::Key::parse("test").unwrap(); + let lazy_set = LazySet::
::open(key); + + // Insert a new value and check that it's added + let key = address::testing::established_address_1(); + lazy_set.insert(&mut storage, key.clone())?; + + assert_eq!(lazy_set.len(&storage)?, 1); + let mut map_it = lazy_set.iter(&storage)?; + assert_eq!(map_it.next().unwrap()?, key); + drop(map_it); + + let key2 = address::testing::established_address_2(); + lazy_set.insert(&mut storage, key2.clone())?; + + assert_eq!(lazy_set.len(&storage)?, 2); + let mut iter = lazy_set.iter(&storage)?; + assert!(key < key2, "sanity check - this influences the iter order"); + assert_eq!(iter.next().unwrap()?, key); + assert_eq!(iter.next().unwrap()?, key2); + assert!(iter.next().is_none()); + drop(iter); + + let storage_key = lazy_set.get_key(&key); + assert_eq!( + lazy_set.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(key)) + ); + + let storage_key2 = lazy_set.get_key(&key2); + assert_eq!( + lazy_set.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(key2)) + ); + Ok(()) } } From a0824fb4a83e52949ddc11bef44535d98eed3e19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 13 Mar 2023 07:45:13 +0000 Subject: [PATCH 07/10] core/lazy_vec: test and fix for address values --- .../storage_api/collections/lazy_vec.rs | 81 +++++++++++++++++-- 1 file changed, 73 insertions(+), 8 deletions(-) diff --git a/core/src/ledger/storage_api/collections/lazy_vec.rs b/core/src/ledger/storage_api/collections/lazy_vec.rs index 47b5c95c75..67b1730c90 100644 --- a/core/src/ledger/storage_api/collections/lazy_vec.rs +++ b/core/src/ledger/storage_api/collections/lazy_vec.rs @@ -12,7 +12,7 @@ use super::LazyCollection; use crate::ledger::storage_api::validation::{self, Data}; use crate::ledger::storage_api::{self, ResultExt, StorageRead, StorageWrite}; use crate::ledger::vp_env::VpEnv; -use crate::types::storage::{self, DbKeySeg}; +use crate::types::storage::{self, DbKeySeg, KeySeg}; /// Subkey pointing to the length of the LazyVec pub const LEN_SUBKEY: &str = "len"; @@ -35,7 +35,7 @@ pub struct LazyVec { } /// Possible sub-keys of a [`LazyVec`] -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum SubKey { /// Length sub-key Len, @@ -144,6 +144,16 @@ where Some(Some(suffix)) => suffix, }; + // A helper to validate the 2nd key segment + let validate_sub_key = |raw_sub_key| { + if let Ok(index) = storage::KeySeg::parse(raw_sub_key) { + Ok(Some(SubKey::Data(index))) + } else { + Err(ValidationError::InvalidSubKey(key.clone())) + .into_storage_result() + } + }; + // Match the suffix against expected sub-keys match &suffix.segments[..] { [DbKeySeg::StringSeg(sub)] if sub == LEN_SUBKEY => { @@ -152,12 +162,12 @@ where [DbKeySeg::StringSeg(sub_a), DbKeySeg::StringSeg(sub_b)] if sub_a == DATA_SUBKEY => { - if let Ok(index) = storage::KeySeg::parse(sub_b.clone()) { - Ok(Some(SubKey::Data(index))) - } else { - Err(ValidationError::InvalidSubKey(key.clone())) - .into_storage_result() - } + validate_sub_key(sub_b.clone()) + } + [DbKeySeg::StringSeg(sub_a), DbKeySeg::AddressSeg(sub_b)] + if sub_a == DATA_SUBKEY => + { + validate_sub_key(sub_b.raw()) } _ => Err(ValidationError::InvalidSubKey(key.clone())) .into_storage_result(), @@ -477,6 +487,7 @@ where mod test { use super::*; use crate::ledger::storage::testing::TestWlStorage; + use crate::types::address::{self, Address}; #[test] fn test_lazy_vec_basics() -> storage_api::Result<()> { @@ -511,6 +522,60 @@ mod test { assert!(lazy_vec.get(&storage, 0)?.is_none()); assert!(lazy_vec.get(&storage, 1)?.is_none()); + let storage_key = lazy_vec.get_data_key(0); + assert_eq!( + lazy_vec.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(0)) + ); + + let storage_key2 = lazy_vec.get_data_key(1); + assert_eq!( + lazy_vec.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(1)) + ); + + Ok(()) + } + + #[test] + fn test_lazy_vec_with_addr() -> storage_api::Result<()> { + let mut storage = TestWlStorage::default(); + + let key = storage::Key::parse("test").unwrap(); + let lazy_vec = LazyVec::
::open(key); + + // Push a new value and check that it's added + let val = address::testing::established_address_1(); + lazy_vec.push(&mut storage, val.clone())?; + assert!(!lazy_vec.is_empty(&storage)?); + assert!(lazy_vec.len(&storage)? == 1); + assert_eq!(lazy_vec.iter(&storage)?.next().unwrap()?, val); + assert_eq!(lazy_vec.get(&storage, 0)?.unwrap(), val); + assert!(lazy_vec.get(&storage, 1)?.is_none()); + + let val2 = address::testing::established_address_2(); + lazy_vec.push(&mut storage, val2.clone())?; + + assert_eq!(lazy_vec.len(&storage)?, 2); + let mut iter = lazy_vec.iter(&storage)?; + // The iterator order follows the indices + assert_eq!(iter.next().unwrap()?, val); + assert_eq!(iter.next().unwrap()?, val2); + assert!(iter.next().is_none()); + drop(iter); + + let storage_key = lazy_vec.get_data_key(0); + assert_eq!( + lazy_vec.is_valid_sub_key(&storage_key).unwrap(), + Some(SubKey::Data(0)) + ); + + let storage_key2 = lazy_vec.get_data_key(1); + assert_eq!( + lazy_vec.is_valid_sub_key(&storage_key2).unwrap(), + Some(SubKey::Data(1)) + ); + Ok(()) } } From bb0a3e98c9efba76d00a9ce6fa50967204ab802e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Tue, 28 Mar 2023 10:01:08 +0100 Subject: [PATCH 08/10] core/wl_storage: remove unnecessary alloc --- core/src/ledger/storage/wl_storage.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/ledger/storage/wl_storage.rs b/core/src/ledger/storage/wl_storage.rs index ff6f454162..f1d8d8b9d9 100644 --- a/core/src/ledger/storage/wl_storage.rs +++ b/core/src/ledger/storage/wl_storage.rs @@ -183,10 +183,9 @@ where what = Next::ReturnStorage; } (Some((storage_key, _, _)), Some((wl_key, _))) => { - let wl_key = wl_key.to_string(); - if &wl_key <= storage_key { + if wl_key <= storage_key { what = Next::ReturnWl { - advance_storage: &wl_key == storage_key, + advance_storage: wl_key == storage_key, }; } else { what = Next::ReturnStorage; From 3c64a8401e4ff8a5e34498eae3647eac47c2d0f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 13 Mar 2023 09:16:40 +0000 Subject: [PATCH 09/10] changelog: add #1212 --- .../unreleased/bug-fixes/1212-lazy-collection-sub-key.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/1212-lazy-collection-sub-key.md diff --git a/.changelog/unreleased/bug-fixes/1212-lazy-collection-sub-key.md b/.changelog/unreleased/bug-fixes/1212-lazy-collection-sub-key.md new file mode 100644 index 0000000000..49d1c5dd57 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1212-lazy-collection-sub-key.md @@ -0,0 +1,3 @@ +- Fixed an issue with lazy collections sub-key validation with the `Address` + type. This issue was also affecting the iterator of nested `LazyMap`. + ([#1212](https://github.com/anoma/namada/pull/1212)) From a1fbe9bff02eb579e893705c46d8430681b5b218 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Wed, 29 Mar 2023 07:11:24 +0000 Subject: [PATCH 10/10] [ci] wasm checksums update --- wasm/checksums.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wasm/checksums.json b/wasm/checksums.json index a336d03400..1ca639c3c8 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,16 +1,16 @@ { - "tx_bond.wasm": "tx_bond.3fbd9f21a7fb1ea3ad9e7e35ceccf0b3cec6f3bfcc192b762e6f02dac36e6dea.wasm", - "tx_change_validator_commission.wasm": "tx_change_validator_commission.a7788841a2a89b81d4e31ee058eabf3a2042642a73297d59b2bbb348ee0f3e99.wasm", + "tx_bond.wasm": "tx_bond.cf30418fef743920c3608b0f8da07a8dc6f9645502fba1db196bf7389fde0552.wasm", + "tx_change_validator_commission.wasm": "tx_change_validator_commission.4a64ce48b48776b2a3d58f9e82d67de9c2b1ac18aafa213385e51fbbc5e38c22.wasm", "tx_ibc.wasm": "tx_ibc.9057055a2ca07ea9c3b0b5d587bc4d7d138c3197a843151b7064c747881770be.wasm", "tx_init_account.wasm": "tx_init_account.05fba94092683984fe52dd0c7dd4d66a9b62a448ca3fe7281bfa711457e19759.wasm", "tx_init_proposal.wasm": "tx_init_proposal.2d952f3068e708fb0f2d5085bee0622c11ac303f73712d15782e4a91e3bc268e.wasm", - "tx_init_validator.wasm": "tx_init_validator.56af521501e22bfe2a8b65c31e6a7424bb6868da81e8f583229aa30c2d23404d.wasm", + "tx_init_validator.wasm": "tx_init_validator.ec8d6e03caa457d4cb421ffdf08469fab7d7b72ff66708290669fadd5a77342b.wasm", "tx_reveal_pk.wasm": "tx_reveal_pk.93ac324e81450fb3ce3852113a17c92328f826319ac979f5d53660c87544013d.wasm", "tx_transfer.wasm": "tx_transfer.cc04b814dfaa9bc0a4f9e5340820173e1a94fed5cdf762d31e6fbfd23ec1f58a.wasm", - "tx_unbond.wasm": "tx_unbond.be25010b518fa098c68319064e1e7492de2953cf8e2d52537ff07150fd8cb580.wasm", + "tx_unbond.wasm": "tx_unbond.1dda3e1a1c180eb4110959d0b1518241e0b86b96f143f9909dd7ba0cd7136db0.wasm", "tx_update_vp.wasm": "tx_update_vp.88c649a97a6d718b8c33e6440ed8b4c3b91aa7072cfc60362f7e490f1909134d.wasm", "tx_vote_proposal.wasm": "tx_vote_proposal.b123b51369102fad83f0b73dfaa65cd6419111dcd090d5342c46a1f3eead3c54.wasm", - "tx_withdraw.wasm": "tx_withdraw.3a4fb67ee3118a550d5648266100ea6f40af5a7495ffacd2b98b1a15ad3d88c2.wasm", + "tx_withdraw.wasm": "tx_withdraw.c21517ffe13cc99a38df1e6fb2e391fabd9d8fd318fc4134c661d6970f6f7cd2.wasm", "vp_implicit.wasm": "vp_implicit.7033fb123369a7cce109ce0670a1dde0d09c72909999ce1012dafedcd593ad39.wasm", "vp_masp.wasm": "vp_masp.3f1deb1034615e33b2d8e68bb507da9416e6cc48b8c076aa259d4a4896e4bfeb.wasm", "vp_testnet_faucet.wasm": "vp_testnet_faucet.dabc8dbf1c3db77788ca9aa07e2c83979d1fc8ecb4a8be4c94f62c86820eaff2.wasm",