diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 06b1b42b10..5a13d30be1 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -3609,6 +3609,7 @@ dependencies = [ "pallet-membership", "pallet-offences", "pallet-offences-benchmarking", + "pallet-proxy", "pallet-randomness-collective-flip", "pallet-recovery", "pallet-scheduler", @@ -4399,6 +4400,22 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-proxy" +version = "2.0.0-rc2" +dependencies = [ + "frame-benchmarking", + "frame-support", + "frame-system", + "pallet-balances", + "parity-scale-codec", + "serde", + "sp-core", + "sp-io", + "sp-runtime", + "sp-std", +] + [[package]] name = "pallet-randomness-collective-flip" version = "2.0.0-rc2" diff --git a/substrate/Cargo.toml b/substrate/Cargo.toml index 8fbe1cf0d8..8dc57b607f 100644 --- a/substrate/Cargo.toml +++ b/substrate/Cargo.toml @@ -86,6 +86,7 @@ members = [ "frame/metadata", "frame/nicks", "frame/offences", + "frame/proxy", "frame/randomness-collective-flip", "frame/recovery", "frame/scheduler", diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index 1bf61c046d..b451ac109e 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -61,6 +61,7 @@ pallet-identity = { version = "2.0.0-rc2", default-features = false, path = "../ pallet-membership = { version = "2.0.0-rc2", default-features = false, path = "../../../frame/membership" } pallet-offences = { version = "2.0.0-rc2", default-features = false, path = "../../../frame/offences" } pallet-offences-benchmarking = { version = "2.0.0-rc2", path = "../../../frame/offences/benchmarking", default-features = false, optional = true } +pallet-proxy = { version = "2.0.0-rc2", default-features = false, path = "../../../frame/proxy" } pallet-randomness-collective-flip = { version = "2.0.0-rc2", default-features = false, path = "../../../frame/randomness-collective-flip" } pallet-recovery = { version = "2.0.0-rc2", default-features = false, path = "../../../frame/recovery" } pallet-session = { version = "2.0.0-rc2", features = ["historical"], path = "../../../frame/session", default-features = false } @@ -111,6 +112,7 @@ std = [ "node-primitives/std", "sp-offchain/std", "pallet-offences/std", + "pallet-proxy/std", "sp-core/std", "pallet-randomness-collective-flip/std", "sp-std/std", @@ -149,6 +151,7 @@ runtime-benchmarks = [ "pallet-elections-phragmen/runtime-benchmarks", "pallet-identity/runtime-benchmarks", "pallet-im-online/runtime-benchmarks", + "pallet-proxy/runtime-benchmarks", "pallet-scheduler/runtime-benchmarks", "pallet-society/runtime-benchmarks", "pallet-staking/runtime-benchmarks", diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 94cdf8bed8..7a036ddf5f 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -24,13 +24,14 @@ use sp_std::prelude::*; use frame_support::{ - construct_runtime, parameter_types, debug, + construct_runtime, parameter_types, debug, RuntimeDebug, weights::{ Weight, IdentityFee, constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, }, traits::{Currency, Imbalance, KeyOwnerProofSystem, OnUnbalanced, Randomness, LockIdentifier}, }; +use codec::{Encode, Decode}; use sp_core::{ crypto::KeyTypeId, u32_trait::{_1, _2, _3, _4}, @@ -60,7 +61,6 @@ use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; use pallet_contracts_rpc_runtime_api::ContractExecResult; use pallet_session::{historical as pallet_session_historical}; use sp_inherents::{InherentData, CheckInherentsResult}; -use codec::Encode; use static_assertions::const_assert; #[cfg(any(feature = "std", test))] @@ -79,6 +79,7 @@ use impls::{CurrencyToVoteHandler, Author, TargetedFeeAdjustment}; /// Constant values used within the runtime. pub mod constants; use constants::{time::*, currency::*}; +use frame_support::traits::InstanceFilter; // Make the WASM binary available. #[cfg(feature = "std")] @@ -168,11 +169,13 @@ impl frame_system::Trait for Runtime { type OnKilledAccount = (); } +const fn deposit(items: u32, bytes: u32) -> Balance { items as Balance * 15 * CENTS + (bytes as Balance) * 6 * CENTS } + parameter_types! { - // One storage item; value is size 4+4+16+32 bytes = 56 bytes. - pub const MultisigDepositBase: Balance = 30 * CENTS; + // One storage item; key size is 32; value is size 4+4+16+32 bytes = 56 bytes. + pub const MultisigDepositBase: Balance = deposit(1, 88); // Additional storage item size of 32 bytes. - pub const MultisigDepositFactor: Balance = 5 * CENTS; + pub const MultisigDepositFactor: Balance = deposit(0, 32); pub const MaxSignatories: u16 = 100; } @@ -186,6 +189,52 @@ impl pallet_utility::Trait for Runtime { type IsCallable = (); } +parameter_types! { + // One storage item; key size 32, value size 8; . + pub const ProxyDepositBase: Balance = deposit(1, 8); + // Additional storage item size of 33 bytes. + pub const ProxyDepositFactor: Balance = deposit(0, 33); + pub const MaxProxies: u16 = 32; +} + +/// The type used to represent the kinds of proxying allowed. +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug)] +pub enum ProxyType { + Any, + NonTransfer, + Governance, + Staking, +} +impl Default for ProxyType { fn default() -> Self { Self::Any } } +impl InstanceFilter for ProxyType { + fn filter(&self, c: &Call) -> bool { + match self { + ProxyType::Any => true, + ProxyType::NonTransfer => !matches!(c, + Call::Balances(..) | Call::Utility(..) + | Call::Vesting(pallet_vesting::Call::vested_transfer(..)) + | Call::Indices(pallet_indices::Call::transfer(..)) + ), + ProxyType::Governance => matches!(c, + Call::Democracy(..) | Call::Council(..) | Call::Society(..) + | Call::TechnicalCommittee(..) | Call::Elections(..) | Call::Treasury(..) + ), + ProxyType::Staking => matches!(c, Call::Staking(..)), + } + } +} + +impl pallet_proxy::Trait for Runtime { + type Event = Event; + type Call = Call; + type Currency = Balances; + type IsCallable = (); + type ProxyType = ProxyType; + type ProxyDepositBase = ProxyDepositBase; + type ProxyDepositFactor = ProxyDepositFactor; + type MaxProxies = MaxProxies; +} + parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * MaximumBlockWeight::get(); } @@ -759,6 +808,7 @@ construct_runtime!( Recovery: pallet_recovery::{Module, Call, Storage, Event}, Vesting: pallet_vesting::{Module, Call, Storage, Event, Config}, Scheduler: pallet_scheduler::{Module, Call, Storage, Event}, + Proxy: pallet_proxy::{Module, Call, Storage, Event}, } ); @@ -1009,6 +1059,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, b"identity", Identity); add_benchmark!(params, batches, b"im-online", ImOnline); add_benchmark!(params, batches, b"offences", OffencesBench::); + add_benchmark!(params, batches, b"proxy", Proxy); add_benchmark!(params, batches, b"scheduler", Scheduler); add_benchmark!(params, batches, b"session", SessionBench::); add_benchmark!(params, batches, b"staking", Staking); diff --git a/substrate/client/network/test/src/block_import.rs b/substrate/client/network/test/src/block_import.rs index 8636cfbc21..46a395700c 100644 --- a/substrate/client/network/test/src/block_import.rs +++ b/substrate/client/network/test/src/block_import.rs @@ -57,8 +57,9 @@ fn import_single_good_block_works() { match import_single_block( &mut substrate_test_runtime_client::new(), - BlockOrigin::File, block, - &mut PassThroughVerifier(true), + BlockOrigin::File, + block, + &mut PassThroughVerifier(true) ) { Ok(BlockImportResult::ImportedUnknown(ref num, ref aux, ref org)) if *num == number && *aux == expected_aux && *org == Some(peer_id) => {} @@ -70,7 +71,8 @@ fn import_single_good_block_works() { fn import_single_good_known_block_is_ignored() { let (mut client, _hash, number, _, block) = prepare_good_block(); match import_single_block( - &mut client, BlockOrigin::File, + &mut client, + BlockOrigin::File, block, &mut PassThroughVerifier(true) ) { @@ -85,8 +87,9 @@ fn import_single_good_block_without_header_fails() { block.header = None; match import_single_block( &mut substrate_test_runtime_client::new(), - BlockOrigin::File, block, - &mut PassThroughVerifier(true), + BlockOrigin::File, + block, + &mut PassThroughVerifier(true) ) { Err(BlockImportError::IncompleteHeader(ref org)) if *org == Some(peer_id) => {} _ => panic!() diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index 225f7f662e..ea7ec92147 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -1257,12 +1257,9 @@ where ) { if amount.is_zero() || reasons.is_none() { return } let mut new_lock = Some(BalanceLock { id, amount, reasons: reasons.into() }); - let mut locks = Self::locks(who).into_iter().filter_map(|l| - if l.id == id { - new_lock.take() - } else { - Some(l) - }).collect::>(); + let mut locks = Self::locks(who).into_iter() + .filter_map(|l| if l.id == id { new_lock.take() } else { Some(l) }) + .collect::>(); if let Some(lock) = new_lock { locks.push(lock) } diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index 9fa619a994..7839618760 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -97,16 +97,6 @@ fn account_vote(b: BalanceOf) -> AccountVote> { } } -fn open_activate_proxy(u: u32) -> Result<(T::AccountId, T::AccountId), &'static str> { - let caller = funded_account::("caller", u); - let voter = funded_account::("voter", u); - - Democracy::::open_proxy(RawOrigin::Signed(caller.clone()).into(), voter.clone())?; - Democracy::::activate_proxy(RawOrigin::Signed(voter.clone()).into(), caller.clone())?; - - Ok((caller, voter)) -} - benchmarks! { _ { } @@ -215,70 +205,6 @@ benchmarks! { assert_eq!(tally.nays, 1000.into(), "changed vote was not recorded"); } - // Basically copy paste of `vote_new` - proxy_vote_new { - let r in 1 .. MAX_REFERENDUMS; - - let (caller, voter) = open_activate_proxy::(0)?; - let account_vote = account_vote::(100.into()); - - // Populate existing direct votes for the voter, they can vote on their own behalf - for i in 0 .. r { - let ref_idx = add_referendum::(i)?; - Democracy::::vote(RawOrigin::Signed(voter.clone()).into(), ref_idx, account_vote.clone())?; - } - - let referendum_index = add_referendum::(r)?; - - }: proxy_vote(RawOrigin::Signed(caller), referendum_index, account_vote) - verify { - let votes = match VotingOf::::get(&voter) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), (r + 1) as usize, "Vote was not recorded."); - } - - // Basically copy paste of `vote_existing` - proxy_vote_existing { - let r in 1 .. MAX_REFERENDUMS; - - let (caller, voter) = open_activate_proxy::(0)?; - let account_vote = account_vote::(100.into()); - - // We need to create existing direct votes - for i in 0 ..=r { - let ref_idx = add_referendum::(i)?; - Democracy::::vote(RawOrigin::Signed(voter.clone()).into(), ref_idx, account_vote.clone())?; - } - let votes = match VotingOf::::get(&voter) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), (r + 1) as usize, "Votes were not recorded."); - - // Change vote from aye to nay - let nay = Vote { aye: false, conviction: Conviction::Locked1x }; - let new_vote = AccountVote::Standard { vote: nay, balance: 1000.into() }; - let referendum_index = Democracy::::referendum_count() - 1; - - // This tests when a user changes a vote - }: proxy_vote(RawOrigin::Signed(caller.clone()), referendum_index, new_vote) - verify { - let votes = match VotingOf::::get(&voter) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), (r + 1) as usize, "Vote was incorrectly added"); - let referendum_info = Democracy::::referendum_info(referendum_index) - .ok_or("referendum doesn't exist")?; - let tally = match referendum_info { - ReferendumInfo::Ongoing(r) => r.tally, - _ => return Err("referendum not ongoing"), - }; - assert_eq!(tally.nays, 1000.into(), "changed vote was not recorded"); - } - emergency_cancel { let r in 1 .. MAX_REFERENDUMS; let origin = T::CancellationOrigin::successful_origin(); @@ -505,33 +431,6 @@ benchmarks! { } } - activate_proxy { - let u in 1 .. MAX_USERS; - - let caller: T::AccountId = funded_account::("caller", u); - let proxy: T::AccountId = funded_account::("proxy", u); - Democracy::::open_proxy(RawOrigin::Signed(proxy.clone()).into(), caller.clone())?; - }: _(RawOrigin::Signed(caller.clone()), proxy.clone()) - verify { - assert_eq!(Democracy::::proxy(proxy), Some(ProxyState::Active(caller))); - } - - close_proxy { - let u in 1 .. MAX_USERS; - let (caller, _) = open_activate_proxy::(u)?; - }: _(RawOrigin::Signed(caller.clone())) - verify { - assert_eq!(Democracy::::proxy(caller), None); - } - - deactivate_proxy { - let u in 1 .. MAX_USERS; - let (caller, voter) = open_activate_proxy::(u)?; - }: _(RawOrigin::Signed(voter.clone()), caller.clone()) - verify { - assert_eq!(Democracy::::proxy(caller), Some(ProxyState::Open(voter))); - } - delegate { let r in 1 .. MAX_REFERENDUMS; @@ -760,14 +659,6 @@ benchmarks! { assert_eq!(voting.locked_balance(), base_balance); } - open_proxy { - let u in 1 .. MAX_USERS; - - let caller: T::AccountId = funded_account::("caller", u); - let proxy: T::AccountId = funded_account::("proxy", u); - - }: _(RawOrigin::Signed(proxy), caller) - remove_vote { let r in 1 .. MAX_REFERENDUMS; @@ -831,131 +722,6 @@ benchmarks! { assert_eq!(votes.len(), (r - 1) as usize, "Vote was not removed"); } - // This is a copy of delegate benchmark, but with `open_activate_proxy` - proxy_delegate { - let r in 1 .. MAX_REFERENDUMS; - - let initial_balance: BalanceOf = 100.into(); - let delegated_balance: BalanceOf = 1000.into(); - - let (caller, voter) = open_activate_proxy::(0)?; - - // Voter will initially delegate to `old_delegate` - let old_delegate: T::AccountId = funded_account::("old_delegate", r); - Democracy::::delegate( - RawOrigin::Signed(voter.clone()).into(), - old_delegate.clone(), - Conviction::Locked1x, - delegated_balance, - )?; - let (target, balance) = match VotingOf::::get(&voter) { - Voting::Delegating { target, balance, .. } => (target, balance), - _ => return Err("Votes are not direct"), - }; - assert_eq!(target, old_delegate, "delegation target didn't work"); - assert_eq!(balance, delegated_balance, "delegation balance didn't work"); - // Voter will now switch to `new_delegate` - let new_delegate: T::AccountId = funded_account::("new_delegate", r); - let account_vote = account_vote::(initial_balance); - // We need to create existing direct votes for the `new_delegate` - for i in 0..r { - let ref_idx = add_referendum::(i)?; - Democracy::::vote(RawOrigin::Signed(new_delegate.clone()).into(), ref_idx, account_vote.clone())?; - } - let votes = match VotingOf::::get(&new_delegate) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), r as usize, "Votes were not recorded."); - }: _(RawOrigin::Signed(caller.clone()), new_delegate.clone(), Conviction::Locked1x, delegated_balance) - verify { - let (target, balance) = match VotingOf::::get(&voter) { - Voting::Delegating { target, balance, .. } => (target, balance), - _ => return Err("Votes are not direct"), - }; - assert_eq!(target, new_delegate, "delegation target didn't work"); - assert_eq!(balance, delegated_balance, "delegation balance didn't work"); - let delegations = match VotingOf::::get(&new_delegate) { - Voting::Direct { delegations, .. } => delegations, - _ => return Err("Votes are not direct"), - }; - assert_eq!(delegations.capital, delegated_balance, "delegation was not recorded."); - } - - // This is a copy of undelegate benchmark, but with `open_activate_proxy` - proxy_undelegate { - let r in 1 .. MAX_REFERENDUMS; - - let initial_balance: BalanceOf = 100.into(); - let delegated_balance: BalanceOf = 1000.into(); - - let (caller, voter) = open_activate_proxy::(0)?; - // Caller will delegate - let the_delegate: T::AccountId = funded_account::("delegate", r); - Democracy::::delegate( - RawOrigin::Signed(voter.clone()).into(), - the_delegate.clone(), - Conviction::Locked1x, - delegated_balance, - )?; - let (target, balance) = match VotingOf::::get(&voter) { - Voting::Delegating { target, balance, .. } => (target, balance), - _ => return Err("Votes are not direct"), - }; - assert_eq!(target, the_delegate, "delegation target didn't work"); - assert_eq!(balance, delegated_balance, "delegation balance didn't work"); - // We need to create votes direct votes for the `delegate` - let account_vote = account_vote::(initial_balance); - for i in 0..r { - let ref_idx = add_referendum::(i)?; - Democracy::::vote( - RawOrigin::Signed(the_delegate.clone()).into(), - ref_idx, - account_vote.clone() - )?; - } - let votes = match VotingOf::::get(&the_delegate) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), r as usize, "Votes were not recorded."); - }: _(RawOrigin::Signed(caller.clone())) - verify { - // Voting should now be direct - match VotingOf::::get(&voter) { - Voting::Direct { .. } => (), - _ => return Err("undelegation failed"), - } - } - - proxy_remove_vote { - let r in 1 .. MAX_REFERENDUMS; - - let (caller, voter) = open_activate_proxy::(0)?; - let account_vote = account_vote::(100.into()); - - for i in 0 .. r { - let ref_idx = add_referendum::(i)?; - Democracy::::vote(RawOrigin::Signed(voter.clone()).into(), ref_idx, account_vote.clone())?; - } - - let votes = match VotingOf::::get(&voter) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), r as usize, "Votes not created"); - - let referendum_index = r - 1; - - }: _(RawOrigin::Signed(caller.clone()), referendum_index) - verify { - let votes = match VotingOf::::get(&voter) { - Voting::Direct { votes, .. } => votes, - _ => return Err("Votes are not direct"), - }; - assert_eq!(votes.len(), (r - 1) as usize, "Vote was not removed"); - } - enact_proposal_execute { // Num of bytes in encoded proposal let b in 0 .. MAX_BYTES; @@ -1012,8 +778,6 @@ mod tests { assert_ok!(test_benchmark_second::()); assert_ok!(test_benchmark_vote_new::()); assert_ok!(test_benchmark_vote_existing::()); - assert_ok!(test_benchmark_proxy_vote_new::()); - assert_ok!(test_benchmark_proxy_vote_existing::()); assert_ok!(test_benchmark_emergency_cancel::()); assert_ok!(test_benchmark_external_propose::()); assert_ok!(test_benchmark_external_propose_majority::()); @@ -1025,10 +789,6 @@ mod tests { assert_ok!(test_benchmark_on_initialize_external::()); assert_ok!(test_benchmark_on_initialize_public::()); assert_ok!(test_benchmark_on_initialize_no_launch_no_maturing::()); - assert_ok!(test_benchmark_open_proxy::()); - assert_ok!(test_benchmark_activate_proxy::()); - assert_ok!(test_benchmark_close_proxy::()); - assert_ok!(test_benchmark_deactivate_proxy::()); assert_ok!(test_benchmark_delegate::()); assert_ok!(test_benchmark_undelegate::()); assert_ok!(test_benchmark_clear_public_proposals::()); @@ -1039,9 +799,6 @@ mod tests { assert_ok!(test_benchmark_unlock_set::()); assert_ok!(test_benchmark_remove_vote::()); assert_ok!(test_benchmark_remove_other_vote::()); - assert_ok!(test_benchmark_proxy_delegate::()); - assert_ok!(test_benchmark_proxy_undelegate::()); - assert_ok!(test_benchmark_proxy_remove_vote::()); assert_ok!(test_benchmark_enact_proposal_execute::()); assert_ok!(test_benchmark_enact_proposal_slash::()); }); diff --git a/substrate/frame/democracy/src/lib.rs b/substrate/frame/democracy/src/lib.rs index 580e80cce0..841281c125 100644 --- a/substrate/frame/democracy/src/lib.rs +++ b/substrate/frame/democracy/src/lib.rs @@ -52,8 +52,6 @@ //! account or an external origin) suggests that the system adopt. //! - **Referendum:** A proposal that is in the process of being voted on for //! either acceptance or rejection as a change to the system. -//! - **Proxy:** An account that has full voting power on behalf of a separate "Stash" account -//! that holds the funds. //! - **Delegation:** The act of granting your voting power to the decisions of another account for //! up to a certain conviction. //! @@ -93,18 +91,6 @@ //! - `reap_vote` - Remove some account's expired votes. //! - `unlock` - Redetermine the account's balance lock, potentially making tokens available. //! -//! Proxy administration: -//! - `activate_proxy` - Activates a proxy that is already open to the sender. -//! - `close_proxy` - Clears the proxy status, called by the proxy. -//! - `deactivate_proxy` - Deactivates a proxy back to the open status, called by the stash. -//! - `open_proxy` - Opens a proxy account on behalf of the sender. -//! -//! Proxy actions: -//! - `proxy_vote` - Votes in a referendum on behalf of a stash account. -//! - `proxy_unvote` - Cancel a previous vote, done on behalf of the voter by a proxy. -//! - `proxy_delegate` - Delegate voting power, done on behalf of the voter by a proxy. -//! - `proxy_undelegate` - Stop delegating voting power, done on behalf of the voter by a proxy. -//! //! Preimage actions: //! - `note_preimage` - Registers the preimage for an upcoming proposal, requires //! a deposit that is returned once the proposal is enacted. @@ -191,7 +177,7 @@ mod types; pub use vote_threshold::{Approved, VoteThreshold}; pub use vote::{Vote, AccountVote, Voting}; pub use conviction::Conviction; -pub use types::{ReferendumInfo, ReferendumStatus, ProxyState, Tally, UnvoteScope, Delegations}; +pub use types::{ReferendumInfo, ReferendumStatus, Tally, UnvoteScope, Delegations}; #[cfg(test)] mod tests; @@ -376,14 +362,6 @@ decl_storage! { /// TWOX-NOTE: SAFE as `AccountId`s are crypto hashes anyway. pub VotingOf: map hasher(twox_64_concat) T::AccountId => Voting, T::AccountId, T::BlockNumber>; - /// Who is able to vote for whom. Value is the fund-holding account, key is the - /// vote-transaction-sending account. - /// - /// TWOX-NOTE: OK ― `AccountId` is a secure hash. - // TODO: Refactor proxy into its own pallet. - // https://github.com/paritytech/substrate/issues/5322 - pub Proxy get(fn proxy): map hasher(twox_64_concat) T::AccountId => Option>; - /// Accounts for which there are locks in action which may be removed at some point in the /// future. The value is the block number at which the lock expires and may be removed. /// @@ -467,8 +445,6 @@ decl_error! { ValueLow, /// Proposal does not exist ProposalMissing, - /// Not a proxy - NotProxy, /// Unknown index BadIndex, /// Cannot cancel the same proposal twice @@ -485,10 +461,6 @@ decl_error! { NoProposal, /// Identity may not veto a proposal twice AlreadyVetoed, - /// Already a proxy - AlreadyProxy, - /// Wrong proxy - WrongProxy, /// Not delegated NotDelegated, /// Preimage already noted @@ -511,12 +483,6 @@ decl_error! { NotLocked, /// The lock on the account to be unlocked has not yet expired. NotExpired, - /// A proxy-pairing was attempted to an account that was not open. - NotOpen, - /// A proxy-pairing was attempted to an account that was open to another account. - WrongOpen, - /// A proxy-de-pairing was attempted to an account that was not active. - NotActive, /// The given account did not vote on the referendum. NotVoter, /// The actor has no permission to conduct the action. @@ -575,27 +541,6 @@ mod weight_for { .saturating_add(votes.saturating_mul(8_000_000)) } - /// Calculate the weight for `proxy_delegate`. - /// same as `delegate with additional: - /// - Db reads: `Proxy`, `proxy account` - /// - Db writes: `proxy account` - /// - Base Weight: 68.61 + 8.039 * R µs - pub fn proxy_delegate(votes: Weight) -> Weight { - T::DbWeight::get().reads_writes(votes.saturating_add(5), votes.saturating_add(4)) - .saturating_add(69_000_000) - .saturating_add(votes.saturating_mul(8_000_000)) - } - - /// Calculate the weight for `proxy_undelegate`. - /// same as `undelegate with additional: - /// Db reads: `Proxy` - /// Base Weight: 39 + 7.958 * R µs - pub fn proxy_undelegate(votes: Weight) -> Weight { - T::DbWeight::get().reads_writes(votes.saturating_add(3), votes.saturating_add(2)) - .saturating_add(40_000_000) - .saturating_add(votes.saturating_mul(8_000_000)) - } - /// Calculate the weight for `note_preimage`. /// # /// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit). @@ -766,34 +711,6 @@ decl_module! { Self::try_vote(&who, ref_index, vote) } - /// Vote in a referendum on behalf of a stash. If `vote.is_aye()`, the vote is to enact - /// the proposal; otherwise it is a vote to keep the status quo. - /// - /// The dispatch origin of this call must be _Signed_. - /// - /// - `ref_index`: The index of the referendum to proxy vote for. - /// - `vote`: The vote configuration. - /// - /// # - /// - Complexity: `O(R)` where R is the number of referendums the proxy has voted on. - /// weight is charged as if maximum votes. - /// - Db reads: `ReferendumInfoOf`, `VotingOf`, `balances locks`, `Proxy`, `proxy account` - /// - Db writes: `ReferendumInfoOf`, `VotingOf`, `balances locks` - /// ------------ - /// - Base Weight: - /// - Proxy Vote New: 54.35 + .344 * R µs - /// - Proxy Vote Existing: 54.35 + .35 * R µs - /// # - #[weight = 55_000_000 + 350_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(5, 3)] - fn proxy_vote(origin, - #[compact] ref_index: ReferendumIndex, - vote: AccountVote>, - ) -> DispatchResult { - let who = ensure_signed(origin)?; - let voter = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - Self::try_vote(&voter, ref_index, vote) - } - /// Schedule an emergency cancellation of a referendum. Cannot happen twice to the same /// referendum. /// @@ -1028,86 +945,6 @@ decl_module! { }) } - /// Specify a proxy that is already open to us. Called by the stash. - /// - /// NOTE: Used to be called `set_proxy`. - /// - /// The dispatch origin of this call must be _Signed_. - /// - /// - `proxy`: The account that will be activated as proxy. - /// - /// # - /// - Complexity: `O(1)` - /// - Db reads: `Proxy` - /// - Db writes: `Proxy` - /// - Base Weight: 7.972 µs - /// # - #[weight = 8_000_000 + T::DbWeight::get().reads_writes(1, 1)] - fn activate_proxy(origin, proxy: T::AccountId) { - let who = ensure_signed(origin)?; - Proxy::::try_mutate(&proxy, |a| match a.take() { - None => Err(Error::::NotOpen), - Some(ProxyState::Active(_)) => Err(Error::::AlreadyProxy), - Some(ProxyState::Open(x)) if &x == &who => { - *a = Some(ProxyState::Active(who)); - Ok(()) - } - Some(ProxyState::Open(_)) => Err(Error::::WrongOpen), - })?; - } - - /// Clear the proxy. Called by the proxy. - /// - /// NOTE: Used to be called `resign_proxy`. - /// - /// The dispatch origin of this call must be _Signed_. - /// - /// # - /// - Complexity: `O(1)` - /// - Db reads: `Proxy`, `sender account` - /// - Db writes: `Proxy`, `sender account` - /// - Base Weight: 15.41 µs - /// # - #[weight = 16_000_000 + T::DbWeight::get().reads_writes(1, 1)] - fn close_proxy(origin) { - let who = ensure_signed(origin)?; - Proxy::::mutate(&who, |a| { - if a.is_some() { - system::Module::::dec_ref(&who); - } - *a = None; - }); - } - - /// Deactivate the proxy, but leave open to this account. Called by the stash. - /// - /// The proxy must already be active. - /// - /// NOTE: Used to be called `remove_proxy`. - /// - /// The dispatch origin of this call must be _Signed_. - /// - /// - `proxy`: The account that will be deactivated as proxy. - /// - /// # - /// - Complexity: `O(1)` - /// - Db reads: `Proxy` - /// - Db writes: `Proxy` - /// - Base Weight: 8.03 µs - /// # - #[weight = 8_000_000 + T::DbWeight::get().reads_writes(1, 1)] - fn deactivate_proxy(origin, proxy: T::AccountId) { - let who = ensure_signed(origin)?; - Proxy::::try_mutate(&proxy, |a| match a.take() { - None | Some(ProxyState::Open(_)) => Err(Error::::NotActive), - Some(ProxyState::Active(x)) if &x == &who => { - *a = Some(ProxyState::Open(who)); - Ok(()) - } - Some(ProxyState::Active(_)) => Err(Error::::WrongProxy), - })?; - } - /// Delegate the voting power (with some given conviction) of the sending account. /// /// The balance delegated is locked for as long as it's delegated, and thereafter for the @@ -1314,33 +1151,6 @@ decl_module! { Self::update_lock(&target); } - /// Become a proxy. - /// - /// This must be called prior to a later `activate_proxy`. - /// - /// Origin must be a Signed. - /// - /// - `target`: The account whose votes will later be proxied. - /// - /// `close_proxy` must be called before the account can be destroyed. - /// - /// # - /// - Complexity: O(1) - /// - Db reads: `Proxy`, `proxy account` - /// - Db writes: `Proxy`, `proxy account` - /// - Base Weight: 14.86 µs - /// # - #[weight = 15_000_000 + T::DbWeight::get().reads_writes(2, 2)] - fn open_proxy(origin, target: T::AccountId) { - let who = ensure_signed(origin)?; - Proxy::::mutate(&who, |a| { - if a.is_none() { - system::Module::::inc_ref(&who); - } - *a = Some(ProxyState::Open(target)); - }); - } - /// Remove a vote for a referendum. /// /// If: @@ -1407,94 +1217,6 @@ decl_module! { Ok(()) } - /// Delegate the voting power (with some given conviction) of a proxied account. - /// - /// The balance delegated is locked for as long as it's delegated, and thereafter for the - /// time appropriate for the conviction's lock period. - /// - /// The dispatch origin of this call must be _Signed_, and the signing account must have - /// been set as the proxy account for `target`. - /// - /// - `target`: The account whole voting power shall be delegated and whose balance locked. - /// This account must either: - /// - be delegating already; or - /// - have no voting activity (if there is, then it will need to be removed/consolidated - /// through `reap_vote` or `unvote`). - /// - `to`: The account whose voting the `target` account's voting power will follow. - /// - `conviction`: The conviction that will be attached to the delegated votes. When the - /// account is undelegated, the funds will be locked for the corresponding period. - /// - `balance`: The amount of the account's balance to be used in delegating. This must - /// not be more than the account's current balance. - /// - /// Emits `Delegated`. - /// - /// # - /// same as `delegate with additional: - /// - Db reads: `Proxy`, `proxy account` - /// - Db writes: `proxy account` - /// - Base Weight: 68.61 + 8.039 * R µs - /// # - #[weight = weight_for::proxy_delegate::(T::MaxVotes::get().into())] - pub fn proxy_delegate(origin, - to: T::AccountId, - conviction: Conviction, - balance: BalanceOf, - ) -> DispatchResultWithPostInfo { - let who = ensure_signed(origin)?; - let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - let votes = Self::try_delegate(target, to, conviction, balance)?; - - Ok(Some(weight_for::proxy_delegate::(votes.into())).into()) - } - - /// Undelegate the voting power of a proxied account. - /// - /// Tokens may be unlocked following once an amount of time consistent with the lock period - /// of the conviction with which the delegation was issued. - /// - /// The dispatch origin of this call must be _Signed_ and the signing account must be a - /// proxy for some other account which is currently delegating. - /// - /// Emits `Undelegated`. - /// - /// # - /// same as `undelegate with additional: - /// Db reads: `Proxy` - /// Base Weight: 39 + 7.958 * R µs - /// # - #[weight = weight_for::proxy_undelegate::(T::MaxVotes::get().into())] - fn proxy_undelegate(origin) -> DispatchResultWithPostInfo { - let who = ensure_signed(origin)?; - let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - let votes = Self::try_undelegate(target)?; - - Ok(Some(weight_for::proxy_undelegate::(votes.into())).into()) - } - - /// Remove a proxied vote for a referendum. - /// - /// Exactly equivalent to `remove_vote` except that it operates on the account that the - /// sender is a proxy for. - /// - /// The dispatch origin of this call must be _Signed_ and the signing account must be a - /// proxy for some other account which has a registered vote for the referendum of `index`. - /// - /// - `index`: The index of referendum of the vote to be removed. - /// - /// # - /// - `O(R + log R)` where R is the number of referenda that `target` has voted on. - /// Weight is calculated for the maximum number of vote. - /// - Db reads: `ReferendumInfoOf`, `VotingOf`, `Proxy` - /// - Db writes: `ReferendumInfoOf`, `VotingOf` - /// - Base Weight: 26.35 + .36 * R µs - /// # - #[weight = 26_000_000 + 360_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(2, 3)] - fn proxy_remove_vote(origin, index: ReferendumIndex) -> DispatchResult { - let who = ensure_signed(origin)?; - let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - Self::try_remove_vote(&target, index, UnvoteScope::Any) - } - /// Enact a proposal from a referendum. For now we just make the weight be the maximum. #[weight = T::MaximumBlockWeight::get()] fn enact_proposal(origin, proposal_hash: T::Hash, index: ReferendumIndex) -> DispatchResult { @@ -1538,16 +1260,6 @@ impl Module { // Exposed mutables. - #[cfg(feature = "std")] - pub fn force_proxy(stash: T::AccountId, proxy: T::AccountId) { - Proxy::::mutate(&proxy, |o| { - if o.is_none() { - system::Module::::inc_ref(&proxy); - } - *o = Some(ProxyState::Active(stash)) - }) - } - /// Start a referendum. pub fn internal_start_referendum( proposal_hash: T::Hash, diff --git a/substrate/frame/democracy/src/tests.rs b/substrate/frame/democracy/src/tests.rs index 103ac6a84b..36c2b7093b 100644 --- a/substrate/frame/democracy/src/tests.rs +++ b/substrate/frame/democracy/src/tests.rs @@ -38,7 +38,6 @@ mod external_proposing; mod fast_tracking; mod lock_voting; mod preimage; -mod proxying; mod public_proposals; mod scheduling; mod voting; diff --git a/substrate/frame/democracy/src/tests/proxying.rs b/substrate/frame/democracy/src/tests/proxying.rs deleted file mode 100644 index 2e39528e75..0000000000 --- a/substrate/frame/democracy/src/tests/proxying.rs +++ /dev/null @@ -1,105 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2017-2020 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! The tests for functionality concerning proxying. - -use super::*; - -#[test] -fn proxy_should_work() { - new_test_ext().execute_with(|| { - assert_eq!(Democracy::proxy(10), None); - assert!(System::allow_death(&10)); - - assert_noop!(Democracy::activate_proxy(Origin::signed(1), 10), Error::::NotOpen); - - assert_ok!(Democracy::open_proxy(Origin::signed(10), 1)); - assert!(!System::allow_death(&10)); - assert_eq!(Democracy::proxy(10), Some(ProxyState::Open(1))); - - assert_noop!(Democracy::activate_proxy(Origin::signed(2), 10), Error::::WrongOpen); - assert_ok!(Democracy::activate_proxy(Origin::signed(1), 10)); - assert_eq!(Democracy::proxy(10), Some(ProxyState::Active(1))); - - // Can't set when already set. - assert_noop!(Democracy::activate_proxy(Origin::signed(2), 10), Error::::AlreadyProxy); - - // But this works because 11 isn't proxying. - assert_ok!(Democracy::open_proxy(Origin::signed(11), 2)); - assert_ok!(Democracy::activate_proxy(Origin::signed(2), 11)); - assert_eq!(Democracy::proxy(10), Some(ProxyState::Active(1))); - assert_eq!(Democracy::proxy(11), Some(ProxyState::Active(2))); - - // 2 cannot fire 1's proxy: - assert_noop!(Democracy::deactivate_proxy(Origin::signed(2), 10), Error::::WrongProxy); - - // 1 deactivates their proxy: - assert_ok!(Democracy::deactivate_proxy(Origin::signed(1), 10)); - assert_eq!(Democracy::proxy(10), Some(ProxyState::Open(1))); - // but the proxy account cannot be killed until the proxy is closed. - assert!(!System::allow_death(&10)); - - // and then 10 closes it completely: - assert_ok!(Democracy::close_proxy(Origin::signed(10))); - assert_eq!(Democracy::proxy(10), None); - assert!(System::allow_death(&10)); - - // 11 just closes without 2's "permission". - assert_ok!(Democracy::close_proxy(Origin::signed(11))); - assert_eq!(Democracy::proxy(11), None); - assert!(System::allow_death(&11)); - }); -} - -#[test] -fn voting_and_removing_votes_should_work_with_proxy() { - new_test_ext().execute_with(|| { - System::set_block_number(0); - assert_ok!(propose_set_balance_and_note(1, 2, 1)); - - fast_forward_to(2); - let r = 0; - assert_ok!(Democracy::open_proxy(Origin::signed(10), 1)); - assert_ok!(Democracy::activate_proxy(Origin::signed(1), 10)); - - assert_ok!(Democracy::proxy_vote(Origin::signed(10), r, aye(1))); - assert_eq!(tally(r), Tally { ayes: 1, nays: 0, turnout: 10 }); - - assert_ok!(Democracy::proxy_remove_vote(Origin::signed(10), r)); - assert_eq!(tally(r), Tally { ayes: 0, nays: 0, turnout: 0 }); - }); -} - -#[test] -fn delegation_and_undelegation_should_work_with_proxy() { - new_test_ext().execute_with(|| { - System::set_block_number(0); - assert_ok!(propose_set_balance_and_note(1, 2, 1)); - fast_forward_to(2); - let r = 0; - assert_ok!(Democracy::open_proxy(Origin::signed(10), 1)); - assert_ok!(Democracy::activate_proxy(Origin::signed(1), 10)); - assert_ok!(Democracy::vote(Origin::signed(2), r, aye(2))); - - assert_ok!(Democracy::proxy_delegate(Origin::signed(10), 2, Conviction::None, 10)); - assert_eq!(tally(r), Tally { ayes: 3, nays: 0, turnout: 30 }); - - assert_ok!(Democracy::proxy_undelegate(Origin::signed(10))); - assert_eq!(tally(r), Tally { ayes: 2, nays: 0, turnout: 20 }); - }); -} - diff --git a/substrate/frame/democracy/src/types.rs b/substrate/frame/democracy/src/types.rs index efd52361f5..8ee0838f8a 100644 --- a/substrate/frame/democracy/src/types.rs +++ b/substrate/frame/democracy/src/types.rs @@ -197,24 +197,6 @@ impl ReferendumInfo { - /// Account is open to becoming a proxy but is not yet assigned. - Open(AccountId), - /// Account is actively being a proxy. - Active(AccountId), -} - -impl ProxyState { - pub (crate) fn as_active(self) -> Option { - match self { - ProxyState::Active(a) => Some(a), - ProxyState::Open(_) => None, - } - } -} - /// Whether an `unvote` operation is able to make actions that are not strictly always in the /// interest of an account. pub enum UnvoteScope { diff --git a/substrate/frame/elections/src/lib.rs b/substrate/frame/elections/src/lib.rs index 1085831373..171a2dbb8b 100644 --- a/substrate/frame/elections/src/lib.rs +++ b/substrate/frame/elections/src/lib.rs @@ -274,10 +274,6 @@ decl_storage! { /// of each entry; It may be the direct summed approval stakes, or a weighted version of it. /// Sorted from low to high. pub Leaderboard get(fn leaderboard): Option, T::AccountId)> >; - - /// Who is able to vote for whom. Value is the fund-holding account, key is the - /// vote-transaction-sending account. - pub Proxy get(fn proxy): map hasher(blake2_128_concat) T::AccountId => Option; } } @@ -292,8 +288,6 @@ decl_error! { CannotReapPresenting, /// Cannot reap during grace period. ReapGrace, - /// Not a proxy. - NotProxy, /// Invalid reporter index. InvalidReporterIndex, /// Invalid target index. @@ -430,23 +424,6 @@ decl_module! { Self::do_set_approvals(who, votes, index, hint, value) } - /// Set candidate approvals from a proxy. Approval slots stay valid as long as candidates in - /// those slots are registered. - /// - /// # - /// - Same as `set_approvals` with one additional storage read. - /// # - #[weight = 2_500_000_000] - fn proxy_set_approvals(origin, - votes: Vec, - #[compact] index: VoteIndex, - hint: SetIndex, - #[compact] value: BalanceOf, - ) -> DispatchResult { - let who = Self::proxy(ensure_signed(origin)?).ok_or(Error::::NotProxy)?; - Self::do_set_approvals(who, votes, index, hint, value) - } - /// Remove a voter. For it not to be a bond-consuming no-op, all approved candidate indices /// must now be either unregistered or registered to a candidate that registered the slot /// after the voter gave their last approval set. diff --git a/substrate/frame/elections/src/tests.rs b/substrate/frame/elections/src/tests.rs index 590266f7fe..8a9f58b54a 100644 --- a/substrate/frame/elections/src/tests.rs +++ b/substrate/frame/elections/src/tests.rs @@ -868,45 +868,6 @@ fn election_voting_should_work() { }); } -#[test] -fn election_proxy_voting_should_work() { - ExtBuilder::default().build().execute_with(|| { - assert_ok!(Elections::submit_candidacy(Origin::signed(5), 0)); - - >::insert(11, 1); - >::insert(12, 2); - >::insert(13, 3); - >::insert(14, 4); - assert_ok!( - Elections::proxy_set_approvals(Origin::signed(11), vec![true], 0, 0, 10) - ); - assert_ok!( - Elections::proxy_set_approvals(Origin::signed(14), vec![true], 0, 1, 40) - ); - - assert_eq!(Elections::all_approvals_of(&1), vec![true]); - assert_eq!(Elections::all_approvals_of(&4), vec![true]); - assert_eq!(voter_ids(), vec![1, 4]); - - assert_ok!(Elections::submit_candidacy(Origin::signed(2), 1)); - assert_ok!(Elections::submit_candidacy(Origin::signed(3), 2)); - - assert_ok!( - Elections::proxy_set_approvals(Origin::signed(12), vec![false, true], 0, 2, 20) - ); - assert_ok!( - Elections::proxy_set_approvals(Origin::signed(13), vec![false, true], 0, 3, 30) - ); - - assert_eq!(Elections::all_approvals_of(&1), vec![true]); - assert_eq!(Elections::all_approvals_of(&4), vec![true]); - assert_eq!(Elections::all_approvals_of(&2), vec![false, true]); - assert_eq!(Elections::all_approvals_of(&3), vec![false, true]); - - assert_eq!(voter_ids(), vec![1, 4, 2, 3]); - }); -} - #[test] fn election_simple_tally_should_work() { ExtBuilder::default().build().execute_with(|| { diff --git a/substrate/frame/proxy/Cargo.toml b/substrate/frame/proxy/Cargo.toml new file mode 100644 index 0000000000..bf76683d6c --- /dev/null +++ b/substrate/frame/proxy/Cargo.toml @@ -0,0 +1,43 @@ +[package] +name = "pallet-proxy" +version = "2.0.0-rc2" +authors = ["Parity Technologies "] +edition = "2018" +license = "Apache-2.0" +homepage = "https://substrate.dev" +repository = "https://github.com/paritytech/substrate/" +description = "FRAME proxying pallet" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +serde = { version = "1.0.101", optional = true } +codec = { package = "parity-scale-codec", version = "1.3.0", default-features = false } +frame-support = { version = "2.0.0-rc2", default-features = false, path = "../support" } +frame-system = { version = "2.0.0-rc2", default-features = false, path = "../system" } +sp-core = { version = "2.0.0-rc2", default-features = false, path = "../../primitives/core" } +sp-runtime = { version = "2.0.0-rc2", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "2.0.0-rc2", default-features = false, path = "../../primitives/std" } + +frame-benchmarking = { version = "2.0.0-rc2", default-features = false, path = "../benchmarking", optional = true } + +[dev-dependencies] +sp-core = { version = "2.0.0-rc2", path = "../../primitives/core" } +pallet-balances = { version = "2.0.0-rc2", path = "../balances" } +sp-io = { version = "2.0.0-rc2", path = "../../primitives/io" } + +[features] +default = ["std"] +std = [ + "serde", + "codec/std", + "sp-runtime/std", + "frame-support/std", + "frame-system/std", + "sp-std/std" +] +runtime-benchmarks = [ + "frame-benchmarking", + "frame-support/runtime-benchmarks", +] diff --git a/substrate/frame/proxy/src/benchmarking.rs b/substrate/frame/proxy/src/benchmarking.rs new file mode 100644 index 0000000000..5c938c12dc --- /dev/null +++ b/substrate/frame/proxy/src/benchmarking.rs @@ -0,0 +1,88 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Benchmarks for Proxy Pallet + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; +use frame_system::RawOrigin; +use frame_benchmarking::{benchmarks, account}; +use sp_runtime::traits::Bounded; +use crate::Module as Proxy; + +const SEED: u32 = 0; + +fn add_proxies(n: u32) -> Result<(), &'static str> { + let caller: T::AccountId = account("caller", 0, SEED); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + for i in 0..n { + Proxy::::add_proxy( + RawOrigin::Signed(caller.clone()).into(), + account("target", i, SEED), + T::ProxyType::default() + )?; + } + Ok(()) +} + +benchmarks! { + _ { + let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p)?; + } + + proxy { + let p in ...; + // In this case the caller is the "target" proxy + let caller: T::AccountId = account("target", p - 1, SEED); + // ... and "real" is the traditional caller. This is not a typo. + let real: T::AccountId = account("caller", 0, SEED); + let call: ::Call = frame_system::Call::::remark(vec![]).into(); + }: _(RawOrigin::Signed(caller), real, Some(T::ProxyType::default()), Box::new(call)) + + add_proxy { + let p in ...; + let caller: T::AccountId = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller), account("target", T::MaxProxies::get().into(), SEED), T::ProxyType::default()) + + remove_proxy { + let p in ...; + let caller: T::AccountId = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller), account("target", 0, SEED), T::ProxyType::default()) + + remove_proxies { + let p in ...; + let caller: T::AccountId = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{new_test_ext, Test}; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext().execute_with(|| { + assert_ok!(test_benchmark_proxy::()); + assert_ok!(test_benchmark_add_proxy::()); + assert_ok!(test_benchmark_remove_proxy::()); + assert_ok!(test_benchmark_remove_proxies::()); + }); + } +} diff --git a/substrate/frame/proxy/src/lib.rs b/substrate/frame/proxy/src/lib.rs new file mode 100644 index 0000000000..94740d7e79 --- /dev/null +++ b/substrate/frame/proxy/src/lib.rs @@ -0,0 +1,271 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Proxy Module +//! A module allowing accounts to give permission to other accounts to dispatch types of calls from +//! their signed origin. +//! +//! - [`proxy::Trait`](./trait.Trait.html) +//! - [`Call`](./enum.Call.html) +//! +//! ## Overview +//! +//! ## Interface +//! +//! ### Dispatchable Functions +//! +//! [`Call`]: ./enum.Call.html +//! [`Trait`]: ./trait.Trait.html + +// Ensure we're `no_std` when compiling for Wasm. +#![cfg_attr(not(feature = "std"), no_std)] + +use sp_std::prelude::*; +use frame_support::{decl_module, decl_event, decl_error, decl_storage, Parameter, ensure}; +use frame_support::{ + traits::{Get, ReservableCurrency, Currency, Filter, InstanceFilter}, + weights::{GetDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}}, + dispatch::{PostDispatchInfo, IsSubType}, +}; +use frame_system::{self as system, ensure_signed}; +use sp_runtime::{DispatchResult, traits::{Dispatchable, Zero}}; +use sp_runtime::traits::Member; + +mod tests; +mod benchmarking; + +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; + +/// Configuration trait. +pub trait Trait: frame_system::Trait { + /// The overarching event type. + type Event: From + Into<::Event>; + + /// The overarching call type. + type Call: Parameter + Dispatchable + + GetDispatchInfo + From> + IsSubType, Self>; + + /// The currency mechanism. + type Currency: ReservableCurrency; + + /// Is a given call compatible with the proxying subsystem? + type IsCallable: Filter<::Call>; + + /// A kind of proxy; specified with the proxy and passed in to the `IsProxyable` fitler. + /// The instance filter determines whether a given call may be proxied under this type. + type ProxyType: Parameter + Member + Ord + PartialOrd + InstanceFilter<::Call> + + Default; + + /// The base amount of currency needed to reserve for creating a proxy. + /// + /// This is held for an additional storage item whose value size is + /// `sizeof(Balance)` bytes and whose key size is `sizeof(AccountId)` bytes. + type ProxyDepositBase: Get>; + + /// The amount of currency needed per proxy added. + /// + /// This is held for adding 32 bytes plus an instance of `ProxyType` more into a pre-existing + /// storage value. + type ProxyDepositFactor: Get>; + + /// The maximum amount of proxies allowed for a single account. + type MaxProxies: Get; +} + +decl_storage! { + trait Store for Module as Proxy { + /// The set of account proxies. Maps the account which has delegated to the accounts + /// which are being delegated to, together with the amount held on deposit. + pub Proxies: map hasher(twox_64_concat) T::AccountId => (Vec<(T::AccountId, T::ProxyType)>, BalanceOf); + } +} + +decl_error! { + pub enum Error for Module { + /// There are too many proxies registered. + TooMany, + /// Proxy registration not found. + NotFound, + /// Sender is not a proxy of the account to be proxied. + NotProxy, + /// A call with a `false` `IsCallable` filter was attempted. + Uncallable, + /// A call which is incompatible with the proxy type's filter was attempted. + Unproxyable, + /// Account is already a proxy. + Duplicate, + /// Call may not be made by proxy because it may escalate its privileges. + NoPermission, + } +} + +decl_event! { + /// Events type. + pub enum Event { + /// A proxy was executed correctly, with the given result. + ProxyExecuted(DispatchResult), + } +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin { + type Error = Error; + + /// Deposit one of this module's events by using the default implementation. + fn deposit_event() = default; + + /// Dispatch the given `call` from an account that the sender is authorised for through + /// `add_proxy`. + /// + /// The dispatch origin for this call must be _Signed_. + /// + /// Parameters: + /// - `real`: The account that the proxy will make a call on behalf of. + /// - `force_proxy_type`: Specify the exact proxy type to be used and checked for this call. + /// - `call`: The call to be made by the `real` account. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 19.87 + .141 * P µs + /// - DB weight: 1 storage read. + /// - Plus the weight of the `call` + /// # + #[weight = { + let di = call.get_dispatch_info(); + (T::DbWeight::get().reads(1) + .saturating_add(di.weight) + .saturating_add(20 * WEIGHT_PER_MICROS) + .saturating_add((140 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())), + di.class) + }] + fn proxy(origin, + real: T::AccountId, + force_proxy_type: Option, + call: Box<::Call> + ) { + let who = ensure_signed(origin)?; + ensure!(T::IsCallable::filter(&call), Error::::Uncallable); + let (_, proxy_type) = Proxies::::get(&real).0.into_iter() + .find(|x| &x.0 == &who && force_proxy_type.as_ref().map_or(true, |y| &x.1 == y)) + .ok_or(Error::::NotProxy)?; + match call.is_sub_type() { + Some(Call::add_proxy(_, ref pt)) | Some(Call::remove_proxy(_, ref pt)) => + ensure!(&proxy_type == pt, Error::::NoPermission), + _ => (), + } + ensure!(proxy_type.filter(&call), Error::::Unproxyable); + let e = call.dispatch(frame_system::RawOrigin::Signed(real).into()); + Self::deposit_event(Event::ProxyExecuted(e.map(|_| ()).map_err(|e| e.error))); + } + + /// Register a proxy account for the sender that is able to make calls on its behalf. + /// + /// The dispatch origin for this call must be _Signed_. + /// + /// Parameters: + /// - `proxy`: The account that the `caller` would like to make a proxy. + /// - `proxy_type`: The permissions allowed for this proxy account. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 17.48 + .176 * P µs + /// - DB weight: 1 storage read and write. + /// # + #[weight = T::DbWeight::get().reads_writes(1, 1) + .saturating_add(18 * WEIGHT_PER_MICROS) + .saturating_add((200 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())) + ] + fn add_proxy(origin, proxy: T::AccountId, proxy_type: T::ProxyType) -> DispatchResult { + let who = ensure_signed(origin)?; + Proxies::::try_mutate(&who, |(ref mut proxies, ref mut deposit)| { + ensure!(proxies.len() < T::MaxProxies::get() as usize, Error::::TooMany); + let typed_proxy = (proxy, proxy_type); + let i = proxies.binary_search(&typed_proxy).err().ok_or(Error::::Duplicate)?; + proxies.insert(i, typed_proxy); + let new_deposit = T::ProxyDepositBase::get() + + T::ProxyDepositFactor::get() * (proxies.len() as u32).into(); + if new_deposit > *deposit { + T::Currency::reserve(&who, new_deposit - *deposit)?; + } else if new_deposit < *deposit { + T::Currency::unreserve(&who, *deposit - new_deposit); + } + *deposit = new_deposit; + Ok(()) + }) + } + + /// Unregister a proxy account for the sender. + /// + /// The dispatch origin for this call must be _Signed_. + /// + /// Parameters: + /// - `proxy`: The account that the `caller` would like to remove as a proxy. + /// - `proxy_type`: The permissions currently enabled for the removed proxy account. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 14.37 + .164 * P µs + /// - DB weight: 1 storage read and write. + /// # + #[weight = T::DbWeight::get().reads_writes(1, 1) + .saturating_add(14 * WEIGHT_PER_MICROS) + .saturating_add((160 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())) + ] + fn remove_proxy(origin, proxy: T::AccountId, proxy_type: T::ProxyType) -> DispatchResult { + let who = ensure_signed(origin)?; + Proxies::::try_mutate_exists(&who, |x| { + let (mut proxies, old_deposit) = x.take().ok_or(Error::::NotFound)?; + let typed_proxy = (proxy, proxy_type); + let i = proxies.binary_search(&typed_proxy).ok().ok_or(Error::::NotFound)?; + proxies.remove(i); + let new_deposit = if proxies.is_empty() { + BalanceOf::::zero() + } else { + T::ProxyDepositBase::get() + T::ProxyDepositFactor::get() * (proxies.len() as u32).into() + }; + if new_deposit > old_deposit { + T::Currency::reserve(&who, new_deposit - old_deposit)?; + } else if new_deposit < old_deposit { + T::Currency::unreserve(&who, old_deposit - new_deposit); + } + if !proxies.is_empty() { + *x = Some((proxies, new_deposit)) + } + Ok(()) + }) + } + + /// Unregister all proxy accounts for the sender. + /// + /// The dispatch origin for this call must be _Signed_. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 13.73 + .129 * P µs + /// - DB weight: 1 storage read and write. + /// # + #[weight = T::DbWeight::get().reads_writes(1, 1) + .saturating_add(14 * WEIGHT_PER_MICROS) + .saturating_add((130 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())) + ] + fn remove_proxies(origin) { + let who = ensure_signed(origin)?; + let (_, old_deposit) = Proxies::::take(&who); + T::Currency::unreserve(&who, old_deposit); + } + } +} diff --git a/substrate/frame/proxy/src/tests.rs b/substrate/frame/proxy/src/tests.rs new file mode 100644 index 0000000000..2d22f2d53c --- /dev/null +++ b/substrate/frame/proxy/src/tests.rs @@ -0,0 +1,220 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Tests for Proxy Pallet + +#![cfg(test)] + +use super::*; + +use frame_support::{ + assert_ok, assert_noop, impl_outer_origin, parameter_types, impl_outer_dispatch, + weights::Weight, impl_outer_event, RuntimeDebug +}; +use codec::{Encode, Decode}; +use sp_core::H256; +use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup}, testing::Header}; +use crate as proxy; + +impl_outer_origin! { + pub enum Origin for Test where system = frame_system {} +} +impl_outer_event! { + pub enum TestEvent for Test { + system, + pallet_balances, + proxy, + } +} +impl_outer_dispatch! { + pub enum Call for Test where origin: Origin { + frame_system::System, + pallet_balances::Balances, + proxy::Proxy, + } +} + +// For testing the pallet, we construct most of a mock runtime. This means +// first constructing a configuration type (`Test`) which `impl`s each of the +// configuration traits of pallets we want to use. +#[derive(Clone, Eq, PartialEq)] +pub struct Test; +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MaximumBlockWeight: Weight = 1024; + pub const MaximumBlockLength: u32 = 2 * 1024; + pub const AvailableBlockRatio: Perbill = Perbill::one(); +} +impl frame_system::Trait for Test { + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Call = Call; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = TestEvent; + type BlockHashCount = BlockHashCount; + type MaximumBlockWeight = MaximumBlockWeight; + type DbWeight = (); + type BlockExecutionWeight = (); + type ExtrinsicBaseWeight = (); + type MaximumExtrinsicWeight = MaximumBlockWeight; + type MaximumBlockLength = MaximumBlockLength; + type AvailableBlockRatio = AvailableBlockRatio; + type Version = (); + type ModuleToIndex = (); + type AccountData = pallet_balances::AccountData; + type OnNewAccount = (); + type OnKilledAccount = (); +} +parameter_types! { + pub const ExistentialDeposit: u64 = 1; +} +impl pallet_balances::Trait for Test { + type Balance = u64; + type Event = TestEvent; + type DustRemoval = (); + type ExistentialDeposit = ExistentialDeposit; + type AccountStore = System; +} +parameter_types! { + pub const ProxyDepositBase: u64 = 1; + pub const ProxyDepositFactor: u64 = 1; + pub const MaxProxies: u16 = 3; +} +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug)] +pub enum ProxyType { + Any, + JustTransfer, +} +impl Default for ProxyType { fn default() -> Self { Self::Any } } +impl InstanceFilter for ProxyType { + fn filter(&self, c: &Call) -> bool { + match self { + ProxyType::Any => true, + ProxyType::JustTransfer => match c { + Call::Balances(pallet_balances::Call::transfer(..)) => true, + _ => false, + } + } + } +} +pub struct TestIsCallable; +impl Filter for TestIsCallable { + fn filter(c: &Call) -> bool { + match *c { + Call::Balances(_) => true, + _ => false, + } + } +} +impl Trait for Test { + type Event = TestEvent; + type Call = Call; + type Currency = Balances; + type IsCallable = TestIsCallable; + type ProxyType = ProxyType; + type ProxyDepositBase = ProxyDepositBase; + type ProxyDepositFactor = ProxyDepositFactor; + type MaxProxies = MaxProxies; +} + +type System = frame_system::Module; +type Balances = pallet_balances::Module; +type Proxy = Module; + +use frame_system::Call as SystemCall; +use pallet_balances::Call as BalancesCall; +use pallet_balances::Error as BalancesError; + +pub fn new_test_ext() -> sp_io::TestExternalities { + let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + pallet_balances::GenesisConfig:: { + balances: vec![(1, 10), (2, 10), (3, 10), (4, 10), (5, 2)], + }.assimilate_storage(&mut t).unwrap(); + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext +} + +fn last_event() -> TestEvent { + system::Module::::events().pop().map(|e| e.event).expect("Event expected") +} + +fn expect_event>(e: E) { + assert_eq!(last_event(), e.into()); +} + +#[test] +fn add_remove_proxies_works() { + new_test_ext().execute_with(|| { + assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::Any)); + assert_noop!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::Any), Error::::Duplicate); + assert_eq!(Balances::reserved_balance(1), 2); + assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::JustTransfer)); + assert_eq!(Balances::reserved_balance(1), 3); + assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::Any)); + assert_eq!(Balances::reserved_balance(1), 4); + assert_noop!(Proxy::add_proxy(Origin::signed(1), 4, ProxyType::Any), Error::::TooMany); + assert_noop!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::JustTransfer), Error::::NotFound); + assert_ok!(Proxy::remove_proxy(Origin::signed(1), 3, ProxyType::Any)); + assert_eq!(Balances::reserved_balance(1), 3); + assert_ok!(Proxy::remove_proxy(Origin::signed(1), 2, ProxyType::Any)); + assert_eq!(Balances::reserved_balance(1), 2); + assert_ok!(Proxy::remove_proxy(Origin::signed(1), 2, ProxyType::JustTransfer)); + assert_eq!(Balances::reserved_balance(1), 0); + }); +} + +#[test] +fn cannot_add_proxy_without_balance() { + new_test_ext().execute_with(|| { + assert_ok!(Proxy::add_proxy(Origin::signed(5), 3, ProxyType::Any)); + assert_eq!(Balances::reserved_balance(5), 2); + assert_noop!( + Proxy::add_proxy(Origin::signed(5), 4, ProxyType::Any), + BalancesError::::InsufficientBalance + ); + }); +} + +#[test] +fn proxying_works() { + new_test_ext().execute_with(|| { + assert_ok!(Proxy::add_proxy(Origin::signed(1), 2, ProxyType::JustTransfer)); + assert_ok!(Proxy::add_proxy(Origin::signed(1), 3, ProxyType::Any)); + + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1))); + assert_noop!(Proxy::proxy(Origin::signed(4), 1, None, call.clone()), Error::::NotProxy); + assert_noop!(Proxy::proxy(Origin::signed(2), 1, Some(ProxyType::Any), call.clone()), Error::::NotProxy); + assert_ok!(Proxy::proxy(Origin::signed(2), 1, None, call.clone())); + expect_event(Event::ProxyExecuted(Ok(()))); + assert_eq!(Balances::free_balance(6), 1); + + let call = Box::new(Call::System(SystemCall::remark(vec![]))); + assert_noop!(Proxy::proxy(Origin::signed(3), 1, None, call.clone()), Error::::Uncallable); + + let call = Box::new(Call::Balances(BalancesCall::transfer_keep_alive(6, 1))); + assert_noop!(Proxy::proxy(Origin::signed(2), 1, None, call.clone()), Error::::Unproxyable); + assert_ok!(Proxy::proxy(Origin::signed(3), 1, None, call.clone())); + expect_event(Event::ProxyExecuted(Ok(()))); + assert_eq!(Balances::free_balance(6), 2); + }); +} diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 05e703a5e8..568401a498 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -43,6 +43,16 @@ impl Filter for () { fn filter(_: &T) -> bool { true } } +/// Simple trait for providing a filter over a reference to some type, given an instance of itself. +pub trait InstanceFilter { + /// Determine if a given value should be allowed through the filter (returns `true`) or not. + fn filter(&self, _: &T) -> bool; +} + +impl InstanceFilter for () { + fn filter(&self, _: &T) -> bool { true } +} + /// An abstraction of a value stored within storage, but possibly as part of a larger composite /// item. pub trait StoredMap { diff --git a/substrate/frame/utility/src/lib.rs b/substrate/frame/utility/src/lib.rs index 546af51bdd..ea56bc4599 100644 --- a/substrate/frame/utility/src/lib.rs +++ b/substrate/frame/utility/src/lib.rs @@ -168,7 +168,7 @@ decl_error! { WrongTimepoint, /// A timepoint was given, yet no multisig operation is underway. UnexpectedTimepoint, - /// A call with a `false` IsCallable filter was attempted. + /// A call with a `false` `IsCallable` filter was attempted. Uncallable, } } diff --git a/substrate/frame/utility/src/tests.rs b/substrate/frame/utility/src/tests.rs index daf3d6c53a..a74d9b3253 100644 --- a/substrate/frame/utility/src/tests.rs +++ b/substrate/frame/utility/src/tests.rs @@ -99,12 +99,11 @@ parameter_types! { pub const MultisigDepositFactor: u64 = 1; pub const MaxSignatories: u16 = 3; } - pub struct TestIsCallable; impl Filter for TestIsCallable { fn filter(c: &Call) -> bool { match *c { - Call::Balances(pallet_balances::Call::transfer(..)) => true, + Call::Balances(_) => true, _ => false, } } @@ -128,7 +127,7 @@ use pallet_balances::Error as BalancesError; pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); pallet_balances::GenesisConfig:: { - balances: vec![(1, 10), (2, 10), (3, 10), (4, 10), (5, 10)], + balances: vec![(1, 10), (2, 10), (3, 10), (4, 10), (5, 2)], }.assimilate_storage(&mut t).unwrap(); let mut ext = sp_io::TestExternalities::new(t); ext.execute_with(|| System::set_block_number(1));