From 9ea1c97c800efa2d19cde7346c60a18bd9c75bf9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 14 Dec 2021 19:32:01 +0100 Subject: [PATCH 1/4] Fix update_lock --- frame/balances/src/lib.rs | 2 +- frame/system/src/lib.rs | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 4471ed91a9afc..6919c66f7211e 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -971,7 +971,7 @@ impl, I: 'static> Pallet { } else { Locks::::insert(who, bounded_locks); if !existed { - if system::Pallet::::inc_consumers(who).is_err() { + if system::Pallet::::inc_consumers_without_limit(who).is_err() { // No providers for the locks. This is impossible under normal circumstances // since the funds that are under the lock will themselves be stored in the // account and therefore will need a reference. diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 6bdaf8bd4b505..ce9b300c6fe37 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -1122,7 +1122,8 @@ impl Pallet { /// Increment the reference counter on an account. /// - /// The account `who`'s `providers` must be non-zero or this will return an error. + /// The account `who`'s `providers` must be non-zero and the current number of consumers must + /// be less than `MaxConsumers` or this will return an error. pub fn inc_consumers(who: &T::AccountId) -> Result<(), DispatchError> { Account::::try_mutate(who, |a| { if a.providers > 0 { @@ -1138,6 +1139,20 @@ impl Pallet { }) } + /// Increment the reference counter on an account, ignoring the `MaxConsumers` limit. + /// + /// The account `who`'s `providers` must be non-zero or this will return an error. + pub fn inc_consumers_without_limit(who: &T::AccountId) -> Result<(), DispatchError> { + Account::::try_mutate(who, |a| { + if a.providers > 0 { + a.consumers = a.consumers.saturating_add(1); + Ok(()) + } else { + Err(DispatchError::NoProviders) + } + }) + } + /// Decrement the reference counter on an account. This *MUST* only be done once for every time /// you called `inc_consumers` on `who`. pub fn dec_consumers(who: &T::AccountId) { From 3a322a6cd80ca33ba8e3e7e20432dc2875070873 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 14 Dec 2021 20:09:57 +0100 Subject: [PATCH 2/4] Fixes --- frame/system/src/lib.rs | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index ce9b300c6fe37..a449cd160b393 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -85,8 +85,8 @@ use frame_support::{ dispatch::{DispatchResult, DispatchResultWithPostInfo}, storage, traits::{ - Contains, EnsureOrigin, Get, HandleLifetime, OnKilledAccount, OnNewAccount, OriginTrait, - PalletInfo, SortedMembers, StoredMap, + ConstU32, Contains, EnsureOrigin, Get, HandleLifetime, OnKilledAccount, OnNewAccount, + OriginTrait, PalletInfo, SortedMembers, StoredMap, }, weights::{ extract_actual_weight, DispatchClass, DispatchInfo, PerDispatchClass, RuntimeDbWeight, @@ -154,6 +154,32 @@ impl SetCode for () { } } +/// Numeric limits over the ability to add a consumer ref using `inc_consumers`. +pub trait ConsumerLimits { + /// The number of consumers over which `inc_consumers` will cease to work. + fn max_consumers() -> RefCount; + /// The maximum number of additional consumers expected to be over be added at once using + /// `inc_consumers_without_limit`. + /// + /// Note: This is not enforced and it's up to the chain's author to ensure this reflects the + /// actual situation. + fn max_overflow() -> RefCount; +} + +impl ConsumerLimits for ConstU32 { + fn max_consumers() -> RefCount { Z } + fn max_overflow() -> RefCount { Z } +} + +impl, MaxOverflow: Get> ConsumerLimits for (MaxNormal, MaxOverflow) { + fn max_consumers() -> RefCount { + MaxNormal::get() + } + fn max_overflow() -> RefCount { + MaxOverflow::get() + } +} + #[frame_support::pallet] pub mod pallet { use crate::{self as frame_system, pallet_prelude::*, *}; @@ -310,8 +336,7 @@ pub mod pallet { type OnSetCode: SetCode; /// The maximum number of consumers allowed on a single account. - #[pallet::constant] - type MaxConsumers: Get; + type MaxConsumers: ConsumerLimits; } #[pallet::pallet] @@ -1123,11 +1148,11 @@ impl Pallet { /// Increment the reference counter on an account. /// /// The account `who`'s `providers` must be non-zero and the current number of consumers must - /// be less than `MaxConsumers` or this will return an error. + /// be less than `MaxConsumers::max_consumers()` or this will return an error. pub fn inc_consumers(who: &T::AccountId) -> Result<(), DispatchError> { Account::::try_mutate(who, |a| { if a.providers > 0 { - if a.consumers < T::MaxConsumers::get() { + if a.consumers < T::MaxConsumers::max_consumers() { a.consumers = a.consumers.saturating_add(1); Ok(()) } else { @@ -1139,7 +1164,7 @@ impl Pallet { }) } - /// Increment the reference counter on an account, ignoring the `MaxConsumers` limit. + /// Increment the reference counter on an account, ignoring the `MaxConsumers` limits. /// /// The account `who`'s `providers` must be non-zero or this will return an error. pub fn inc_consumers_without_limit(who: &T::AccountId) -> Result<(), DispatchError> { @@ -1187,7 +1212,7 @@ impl Pallet { /// True if the account has at least one provider reference. pub fn can_inc_consumer(who: &T::AccountId) -> bool { let a = Account::::get(who); - a.providers > 0 && a.consumers < T::MaxConsumers::get() + a.providers > 0 && a.consumers < T::MaxConsumers::max_consumers() } /// Deposits an event into this block's event record. From 87345f0b1dd15991a853408bdff7340ff6a83c5d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 14 Dec 2021 20:14:16 +0100 Subject: [PATCH 3/4] Formatting --- frame/system/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index a449cd160b393..e167f44ffd88c 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -167,8 +167,12 @@ pub trait ConsumerLimits { } impl ConsumerLimits for ConstU32 { - fn max_consumers() -> RefCount { Z } - fn max_overflow() -> RefCount { Z } + fn max_consumers() -> RefCount { + Z + } + fn max_overflow() -> RefCount { + Z + } } impl, MaxOverflow: Get> ConsumerLimits for (MaxNormal, MaxOverflow) { From 77a5ff3afec848309b0c2a149b3d1be201675363 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 14 Dec 2021 17:07:20 -0400 Subject: [PATCH 4/4] add `inc_consumers_without_limits` to session too --- frame/assets/src/lib.rs | 5 +++-- frame/session/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index b86661d9fa4fd..6643cc177460c 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -467,8 +467,9 @@ pub mod pallet { BadWitness, /// Minimum balance should be non-zero. MinBalanceZero, - /// No provider reference exists to allow a non-zero balance of a non-self-sufficient - /// asset. + /// Unable to increment the consumer reference counters on the account. Either no provider + /// reference exists to allow a non-zero balance of a non-self-sufficient asset, or the + /// maximum number of consumers has been reached. NoProvider, /// Invalid metadata given. BadMetadata, diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 0f80494550849..e88f143065052 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -448,7 +448,7 @@ pub mod pallet { for (account, val, keys) in self.keys.iter().cloned() { >::inner_set_keys(&val, keys) .expect("genesis config must not contain duplicates; qed"); - if frame_system::Pallet::::inc_consumers(&account).is_err() { + if frame_system::Pallet::::inc_consumers_without_limit(&account).is_err() { // This will leak a provider reference, however it only happens once (at // genesis) so it's really not a big deal and we assume that the user wants to // do this since it's the only way a non-endowed account can contain a session