diff --git a/Cargo.lock b/Cargo.lock index df01a143efb61..adb2843d0a7a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18834,9 +18834,9 @@ checksum = "f97841a747eef040fcd2e7b3b9a220a7205926e60488e673d9e4926d27772ce5" [[package]] name = "serde" -version = "1.0.206" +version = "1.0.207" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b3e4cd94123dd520a128bcd11e34d9e9e423e7e3e50425cb1b4b1e3549d0284" +checksum = "5665e14a49a4ea1b91029ba7d3bca9f299e1f7cfa194388ccc20f14743e784f2" dependencies = [ "serde_derive", ] @@ -18861,9 +18861,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.206" +version = "1.0.207" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fabfb6138d2383ea8208cf98ccf69cdfb1aff4088460681d84189aa259762f97" +checksum = "6aea2634c86b0e8ef2cfdc0c340baede54ec27b1e46febd7f80dffb2aa44a00e" dependencies = [ "proc-macro2 1.0.82", "quote 1.0.36", diff --git a/prdoc/pr_5336.prdoc b/prdoc/pr_5336.prdoc new file mode 100644 index 0000000000000..7992b9b27d087 --- /dev/null +++ b/prdoc/pr_5336.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Refactor proxy pallet to use Consideration trait + +doc: + - audience: Runtime Dev + description: | + Before this pallet would reserve its storage value manually, `Consideration` + trait simplifies the implementation. + +crates: + - name: pallet-proxy + bump: major diff --git a/substrate/frame/proxy/src/benchmarking.rs b/substrate/frame/proxy/src/benchmarking.rs index 4081af49c2435..7cfcc44df48ff 100644 --- a/substrate/frame/proxy/src/benchmarking.rs +++ b/substrate/frame/proxy/src/benchmarking.rs @@ -23,6 +23,7 @@ use super::*; use crate::Pallet as Proxy; use alloc::{boxed::Box, vec}; use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller}; +use frame_support::traits::Currency; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use sp_runtime::traits::Bounded; @@ -136,7 +137,7 @@ benchmarks! { add_announcements::(a, Some(caller.clone()), None)?; }: _(RawOrigin::Signed(caller.clone()), real_lookup, T::CallHasher::hash_of(&call)) verify { - let (announcements, _) = Announcements::::get(&caller); + let (announcements, _) = Announcements::::get(&caller).unwrap(); assert_eq!(announcements.len() as u32, a); } @@ -159,7 +160,7 @@ benchmarks! { add_announcements::(a, Some(caller.clone()), None)?; }: _(RawOrigin::Signed(real), caller_lookup, T::CallHasher::hash_of(&call)) verify { - let (announcements, _) = Announcements::::get(&caller); + let (announcements, _) = Announcements::::get(&caller).unwrap(); assert_eq!(announcements.len() as u32, a); } @@ -191,7 +192,7 @@ benchmarks! { BlockNumberFor::::zero() ) verify { - let (proxies, _) = Proxies::::get(caller); + let (proxies, _) = Proxies::::get(caller).unwrap(); assert_eq!(proxies.len() as u32, p + 1); } @@ -206,8 +207,8 @@ benchmarks! { BlockNumberFor::::zero() ) verify { - let (proxies, _) = Proxies::::get(caller); - assert_eq!(proxies.len() as u32, p - 1); + let len = Proxies::::get(caller).map(|x| x.0.len()).unwrap_or(0); + assert_eq!(len as u32, p - 1); } remove_proxies { @@ -215,8 +216,8 @@ benchmarks! { let caller: T::AccountId = whitelisted_caller(); }: _(RawOrigin::Signed(caller.clone())) verify { - let (proxies, _) = Proxies::::get(caller); - assert_eq!(proxies.len() as u32, 0); + let len = Proxies::::get(caller).map(|x| x.0.len()).unwrap_or(0); + assert_eq!(len as u32, 0); } create_pure { diff --git a/substrate/frame/proxy/src/lib.rs b/substrate/frame/proxy/src/lib.rs index 016f2cf225e05..d0db482fc1900 100644 --- a/substrate/frame/proxy/src/lib.rs +++ b/substrate/frame/proxy/src/lib.rs @@ -40,7 +40,10 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::GetDispatchInfo, ensure, - traits::{Currency, Get, InstanceFilter, IsSubType, IsType, OriginTrait, ReservableCurrency}, + traits::{ + Consideration, Footprint, Get, InstanceFilter, IsSubType, IsType, OriginTrait, + ReservableCurrency, Currency, + }, BoundedVec, }; use frame_system::{self as system, ensure_signed, pallet_prelude::BlockNumberFor}; @@ -55,8 +58,11 @@ pub use weights::WeightInfo; type CallHashOf = <::CallHasher as Hash>::Output; -type BalanceOf = - <::Currency as Currency<::AccountId>>::Balance; +type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; + +type AnnouncementTicketOf = ::AnnouncementConsideration; + +type ProxyTicketOf = ::ProxyConsideration; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; @@ -99,7 +105,10 @@ pub struct Announcement { #[frame_support::pallet] pub mod pallet { use super::{DispatchResult, *}; - use frame_support::pallet_prelude::*; + use frame_support::{ + pallet_prelude::{OptionQuery, *}, + traits::Footprint, + }; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -134,20 +143,15 @@ pub mod pallet { + Default + MaxEncodedLen; - /// 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. - #[pallet::constant] - type ProxyDepositBase: Get>; + /// A means of providing some cost for storing annoucement data on-chain. + type AnnouncementConsideration: Consideration; /// 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. Thus, when configuring `ProxyDepositFactor` one should take /// into account `32 + proxy_type.encode().len()` bytes of data. - #[pallet::constant] - type ProxyDepositFactor: Get>; + type ProxyConsideration: Consideration; /// The maximum amount of proxies allowed for a single account. #[pallet::constant] @@ -162,20 +166,15 @@ pub mod pallet { /// The type of hash used for hashing the call. type CallHasher: Hash; + } - /// The base amount of currency needed to reserve for creating an announcement. - /// - /// This is held when a new storage item holding a `Balance` is created (typically 16 - /// bytes). - #[pallet::constant] - type AnnouncementDepositBase: Get>; - - /// The amount of currency needed per announcement made. - /// - /// This is held for adding an `AccountId`, `Hash` and `BlockNumber` (typically 68 bytes) - /// into a pre-existing storage value. - #[pallet::constant] - type AnnouncementDepositFactor: Get>; + /// Reasons the pallet may place funds on hold. + #[pallet::composite_enum] + pub enum HoldReason { + /// The funds are held as storage deposit for a pure Proxy. + Proxy, + /// The funds are held as storage deposit for an announcement. + Announcement, } #[pallet::call] @@ -266,7 +265,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::remove_proxies(T::MaxProxies::get()))] pub fn remove_proxies(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; - Self::remove_all_proxy_delegates(&who); + Self::remove_all_proxy_delegates(&who)?; Ok(()) } @@ -306,10 +305,12 @@ pub mod pallet { let bounded_proxies: BoundedVec<_, T::MaxProxies> = vec![proxy_def].try_into().map_err(|_| Error::::TooMany)?; - let deposit = T::ProxyDepositBase::get() + T::ProxyDepositFactor::get(); - T::Currency::reserve(&who, deposit)?; + let ticket = T::ProxyConsideration::new( + &who, + Footprint::from_parts(bounded_proxies.len(), Self::proxy_def_size_bytes()), + )?; + Proxies::::insert(&pure, (bounded_proxies, ticket)); - Proxies::::insert(&pure, (bounded_proxies, deposit)); Self::deposit_event(Event::PureCreated { pure, who, @@ -353,8 +354,9 @@ pub mod pallet { let proxy = Self::pure_account(&spawner, &proxy_type, index, Some(when)); ensure!(proxy == who, Error::::NoPermission); - let (_, deposit) = Proxies::::take(&who); - T::Currency::unreserve(&spawner, deposit); + if let Some((_, ticket)) = Proxies::::take(&who) { + ticket.drop(&spawner)?; + } Ok(()) } @@ -383,11 +385,10 @@ pub mod pallet { ) -> DispatchResult { let who = ensure_signed(origin)?; let real = T::Lookup::lookup(real)?; - Proxies::::get(&real) - .0 - .into_iter() - .find(|x| x.delegate == who) - .ok_or(Error::::NotProxy)?; + + // if proxy is not found then same error of NotProxy still applies + let (proxy, _) = Proxies::::try_get(&real).map_err(|_| Error::::NotProxy)?; + proxy.into_iter().find(|x| x.delegate == who).ok_or(Error::::NotProxy)?; let announcement = Announcement { real: real.clone(), @@ -395,20 +396,40 @@ pub mod pallet { height: system::Pallet::::block_number(), }; - Announcements::::try_mutate(&who, |(ref mut pending, ref mut deposit)| { - pending.try_push(announcement).map_err(|_| Error::::TooMany)?; - Self::rejig_deposit( - &who, - *deposit, - T::AnnouncementDepositBase::get(), - T::AnnouncementDepositFactor::get(), - pending.len(), - ) - .map(|d| { - d.expect("Just pushed; pending.len() > 0; rejig_deposit returns Some; qed") - }) - .map(|d| *deposit = d) - })?; + Announcements::::mutate( + &who, + |value: &mut Option<( + BoundedVec< + Announcement, BlockNumberFor>, + T::MaxPending, + >, + AnnouncementTicketOf, + )>| + -> Result<(), DispatchError> { + let (mut pending, ticket) = if let Some(v) = value.take() { + v + } else { + let bounded_announcements: BoundedVec< + Announcement, BlockNumberFor>, + T::MaxPending, + > = Default::default(); + ( + bounded_announcements, + T::AnnouncementConsideration::new( + &who, + Footprint::from_parts(0, Self::annoucement_size_bytes()), + )?, + ) + }; + pending.try_push(announcement).map_err(|_| Error::::TooMany)?; + let new_ticket = ticket.clone().update( + &who, + Footprint::from_parts(pending.len(), Self::annoucement_size_bytes()), + )?; + *value = Some((pending, new_ticket)); + Ok::<(), DispatchError>(()) + }, + )?; Self::deposit_event(Event::Announced { real, proxy: who, call_hash }); Ok(()) @@ -505,9 +526,9 @@ pub mod pallet { let call_hash = T::CallHasher::hash_of(&call); let now = system::Pallet::::block_number(); Self::edit_announcements(&delegate, |ann| { - ann.real != real || - ann.call_hash != call_hash || - now.saturating_sub(ann.height) < def.delay + ann.real != real + || ann.call_hash != call_hash + || now.saturating_sub(ann.height) < def.delay }) .map_err(|_| Error::::Unannounced)?; @@ -580,9 +601,9 @@ pub mod pallet { ProxyDefinition>, T::MaxProxies, >, - BalanceOf, + ProxyTicketOf, ), - ValueQuery, + OptionQuery, >; /// The announcements made by the proxy (key). @@ -593,30 +614,38 @@ pub mod pallet { T::AccountId, ( BoundedVec, BlockNumberFor>, T::MaxPending>, - BalanceOf, + AnnouncementTicketOf, ), - ValueQuery, + OptionQuery, >; } impl Pallet { + const fn annoucement_size_bytes() -> usize { + core::mem::size_of::, BlockNumberFor>>() + } + + const fn proxy_def_size_bytes() -> usize { + core::mem::size_of::>>() + } + /// Public function to proxies storage. pub fn proxies( account: T::AccountId, - ) -> ( + ) -> Option<( BoundedVec>, T::MaxProxies>, - BalanceOf, - ) { + ProxyTicketOf, + )> { Proxies::::get(account) } /// Public function to announcements storage. pub fn announcements( account: T::AccountId, - ) -> ( + ) -> Option<( BoundedVec, BlockNumberFor>, T::MaxPending>, - BalanceOf, - ) { + AnnouncementTicketOf, + )> { Announcements::::get(account) } @@ -664,7 +693,23 @@ impl Pallet { delay: BlockNumberFor, ) -> DispatchResult { ensure!(delegator != &delegatee, Error::::NoSelfProxy); - Proxies::::try_mutate(delegator, |(ref mut proxies, ref mut deposit)| { + Proxies::::mutate_exists(delegator, |value| { + let (mut proxies, ticket) = if let Some(v) = value.take() { + v + } else { + let bounded_proxies: BoundedVec< + ProxyDefinition>, + T::MaxProxies, + > = Default::default(); + ( + bounded_proxies, + T::ProxyConsideration::new( + delegator, + Footprint::from_parts(0, Self::proxy_def_size_bytes()), + )?, + ) + }; + let proxy_def = ProxyDefinition { delegate: delegatee.clone(), proxy_type: proxy_type.clone(), @@ -672,13 +717,14 @@ impl Pallet { }; let i = proxies.binary_search(&proxy_def).err().ok_or(Error::::Duplicate)?; proxies.try_insert(i, proxy_def).map_err(|_| Error::::TooMany)?; - let new_deposit = Self::deposit(proxies.len() as u32); - if new_deposit > *deposit { - T::Currency::reserve(delegator, new_deposit - *deposit)?; - } else if new_deposit < *deposit { - T::Currency::unreserve(delegator, *deposit - new_deposit); - } - *deposit = new_deposit; + + let new_ticket = ticket.update( + delegator, + Footprint::from_parts(proxies.len(), Self::proxy_def_size_bytes()), + )?; + + *value = Some((proxies, new_ticket)); + Self::deposit_event(Event::::ProxyAdded { delegator: delegator.clone(), delegatee, @@ -704,7 +750,7 @@ impl Pallet { delay: BlockNumberFor, ) -> DispatchResult { Proxies::::try_mutate_exists(delegator, |x| { - let (mut proxies, old_deposit) = x.take().ok_or(Error::::NotFound)?; + let (mut proxies, ticket) = x.take().ok_or(Error::::NotFound)?; let proxy_def = ProxyDefinition { delegate: delegatee.clone(), proxy_type: proxy_type.clone(), @@ -712,14 +758,13 @@ impl Pallet { }; let i = proxies.binary_search(&proxy_def).ok().ok_or(Error::::NotFound)?; proxies.remove(i); - let new_deposit = Self::deposit(proxies.len() as u32); - if new_deposit > old_deposit { - T::Currency::reserve(delegator, new_deposit - old_deposit)?; - } else if new_deposit < old_deposit { - T::Currency::unreserve(delegator, old_deposit - new_deposit); - } + // If this was the last proxy, the ticket will be updated to 0. + let new_ticket = ticket.update( + delegator, + Footprint::from_parts(proxies.len(), Self::proxy_def_size_bytes()), + )?; if !proxies.is_empty() { - *x = Some((proxies, new_deposit)) + *x = Some((proxies, new_ticket)) } Self::deposit_event(Event::::ProxyRemoved { delegator: delegator.clone(), @@ -731,31 +776,6 @@ impl Pallet { }) } - pub fn deposit(num_proxies: u32) -> BalanceOf { - if num_proxies == 0 { - Zero::zero() - } else { - T::ProxyDepositBase::get() + T::ProxyDepositFactor::get() * num_proxies.into() - } - } - - fn rejig_deposit( - who: &T::AccountId, - old_deposit: BalanceOf, - base: BalanceOf, - factor: BalanceOf, - len: usize, - ) -> Result>, DispatchError> { - let new_deposit = - if len == 0 { BalanceOf::::zero() } else { base + factor * (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); - } - Ok(if len == 0 { None } else { Some(new_deposit) }) - } - fn edit_announcements< F: FnMut(&Announcement, BlockNumberFor>) -> bool, >( @@ -763,18 +783,15 @@ impl Pallet { f: F, ) -> DispatchResult { Announcements::::try_mutate_exists(delegate, |x| { - let (mut pending, old_deposit) = x.take().ok_or(Error::::NotFound)?; + let (mut pending, ticket) = x.take().ok_or(Error::::NotFound)?; let orig_pending_len = pending.len(); pending.retain(f); ensure!(orig_pending_len > pending.len(), Error::::NotFound); - *x = Self::rejig_deposit( + let new_ticket = ticket.update( delegate, - old_deposit, - T::AnnouncementDepositBase::get(), - T::AnnouncementDepositFactor::get(), - pending.len(), - )? - .map(|deposit| (pending, deposit)); + Footprint::from_parts(pending.len(), Self::annoucement_size_bytes()), + )?; + *x = Some((pending, new_ticket)); Ok(()) }) } @@ -785,10 +802,11 @@ impl Pallet { force_proxy_type: Option, ) -> Result>, DispatchError> { let f = |x: &ProxyDefinition>| -> bool { - &x.delegate == delegate && - force_proxy_type.as_ref().map_or(true, |y| &x.proxy_type == y) + &x.delegate == delegate + && force_proxy_type.as_ref().map_or(true, |y| &x.proxy_type == y) }; - Ok(Proxies::::get(real).0.into_iter().find(f).ok_or(Error::::NotProxy)?) + let (proxy, _) = Proxies::::get(real).ok_or(Error::::NotProxy)?; + Ok(proxy.into_iter().find(f).ok_or(Error::::NotProxy)?) } fn do_proxy( @@ -804,15 +822,19 @@ impl Pallet { match c.is_sub_type() { // Proxy call cannot add or remove a proxy with more permissions than it already // has. - Some(Call::add_proxy { ref proxy_type, .. }) | - Some(Call::remove_proxy { ref proxy_type, .. }) + Some(Call::add_proxy { ref proxy_type, .. }) + | Some(Call::remove_proxy { ref proxy_type, .. }) if !def.proxy_type.is_superset(proxy_type) => - false, + { + false + }, // Proxy call cannot remove all proxies or kill pure proxies unless it has full // permissions. Some(Call::remove_proxies { .. }) | Some(Call::kill_pure { .. }) if def.proxy_type != T::ProxyType::default() => - false, + { + false + }, _ => def.proxy_type.filter(c), } }); @@ -824,8 +846,10 @@ impl Pallet { /// /// Parameters: /// - `delegator`: The delegator account. - pub fn remove_all_proxy_delegates(delegator: &T::AccountId) { - let (_, old_deposit) = Proxies::::take(&delegator); - T::Currency::unreserve(&delegator, old_deposit); + pub fn remove_all_proxy_delegates(delegator: &T::AccountId) -> DispatchResult { + if let Some((_, ticket)) = Proxies::::take(&delegator) { + ticket.drop(&delegator)?; + } + Ok(()) } } diff --git a/substrate/frame/proxy/src/tests.rs b/substrate/frame/proxy/src/tests.rs index 3edb96026a82b..4ceeedbfaa30c 100644 --- a/substrate/frame/proxy/src/tests.rs +++ b/substrate/frame/proxy/src/tests.rs @@ -26,10 +26,14 @@ use alloc::{vec, vec::Vec}; use codec::{Decode, Encode}; use frame_support::{ assert_noop, assert_ok, derive_impl, - traits::{ConstU32, ConstU64, Contains}, + traits::{fungible::HoldConsideration, ConstU32, Contains, Currency}, }; use sp_core::H256; -use sp_runtime::{traits::BlakeTwo256, BuildStorage, DispatchError, RuntimeDebug}; +use sp_runtime::{ + traits::{BlakeTwo256, Convert}, + BuildStorage, DispatchError, RuntimeDebug, + TokenError::FundsUnavailable, +}; type Block = frame_system::mocking::MockBlock; @@ -114,24 +118,57 @@ impl Contains for BaseFilter { } } } + +#[derive(Default)] +pub struct ConvertDeposit; +impl Convert::Balance> for ConvertDeposit { + fn convert(a: Footprint) -> ::Balance { + a.count + } +} + +#[derive(Default)] +pub struct AnnouncementHoldReason; +impl Get for AnnouncementHoldReason { + fn get() -> RuntimeHoldReason { + RuntimeHoldReason::Proxy(crate::HoldReason::Announcement) + } +} + +#[derive(Default)] +pub struct ProxyHoldReason; +impl Get for ProxyHoldReason { + fn get() -> RuntimeHoldReason { + RuntimeHoldReason::Proxy(crate::HoldReason::Proxy) + } +} + impl Config for Test { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; type Currency = Balances; type ProxyType = ProxyType; - type ProxyDepositBase = ConstU64<1>; - type ProxyDepositFactor = ConstU64<1>; type MaxProxies = ConstU32<4>; type WeightInfo = (); type CallHasher = BlakeTwo256; type MaxPending = ConstU32<2>; - type AnnouncementDepositBase = ConstU64<1>; - type AnnouncementDepositFactor = ConstU64<1>; + type ProxyConsideration = HoldConsideration< + ::AccountId, + Balances, + ProxyHoldReason, + ConvertDeposit, + >; + type AnnouncementConsideration = HoldConsideration< + ::AccountId, + Balances, + AnnouncementHoldReason, + ConvertDeposit, + >; } use super::{Call as ProxyCall, Event as ProxyEvent}; use frame_system::Call as SystemCall; -use pallet_balances::{Call as BalancesCall, Event as BalancesEvent}; +use pallet_balances::{Call as BalancesCall}; use pallet_utility::{Call as UtilityCall, Event as UtilityEvent}; type SystemError = frame_system::Error; @@ -139,7 +176,7 @@ type SystemError = frame_system::Error; 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, 3)], + balances: vec![(1, 10), (2, 10), (3, 10), (4, 10), (5, 2)], } .assimilate_storage(&mut t) .unwrap(); @@ -180,18 +217,20 @@ fn announcement_works() { .into(), ); assert_ok!(Proxy::add_proxy(RuntimeOrigin::signed(2), 3, ProxyType::Any, 1)); + assert_eq!(Balances::reserved_balance(1), 1); + assert_eq!(Balances::reserved_balance(2), 1); assert_eq!(Balances::reserved_balance(3), 0); assert_ok!(Proxy::announce(RuntimeOrigin::signed(3), 1, [1; 32].into())); - let announcements = Announcements::::get(3); + let announcements = Announcements::::get(3).unwrap(); assert_eq!( announcements.0, vec![Announcement { real: 1, call_hash: [1; 32].into(), height: 1 }] ); - assert_eq!(Balances::reserved_balance(3), announcements.1); + assert_eq!(Balances::reserved_balance(3), 1); assert_ok!(Proxy::announce(RuntimeOrigin::signed(3), 2, [2; 32].into())); - let announcements = Announcements::::get(3); + let announcements = Announcements::::get(3).unwrap(); assert_eq!( announcements.0, vec![ @@ -199,7 +238,7 @@ fn announcement_works() { Announcement { real: 2, call_hash: [2; 32].into(), height: 1 }, ] ); - assert_eq!(Balances::reserved_balance(3), announcements.1); + assert_eq!(Balances::reserved_balance(3), 2); assert_noop!( Proxy::announce(RuntimeOrigin::signed(3), 2, [3; 32].into()), @@ -218,12 +257,12 @@ fn remove_announcement_works() { let e = Error::::NotFound; assert_noop!(Proxy::remove_announcement(RuntimeOrigin::signed(3), 1, [0; 32].into()), e); assert_ok!(Proxy::remove_announcement(RuntimeOrigin::signed(3), 1, [1; 32].into())); - let announcements = Announcements::::get(3); + let announcements = Announcements::::get(3).unwrap(); assert_eq!( announcements.0, vec![Announcement { real: 2, call_hash: [2; 32].into(), height: 1 }] ); - assert_eq!(Balances::reserved_balance(3), announcements.1); + assert_eq!(Balances::reserved_balance(3), 1); }); } @@ -239,12 +278,12 @@ fn reject_announcement_works() { let e = Error::::NotFound; assert_noop!(Proxy::reject_announcement(RuntimeOrigin::signed(4), 3, [1; 32].into()), e); assert_ok!(Proxy::reject_announcement(RuntimeOrigin::signed(1), 3, [1; 32].into())); - let announcements = Announcements::::get(3); + let announcements = Announcements::::get(3).unwrap(); assert_eq!( announcements.0, vec![Announcement { real: 2, call_hash: [2; 32].into(), height: 1 }] ); - assert_eq!(Balances::reserved_balance(3), announcements.1); + assert_eq!(Balances::reserved_balance(3), 1); }); } @@ -270,7 +309,7 @@ fn calling_proxy_doesnt_remove_announcement() { assert_ok!(Proxy::proxy(RuntimeOrigin::signed(2), 1, None, call)); // The announcement is not removed by calling proxy. - let announcements = Announcements::::get(2); + let announcements = Announcements::::get(2).unwrap(); assert_eq!(announcements.0, vec![Announcement { real: 1, call_hash, height: 1 }]); }); } @@ -306,9 +345,9 @@ fn proxy_announced_removes_announcement_and_returns_deposit() { system::Pallet::::set_block_number(2); assert_ok!(Proxy::proxy_announced(RuntimeOrigin::signed(0), 3, 1, None, call.clone())); - let announcements = Announcements::::get(3); + let announcements = Announcements::::get(3).unwrap(); assert_eq!(announcements.0, vec![Announcement { real: 2, call_hash, height: 1 }]); - assert_eq!(Balances::reserved_balance(3), announcements.1); + assert_eq!(Balances::reserved_balance(3), 1); }); } @@ -398,10 +437,7 @@ fn filtering_works() { ProxyEvent::ProxyExecuted { result: Err(SystemError::CallFiltered.into()) }.into(), ); assert_ok!(Proxy::proxy(RuntimeOrigin::signed(2), 1, None, call.clone())); - expect_events(vec![ - BalancesEvent::::Unreserved { who: 1, amount: 5 }.into(), - ProxyEvent::ProxyExecuted { result: Ok(()) }.into(), - ]); + expect_events(vec![ProxyEvent::ProxyExecuted { result: Ok(()) }.into()]); }); } @@ -413,13 +449,13 @@ fn add_remove_proxies_works() { Proxy::add_proxy(RuntimeOrigin::signed(1), 2, ProxyType::Any, 0), Error::::Duplicate ); - assert_eq!(Balances::reserved_balance(1), 2); + assert_eq!(Balances::reserved_balance(1), 1); assert_ok!(Proxy::add_proxy(RuntimeOrigin::signed(1), 2, ProxyType::JustTransfer, 0)); - assert_eq!(Balances::reserved_balance(1), 3); + assert_eq!(Balances::reserved_balance(1), 2); assert_ok!(Proxy::add_proxy(RuntimeOrigin::signed(1), 3, ProxyType::Any, 0)); - assert_eq!(Balances::reserved_balance(1), 4); + assert_eq!(Balances::reserved_balance(1), 3); assert_ok!(Proxy::add_proxy(RuntimeOrigin::signed(1), 4, ProxyType::JustUtility, 0)); - assert_eq!(Balances::reserved_balance(1), 5); + assert_eq!(Balances::reserved_balance(1), 4); assert_noop!( Proxy::add_proxy(RuntimeOrigin::signed(1), 4, ProxyType::Any, 0), Error::::TooMany @@ -438,9 +474,9 @@ fn add_remove_proxies_works() { } .into(), ); - assert_eq!(Balances::reserved_balance(1), 4); - assert_ok!(Proxy::remove_proxy(RuntimeOrigin::signed(1), 3, ProxyType::Any, 0)); assert_eq!(Balances::reserved_balance(1), 3); + assert_ok!(Proxy::remove_proxy(RuntimeOrigin::signed(1), 3, ProxyType::Any, 0)); + assert_eq!(Balances::reserved_balance(1), 2); System::assert_last_event( ProxyEvent::ProxyRemoved { delegator: 1, @@ -451,7 +487,7 @@ fn add_remove_proxies_works() { .into(), ); assert_ok!(Proxy::remove_proxy(RuntimeOrigin::signed(1), 2, ProxyType::Any, 0)); - assert_eq!(Balances::reserved_balance(1), 2); + assert_eq!(Balances::reserved_balance(1), 1); System::assert_last_event( ProxyEvent::ProxyRemoved { delegator: 1, @@ -476,6 +512,7 @@ fn add_remove_proxies_works() { Proxy::add_proxy(RuntimeOrigin::signed(1), 1, ProxyType::Any, 0), Error::::NoSelfProxy ); + assert_eq!(Balances::reserved_balance(1), 0); }); } @@ -483,10 +520,10 @@ fn add_remove_proxies_works() { fn cannot_add_proxy_without_balance() { new_test_ext().execute_with(|| { assert_ok!(Proxy::add_proxy(RuntimeOrigin::signed(5), 3, ProxyType::Any, 0)); - assert_eq!(Balances::reserved_balance(5), 2); + assert_eq!(Balances::reserved_balance(5), 1); assert_noop!( Proxy::add_proxy(RuntimeOrigin::signed(5), 4, ProxyType::Any, 0), - DispatchError::ConsumerRemaining, + DispatchError::Token(FundsUnavailable), ); }); } @@ -582,9 +619,9 @@ fn pure_works() { Proxy::kill_pure(RuntimeOrigin::signed(1), 1, ProxyType::Any, 0, 1, 0), Error::::NoPermission ); - assert_eq!(Balances::free_balance(1), 1); + assert_eq!(Balances::free_balance(1), 6); assert_ok!(Proxy::proxy(RuntimeOrigin::signed(1), anon, None, call.clone())); - assert_eq!(Balances::free_balance(1), 3); + assert_eq!(Balances::free_balance(1), 7); assert_noop!( Proxy::proxy(RuntimeOrigin::signed(1), anon, None, call.clone()), Error::::NotProxy diff --git a/substrate/frame/support/src/storage/types/map.rs b/substrate/frame/support/src/storage/types/map.rs index b70026eea50e1..ac993d7d18aa6 100644 --- a/substrate/frame/support/src/storage/types/map.rs +++ b/substrate/frame/support/src/storage/types/map.rs @@ -216,7 +216,7 @@ where >::try_mutate(key, f) } - /// Mutate the value under a key iff it exists. Do nothing and return the default value if not. + /// Mutate the value under a key if it exists. Do nothing and return the default value if not. pub fn mutate_extant, R: Default, F: FnOnce(&mut Value) -> R>( key: KeyArg, f: F,