diff --git a/substrate/frame/collective/Cargo.toml b/substrate/frame/collective/Cargo.toml index 8e53000352aea..748cfcc417e11 100644 --- a/substrate/frame/collective/Cargo.toml +++ b/substrate/frame/collective/Cargo.toml @@ -29,6 +29,7 @@ sp-runtime = { workspace = true } [dev-dependencies] pallet-balances = { workspace = true, default-features = false } +pallet-preimage = { workspace = true, default-features = false} [features] default = ["std"] diff --git a/substrate/frame/collective/src/lib.rs b/substrate/frame/collective/src/lib.rs index 54144836f1708..cdbba5b240185 100644 --- a/substrate/frame/collective/src/lib.rs +++ b/substrate/frame/collective/src/lib.rs @@ -60,7 +60,8 @@ use frame_support::{ ensure, traits::{ Backing, ChangeMembers, Consideration, EnsureOrigin, EnsureOriginWithArg, Get, GetBacking, - InitializeMembers, MaybeConsideration, OriginTrait, StorageVersion, + InitializeMembers, MaybeConsideration, OriginTrait, QueryPreimage, StorageVersion, + StorePreimage, }, weights::Weight, }; @@ -398,6 +399,8 @@ pub mod pallet { /// consider using a constant cost (e.g., [`crate::deposit::Constant`]) equal to the minimum /// balance under the `runtime-benchmarks` feature. type Consideration: MaybeConsideration; + + type Preimages: QueryPreimage + StorePreimage; } #[pallet::genesis_config] @@ -439,7 +442,7 @@ pub mod pallet { /// Actual proposal for a given hash, if it's current. #[pallet::storage] pub type ProposalOf, I: 'static = ()> = - StorageMap<_, Identity, T::Hash, >::Proposal, OptionQuery>; + StorageMap<_, Identity, T::Hash, (), OptionQuery>; /// Consideration cost created for publishing and storing a proposal. /// @@ -976,7 +979,8 @@ impl, I: 'static> Pallet { let index = ProposalCount::::get(); >::mutate(|i| *i += 1); - >::insert(proposal_hash, proposal); + T::Preimages::note(proposal.encode().into())?; + >::insert(proposal_hash, ()); let votes = { let end = frame_system::Pallet::::block_number() + T::MotionDuration::get(); Votes { index, threshold, ayes: vec![], nays: vec![], end } @@ -1135,12 +1139,13 @@ impl, I: 'static> Pallet { length_bound: u32, weight_bound: Weight, ) -> Result<(>::Proposal, usize), DispatchError> { - let key = ProposalOf::::hashed_key_for(hash); - // read the length of the proposal storage entry directly - let proposal_len = - storage::read(&key, &mut [0; 0], 0).ok_or(Error::::ProposalMissing)?; + let proposal_len = T::Preimages::len(hash).ok_or(Error::::ProposalMissing)?; ensure!(proposal_len <= length_bound, Error::::WrongProposalLength); - let proposal = ProposalOf::::get(hash).ok_or(Error::::ProposalMissing)?; + + let bytes = T::Preimages::fetch(hash, Some(proposal_len)) + .map_err(|_| Error::::ProposalMissing)?; + let proposal = >::Proposal::decode(&mut &bytes[..]) + .map_err(|_| Error::::ProposalMissing)?; let proposal_weight = proposal.get_dispatch_info().call_weight; ensure!(proposal_weight.all_lte(weight_bound), Error::::WrongProposalWeight); Ok((proposal, proposal_len as usize)) diff --git a/substrate/frame/collective/src/tests.rs b/substrate/frame/collective/src/tests.rs index baac228254f55..f5ec9ef8a65dc 100644 --- a/substrate/frame/collective/src/tests.rs +++ b/substrate/frame/collective/src/tests.rs @@ -25,7 +25,6 @@ use frame_support::{ fungible::{HoldConsideration, Inspect, Mutate}, ConstU32, ConstU64, StorageVersion, }, - Hashable, }; use frame_system::{EnsureRoot, EventRecord, Phase}; use sp_core::{ConstU128, H256}; @@ -34,6 +33,7 @@ use sp_runtime::{ traits::{BlakeTwo256, Convert, Zero}, BuildStorage, FixedU128, }; +use std::{borrow::Cow, cell::RefCell, collections::HashMap}; pub type Block = sp_runtime::generic::Block; pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; @@ -47,6 +47,7 @@ frame_support::construct_runtime!( CollectiveMajority: pallet_collective::, DefaultCollective: pallet_collective, Democracy: mock_democracy, + Preimage: pallet_preimage, } ); @@ -85,6 +86,14 @@ mod mock_democracy { } } +impl pallet_preimage::Config for Test { + type RuntimeEvent = RuntimeEvent; + type WeightInfo = (); + type Currency = Balances; + type ManagerOrigin = EnsureRoot; + type Consideration = (); +} + pub type MaxMembers = ConstU32<100>; type AccountId = u64; @@ -134,6 +143,7 @@ impl Config for Test { type KillOrigin = EnsureRoot; type Consideration = HoldConsideration; + type Preimages = Preimage; } type CollectiveMajorityDeposit = deposit::Linear, ProposalDepositBase>; @@ -154,6 +164,7 @@ impl Config for Test { type KillOrigin = EnsureRoot; type Consideration = HoldConsideration; + type Preimages = Preimage; } impl mock_democracy::Config for Test { type RuntimeEvent = RuntimeEvent; @@ -182,6 +193,7 @@ impl Config for Test { type KillOrigin = EnsureRoot; type Consideration = HoldConsideration; + type Preimages = Preimage; } pub struct ExtBuilder { @@ -227,6 +239,7 @@ impl ExtBuilder { pub fn build_and_execute(self, test: impl FnOnce() -> ()) { self.build().execute_with(|| { + PREIMAGES.with(|m| m.borrow_mut().clear()); test(); Collective::do_try_state().unwrap(); }) @@ -357,6 +370,7 @@ fn close_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -504,6 +518,7 @@ fn close_with_prime_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -579,6 +594,7 @@ fn close_with_voting_prime_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -672,6 +688,7 @@ fn close_with_no_prime_but_majority_works() { who: 5, amount: 2, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::CollectiveMajority(CollectiveEvent::Proposed { account: 5, proposal_index: 0, @@ -847,7 +864,7 @@ fn propose_works() { proposal_len )); assert_eq!(*Proposals::::get(), vec![hash]); - assert_eq!(ProposalOf::::get(&hash), Some(proposal)); + assert_eq!(ProposalOf::::get(&hash), Some(())); assert_eq!( Voting::::get(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![], nays: vec![], end }) @@ -863,6 +880,7 @@ fn propose_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -1058,6 +1076,7 @@ fn motions_vote_after_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -1210,6 +1229,7 @@ fn motions_approval_with_enough_votes_and_lower_voting_threshold_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, // 0 is the proposal that failed to execute? @@ -1272,6 +1292,7 @@ fn motions_approval_with_enough_votes_and_lower_voting_threshold_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 1, @@ -1350,6 +1371,7 @@ fn motions_disapproval_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -1416,6 +1438,7 @@ fn motions_approval_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, @@ -1497,15 +1520,19 @@ fn motion_with_no_votes_closes_with_disapproval() { // Events show that the close ended in a disapproval. assert_eq!( System::events()[1], + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), + ); + assert_eq!( + System::events()[2], record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0, proposal_hash: hash, threshold: 3 - })) + })), ); assert_eq!( - System::events()[2], + System::events()[3], record(RuntimeEvent::Collective(CollectiveEvent::Closed { proposal_hash: hash, yes: 0, @@ -1513,7 +1540,7 @@ fn motion_with_no_votes_closes_with_disapproval() { })) ); assert_eq!( - System::events()[3], + System::events()[4], record(RuntimeEvent::Collective(CollectiveEvent::Disapproved { proposal_hash: hash })) ); }) @@ -1581,6 +1608,7 @@ fn disapprove_proposal_works() { who: 1, amount: 0, })), + record(RuntimeEvent::Preimage(pallet_preimage::Event::Noted { hash })), record(RuntimeEvent::Collective(CollectiveEvent::Proposed { account: 1, proposal_index: 0,