Self-sufficient account ref-counting (#8221)

* Self-sufficient account ref-counting

* Fixes

* Update frame/system/src/lib.rs

Co-authored-by: Jaco Greeff <jacogr@gmail.com>

* Fixes

* Fixes

* Fixes

* Fixes

* Fixes

* Update frame/system/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Update frame/system/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Update frame/system/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* fix build

* Update frame/system/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

Co-authored-by: Jaco Greeff <jacogr@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This commit is contained in:
Gavin Wood
2021-03-03 22:28:05 +01:00
committed by GitHub
parent 274e7f0652
commit e993f884fc
7 changed files with 260 additions and 53 deletions
@@ -128,6 +128,7 @@ mod tests {
nonce: 1,
consumers: 0,
providers: 0,
sufficients: 0,
data: 0,
});
let info = DispatchInfo::default();
+126 -37
View File
@@ -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>()
} else {
0
}
@@ -594,10 +594,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]
@@ -627,7 +627,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());
@@ -642,16 +642,29 @@ mod migrations {
use super::*;
#[allow(dead_code)]
/// Migrate from unique `u8` reference counting to triple `u32` reference counting.
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)]
/// Migrate from unique `u32` reference counting to triple `u32` reference counting.
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
}
/// Migrate from dual `u32` reference counting to triple `u32` reference counting.
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
}
@@ -762,8 +775,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,
@@ -974,8 +990,8 @@ 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,
@@ -983,8 +999,8 @@ pub enum IncRefStatus {
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,
@@ -993,14 +1009,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,
@@ -1036,11 +1052,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);
@@ -1051,30 +1065,34 @@ 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.
log::error!(
target: "runtime::system",
"Logic error: Unexpected underflow in reducing provider",
);
Ok(DecRefStatus::Reaped)
},
(1, 0) => {
if account.providers == 0 {
// Logic error - cannot decrement beyond zero.
log::error!(
target: "runtime::system",
"Logic error: Unexpected underflow in reducing provider",
);
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)
@@ -1090,11 +1108,69 @@ 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.
log::error!(
target: "runtime::system",
"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 {
log::error!(
target: "runtime::system",
"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 sufficient 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.
@@ -1451,6 +1527,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> {
+101 -6
View File
@@ -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() {
@@ -37,7 +34,13 @@ 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));
@@ -54,6 +57,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(|| {
@@ -403,7 +498,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::default();
System::on_created_account(Default::default(), &mut account_data);
assert!(System::events().is_empty());
// Events will be emitted starting on block 1