diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7a036ddf5f1ed..748403c57cc44 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -808,7 +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}, + Proxy: pallet_proxy::{Module, Call, Storage, Event}, } ); diff --git a/frame/democracy/src/benchmarking.rs b/frame/democracy/src/benchmarking.rs index 7839618760d7a..3957b38f42926 100644 --- a/frame/democracy/src/benchmarking.rs +++ b/frame/democracy/src/benchmarking.rs @@ -30,7 +30,6 @@ use sp_runtime::traits::{Bounded, One}; use crate::Module as Democracy; const SEED: u32 = 0; -const MAX_USERS: u32 = 1000; const MAX_REFERENDUMS: u32 = 100; const MAX_PROPOSALS: u32 = 100; const MAX_SECONDERS: u32 = 100; diff --git a/frame/proxy/Cargo.toml b/frame/proxy/Cargo.toml index bf76683d6c562..ee776951fdfdc 100644 --- a/frame/proxy/Cargo.toml +++ b/frame/proxy/Cargo.toml @@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "1.3.0", default-features = 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-io = { version = "2.0.0-rc2", default-features = false, path = "../../primitives/io" } 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" } @@ -25,7 +26,6 @@ frame-benchmarking = { version = "2.0.0-rc2", default-features = false, path = " [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"] @@ -35,7 +35,8 @@ std = [ "sp-runtime/std", "frame-support/std", "frame-system/std", - "sp-std/std" + "sp-std/std", + "sp-io/std" ] runtime-benchmarks = [ "frame-benchmarking", diff --git a/frame/proxy/src/benchmarking.rs b/frame/proxy/src/benchmarking.rs index 5c938c12dc9b8..3cbe517dfd7da 100644 --- a/frame/proxy/src/benchmarking.rs +++ b/frame/proxy/src/benchmarking.rs @@ -27,8 +27,8 @@ 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); +fn add_proxies(n: u32, maybe_who: Option) -> Result<(), &'static str> { + let caller = maybe_who.unwrap_or_else(|| account("caller", 0, SEED)); T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); for i in 0..n { Proxy::::add_proxy( @@ -42,7 +42,7 @@ fn add_proxies(n: u32) -> Result<(), &'static str> { benchmarks! { _ { - let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p)?; + let p in 1 .. (T::MaxProxies::get() - 1).into() => add_proxies::(p, None)?; } proxy { @@ -68,6 +68,24 @@ benchmarks! { let p in ...; let caller: T::AccountId = account("caller", 0, SEED); }: _(RawOrigin::Signed(caller)) + + anonymous { + let p in ...; + }: _(RawOrigin::Signed(account("caller", 0, SEED)), T::ProxyType::default(), 0) + + kill_anonymous { + let p in 0 .. (T::MaxProxies::get() - 2).into(); + + let caller: T::AccountId = account("caller", 0, SEED); + T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + Module::::anonymous(RawOrigin::Signed(account("caller", 0, SEED)).into(), T::ProxyType::default(), 0)?; + let height = system::Module::::block_number(); + let ext_index = system::Module::::extrinsic_index().unwrap_or(0); + let anon = Module::::anonymous_account(&caller, &T::ProxyType::default(), 0, None); + + add_proxies::(p, Some(anon.clone()))?; + + }: _(RawOrigin::Signed(anon), caller, T::ProxyType::default(), 0, height, ext_index) } #[cfg(test)] @@ -83,6 +101,8 @@ mod tests { assert_ok!(test_benchmark_add_proxy::()); assert_ok!(test_benchmark_remove_proxy::()); assert_ok!(test_benchmark_remove_proxies::()); + assert_ok!(test_benchmark_anonymous::()); + assert_ok!(test_benchmark_kill_anonymous::()); }); } } diff --git a/frame/proxy/src/lib.rs b/frame/proxy/src/lib.rs index 94740d7e793cc..0c5b9c494c9f1 100644 --- a/frame/proxy/src/lib.rs +++ b/frame/proxy/src/lib.rs @@ -35,15 +35,17 @@ #![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 codec::{Encode, Decode}; +use sp_io::hashing::blake2_256; +use sp_runtime::{DispatchResult, traits::{Dispatchable, Zero}}; +use sp_runtime::traits::Member; use frame_support::{ + decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, 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; @@ -53,7 +55,7 @@ type BalanceOf = <::Currency as Currency< + Into<::Event>; + type Event: From> + Into<::Event>; /// The overarching call type. type Call: Parameter + Dispatchable @@ -115,9 +117,15 @@ decl_error! { decl_event! { /// Events type. - pub enum Event { + pub enum Event where + AccountId = ::AccountId, + ProxyType = ::ProxyType + { /// A proxy was executed correctly, with the given result. ProxyExecuted(DispatchResult), + /// Anonymous account (first parameter) has been created by new proxy (second) with given + /// disambiguation index and proxy type. + AnonymousCreated(AccountId, AccountId, ProxyType, u16), } } @@ -164,12 +172,12 @@ decl_module! { .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!(pt.is_no_more_permissive(&proxy_type), 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))); + Self::deposit_event(RawEvent::ProxyExecuted(e.map(|_| ()).map_err(|e| e.error))); } /// Register a proxy account for the sender that is able to make calls on its behalf. @@ -186,8 +194,8 @@ decl_module! { /// - 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())) + .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)?; @@ -222,8 +230,8 @@ decl_module! { /// - 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())) + .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)?; @@ -253,19 +261,119 @@ decl_module! { /// /// The dispatch origin for this call must be _Signed_. /// + /// WARNING: This may be called on accounts created by `anonymous`, however if done, then + /// the unreserved fees will be inaccessible. **All access to this account will be lost.** + /// /// # /// 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())) + .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); } + + /// Spawn a fresh new account that is guaranteed to be otherwise inaccessible, and + /// initialize it with a proxy of `proxy_type` for `origin` sender. + /// + /// Requires a `Signed` origin. + /// + /// - `proxy_type`: The type of the proxy that the sender will be registered as over the + /// new account. This will almost always be the most permissive `ProxyType` possible to + /// allow for maximum flexibility. + /// - `index`: A disambiguation index, in case this is called multiple times in the same + /// transaction (e.g. with `utility::batch`). Unless you're using `batch` you probably just + /// want to use `0`. + /// + /// Fails with `Duplicate` if this has already been called in this transaction, from the + /// same sender, with the same parameters. + /// + /// Fails if there are insufficient funds to pay for deposit. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 36.48 + .039 * P µs + /// - DB weight: 1 storage read and write. + /// # + #[weight = T::DbWeight::get().reads_writes(1, 1) + .saturating_add(36 * WEIGHT_PER_MICROS) + .saturating_add((40 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())) + ] + fn anonymous(origin, proxy_type: T::ProxyType, index: u16) { + let who = ensure_signed(origin)?; + + let anonymous = Self::anonymous_account(&who, &proxy_type, index, None); + ensure!(!Proxies::::contains_key(&anonymous), Error::::Duplicate); + let deposit = T::ProxyDepositBase::get() + T::ProxyDepositFactor::get(); + T::Currency::reserve(&who, deposit)?; + Proxies::::insert(&anonymous, (vec![(who.clone(), proxy_type.clone())], deposit)); + Self::deposit_event(RawEvent::AnonymousCreated(anonymous, who, proxy_type, index)); + } + + /// Removes a previously spawned anonymous proxy. + /// + /// WARNING: **All access to this account will be lost.** Any funds held in it will be + /// inaccessible. + /// + /// Requires a `Signed` origin, and the sender account must have been created by a call to + /// `anonymous` with corresponding parameters. + /// + /// - `spawner`: The account that originally called `anonymous` to create this account. + /// - `index`: The disambiguation index originally passed to `anonymous`. Probably `0`. + /// - `proxy_type`: The proxy type originally passed to `anonymous`. + /// - `height`: The height of the chain when the call to `anonymous` was processed. + /// - `ext_index`: The extrinsic index in which the call to `anonymous` was processed. + /// + /// Fails with `NoPermission` in case the caller is not a previously created anonymous + /// account whose `anonymous` call has corresponding parameters. + /// + /// # + /// P is the number of proxies the user has + /// - Base weight: 15.65 + .137 * P µs + /// - DB weight: 1 storage read and write. + /// # + #[weight = T::DbWeight::get().reads_writes(1, 1) + .saturating_add(15 * WEIGHT_PER_MICROS) + .saturating_add((140 * WEIGHT_PER_NANOS).saturating_mul(T::MaxProxies::get().into())) + ] + fn kill_anonymous(origin, + spawner: T::AccountId, + proxy_type: T::ProxyType, + index: u16, + #[compact] height: T::BlockNumber, + #[compact] ext_index: u32, + ) { + let who = ensure_signed(origin)?; + + let when = (height, ext_index); + let proxy = Self::anonymous_account(&spawner, &proxy_type, index, Some(when)); + ensure!(proxy == who, Error::::NoPermission); + + let (_, deposit) = Proxies::::take(&who); + T::Currency::unreserve(&spawner, deposit); + } + } +} + +impl Module { + pub fn anonymous_account( + who: &T::AccountId, + proxy_type: &T::ProxyType, + index: u16, + maybe_when: Option<(T::BlockNumber, u32)>, + ) -> T::AccountId { + let (height, ext_index) = maybe_when.unwrap_or_else(|| ( + system::Module::::block_number(), + system::Module::::extrinsic_index().unwrap_or_default() + )); + let entropy = (b"modlpy/proxy____", who, height, ext_index, proxy_type, index) + .using_encoded(blake2_256); + T::AccountId::decode(&mut &entropy[..]).unwrap_or_default() } } diff --git a/frame/proxy/src/tests.rs b/frame/proxy/src/tests.rs index 2d22f2d53c01d..c331195c26287 100644 --- a/frame/proxy/src/tests.rs +++ b/frame/proxy/src/tests.rs @@ -23,7 +23,7 @@ use super::*; use frame_support::{ assert_ok, assert_noop, impl_outer_origin, parameter_types, impl_outer_dispatch, - weights::Weight, impl_outer_event, RuntimeDebug + weights::Weight, impl_outer_event, RuntimeDebug, dispatch::DispatchError }; use codec::{Encode, Decode}; use sp_core::H256; @@ -37,7 +37,7 @@ impl_outer_event! { pub enum TestEvent for Test { system, pallet_balances, - proxy, + proxy, } } impl_outer_dispatch! { @@ -120,8 +120,10 @@ pub struct TestIsCallable; impl Filter for TestIsCallable { fn filter(c: &Call) -> bool { match *c { - Call::Balances(_) => true, - _ => false, + // Remark is used as a no-op call in the benchmarking + Call::System(SystemCall::remark(_)) => true, + Call::System(_) => false, + _ => true, } } } @@ -143,6 +145,7 @@ type Proxy = Module; use frame_system::Call as SystemCall; use pallet_balances::Call as BalancesCall; use pallet_balances::Error as BalancesError; +use super::Call as ProxyCall; pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); @@ -155,7 +158,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities { } fn last_event() -> TestEvent { - system::Module::::events().pop().map(|e| e.event).expect("Event expected") + system::Module::::events().pop().expect("Event expected").event } fn expect_event>(e: E) { @@ -203,18 +206,61 @@ fn proxying_works() { 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_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(()))); + expect_event(RawEvent::ProxyExecuted(Ok(()))); assert_eq!(Balances::free_balance(6), 1); - let call = Box::new(Call::System(SystemCall::remark(vec![]))); + let call = Box::new(Call::System(SystemCall::set_code(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(()))); + expect_event(RawEvent::ProxyExecuted(Ok(()))); assert_eq!(Balances::free_balance(6), 2); }); } + +#[test] +fn anonymous_works() { + new_test_ext().execute_with(|| { + assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 0)); + let anon = Proxy::anonymous_account(&1, &ProxyType::Any, 0, None); + expect_event(RawEvent::AnonymousCreated(anon.clone(), 1, ProxyType::Any, 0)); + + // other calls to anonymous allowed as long as they're not exactly the same. + assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::JustTransfer, 0)); + assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 1)); + let anon2 = Proxy::anonymous_account(&2, &ProxyType::Any, 0, None); + assert_ok!(Proxy::anonymous(Origin::signed(2), ProxyType::Any, 0)); + assert_noop!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 0), Error::::Duplicate); + System::set_extrinsic_index(1); + assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 0)); + System::set_extrinsic_index(0); + System::set_block_number(2); + assert_ok!(Proxy::anonymous(Origin::signed(1), ProxyType::Any, 0)); + + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 1))); + assert_ok!(Balances::transfer(Origin::signed(3), anon, 5)); + assert_ok!(Proxy::proxy(Origin::signed(1), anon, None, call)); + expect_event(RawEvent::ProxyExecuted(Ok(()))); + assert_eq!(Balances::free_balance(6), 1); + + let call = Box::new(Call::Proxy(ProxyCall::kill_anonymous(1, ProxyType::Any, 0, 1, 0))); + assert_ok!(Proxy::proxy(Origin::signed(2), anon2, None, call.clone())); + let de = DispatchError::from(Error::::NoPermission).stripped(); + expect_event(RawEvent::ProxyExecuted(Err(de))); + assert_noop!( + Proxy::kill_anonymous(Origin::signed(1), 1, ProxyType::Any, 0, 1, 0), + Error::::NoPermission + ); + assert_eq!(Balances::free_balance(1), 0); + assert_ok!(Proxy::proxy(Origin::signed(1), anon, None, call.clone())); + assert_eq!(Balances::free_balance(1), 2); + assert_noop!(Proxy::proxy(Origin::signed(1), anon, None, call.clone()), Error::::NotProxy); + }); +} diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 568401a498fd3..979f021e03dba 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -47,10 +47,25 @@ impl Filter for () { pub trait InstanceFilter { /// Determine if a given value should be allowed through the filter (returns `true`) or not. fn filter(&self, _: &T) -> bool; + + /// Determines whether `self` matches at least all items that `o` does. + fn is_no_less_permissive(&self, o: &Self) -> bool { !self.is_less_permissive(o) } + + /// Determines whether `self` matches at most only the items that `o` does. + fn is_no_more_permissive(&self, o: &Self) -> bool { !o.is_less_permissive(&self) } + + /// Determines whether `self` matches all the items that `o` does and others. + fn is_more_permissive(&self, o: &Self) -> bool { o.is_less_permissive(self) } + + /// Determines whether `self` does not match all the items that `_o` does, nor any others. + /// + /// NOTE: This is the only `*permissive` function that needs to be reimplemented. + fn is_less_permissive(&self, _o: &Self) -> bool { true } } impl InstanceFilter for () { fn filter(&self, _: &T) -> bool { true } + fn is_less_permissive(&self, _o: &Self) -> bool { false } } /// An abstraction of a value stored within storage, but possibly as part of a larger composite