From 5742b9cd3fc9d9c443cd2624e37cfd47c005a1f3 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Wed, 25 Jan 2023 15:23:35 +0000 Subject: [PATCH 1/2] refactor: use bounded storage items in democracy pallet Signed-off-by: Gregory Hill --- crates/democracy/src/lib.rs | 30 ++++++++++++------- crates/democracy/src/tests.rs | 1 + crates/democracy/src/tests/decoders.rs | 14 +++++++-- parachain/runtime/interlay/src/lib.rs | 1 + parachain/runtime/kintsugi/src/lib.rs | 1 + parachain/runtime/testnet-interlay/src/lib.rs | 1 + parachain/runtime/testnet-kintsugi/src/lib.rs | 1 + standalone/runtime/src/lib.rs | 1 + 8 files changed, 38 insertions(+), 12 deletions(-) diff --git a/crates/democracy/src/lib.rs b/crates/democracy/src/lib.rs index 2cfc7b9059..602845859a 100644 --- a/crates/democracy/src/lib.rs +++ b/crates/democracy/src/lib.rs @@ -241,6 +241,10 @@ pub mod pallet { #[pallet::constant] type MaxProposals: Get; + /// The maximum number of deposits a public proposal may have at any time. + #[pallet::constant] + type MaxDeposits: Get; + /// Unix time type UnixTime: UnixTime; @@ -258,14 +262,16 @@ pub mod pallet { /// The public proposals. Unsorted. The second item is the proposal's hash. #[pallet::storage] #[pallet::getter(fn public_props)] - pub type PublicProps = StorageValue<_, Vec<(PropIndex, T::Hash, T::AccountId)>, ValueQuery>; + pub type PublicProps = + StorageValue<_, BoundedVec<(PropIndex, T::Hash, T::AccountId), T::MaxProposals>, ValueQuery>; /// Those who have locked a deposit. /// /// TWOX-NOTE: Safe, as increasing integer keys are safe. #[pallet::storage] #[pallet::getter(fn deposit_of)] - pub type DepositOf = StorageMap<_, Twox64Concat, PropIndex, (Vec, BalanceOf)>; + pub type DepositOf = + StorageMap<_, Twox64Concat, PropIndex, (BoundedVec, BalanceOf)>; /// Map of hashes to the proposal preimage, along with who registered it and their deposit. /// The block number is the block at which it was deposited. @@ -409,8 +415,8 @@ pub mod pallet { WrongUpperBound, /// Maximum number of votes reached. MaxVotesReached, - /// Maximum number of proposals reached. - TooManyProposals, + /// Maximum number of items reached. + TooMany, /// Unable to convert value. TryIntoIntError, } @@ -458,13 +464,15 @@ pub mod pallet { let index = Self::public_prop_count(); let real_prop_count = PublicProps::::decode_len().unwrap_or(0) as u32; let max_proposals = T::MaxProposals::get(); - ensure!(real_prop_count < max_proposals, Error::::TooManyProposals); + ensure!(real_prop_count < max_proposals, Error::::TooMany); T::Currency::reserve(&who, value)?; PublicPropCount::::put(index + 1); - >::insert(index, (&[&who][..], value)); - >::append((index, proposal_hash, who)); + let depositors = BoundedVec::<_, T::MaxDeposits>::truncate_from(vec![who.clone()]); + DepositOf::::insert(index, (depositors, value)); + + PublicProps::::try_append((index, proposal_hash, who)).map_err(|_| Error::::TooMany)?; Self::deposit_event(Event::::Proposed(index, value)); Ok(()) @@ -490,10 +498,12 @@ pub mod pallet { let who = ensure_signed(origin)?; let seconds = Self::len_of_deposit_of(proposal).ok_or_else(|| Error::::ProposalMissing)?; + ensure!(seconds < T::MaxDeposits::get(), Error::::TooMany); ensure!(seconds <= seconds_upper_bound, Error::::WrongUpperBound); let mut deposit = Self::deposit_of(proposal).ok_or(Error::::ProposalMissing)?; T::Currency::reserve(&who, deposit.1)?; - deposit.0.push(who); + let ok = deposit.0.try_push(who.clone()).is_ok(); + debug_assert!(ok, "`seconds` is below static limit; `try_insert` should succeed; qed"); >::insert(proposal, deposit); Ok(()) } @@ -852,7 +862,7 @@ impl Pallet { delay: T::BlockNumber, voting_period: T::BlockNumber, ) -> DispatchResult { - let mut public_props = >::get(); + let mut public_props = Self::public_props(); let (winner_index, _) = public_props .iter() .enumerate() @@ -994,7 +1004,7 @@ impl Pallet { for d in &depositors { T::Currency::unreserve(d, deposit); } - Self::deposit_event(Event::::Tabled(prop_index, deposit, depositors)); + Self::deposit_event(Event::::Tabled(prop_index, deposit, depositors.to_vec())); Self::inject_referendum( now.saturating_add(T::VotingPeriod::get()), proposal, diff --git a/crates/democracy/src/tests.rs b/crates/democracy/src/tests.rs index add5154247..a584d217b8 100644 --- a/crates/democracy/src/tests.rs +++ b/crates/democracy/src/tests.rs @@ -175,6 +175,7 @@ impl Config for Test { type PalletsOrigin = OriginCaller; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<1000>; type UnixTime = Timestamp; type Moment = u64; type LaunchOffsetMillis = LaunchOffsetMillis; diff --git a/crates/democracy/src/tests/decoders.rs b/crates/democracy/src/tests/decoders.rs index c591a90384..0944b22acd 100644 --- a/crates/democracy/src/tests/decoders.rs +++ b/crates/democracy/src/tests/decoders.rs @@ -1,7 +1,10 @@ //! The for various partial storage decoders use super::*; -use frame_support::storage::{migration, unhashed}; +use frame_support::{ + storage::{migration, unhashed}, + BoundedVec, +}; #[test] fn test_decode_compact_u32_at() { @@ -25,7 +28,14 @@ fn test_decode_compact_u32_at() { fn len_of_deposit_of() { new_test_ext().execute_with(|| { for l in vec![0, 1, 200, 1000] { - let value: (Vec, u64) = ((0..l).map(|_| Default::default()).collect(), 3u64); + let value: (BoundedVec, u64) = ( + (0..l) + .map(|_| Default::default()) + .collect::>() + .try_into() + .unwrap(), + 3u64, + ); DepositOf::::insert(2, value); assert_eq!(Democracy::len_of_deposit_of(2), Some(l)); } diff --git a/parachain/runtime/interlay/src/lib.rs b/parachain/runtime/interlay/src/lib.rs index 5c7ec0c9f3..b2b9f9b136 100644 --- a/parachain/runtime/interlay/src/lib.rs +++ b/parachain/runtime/interlay/src/lib.rs @@ -496,6 +496,7 @@ impl democracy::Config for Runtime { type MaxVotes = MaxVotes; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<100>; type UnixTime = Timestamp; type Moment = Moment; type LaunchOffsetMillis = LaunchOffsetMillis; diff --git a/parachain/runtime/kintsugi/src/lib.rs b/parachain/runtime/kintsugi/src/lib.rs index 01011b4c29..abf1cda8e9 100644 --- a/parachain/runtime/kintsugi/src/lib.rs +++ b/parachain/runtime/kintsugi/src/lib.rs @@ -494,6 +494,7 @@ impl democracy::Config for Runtime { type MaxVotes = MaxVotes; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<100>; type UnixTime = Timestamp; type Moment = Moment; type LaunchOffsetMillis = LaunchOffsetMillis; diff --git a/parachain/runtime/testnet-interlay/src/lib.rs b/parachain/runtime/testnet-interlay/src/lib.rs index 5c1fefe81b..d89f0468f7 100644 --- a/parachain/runtime/testnet-interlay/src/lib.rs +++ b/parachain/runtime/testnet-interlay/src/lib.rs @@ -465,6 +465,7 @@ impl democracy::Config for Runtime { type MaxVotes = MaxVotes; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<100>; type UnixTime = Timestamp; type Moment = Moment; type LaunchOffsetMillis = LaunchOffsetMillis; diff --git a/parachain/runtime/testnet-kintsugi/src/lib.rs b/parachain/runtime/testnet-kintsugi/src/lib.rs index c2be4cf968..3c3eefbab7 100644 --- a/parachain/runtime/testnet-kintsugi/src/lib.rs +++ b/parachain/runtime/testnet-kintsugi/src/lib.rs @@ -465,6 +465,7 @@ impl democracy::Config for Runtime { type MaxVotes = MaxVotes; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<100>; type UnixTime = Timestamp; type Moment = Moment; type LaunchOffsetMillis = LaunchOffsetMillis; diff --git a/standalone/runtime/src/lib.rs b/standalone/runtime/src/lib.rs index 8e2f3594fa..8a7e62ecaa 100644 --- a/standalone/runtime/src/lib.rs +++ b/standalone/runtime/src/lib.rs @@ -461,6 +461,7 @@ impl democracy::Config for Runtime { type MaxVotes = MaxVotes; type WeightInfo = (); type MaxProposals = MaxProposals; + type MaxDeposits = ConstU32<100>; type UnixTime = Timestamp; type Moment = Moment; type LaunchOffsetMillis = LaunchOffsetMillis; From ab38f2212401d1d1e314c90b780f526f247b12e5 Mon Sep 17 00:00:00 2001 From: Gregory Hill Date: Tue, 7 Feb 2023 08:13:18 +0000 Subject: [PATCH 2/2] chore: remove debug_assert in favour of explicit error Signed-off-by: Gregory Hill --- crates/democracy/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/democracy/src/lib.rs b/crates/democracy/src/lib.rs index 602845859a..361a5c96d0 100644 --- a/crates/democracy/src/lib.rs +++ b/crates/democracy/src/lib.rs @@ -502,8 +502,7 @@ pub mod pallet { ensure!(seconds <= seconds_upper_bound, Error::::WrongUpperBound); let mut deposit = Self::deposit_of(proposal).ok_or(Error::::ProposalMissing)?; T::Currency::reserve(&who, deposit.1)?; - let ok = deposit.0.try_push(who.clone()).is_ok(); - debug_assert!(ok, "`seconds` is below static limit; `try_insert` should succeed; qed"); + deposit.0.try_push(who.clone()).map_err(|_| Error::::TooMany)?; >::insert(proposal, deposit); Ok(()) }