From 565011077e9ee71b26a47e5d4059d7ebbf0c0231 Mon Sep 17 00:00:00 2001 From: mariari Date: Wed, 10 May 2023 17:12:45 +0800 Subject: [PATCH 1/3] Fix Key from always parsing Previously the key would always parse, even if the string is empty, this causes an issue where if we wanted an Optional Key, then it would always be parsed as Some, causing issues for various parts of the system. --- core/src/types/storage.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index e61e1e98dc..7140d3bc2c 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -480,11 +480,16 @@ impl Value for TreeBytes { impl Key { /// Parses string and returns a key pub fn parse(string: impl AsRef) -> Result { - let mut segments = Vec::new(); - for s in string.as_ref().split(KEY_SEGMENT_SEPARATOR) { - segments.push(DbKeySeg::parse(s.to_owned())?); + let string = string.as_ref(); + if string.is_empty() { + Err(Error::ParseKeySeg(string.to_string())) + } else { + let mut segments = Vec::new(); + for s in string.split(KEY_SEGMENT_SEPARATOR) { + segments.push(DbKeySeg::parse(s.to_owned())?); + } + Ok(Key { segments }) } - Ok(Key { segments }) } /// Returns a new key with segments of `Self` and the given segment From 05c133bc377da5b6867f7286e95a5019c6f29edb Mon Sep 17 00:00:00 2001 From: satan Date: Fri, 9 Jun 2023 11:19:26 +0200 Subject: [PATCH 2/3] [fix]: Fixed unit tests that failed on parsing empty strings as keys --- apps/src/lib/node/ledger/shell/finalize_block.rs | 4 +--- apps/src/lib/node/ledger/shell/init_chain.rs | 5 +---- apps/src/lib/node/ledger/storage/rocksdb.rs | 8 ++++---- core/src/ledger/storage/mod.rs | 5 ++++- core/src/ledger/storage/wl_storage.rs | 4 ++-- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs index e992544051..f0fa9e9434 100644 --- a/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -895,7 +895,6 @@ fn pos_votes_from_abci( #[cfg(test)] mod test_finalize_block { use std::collections::{BTreeMap, BTreeSet}; - use std::str::FromStr; use data_encoding::HEXUPPER; use namada::ledger::parameters::EpochDuration; @@ -1340,12 +1339,11 @@ mod test_finalize_block { // Collect all storage key-vals into a sorted map let store_block_state = |shell: &TestShell| -> BTreeMap<_, _> { - let prefix: Key = FromStr::from_str("").unwrap(); shell .wl_storage .storage .db - .iter_prefix(&prefix) + .iter_optional_prefix(None) .map(|(key, val, _gas)| (key, val)) .collect() }; diff --git a/apps/src/lib/node/ledger/shell/init_chain.rs b/apps/src/lib/node/ledger/shell/init_chain.rs index 3aabed3196..aa23947509 100644 --- a/apps/src/lib/node/ledger/shell/init_chain.rs +++ b/apps/src/lib/node/ledger/shell/init_chain.rs @@ -459,11 +459,9 @@ where #[cfg(test)] mod test { use std::collections::BTreeMap; - use std::str::FromStr; use namada::ledger::storage::DBIter; use namada::types::chain::ChainId; - use namada::types::storage; use crate::facade::tendermint_proto::abci::RequestInitChain; use crate::facade::tendermint_proto::google::protobuf::Timestamp; @@ -477,12 +475,11 @@ mod test { // Collect all storage key-vals into a sorted map let store_block_state = |shell: &TestShell| -> BTreeMap<_, _> { - let prefix: storage::Key = FromStr::from_str("").unwrap(); shell .wl_storage .storage .db - .iter_prefix(&prefix) + .iter_optional_prefix(None) .map(|(key, val, _gas)| (key, val)) .collect() }; diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 615ee0e4b9..6108cdf289 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -413,9 +413,8 @@ impl RocksDB { let batch = Mutex::new(batch); tracing::info!("Restoring previous hight subspace diffs"); - self.iter_prefix(&Key::default()) - .par_bridge() - .try_for_each(|(key, _value, _gas)| -> Result<()> { + self.iter_optional_prefix(None).par_bridge().try_for_each( + |(key, _value, _gas)| -> Result<()> { // Restore previous height diff if present, otherwise delete the // subspace key let subspace_cf = self.get_column_family(SUBSPACE_CF)?; @@ -433,7 +432,8 @@ impl RocksDB { } Ok(()) - })?; + }, + )?; tracing::info!("Deleting keys prepended with the last height"); let mut batch = batch.into_inner().unwrap(); diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index bece612b56..137c7b9e53 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -600,7 +600,10 @@ where &self, prefix: &Key, ) -> (>::PrefixIter, u64) { - (self.db.iter_prefix(prefix), prefix.len() as _) + ( + self.db.iter_optional_prefix(Some(prefix)), + prefix.len() as _, + ) } /// Returns a prefix iterator and the gas cost diff --git a/core/src/ledger/storage/wl_storage.rs b/core/src/ledger/storage/wl_storage.rs index 82bb799f39..4ca76aa379 100644 --- a/core/src/ledger/storage/wl_storage.rs +++ b/core/src/ledger/storage/wl_storage.rs @@ -236,7 +236,7 @@ where D: DB + for<'iter_> DBIter<'iter_>, H: StorageHasher, { - let storage_iter = storage.db.iter_prefix(prefix).peekable(); + let storage_iter = storage.db.iter_optional_prefix(Some(prefix)).peekable(); let write_log_iter = write_log.iter_prefix_pre(prefix).peekable(); ( PrefixIter { @@ -261,7 +261,7 @@ where D: DB + for<'iter_> DBIter<'iter_>, H: StorageHasher, { - let storage_iter = storage.db.iter_prefix(prefix).peekable(); + let storage_iter = storage.db.iter_optional_prefix(Some(prefix)).peekable(); let write_log_iter = write_log.iter_prefix_post(prefix).peekable(); ( PrefixIter { From 4232b87522378b4f69908a3613851d9980f2e97f Mon Sep 17 00:00:00 2001 From: Gianmarco Fraccaroli Date: Mon, 12 Jun 2023 08:59:50 +0200 Subject: [PATCH 3/3] added changelog --- .changelog/unreleased/bug-fixes/1345-key-parsing-fix.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/1345-key-parsing-fix.md diff --git a/.changelog/unreleased/bug-fixes/1345-key-parsing-fix.md b/.changelog/unreleased/bug-fixes/1345-key-parsing-fix.md new file mode 100644 index 0000000000..e7d8f19ee8 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1345-key-parsing-fix.md @@ -0,0 +1,2 @@ +- Correctly handle parsing storage key if they are empty. + ([#1345](https://github.com/anoma/namada/pull/1345)) \ No newline at end of file