Skip to content

Commit

Permalink
Fix update_lock (paritytech#10485)
Browse files Browse the repository at this point in the history
* Fix update_lock

* Fixes

* Formatting

* add `inc_consumers_without_limits` to session too

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
  • Loading branch information
2 people authored and ark0f committed Feb 27, 2023
1 parent 24b8a1c commit f5316e9
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 11 deletions.
5 changes: 3 additions & 2 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
} else {
Locks::<T, I>::insert(who, bounded_locks);
if !existed {
if system::Pallet::<T>::inc_consumers(who).is_err() {
if system::Pallet::<T>::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.
Expand Down
2 changes: 1 addition & 1 deletion frame/session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ pub mod pallet {
for (account, val, keys) in self.keys.iter().cloned() {
<Pallet<T>>::inner_set_keys(&val, keys)
.expect("genesis config must not contain duplicates; qed");
if frame_system::Pallet::<T>::inc_consumers(&account).is_err() {
if frame_system::Pallet::<T>::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
Expand Down
58 changes: 51 additions & 7 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -154,6 +154,36 @@ impl<T: Config> SetCode<T> 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<const Z: u32> ConsumerLimits for ConstU32<Z> {
fn max_consumers() -> RefCount {
Z
}
fn max_overflow() -> RefCount {
Z
}
}

impl<MaxNormal: Get<u32>, MaxOverflow: Get<u32>> 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::*, *};
Expand Down Expand Up @@ -310,8 +340,7 @@ pub mod pallet {
type OnSetCode: SetCode<Self>;

/// The maximum number of consumers allowed on a single account.
#[pallet::constant]
type MaxConsumers: Get<RefCount>;
type MaxConsumers: ConsumerLimits;
}

#[pallet::pallet]
Expand Down Expand Up @@ -1122,11 +1151,12 @@ impl<T: Config> Pallet<T> {

/// 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::max_consumers()` or this will return an error.
pub fn inc_consumers(who: &T::AccountId) -> Result<(), DispatchError> {
Account::<T>::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 {
Expand All @@ -1138,6 +1168,20 @@ impl<T: Config> Pallet<T> {
})
}

/// 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> {
Account::<T>::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) {
Expand Down Expand Up @@ -1172,7 +1216,7 @@ impl<T: Config> Pallet<T> {
/// True if the account has at least one provider reference.
pub fn can_inc_consumer(who: &T::AccountId) -> bool {
let a = Account::<T>::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.
Expand Down

0 comments on commit f5316e9

Please sign in to comment.