Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Next Next commit
Self-sufficient account ref-counting
  • Loading branch information
gavofyork committed Feb 27, 2021
commit 1a5e42cc6942a93893e9bcd7d5539edfd817e346
146 changes: 112 additions & 34 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
if !UpgradedToDualRefCount::<T>::get() {
UpgradedToDualRefCount::<T>::put(true);
migrations::migrate_to_dual_ref_count::<T>()
if !UpgradedToTripleRefCount::<T>::get() {
UpgradedToTripleRefCount::<T>::put(true);
migrations::migrate_to_triple_ref_count::<T>()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not call the migration explicitly in the particular runtimes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure - i'm just concerned about it being forgotten.

} else {
0
}
Expand Down Expand Up @@ -580,10 +580,10 @@ pub mod pallet {
#[pallet::storage]
pub(super) type UpgradedToU32RefCount<T: Config> = StorageValue<_, bool, ValueQuery>;

/// True if we have upgraded so that AccountInfo contains two types of `RefCount`. False
/// True if we have upgraded so that AccountInfo contains three types of `RefCount`. False
/// (default) if not.
#[pallet::storage]
pub(super) type UpgradedToDualRefCount<T: Config> = StorageValue<_, bool, ValueQuery>;
pub(super) type UpgradedToTripleRefCount<T: Config> = StorageValue<_, bool, ValueQuery>;

/// The execution phase of the block.
#[pallet::storage]
Expand Down Expand Up @@ -613,7 +613,7 @@ pub mod pallet {
<ParentHash<T>>::put::<T::Hash>(hash69());
<LastRuntimeUpgrade<T>>::put(LastRuntimeUpgradeInfo::from(T::Version::get()));
<UpgradedToU32RefCount<T>>::put(true);
<UpgradedToDualRefCount<T>>::put(true);
<UpgradedToTripleRefCount<T>>::put(true);

sp_io::storage::set(well_known_keys::CODE, &self.code);
sp_io::storage::set(well_known_keys::EXTRINSIC_INDEX, &0u32.encode());
Expand All @@ -630,14 +630,23 @@ mod migrations {
#[allow(dead_code)]
pub fn migrate_all<T: Config>() -> frame_support::weights::Weight {
Account::<T>::translate::<(T::Index, u8, T::AccountData), _>(|_key, (nonce, rc, data)|
Some(AccountInfo { nonce, consumers: rc as RefCount, providers: 1, data })
Some(AccountInfo { nonce, consumers: rc as RefCount, providers: 1, sufficients: 0, data })
);
T::BlockWeights::get().max_block
}

#[allow(dead_code)]
pub fn migrate_to_dual_ref_count<T: Config>() -> frame_support::weights::Weight {
Account::<T>::translate::<(T::Index, RefCount, T::AccountData), _>(|_key, (nonce, rc, data)|
Some(AccountInfo { nonce, consumers: rc as RefCount, providers: 1, data })
Account::<T>::translate::<(T::Index, RefCount, T::AccountData), _>(|_key, (nonce, consumers, data)|
Some(AccountInfo { nonce, consumers, providers: 1, sufficients: 0, data })
);
T::BlockWeights::get().max_block
}

/// Must be used.
pub fn migrate_to_triple_ref_count<T: Config>() -> frame_support::weights::Weight {
Account::<T>::translate::<(T::Index, RefCount, RefCount, T::AccountData), _>(|_key, (nonce, consumers, providers, data)|
Some(AccountInfo { nonce, consumers, providers, sufficients: 0, data })
);
T::BlockWeights::get().max_block
}
Expand Down Expand Up @@ -748,8 +757,11 @@ pub struct AccountInfo<Index, AccountData> {
/// cannot be reaped until this is zero.
pub consumers: RefCount,
/// The number of other modules that allow this account to exist. The account may not be reaped
/// until this is zero.
/// until this and `sufficients` are both zero.
pub providers: RefCount,
/// The number of modules that allow this account to exist for their own purposes only. The
/// account may not be reaped until this and `providers` are both zero.
pub sufficients: RefCount,
/// The additional data that belongs to this account. Used to store the balance(s) in a lot of
/// chains.
pub data: AccountData,
Expand Down Expand Up @@ -960,17 +972,17 @@ pub enum RefStatus {
Unreferenced,
}

/// Some resultant status relevant to incrementing a provider reference.
#[derive(RuntimeDebug)]
/// Some resultant status relevant to incrementing a provider/self-sufficient reference.
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum IncRefStatus {
/// Account was created.
Created,
/// Account already existed.
Existed,
}

/// Some resultant status relevant to decrementing a provider reference.
#[derive(RuntimeDebug)]
/// Some resultant status relevant to decrementing a provider/self-sufficient reference.
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum DecRefStatus {
/// Account was destroyed.
Reaped,
Expand All @@ -979,14 +991,14 @@ pub enum DecRefStatus {
}

/// Some resultant status relevant to decrementing a provider reference.
#[derive(RuntimeDebug)]
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum DecRefError {
/// Account cannot have the last provider reference removed while there is a consumer.
ConsumerRemaining,
}

/// Some resultant status relevant to incrementing a provider reference.
#[derive(RuntimeDebug)]
/// Some resultant status relevant to incrementing a consumer reference.
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum IncRefError {
/// Account cannot introduce a consumer while there are no providers.
NoProviders,
Expand Down Expand Up @@ -1022,11 +1034,9 @@ impl<T: Config> Module<T> {
!Self::is_provider_required(who)
}

/// Increment the reference counter on an account.
///
/// The account `who`'s `providers` must be non-zero or this will return an error.
/// Increment the provider reference counter on an account.
pub fn inc_providers(who: &T::AccountId) -> IncRefStatus {
Account::<T>::mutate(who, |a| if a.providers == 0 {
Account::<T>::mutate(who, |a| if a.providers == 0 && a.sufficients == 0 {
// Account is being created.
a.providers = 1;
Self::on_created_account(who.clone(), a);
Expand All @@ -1037,27 +1047,30 @@ impl<T: Config> Module<T> {
})
}

/// Decrement the reference counter on an account. This *MUST* only be done once for every time
/// you called `inc_consumers` on `who`.
/// Decrement the provider reference counter on an account.
///
/// This *MUST* only be done once for every time you called `inc_providers` on `who`.
pub fn dec_providers(who: &T::AccountId) -> Result<DecRefStatus, DecRefError> {
Account::<T>::try_mutate_exists(who, |maybe_account| {
if let Some(mut account) = maybe_account.take() {
match (account.providers, account.consumers) {
(0, _) => {
// Logic error - cannot decrement beyond zero and no item should
// exist with zero providers.
debug::print!("Logic error: Unexpected underflow in reducing provider");
Ok(DecRefStatus::Reaped)
},
(1, 0) => {
if account.providers == 0 {
// Logic error - cannot decrement beyond zero.
debug::print!("Logic error: Unexpected underflow in reducing sufficients");
account.providers = 1;
}
match (account.providers, account.consumers, account.sufficients) {
(1, 0, 0) => {
// No providers left (and no consumers) and no sufficients. Account dead.
Module::<T>::on_killed_account(who.clone());
Ok(DecRefStatus::Reaped)
}
(1, _) => {
(1, c, _) if c > 0 => {
// Cannot remove last provider if there are consumers.
Err(DecRefError::ConsumerRemaining)
}
(x, _) => {
(x, _, _) => {
// Account will continue to exist as there is either > 1 provider or
// > 0 sufficients.
account.providers = x - 1;
*maybe_account = Some(account);
Ok(DecRefStatus::Exists)
Expand All @@ -1070,11 +1083,63 @@ impl<T: Config> Module<T> {
})
}

/// The number of outstanding references for the account `who`.
/// Increment the self-sufficient reference counter on an account.
pub fn inc_sufficients(who: &T::AccountId) -> IncRefStatus {
Account::<T>::mutate(who, |a| if a.providers + a.sufficients == 0 {
// Account is being created.
a.sufficients = 1;
Self::on_created_account(who.clone(), a);
IncRefStatus::Created
} else {
a.sufficients = a.sufficients.saturating_add(1);
IncRefStatus::Existed
})
}

/// Decrement the sufficients reference counter on an account.
///
/// This *MUST* only be done once for every time you called `inc_sufficients` on `who`.
pub fn dec_sufficients(who: &T::AccountId) -> DecRefStatus {
Account::<T>::mutate_exists(who, |maybe_account| {
if let Some(mut account) = maybe_account.take() {
if account.sufficients == 0 {
// Logic error - cannot decrement beyond zero.
debug::print!("Logic error: Unexpected underflow in reducing sufficients");
}
match (account.sufficients, account.providers) {
(0, 0) | (1, 0) => {
Module::<T>::on_killed_account(who.clone());
DecRefStatus::Reaped
}
(x, _) => {
account.sufficients = x - 1;
*maybe_account = Some(account);
DecRefStatus::Exists
}
}
} else {
debug::print!("Logic error: Account already dead when reducing provider");
DecRefStatus::Reaped
}
})
}

/// The number of outstanding provider references for the account `who`.
pub fn providers(who: &T::AccountId) -> RefCount {
Account::<T>::get(who).providers
}

/// The number of outstanding provider references for the account `who`.
pub fn sufficients(who: &T::AccountId) -> RefCount {
Account::<T>::get(who).sufficients
}

/// The number of outstanding provider and sufficient references for the account `who`.
pub fn reference_count(who: &T::AccountId) -> RefCount {
let a = Account::<T>::get(who);
a.providers + a.sufficients
}

/// Increment the reference counter on an account.
///
/// The account `who`'s `providers` must be non-zero or this will return an error.
Expand Down Expand Up @@ -1428,6 +1493,19 @@ impl<T: Config> HandleLifetime<T::AccountId> for Provider<T> {
}
}

/// Event handler which registers a self-sufficient when created.
pub struct SelfSufficient<T>(PhantomData<T>);
impl<T: Config> HandleLifetime<T::AccountId> for SelfSufficient<T> {
fn created(t: &T::AccountId) -> Result<(), StoredMapError> {
Module::<T>::inc_sufficients(t);
Ok(())
}
fn killed(t: &T::AccountId) -> Result<(), StoredMapError> {
Module::<T>::dec_sufficients(t);
Ok(())
}
}

/// Event handler which registers a consumer when created.
pub struct Consumer<T>(PhantomData<T>);
impl<T: Config> HandleLifetime<T::AccountId> for Consumer<T> {
Expand Down
101 changes: 95 additions & 6 deletions frame/system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ use crate::*;
use mock::{*, Origin};
use sp_core::H256;
use sp_runtime::{DispatchError, DispatchErrorWithPostInfo, traits::{Header, BlakeTwo256}};
use frame_support::{
weights::WithPostDispatchInfo,
dispatch::PostDispatchInfo,
};
use frame_support::{assert_noop, weights::WithPostDispatchInfo, dispatch::PostDispatchInfo};

#[test]
fn origin_works() {
Expand All @@ -37,7 +34,7 @@ fn stored_map_works() {
assert!(System::insert(&0, 42).is_ok());
assert!(!System::is_provider_required(&0));

assert_eq!(Account::<Test>::get(0), AccountInfo { nonce: 0, providers: 1, consumers: 0, data: 42 });
assert_eq!(Account::<Test>::get(0), AccountInfo { nonce: 0, providers: 1, consumers: 0, sufficients: 0, data: 42 });

assert!(System::inc_consumers(&0).is_ok());
assert!(System::is_provider_required(&0));
Expand All @@ -54,6 +51,98 @@ fn stored_map_works() {
});
}

#[test]
fn provider_ref_handover_to_self_sufficient_ref_works() {
new_test_ext().execute_with(|| {
assert_eq!(System::inc_providers(&0), IncRefStatus::Created);
System::inc_account_nonce(&0);
assert_eq!(System::account_nonce(&0), 1);

// a second reference coming and going doesn't change anything.
assert_eq!(System::inc_sufficients(&0), IncRefStatus::Existed);
assert_eq!(System::dec_sufficients(&0), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// a provider reference coming and going doesn't change anything.
assert_eq!(System::inc_providers(&0), IncRefStatus::Existed);
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// decreasing the providers with a self-sufficient present should not delete the account
assert_eq!(System::inc_sufficients(&0), IncRefStatus::Existed);
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// decreasing the sufficients should delete the account
assert_eq!(System::dec_sufficients(&0), DecRefStatus::Reaped);
assert_eq!(System::account_nonce(&0), 0);
});
}

#[test]
fn self_sufficient_ref_handover_to_provider_ref_works() {
new_test_ext().execute_with(|| {
assert_eq!(System::inc_sufficients(&0), IncRefStatus::Created);
System::inc_account_nonce(&0);
assert_eq!(System::account_nonce(&0), 1);

// a second reference coming and going doesn't change anything.
assert_eq!(System::inc_providers(&0), IncRefStatus::Existed);
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// a sufficient reference coming and going doesn't change anything.
assert_eq!(System::inc_sufficients(&0), IncRefStatus::Existed);
assert_eq!(System::dec_sufficients(&0), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// decreasing the sufficients with a provider present should not delete the account
assert_eq!(System::inc_providers(&0), IncRefStatus::Existed);
assert_eq!(System::dec_sufficients(&0), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

// decreasing the providers should delete the account
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Reaped);
assert_eq!(System::account_nonce(&0), 0);
});
}

#[test]
fn sufficient_cannot_support_consumer() {
new_test_ext().execute_with(|| {
assert_eq!(System::inc_sufficients(&0), IncRefStatus::Created);
System::inc_account_nonce(&0);
assert_eq!(System::account_nonce(&0), 1);
assert_noop!(System::inc_consumers(&0), IncRefError::NoProviders);

assert_eq!(System::inc_providers(&0), IncRefStatus::Existed);
assert!(System::inc_consumers(&0).is_ok());
assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining);
});
}

#[test]
fn provider_required_to_support_consumer() {
new_test_ext().execute_with(|| {
assert_noop!(System::inc_consumers(&0), IncRefError::NoProviders);

assert_eq!(System::inc_providers(&0), IncRefStatus::Created);
System::inc_account_nonce(&0);
assert_eq!(System::account_nonce(&0), 1);

assert_eq!(System::inc_providers(&0), IncRefStatus::Existed);
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Exists);
assert_eq!(System::account_nonce(&0), 1);

assert!(System::inc_consumers(&0).is_ok());
assert_noop!(System::dec_providers(&0), DecRefError::ConsumerRemaining);

System::dec_consumers(&0);
assert_eq!(System::dec_providers(&0).unwrap(), DecRefStatus::Reaped);
assert_eq!(System::account_nonce(&0), 0);
});
}

#[test]
fn deposit_event_should_work() {
new_test_ext().execute_with(|| {
Expand Down Expand Up @@ -403,7 +492,7 @@ fn events_not_emitted_during_genesis() {
new_test_ext().execute_with(|| {
// Block Number is zero at genesis
assert!(System::block_number().is_zero());
let mut account_data = AccountInfo { nonce: 0, consumers: 0, providers: 0, data: 0 };
let mut account_data = AccountInfo { nonce: 0, consumers: 0, providers: 0, sufficients: 0, data: 0 };
System::on_created_account(Default::default(), &mut account_data);
assert!(System::events().is_empty());
// Events will be emitted starting on block 1
Expand Down