From 66bff1e954ebd575cc53a946685fff2c33b47ea8 Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 01:00:53 +0200 Subject: [PATCH 1/9] WIP: add try append for nmap --- substrate/frame/support/src/storage/mod.rs | 35 ++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 7fb991d37792..97544fa03e8d 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -1648,6 +1648,41 @@ where } } + +/// Storage N map that is capable of [`StorageTryAppend`]. +pub trait TryAppendNMap + TupleToEncodedIter, T: StorageTryAppend, I: Encode> { + /// Try and append the `item` into the storage map at the given `key`. + /// + /// This might fail if bounds are not respected. + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()>; +} + +impl TryAppendMap for StorageMapT +where + K: KeyGenerator, + T: FullCodec + StorageTryAppend, + I: Encode, + StorageNMapT: generator::StorageNMap, +{ + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()> { + let bound = T::bound(); + let current = Self::decode_len(key.clone()).unwrap_or_default(); + if current < bound { + let key = Self::storage_n_map_final_key(key); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + /// Storage double map that is capable of [`StorageTryAppend`]. pub trait TryAppendDoubleMap, I: Encode> { /// Try and append the `item` into the storage double map at the given `key`. From b063092a178662453506d43ff73afc9389fc7b4d Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 01:09:56 +0200 Subject: [PATCH 2/9] WIP: add try append tests --- substrate/frame/support/src/storage/mod.rs | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 97544fa03e8d..5bfd01cf3a58 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -1660,7 +1660,7 @@ pub trait TryAppendNMap + TupleToEncodedIter, T: Sto ) -> Result<(), ()>; } -impl TryAppendMap for StorageMapT +impl TryAppendNMap for StorageNMapT where K: KeyGenerator, T: FullCodec + StorageTryAppend, @@ -2144,6 +2144,31 @@ mod test { BoundedVec::>::try_from(vec![4, 5]).unwrap(), ); }); + + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec> = vec![1, 2, 3].try_into().unwrap(); + FooTripleMap::insert((1, 1, 1), bounded); + + assert_ok!(FooTripleMap::try_append((1, 1, 1), 4)); + assert_ok!(FooTripleMap::try_append((1, 1, 1), 5)); + assert_ok!(FooTripleMap::try_append((1, 1, 1), 6)); + assert_ok!(FooTripleMap::try_append((1, 1, 1), 7)); + assert_eq!(FooTripleMap::decode_len((1, 1, 1)).unwrap(), 7); + assert!(FooTripleMap::try_append((1, 1, 1), 8).is_err()); + + // append to a non-existing + assert!(FooTripleMap::get((2, 1, 1)).is_none()); + assert_ok!(FooTripleMap::try_append((2, 1, 1), 4)); + assert_eq!( + FooTripleMap::get((2, 1, 1)).unwrap(), + BoundedVec::>::try_from(vec![4]).unwrap(), + ); + assert_ok!(FooTripleMap::try_append((2, 1, 1), 5)); + assert_eq!( + FooTripleMap::get((2, 1, 1)).unwrap(), + BoundedVec::>::try_from(vec![4, 5]).unwrap(), + ); + }); } #[crate::storage_alias] From 604ec05373070cfe5af1354892a4689a6b312e27 Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 01:54:22 +0200 Subject: [PATCH 3/9] WIP: fix generic --- substrate/frame/support/src/storage/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 5bfd01cf3a58..6bb9fd8f51fc 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -1650,11 +1650,11 @@ where /// Storage N map that is capable of [`StorageTryAppend`]. -pub trait TryAppendNMap + TupleToEncodedIter, T: StorageTryAppend, I: Encode> { +pub trait TryAppendNMap, I: Encode> { /// Try and append the `item` into the storage map at the given `key`. /// /// This might fail if bounds are not respected. - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( key: LikeK, item: LikeI, ) -> Result<(), ()>; @@ -1667,14 +1667,14 @@ where I: Encode, StorageNMapT: generator::StorageNMap, { - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( key: LikeK, item: LikeI, ) -> Result<(), ()> { let bound = T::bound(); let current = Self::decode_len(key.clone()).unwrap_or_default(); if current < bound { - let key = Self::storage_n_map_final_key(key); + let key = Self::storage_n_map_final_key::(key); sp_io::storage::append(&key, item.encode()); Ok(()) } else { From f8872c2671a116796d6b66f893ea047706bebfec Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 02:15:44 +0200 Subject: [PATCH 4/9] extend try_append tests --- substrate/frame/support/src/storage/mod.rs | 30 +++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 6bb9fd8f51fc..85848950549d 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -2054,6 +2054,12 @@ mod test { (NMapKey, NMapKey, NMapKey), u64, >; + #[crate::storage_alias] + type FooQuadMap = StorageNMap< + Prefix, + (NMapKey, NMapKey, NMapKey, NMapKey), + BoundedVec>, + >; #[test] fn contains_prefix_works() { @@ -2147,25 +2153,25 @@ mod test { TestExternalities::default().execute_with(|| { let bounded: BoundedVec> = vec![1, 2, 3].try_into().unwrap(); - FooTripleMap::insert((1, 1, 1), bounded); + FooQuadMap::insert((1, 1, 1, 1), bounded); - assert_ok!(FooTripleMap::try_append((1, 1, 1), 4)); - assert_ok!(FooTripleMap::try_append((1, 1, 1), 5)); - assert_ok!(FooTripleMap::try_append((1, 1, 1), 6)); - assert_ok!(FooTripleMap::try_append((1, 1, 1), 7)); - assert_eq!(FooTripleMap::decode_len((1, 1, 1)).unwrap(), 7); - assert!(FooTripleMap::try_append((1, 1, 1), 8).is_err()); + assert_ok!(FooQuadMap::try_append((1, 1, 1, 1), 4)); + assert_ok!(FooQuadMap::try_append((1, 1, 1, 1), 5)); + assert_ok!(FooQuadMap::try_append((1, 1, 1, 1), 6)); + assert_ok!(FooQuadMap::try_append((1, 1, 1, 1), 7)); + assert_eq!(FooQuadMap::decode_len((1, 1, 1, 1)).unwrap(), 7); + assert!(FooQuadMap::try_append((1, 1, 1, 1), 8).is_err()); // append to a non-existing - assert!(FooTripleMap::get((2, 1, 1)).is_none()); - assert_ok!(FooTripleMap::try_append((2, 1, 1), 4)); + assert!(FooQuadMap::get((2, 1, 1, 1)).is_none()); + assert_ok!(FooQuadMap::try_append((2, 1, 1, 1), 4)); assert_eq!( - FooTripleMap::get((2, 1, 1)).unwrap(), + FooQuadMap::get((2, 1, 1, 1)).unwrap(), BoundedVec::>::try_from(vec![4]).unwrap(), ); - assert_ok!(FooTripleMap::try_append((2, 1, 1), 5)); + assert_ok!(FooQuadMap::try_append((2, 1, 1, 1), 5)); assert_eq!( - FooTripleMap::get((2, 1, 1)).unwrap(), + FooQuadMap::get((2, 1, 1, 1)).unwrap(), BoundedVec::>::try_from(vec![4, 5]).unwrap(), ); }); From 8a94b764c9e923aed6bbf6193139696928f7afc7 Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 02:37:12 +0200 Subject: [PATCH 5/9] reorder code block --- substrate/frame/support/src/storage/mod.rs | 69 +++++++++++----------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 85848950549d..020d833e5d12 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -1648,41 +1648,6 @@ where } } - -/// Storage N map that is capable of [`StorageTryAppend`]. -pub trait TryAppendNMap, I: Encode> { - /// Try and append the `item` into the storage map at the given `key`. - /// - /// This might fail if bounds are not respected. - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( - key: LikeK, - item: LikeI, - ) -> Result<(), ()>; -} - -impl TryAppendNMap for StorageNMapT -where - K: KeyGenerator, - T: FullCodec + StorageTryAppend, - I: Encode, - StorageNMapT: generator::StorageNMap, -{ - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( - key: LikeK, - item: LikeI, - ) -> Result<(), ()> { - let bound = T::bound(); - let current = Self::decode_len(key.clone()).unwrap_or_default(); - if current < bound { - let key = Self::storage_n_map_final_key::(key); - sp_io::storage::append(&key, item.encode()); - Ok(()) - } else { - Err(()) - } - } -} - /// Storage double map that is capable of [`StorageTryAppend`]. pub trait TryAppendDoubleMap, I: Encode> { /// Try and append the `item` into the storage double map at the given `key`. @@ -1728,6 +1693,40 @@ where } } +/// Storage N map that is capable of [`StorageTryAppend`]. +pub trait TryAppendNMap, I: Encode> { + /// Try and append the `item` into the storage N map at the given `key`. + /// + /// This might fail if bounds are not respected. + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()>; +} + +impl TryAppendNMap for StorageNMapT +where + K: KeyGenerator, + T: FullCodec + StorageTryAppend, + I: Encode, + StorageNMapT: generator::StorageNMap, +{ + fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()> { + let bound = T::bound(); + let current = Self::decode_len(key.clone()).unwrap_or_default(); + if current < bound { + let key = Self::storage_n_map_final_key::(key); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + /// Returns the storage prefix for a specific pallet name and storage name. /// /// The storage prefix is `concat(twox_128(pallet_name), twox_128(storage_name))`. From daf60098f2a516eb5038e0d879f1334f2990e1b0 Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 03:08:01 +0200 Subject: [PATCH 6/9] extend type definition --- substrate/frame/support/src/storage/types/nmap.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/substrate/frame/support/src/storage/types/nmap.rs b/substrate/frame/support/src/storage/types/nmap.rs index 9ee012f86286..5de51cc2423a 100755 --- a/substrate/frame/support/src/storage/types/nmap.rs +++ b/substrate/frame/support/src/storage/types/nmap.rs @@ -338,6 +338,19 @@ where >::append(key, item) } + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. + pub fn try_append(key: KArg, item: EncodeLikeItem) -> Result<(), ()> + where + KArg: EncodeLikeTuple + TupleToEncodedIter + Clone, + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageTryAppend, + { + >::try_append(key, item) + } + /// Read the length of the storage value without decoding the entire value under the /// given `key1` and `key2`. /// From 84f08f4cc8aa94ed71cd551941aea6d98c7e99e8 Mon Sep 17 00:00:00 2001 From: Connor Campbell Date: Wed, 18 Sep 2024 10:19:11 +0200 Subject: [PATCH 7/9] update import --- substrate/frame/support/src/storage/types/nmap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/src/storage/types/nmap.rs b/substrate/frame/support/src/storage/types/nmap.rs index 5de51cc2423a..07bf0c14f4a3 100755 --- a/substrate/frame/support/src/storage/types/nmap.rs +++ b/substrate/frame/support/src/storage/types/nmap.rs @@ -24,7 +24,7 @@ use crate::{ EncodeLikeTuple, HasKeyPrefix, HasReversibleKeyPrefix, OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder, TupleToEncodedIter, }, - KeyGenerator, PrefixIterator, StorageAppend, StorageDecodeLength, StoragePrefixedMap, + KeyGenerator, PrefixIterator, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; From e0588cdc83737e24ef691d348ff90d48cd491c90 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 23 Sep 2024 12:37:08 -0400 Subject: [PATCH 8/9] add pr doc --- prdoc/pr_5745.prdoc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 prdoc/pr_5745.prdoc diff --git a/prdoc/pr_5745.prdoc b/prdoc/pr_5745.prdoc new file mode 100644 index 000000000000..7463589378a0 --- /dev/null +++ b/prdoc/pr_5745.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Implement `try_append` for `StorageNMap` + +doc: + - audience: Runtime Dev + description: | + This PR introduces the `try_append` api which is available on other storage map types, + but missing on `StorageNMap`. + +crates: + - name: frame-support + bump: minor From 1940faac02c3810bb0bf36934f780c10bc6ac762 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Mon, 23 Sep 2024 15:57:49 -0400 Subject: [PATCH 9/9] fmt --- substrate/frame/support/src/storage/mod.rs | 17 ++++++++++++++--- .../frame/support/src/storage/types/nmap.rs | 3 ++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index 020d833e5d12..619392563035 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -1698,7 +1698,10 @@ pub trait TryAppendNMap, I: Encode> { /// Try and append the `item` into the storage N map at the given `key`. /// /// This might fail if bounds are not respected. - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + fn try_append< + LikeK: EncodeLikeTuple + TupleToEncodedIter + Clone, + LikeI: EncodeLike, + >( key: LikeK, item: LikeI, ) -> Result<(), ()>; @@ -1711,7 +1714,10 @@ where I: Encode, StorageNMapT: generator::StorageNMap, { - fn try_append + TupleToEncodedIter + Clone, LikeI: EncodeLike>( + fn try_append< + LikeK: EncodeLikeTuple + TupleToEncodedIter + Clone, + LikeI: EncodeLike, + >( key: LikeK, item: LikeI, ) -> Result<(), ()> { @@ -2056,7 +2062,12 @@ mod test { #[crate::storage_alias] type FooQuadMap = StorageNMap< Prefix, - (NMapKey, NMapKey, NMapKey, NMapKey), + ( + NMapKey, + NMapKey, + NMapKey, + NMapKey, + ), BoundedVec>, >; diff --git a/substrate/frame/support/src/storage/types/nmap.rs b/substrate/frame/support/src/storage/types/nmap.rs index 07bf0c14f4a3..0fc22b35352d 100755 --- a/substrate/frame/support/src/storage/types/nmap.rs +++ b/substrate/frame/support/src/storage/types/nmap.rs @@ -24,7 +24,8 @@ use crate::{ EncodeLikeTuple, HasKeyPrefix, HasReversibleKeyPrefix, OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder, TupleToEncodedIter, }, - KeyGenerator, PrefixIterator, StorageAppend, StorageDecodeLength, StoragePrefixedMap, StorageTryAppend + KeyGenerator, PrefixIterator, StorageAppend, StorageDecodeLength, StoragePrefixedMap, + StorageTryAppend, }, traits::{Get, GetDefault, StorageInfo, StorageInstance}, };