From 05ab1e03e374e619dba30d429c3d95edffbb2d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 3 Jul 2023 08:40:34 +0100 Subject: [PATCH 1/7] ledger/db: ensure that prefix iter only matches full key segments --- apps/src/lib/node/ledger/storage/rocksdb.rs | 14 ++++++++++++-- core/src/ledger/storage/mockdb.rs | 16 ++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 460f310ba4..b052cd2321 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -1312,8 +1312,18 @@ fn iter_subspace_prefix<'iter>( .get_column_family(SUBSPACE_CF) .expect("{SUBSPACE_CF} column family should exist"); let db_prefix = "".to_owned(); - let prefix = prefix.map(|k| k.to_string()).unwrap_or_default(); - iter_prefix(db, subspace_cf, db_prefix, Some(prefix)) + iter_prefix( + db, + subspace_cf, + db_prefix, + prefix.map(|k| { + if k == &Key::default() { + k.to_string() + } else { + format!("{k}/") + } + }), + ) } fn iter_diffs_prefix( diff --git a/core/src/ledger/storage/mockdb.rs b/core/src/ledger/storage/mockdb.rs index 9955456830..134e4557e4 100644 --- a/core/src/ledger/storage/mockdb.rs +++ b/core/src/ledger/storage/mockdb.rs @@ -516,8 +516,20 @@ impl<'iter> DBIter<'iter> for MockDB { fn iter_prefix(&'iter self, prefix: Option<&Key>) -> MockPrefixIterator { let db_prefix = "subspace/".to_owned(); - let prefix_str = prefix.map(|k| k.to_string()).unwrap_or_default(); - let prefix = format!("{}{}", db_prefix, prefix_str); + let prefix = format!( + "{}{}", + db_prefix, + match prefix { + Some(prefix) => { + if prefix == &Key::default() { + prefix.to_string() + } else { + format!("{prefix}/") + } + } + None => "".to_string(), + } + ); let iter = self.0.borrow().clone().into_iter(); MockPrefixIterator::new(MockIterator { prefix, iter }, db_prefix) } From f8d765c2ac9042917045655f0a8a65bdebcf6d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 3 Jul 2023 09:20:49 +0100 Subject: [PATCH 2/7] test/storage/rocksdb: check that prefix_iter matches only full segments --- apps/src/lib/node/ledger/storage/rocksdb.rs | 26 +++++++++------------ 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index b052cd2321..fa2acc6615 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -1619,28 +1619,24 @@ mod test { let key_1_a = prefix_1.push(&"a".to_string()).unwrap(); let key_1_b = prefix_1.push(&"b".to_string()).unwrap(); let key_1_c = prefix_1.push(&"c".to_string()).unwrap(); + let prefix_01 = Key::parse("01").unwrap(); + let key_01_a = prefix_01.push(&"a".to_string()).unwrap(); - let keys_0 = vec![key_0_a.clone(), key_0_b.clone(), key_0_c.clone()]; - let keys_1 = vec![key_1_a.clone(), key_1_b.clone(), key_1_c.clone()]; - let all_keys = vec![keys_0.clone(), keys_1.clone()].concat(); + let keys_0 = vec![key_0_a, key_0_b, key_0_c]; + let keys_1 = vec![key_1_a, key_1_b, key_1_c]; + let keys_01 = vec![key_01_a]; + let all_keys = vec![keys_0.clone(), keys_01, keys_1.clone()].concat(); // Write the keys let mut batch = RocksDB::batch(); let height = BlockHeight(1); - db.batch_write_subspace_val(&mut batch, height, &key_0_a, [0_u8]) - .unwrap(); - db.batch_write_subspace_val(&mut batch, height, &key_0_b, [0_u8]) - .unwrap(); - db.batch_write_subspace_val(&mut batch, height, &key_0_c, [0_u8]) - .unwrap(); - db.batch_write_subspace_val(&mut batch, height, &key_1_a, [0_u8]) - .unwrap(); - db.batch_write_subspace_val(&mut batch, height, &key_1_b, [0_u8]) - .unwrap(); - db.batch_write_subspace_val(&mut batch, height, &key_1_c, [0_u8]) - .unwrap(); + for key in &all_keys { + db.batch_write_subspace_val(&mut batch, height, key, [0_u8]) + .unwrap(); + } db.exec_batch(batch.0).unwrap(); + // Prefix "0" shouldn't match prefix "01" let itered_keys: Vec = db .iter_prefix(Some(&prefix_0)) .map(|(key, _val, _)| Key::parse(key).unwrap()) From fd9d849e56ea5c04d00b9ce8e072be12c0fd6b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 3 Jul 2023 09:25:31 +0100 Subject: [PATCH 3/7] changelog: add #1642 --- .../unreleased/improvements/1642-iter-prefix-full-match.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/1642-iter-prefix-full-match.md diff --git a/.changelog/unreleased/improvements/1642-iter-prefix-full-match.md b/.changelog/unreleased/improvements/1642-iter-prefix-full-match.md new file mode 100644 index 0000000000..935e027711 --- /dev/null +++ b/.changelog/unreleased/improvements/1642-iter-prefix-full-match.md @@ -0,0 +1,3 @@ +- Storage: Ensure that prefix iterator only returns key- + vals in which the prefix key segments are matched fully. + ([\#1642](https://github.com/anoma/namada/pull/1642)) \ No newline at end of file From 4d19abe6d1f2be5e6803bb889adf7a84189074dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Jun 2023 08:47:19 +0100 Subject: [PATCH 4/7] core/storage_api: add `StorageWrite::delete_prefix` --- core/src/ledger/storage_api/mod.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/core/src/ledger/storage_api/mod.rs b/core/src/ledger/storage_api/mod.rs index 2475db3197..451996d0e5 100644 --- a/core/src/ledger/storage_api/mod.rs +++ b/core/src/ledger/storage_api/mod.rs @@ -120,6 +120,26 @@ pub trait StorageWrite { /// Delete a value at the given key from storage. fn delete(&mut self, key: &storage::Key) -> Result<()>; + + /// Delete all key-vals with a matching prefix. + fn delete_prefix(&mut self, prefix: &storage::Key) -> Result<()> + where + Self: StorageRead + Sized, + { + let keys = iter_prefix_bytes(self, prefix)? + .map(|res| { + let (key, _val) = res?; + Ok(key) + }) + .collect::>>(); + for key in keys? { + // Skip validity predicates as they cannot be deleted + if key.is_validity_predicate().is_none() { + self.delete(&key)?; + } + } + Ok(()) + } } /// Iterate items matching the given prefix, ordered by the storage keys. From 03a8a74e2447097eae2c66627c9d6f1044f5e69c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Jun 2023 10:22:09 +0100 Subject: [PATCH 5/7] test/core/wl_storage: extend prefix iter test to include delete prefix --- .../ledger/storage/wl_storage.txt | 8 ++ core/src/ledger/storage/wl_storage.rs | 85 +++++++++++++++++-- 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 core/proptest-regressions/ledger/storage/wl_storage.txt diff --git a/core/proptest-regressions/ledger/storage/wl_storage.txt b/core/proptest-regressions/ledger/storage/wl_storage.txt new file mode 100644 index 0000000000..2a372cf111 --- /dev/null +++ b/core/proptest-regressions/ledger/storage/wl_storage.txt @@ -0,0 +1,8 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc dda57e313536d5b8da109e808f0ea8af87981e843db9907ab75adcf58c3ad7a0 # shrinks to key_vals = [(Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("_"), StringSeg("0")] }, Storage(0)), (Key { segments: [StringSeg("_"), StringSeg("_")] }, Storage(0)), (Key { segments: [StringSeg("_A"), StringSeg("A")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("_"), StringSeg("0")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [StringSeg("_"), StringSeg("0")] }, BlockWriteLog(Write(0))), (Key { segments: [StringSeg("_"), StringSeg("0")] }, BlockWriteLog(Delete)), (Key { segments: [StringSeg("_"), StringSeg("0")] }, BlockWriteLog(Delete)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [StringSeg("_")] }, BlockWriteLog(DeletePrefix))] +cc be5b2533eaa2312359cefbe482c60fa93524e9decae0905af2b9aec936c05a56 # shrinks to key_vals = [(Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("a"), StringSeg("0")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("_"), StringSeg("_")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("a"), StringSeg("A")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [StringSeg("_0"), StringSeg("_")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, Storage(0)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu), StringSeg("?")] }, BlockWriteLog(Write(0))), (Key { segments: [StringSeg("_"), StringSeg("_")] }, BlockWriteLog(Delete)), (Key { segments: [StringSeg("a"), StringSeg("0")] }, BlockWriteLog(Delete)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [AddressSeg(Established: atest1v4ehgw36gym52vfnxqmrjdp3xcmyx3zz8y65yv29x9pyys69xdrryv29x3zyy3pkxdrrgdjylcyclu)] }, BlockWriteLog(DeletePrefix)), (Key { segments: [StringSeg("_")] }, TxWriteLog(DeletePrefix))] diff --git a/core/src/ledger/storage/wl_storage.rs b/core/src/ledger/storage/wl_storage.rs index cdb29668bf..357cd85f54 100644 --- a/core/src/ledger/storage/wl_storage.rs +++ b/core/src/ledger/storage/wl_storage.rs @@ -548,6 +548,12 @@ mod tests { // Deletes have to be applied last if let Level::BlockWriteLog(WlMod::Delete) = val { expected_pre.remove(key); + } else if let Level::BlockWriteLog(WlMod::DeletePrefix) = val { + expected_pre.retain(|expected_key, _val| { + // Remove matching prefixes except for VPs + expected_key.is_validity_predicate().is_some() + || expected_key.split_prefix(key).is_none() + }) } } @@ -582,6 +588,12 @@ mod tests { // Deletes have to be applied last if let Level::TxWriteLog(WlMod::Delete) = val { expected_post.remove(key); + } else if let Level::TxWriteLog(WlMod::DeletePrefix) = val { + expected_post.retain(|expected_key, _val| { + // Remove matching prefixes except for VPs + expected_key.is_validity_predicate().is_some() + || expected_key.split_prefix(key).is_none() + }) } } @@ -602,10 +614,12 @@ mod tests { } fn apply_to_wl_storage(s: &mut TestWlStorage, kvs: &[KeyVal]) { + // Apply writes first for (key, val) in kvs { match val { - Level::TxWriteLog(WlMod::Delete) - | Level::BlockWriteLog(WlMod::Delete) => {} + Level::TxWriteLog(WlMod::Delete | WlMod::DeletePrefix) + | Level::BlockWriteLog(WlMod::Delete | WlMod::DeletePrefix) => { + } Level::TxWriteLog(WlMod::Write(val)) => { s.write_log.write(key, val.try_to_vec().unwrap()).unwrap(); } @@ -620,13 +634,34 @@ mod tests { } } } + // Then apply deletions for (key, val) in kvs { match val { Level::TxWriteLog(WlMod::Delete) => { s.write_log.delete(key).unwrap(); } Level::BlockWriteLog(WlMod::Delete) => { - s.write_log.protocol_delete(key).unwrap(); + s.delete(key).unwrap(); + } + Level::TxWriteLog(WlMod::DeletePrefix) => { + // Find keys matching the prefix + let keys = storage_api::iter_prefix_bytes(s, key) + .unwrap() + .map(|res| { + let (key, _val) = res.unwrap(); + key + }) + .collect::>(); + // Delete the matching keys + for key in keys { + // Skip validity predicates which cannot be deleted + if key.is_validity_predicate().is_none() { + s.write_log.delete(&key).unwrap(); + } + } + } + Level::BlockWriteLog(WlMod::DeletePrefix) => { + s.delete_prefix(key).unwrap(); } _ => {} } @@ -649,6 +684,7 @@ mod tests { enum WlMod { Write(VAL), Delete, + DeletePrefix, } fn arb_key_vals(len: usize) -> impl Strategy>> { @@ -675,9 +711,22 @@ mod tests { 1..len / 3, ); + // Select some indices to delete prefix + let delete_prefix = prop::collection::vec( + ( + any::(), + any::(), + // An arbitrary number of key segments to drop from a selected + // key to obtain the prefix. Because `arb_key` generates `2..5` + // segments, we can drop one less of its upper bound. + (2_usize..4), + ), + 1..len / 4, + ); + // Combine them all together - (storage_kvs, overrides, deletes).prop_map( - |(mut kvs, overrides, deletes)| { + (storage_kvs, overrides, deletes, delete_prefix).prop_map( + |(mut kvs, overrides, deletes, delete_prefix)| { for (ix, val, is_tx) in overrides { let (key, _) = ix.get(&kvs); let wl_mod = WlMod::Write(val); @@ -703,6 +752,32 @@ mod tests { }; kvs.push((key.clone(), lvl)); } + for (ix, is_tx, num_of_seg_to_drop) in delete_prefix { + let (key, _) = ix.get(&kvs); + let wl_mod = WlMod::DeletePrefix; + let lvl = if is_tx { + Level::TxWriteLog(wl_mod) + } else { + Level::BlockWriteLog(wl_mod) + }; + // Keep at least one segment + let num_of_seg_to_keep = std::cmp::max( + 1, + key.segments + .len() + .checked_sub(num_of_seg_to_drop) + .unwrap_or_default(), + ); + let prefix = storage::Key { + segments: key + .segments + .iter() + .take(num_of_seg_to_keep) + .cloned() + .collect(), + }; + kvs.push((prefix, lvl)); + } kvs }, ) From efe7ceb66b78081774185f0befe3d44a10c0a624 Mon Sep 17 00:00:00 2001 From: brentstone Date: Sun, 2 Jul 2023 15:24:08 +0200 Subject: [PATCH 6/7] storage_api/lazy_map: remove entries by prefixed key --- .../storage_api/collections/lazy_map.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/core/src/ledger/storage_api/collections/lazy_map.rs b/core/src/ledger/storage_api/collections/lazy_map.rs index 5870fd7b3f..4f9aeb426d 100644 --- a/core/src/ledger/storage_api/collections/lazy_map.rs +++ b/core/src/ledger/storage_api/collections/lazy_map.rs @@ -392,6 +392,19 @@ where V::open(self.get_data_key(key)) } + /// Remove all map entries at a given key prefix + pub fn remove_all(&self, storage: &mut S, key: &K) -> Result + where + S: StorageRead + StorageWrite, + { + let is_data = self.contains(storage, key)?; + + let data_prefix = self.get_data_key(key); + storage.delete_prefix(&data_prefix)?; + + Ok(is_data) + } + /// An iterator visiting all key-value elements, where the values are from /// the inner-most collection. The iterator element type is `Result<_>`, /// because iterator's call to `next` may fail with e.g. out of gas or @@ -855,4 +868,35 @@ mod test { Ok(()) } + + #[test] + fn test_nested_map_key_prefix_removal() { + let mut storage = TestWlStorage::default(); + let key = storage::Key::parse("testing").unwrap(); + + // A nested map from u32 -> String -> u32 + let nested_map = NestedMap::>::open(key); + nested_map + .at(&0) + .insert(&mut storage, "dingus".to_string(), 5) + .unwrap(); + nested_map + .at(&0) + .insert(&mut storage, "zingus".to_string(), 3) + .unwrap(); + nested_map + .at(&1) + .insert(&mut storage, "dingus".to_string(), 4) + .unwrap(); + + assert_eq!(nested_map.iter(&storage).unwrap().count(), 3); + + nested_map.remove_all(&mut storage, &0).unwrap(); + assert!(!nested_map.contains(&storage, &0).unwrap()); + assert_eq!(nested_map.iter(&storage).unwrap().count(), 1); + + nested_map.remove_all(&mut storage, &1).unwrap(); + assert!(!nested_map.contains(&storage, &1).unwrap()); + assert!(nested_map.is_empty(&storage).unwrap()); + } } From d9b6dd910792d1cac26231dcf19e471f303f478b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Mon, 3 Jul 2023 10:39:32 +0100 Subject: [PATCH 7/7] changelog: add #1632 --- .changelog/unreleased/features/1632-delete-prefix.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/1632-delete-prefix.md diff --git a/.changelog/unreleased/features/1632-delete-prefix.md b/.changelog/unreleased/features/1632-delete-prefix.md new file mode 100644 index 0000000000..9b6b229a40 --- /dev/null +++ b/.changelog/unreleased/features/1632-delete-prefix.md @@ -0,0 +1,2 @@ +- Storage: Add a function to delete key-vals matching a given prefix. + ([\#1632](https://github.com/anoma/namada/pull/1632)) \ No newline at end of file