From a82543f04ad239d9977dc6f5074f74976e727cfc Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Thu, 3 Aug 2023 17:06:13 +0200 Subject: [PATCH 1/6] WIP --- bin/node/runtime/src/lib.rs | 49 ++++++++++----- frame/assets/src/impl_fungibles.rs | 95 ++++++++++++++++++++++++++++++ frame/assets/src/lib.rs | 25 +++++++- frame/assets/src/types.rs | 13 +++- 4 files changed, 165 insertions(+), 17 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 94ae7e53237e2..4f921da4ac5df 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -311,21 +311,21 @@ impl InstanceFilter for ProxyType { ProxyType::Any => true, ProxyType::NonTransfer => !matches!( c, - RuntimeCall::Balances(..) | - RuntimeCall::Assets(..) | - RuntimeCall::Uniques(..) | - RuntimeCall::Nfts(..) | - RuntimeCall::Vesting(pallet_vesting::Call::vested_transfer { .. }) | - RuntimeCall::Indices(pallet_indices::Call::transfer { .. }) + RuntimeCall::Balances(..) + | RuntimeCall::Assets(..) + | RuntimeCall::Uniques(..) + | RuntimeCall::Nfts(..) + | RuntimeCall::Vesting(pallet_vesting::Call::vested_transfer { .. }) + | RuntimeCall::Indices(pallet_indices::Call::transfer { .. }) ), ProxyType::Governance => matches!( c, - RuntimeCall::Democracy(..) | - RuntimeCall::Council(..) | - RuntimeCall::Society(..) | - RuntimeCall::TechnicalCommittee(..) | - RuntimeCall::Elections(..) | - RuntimeCall::Treasury(..) + RuntimeCall::Democracy(..) + | RuntimeCall::Council(..) + | RuntimeCall::Society(..) + | RuntimeCall::TechnicalCommittee(..) + | RuntimeCall::Elections(..) + | RuntimeCall::Treasury(..) ), ProxyType::Staking => { matches!(c, RuntimeCall::Staking(..) | RuntimeCall::FastUnstake(..)) @@ -693,8 +693,8 @@ impl Get> for OffchainRandomBalancing { max => { let seed = sp_io::offchain::random_seed(); let random = ::decode(&mut TrailingZeroInput::new(&seed)) - .expect("input is padded with zeroes; qed") % - max.saturating_add(1); + .expect("input is padded with zeroes; qed") + % max.saturating_add(1); random as usize }, }; @@ -1486,6 +1486,23 @@ impl pallet_lottery::Config for Runtime { type WeightInfo = pallet_lottery::weights::SubstrateWeight; } +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum AssetRuntimeHoldReasons { + TransferHold, +} + parameter_types! { pub const AssetDeposit: Balance = 100 * DOLLARS; pub const ApprovalDeposit: Balance = 1 * DOLLARS; @@ -1513,6 +1530,8 @@ impl pallet_assets::Config for Runtime { type CallbackHandle = (); type WeightInfo = pallet_assets::weights::SubstrateWeight; type RemoveItemsLimit = ConstU32<1000>; + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = AssetRuntimeHoldReasons; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } @@ -1540,6 +1559,8 @@ impl pallet_assets::Config for Runtime { type WeightInfo = pallet_assets::weights::SubstrateWeight; type RemoveItemsLimit = ConstU32<1000>; type CallbackHandle = (); + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = AssetRuntimeHoldReasons; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } diff --git a/frame/assets/src/impl_fungibles.rs b/frame/assets/src/impl_fungibles.rs index 123abeba8283f..0a14b8aa8d10b 100644 --- a/frame/assets/src/impl_fungibles.rs +++ b/frame/assets/src/impl_fungibles.rs @@ -308,3 +308,98 @@ impl, I: 'static> fungibles::InspectEnumerable for Pa Asset::::iter_keys() } } + +impl, I: 'static> fungibles::MutateHold for Pallet {} + +impl, I: 'static> fungibles::InspectHold for Pallet { + type Reason = T::RuntimeHoldReason; + + fn total_balance_on_hold(asset: T::AssetId, who: &T::AccountId) -> T::Balance { + Holds::::get(who, asset) + .iter() + .map(|x| x.amount) + .fold(T::Balance::zero(), |acc, x| acc.saturating_add(x)) + } + + fn reducible_total_balance_on_hold( + asset: T::AssetId, + who: &T::AccountId, + _force: Fortitude, + ) -> Self::Balance { + let total_hold = Self::total_balance_on_hold(asset.clone(), who); + let free = Account::::get(asset.clone(), who) + .map(|account| account.balance) + .unwrap_or(Self::Balance::zero()); + // take alternative of unwrap + let ed = Asset::::get(asset).map(|x| x.min_balance).unwrap(); + + if free.saturating_sub(total_hold) < ed { + return total_hold.saturating_sub(ed); + } + total_hold + } + fn balance_on_hold(asset: T::AssetId, reason: &Self::Reason, who: &T::AccountId) -> T::Balance { + Holds::::get(who, asset) + .iter() + .find(|x| &x.id == reason) + .map_or_else(Zero::zero, |x| x.amount) + } + fn hold_available(asset: T::AssetId, reason: &Self::Reason, who: &T::AccountId) -> bool { + if frame_system::Pallet::::providers(who) == 0 { + return false; + } + let holds = Holds::::get(who, asset); + if holds.is_full() && !holds.iter().any(|x| &x.id == reason) { + return false; + } + true + } +} + +impl, I: 'static> fungibles::UnbalancedHold for Pallet { + fn set_balance_on_hold( + asset: T::AssetId, + reason: &Self::Reason, + who: &T::AccountId, + amount: Self::Balance, + ) -> DispatchResult { + let mut holds = Holds::::get(who, asset.clone()); + + if let Some(item) = holds.iter_mut().find(|x| &x.id == reason) { + let delta = item.amount.max(amount) - item.amount.min(amount); + let increase = amount > item.amount; + item.amount = amount; + if increase { + item.amount.checked_add(&delta).ok_or(ArithmeticError::Overflow)? + } else { + item.amount.checked_sub(&delta).ok_or(ArithmeticError::Underflow)? + }; + holds.retain(|x| !x.amount.is_zero()); + } else { + if !amount.is_zero() { + holds + .try_push(IdAmount { id: *reason, amount }) + .map_err(|_| Error::::TooManyHolds)?; + } + } + + let account: Option> = Account::::get(&asset, &who); + if let None = account { + let mut details = Asset::::get(&asset).ok_or(Error::::Unknown)?; + let new_account = AssetAccountOf:: { + balance: Zero::zero(), + status: AccountStatus::Blocked, + reason: Self::new_account(who, &mut details, None)?, + extra: T::Extra::default(), + }; + Account::::insert(&asset, &who, new_account); + } + + /* debug_assert!( + maybe_dust.is_none(), + "Does not alter main balance; dust only happens when it is altered; qed" + ); */ + Holds::::insert(who, asset, holds); + Ok(()) + } +} diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index 363a99701b56a..dd72bf20d6f56 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -280,6 +280,9 @@ pub mod pallet { /// attributes. type ForceOrigin: EnsureOrigin; + /// The overarching hold reason. + type RuntimeHoldReason: Parameter + Member + MaxEncodedLen + Ord + Copy; + /// The basic amount of funds that must be reserved for an asset. #[pallet::constant] type AssetDeposit: Get>; @@ -306,6 +309,10 @@ pub mod pallet { #[pallet::constant] type StringLimit: Get; + /// The maximum number of holds that can exist on an account at any time. + #[pallet::constant] + type MaxHolds: Get; + /// A hook to allow a per-asset, per-account minimum balance to be enforced. This must be /// respected in all permissionless operations. type Freezer: FrozenBalance; @@ -368,6 +375,18 @@ pub mod pallet { ValueQuery, >; + /// Holds on account balances. + #[pallet::storage] + pub type Holds, I: 'static = ()> = StorageDoubleMap< + _, + Blake2_128Concat, + T::AccountId, + Blake2_128Concat, + T::AssetId, + BoundedVec, T::MaxHolds>, + ValueQuery, + >; + #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] pub struct GenesisConfig, I: 'static = ()> { @@ -571,6 +590,10 @@ pub mod pallet { NotFrozen, /// Callback action resulted in error CallbackFailed, + /// Number of holds exceed `MaxHolds` + TooManyHolds, + /// Error to update holds + HoldsNotUpdated, } #[pallet::call(weight(>::WeightInfo))] @@ -1069,7 +1092,7 @@ pub mod pallet { ensure!(details.status == AssetStatus::Live, Error::::LiveAsset); ensure!(origin == details.owner, Error::::NoPermission); if details.owner == owner { - return Ok(()) + return Ok(()); } let metadata_deposit = Metadata::::get(&id).deposit; diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index 559afccb946c5..2a2b236c344bc 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -87,6 +87,15 @@ pub struct Approval { pub(super) deposit: DepositBalance, } +/// An identifier and balance. +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, TypeInfo)] +pub struct IdAmount { + /// An identifier for this item. + pub id: Id, + /// Some amount for this item. + pub amount: Balance, +} + #[test] fn ensure_bool_decodes_to_consumer_or_sufficient() { assert_eq!(false.encode(), ExistenceReason::<(), ()>::Consumer.encode()); @@ -120,7 +129,7 @@ where { pub(crate) fn take_deposit(&mut self) -> Option { if !matches!(self, ExistenceReason::DepositHeld(_)) { - return None + return None; } if let ExistenceReason::DepositHeld(deposit) = sp_std::mem::replace(self, ExistenceReason::DepositRefunded) @@ -133,7 +142,7 @@ where pub(crate) fn take_deposit_from(&mut self) -> Option<(AccountId, Balance)> { if !matches!(self, ExistenceReason::DepositFrom(..)) { - return None + return None; } if let ExistenceReason::DepositFrom(depositor, deposit) = sp_std::mem::replace(self, ExistenceReason::DepositRefunded) From 2b9a16653a8f04af7a478b2996b7a46b59304380 Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Thu, 3 Aug 2023 17:08:53 +0200 Subject: [PATCH 2/6] wip --- frame/assets/src/impl_fungibles.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/assets/src/impl_fungibles.rs b/frame/assets/src/impl_fungibles.rs index 0a14b8aa8d10b..f2fa36dd5e452 100644 --- a/frame/assets/src/impl_fungibles.rs +++ b/frame/assets/src/impl_fungibles.rs @@ -395,10 +395,7 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle Account::::insert(&asset, &who, new_account); } - /* debug_assert!( - maybe_dust.is_none(), - "Does not alter main balance; dust only happens when it is altered; qed" - ); */ + /// Here the balance pallet calls try_mutate_account that calculates then dust. We should do something similar. Holds::::insert(who, asset, holds); Ok(()) } From f9b8cd427488a5ffd66aa38e0cc6a83fb56ce9b2 Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Fri, 4 Aug 2023 13:26:16 +0200 Subject: [PATCH 3/6] First test working --- frame/assets/src/impl_fungibles.rs | 11 +++-- frame/assets/src/mock.rs | 24 +++++++++- frame/assets/src/tests.rs | 74 +++++++++++++++++++++++++++++- 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/frame/assets/src/impl_fungibles.rs b/frame/assets/src/impl_fungibles.rs index f2fa36dd5e452..da73188d17eae 100644 --- a/frame/assets/src/impl_fungibles.rs +++ b/frame/assets/src/impl_fungibles.rs @@ -368,12 +368,13 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle if let Some(item) = holds.iter_mut().find(|x| &x.id == reason) { let delta = item.amount.max(amount) - item.amount.min(amount); let increase = amount > item.amount; - item.amount = amount; + if increase { - item.amount.checked_add(&delta).ok_or(ArithmeticError::Overflow)? + item.amount = item.amount.checked_add(&delta).ok_or(ArithmeticError::Overflow)? } else { - item.amount.checked_sub(&delta).ok_or(ArithmeticError::Underflow)? + item.amount = item.amount.checked_sub(&delta).ok_or(ArithmeticError::Underflow)? }; + holds.retain(|x| !x.amount.is_zero()); } else { if !amount.is_zero() { @@ -383,6 +384,8 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle } } + println!("holds: {:?}", holds); + let account: Option> = Account::::get(&asset, &who); if let None = account { let mut details = Asset::::get(&asset).ok_or(Error::::Unknown)?; @@ -395,7 +398,7 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle Account::::insert(&asset, &who, new_account); } - /// Here the balance pallet calls try_mutate_account that calculates then dust. We should do something similar. + // Here the balance pallet calls try_mutate_account that calculates then dust. We should do something similar. Holds::::insert(who, asset, holds); Ok(()) } diff --git a/frame/assets/src/mock.rs b/frame/assets/src/mock.rs index 32ad02da90412..2eb3c9ddee492 100644 --- a/frame/assets/src/mock.rs +++ b/frame/assets/src/mock.rs @@ -19,11 +19,12 @@ use super::*; use crate as pallet_assets; +use codec::{Decode, Encode, MaxEncodedLen}; -use codec::Encode; use frame_support::{ construct_runtime, parameter_types, traits::{AsEnsureOriginWithArg, ConstU32, ConstU64}, + RuntimeDebug, }; use sp_core::H256; use sp_io::storage; @@ -131,6 +132,25 @@ impl AssetsCallbackHandle { } } +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum TestId { + Foo, + Bar, + Baz, +} + impl Config for Test { type RuntimeEvent = RuntimeEvent; type Balance = u64; @@ -150,6 +170,8 @@ impl Config for Test { type CallbackHandle = AssetsCallbackHandle; type Extra = (); type RemoveItemsLimit = ConstU32<5>; + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index 06d4ec1211737..a84469727b554 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -22,7 +22,11 @@ use crate::{mock::*, Error}; use frame_support::{ assert_noop, assert_ok, dispatch::GetDispatchInfo, - traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency}, + traits::{ + fungibles::InspectEnumerable, + tokens::{Precision::Exact, Preservation::Protect}, + Currency, + }, }; use pallet_balances::Error as BalancesError; use sp_io::storage; @@ -1775,3 +1779,71 @@ fn asset_destroy_refund_existence_deposit() { assert_eq!(Balances::reserved_balance(&admin), 0); }); } + +#[test] +fn unbalanced_trait_set_balance_works() { + new_test_ext().execute_with(|| { + let asset = 0; + assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset, 1, false, 1)); + let admin = 1; + let receiver = 2; // account with own deposit + Balances::make_free_balance_be(&admin, 100); + Balances::make_free_balance_be(&receiver, 100); + + assert_eq!(>::balance(asset, &receiver), 0); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), asset, receiver, 100)); + assert_eq!(>::balance(asset, &receiver), 100); + + assert_ok!(>::hold(asset, &TestId::Foo, &receiver, 60)); + assert_eq!(>::balance(asset, &receiver), 40); + assert_eq!( + >::total_balance_on_hold(asset, &receiver), + 60 + ); + assert_eq!( + >::balance_on_hold(asset, &TestId::Foo, &receiver), + 60 + ); + + assert_eq!( + >::balance_on_hold(asset, &TestId::Foo, &receiver), + 60 + ); + + assert_ok!(>::release( + asset, + &TestId::Foo, + &receiver, + 30, + Exact + )); + + assert_eq!( + >::balance_on_hold(asset, &TestId::Foo, &receiver), + 30 + ); + assert_eq!( + >::total_balance_on_hold(asset, &receiver), + 30 + ); + + assert_ok!(>::release( + asset, + &TestId::Foo, + &receiver, + 30, + Exact + )); + + assert_eq!( + >::balance_on_hold(asset, &TestId::Foo, &receiver), + 0 + ); + assert_eq!( + >::total_balance_on_hold(asset, &receiver), + 0 + ); + let holds = Holds::::get(&receiver, asset); + assert_eq!(holds.len(), 0); + }); +} From 29bcffcc2288d4fab8bbed9d208a07b2b982a84b Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Fri, 4 Aug 2023 18:30:39 +0200 Subject: [PATCH 4/6] Potential initial working version --- frame/assets/src/functions.rs | 48 +++++++-------- frame/assets/src/impl_fungibles.rs | 13 +++-- frame/assets/src/tests.rs | 93 +++++++++++++++++++++--------- 3 files changed, 101 insertions(+), 53 deletions(-) diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index c2c1b6839060e..e0d7313285217 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -86,7 +86,7 @@ impl, I: 'static> Pallet { // allow accidental usage of all consumer references which could cause grief. if !frame_system::Pallet::::can_inc_consumer(who) { frame_system::Pallet::::dec_consumers(who); - return Err(Error::::UnavailableConsumer.into()) + return Err(Error::::UnavailableConsumer.into()); } ExistenceReason::Consumer }; @@ -132,25 +132,27 @@ impl, I: 'static> Pallet { Some(details) => details, None => return DepositConsequence::UnknownAsset, }; + if increase_supply && details.supply.checked_add(&amount).is_none() { - return DepositConsequence::Overflow + return DepositConsequence::Overflow; } + if let Some(account) = Account::::get(id, who) { if account.status.is_blocked() { - return DepositConsequence::Blocked + return DepositConsequence::Blocked; } if account.balance.checked_add(&amount).is_none() { - return DepositConsequence::Overflow + return DepositConsequence::Overflow; } } else { if amount < details.min_balance { - return DepositConsequence::BelowMinimum + return DepositConsequence::BelowMinimum; } if !details.is_sufficient && !frame_system::Pallet::::can_accrue_consumers(who, 2) { - return DepositConsequence::CannotCreate + return DepositConsequence::CannotCreate; } if details.is_sufficient && details.sufficients.checked_add(1).is_none() { - return DepositConsequence::Overflow + return DepositConsequence::Overflow; } } @@ -170,20 +172,20 @@ impl, I: 'static> Pallet { None => return UnknownAsset, }; if details.supply.checked_sub(&amount).is_none() { - return Underflow + return Underflow; } if details.status == AssetStatus::Frozen { - return Frozen + return Frozen; } if amount.is_zero() { - return Success + return Success; } let account = match Account::::get(&id, who) { Some(a) => a, None => return BalanceLow, }; if account.status.is_frozen() { - return Frozen + return Frozen; } if let Some(rest) = account.balance.checked_sub(&amount) { if let Some(frozen) = T::Freezer::frozen_balance(id.clone(), who) { @@ -267,7 +269,7 @@ impl, I: 'static> Pallet { Ok(dust) => actual.saturating_add(dust), //< guaranteed by reducible_balance Err(e) => { debug_assert!(false, "passed from reducible_balance; qed"); - return Err(e) + return Err(e); }, }; @@ -360,7 +362,7 @@ impl, I: 'static> Pallet { debug_assert!(false, "refund did not result in dead account?!"); // deposit may have been refunded, need to update `Account` Account::::insert(id, &who, account); - return Ok(()) + return Ok(()); } Asset::::insert(&id, details); // Executing a hook here is safe, since it is not in a `mutate`. @@ -391,12 +393,12 @@ impl, I: 'static> Pallet { debug_assert!(false, "refund did not result in dead account?!"); // deposit may have been refunded, need to update `Account` Account::::insert(&id, &who, account); - return Ok(()) + return Ok(()); } Asset::::insert(&id, details); // Executing a hook here is safe, since it is not in a `mutate`. T::Freezer::died(id, &who); - return Ok(()) + return Ok(()); } /// Increases the asset `id` balance of `beneficiary` by `amount`. @@ -441,7 +443,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult, ) -> DispatchResult { if amount.is_zero() { - return Ok(()) + return Ok(()); } Self::can_increase(id.clone(), beneficiary, amount, true).into_result()?; @@ -528,7 +530,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult, ) -> Result { if amount.is_zero() { - return Ok(amount) + return Ok(amount); } let details = Asset::::get(&id).ok_or(Error::::Unknown)?; @@ -551,7 +553,7 @@ impl, I: 'static> Pallet { debug_assert!(account.balance.is_zero(), "checked in prep; qed"); target_died = Some(Self::dead_account(target, details, &account.reason, false)); if let Some(Remove) = target_died { - return Ok(()) + return Ok(()); } }; *maybe_account = Some(account); @@ -604,7 +606,7 @@ impl, I: 'static> Pallet { ) -> Result<(T::Balance, Option), DispatchError> { // Early exit if no-op. if amount.is_zero() { - return Ok((amount, None)) + return Ok((amount, None)); } let details = Asset::::get(&id).ok_or(Error::::Unknown)?; ensure!(details.status == AssetStatus::Live, Error::::AssetNotLive); @@ -627,7 +629,7 @@ impl, I: 'static> Pallet { // Skip if source == dest if source == dest { - return Ok(()) + return Ok(()); } // Burn any dust if needed. @@ -672,7 +674,7 @@ impl, I: 'static> Pallet { Some(Self::dead_account(source, details, &source_account.reason, false)); if let Some(Remove) = source_died { Account::::remove(&id, &source); - return Ok(()) + return Ok(()); } } Account::::insert(&id, &source, &source_account); @@ -776,7 +778,7 @@ impl, I: 'static> Pallet { defensive!("destroy did not result in dead account?!"); } if i + 1 >= (max_items as usize) { - break + break; } } remaining_accounts = details.accounts; @@ -817,7 +819,7 @@ impl, I: 'static> Pallet { removed_approvals = removed_approvals.saturating_add(1); details.approvals = details.approvals.saturating_sub(1); if removed_approvals >= max_items { - break + break; } } Self::deposit_event(Event::ApprovalsDestroyed { diff --git a/frame/assets/src/impl_fungibles.rs b/frame/assets/src/impl_fungibles.rs index da73188d17eae..a19bfcf57c944 100644 --- a/frame/assets/src/impl_fungibles.rs +++ b/frame/assets/src/impl_fungibles.rs @@ -345,10 +345,16 @@ impl, I: 'static> fungibles::InspectHold for Pallet bool { + let asset_details = Asset::::get(asset.clone()).unwrap(); + let holds = Holds::::get(who, asset); + if !holds.is_full() && asset_details.is_sufficient == true { + return true; + } + if frame_system::Pallet::::providers(who) == 0 { return false; } - let holds = Holds::::get(who, asset); + if holds.is_full() && !holds.iter().any(|x| &x.id == reason) { return false; } @@ -384,14 +390,13 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle } } - println!("holds: {:?}", holds); - let account: Option> = Account::::get(&asset, &who); + if let None = account { let mut details = Asset::::get(&asset).ok_or(Error::::Unknown)?; let new_account = AssetAccountOf:: { balance: Zero::zero(), - status: AccountStatus::Blocked, + status: AccountStatus::Liquid, reason: Self::new_account(who, &mut details, None)?, extra: T::Extra::default(), }; diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index a84469727b554..20ecae82ab7ce 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -24,7 +24,7 @@ use frame_support::{ dispatch::GetDispatchInfo, traits::{ fungibles::InspectEnumerable, - tokens::{Precision::Exact, Preservation::Protect}, + tokens::{Fortitude::Polite, Precision::Exact, Preservation::Protect}, Currency, }, }; @@ -1786,64 +1786,105 @@ fn unbalanced_trait_set_balance_works() { let asset = 0; assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset, 1, false, 1)); let admin = 1; - let receiver = 2; // account with own deposit + let dest = 2; // account with own deposit Balances::make_free_balance_be(&admin, 100); - Balances::make_free_balance_be(&receiver, 100); + Balances::make_free_balance_be(&dest, 100); - assert_eq!(>::balance(asset, &receiver), 0); - assert_ok!(Assets::mint(RuntimeOrigin::signed(1), asset, receiver, 100)); - assert_eq!(>::balance(asset, &receiver), 100); + assert_eq!(>::balance(asset, &dest), 0); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), asset, dest, 100)); + assert_eq!(>::balance(asset, &dest), 100); - assert_ok!(>::hold(asset, &TestId::Foo, &receiver, 60)); - assert_eq!(>::balance(asset, &receiver), 40); + assert_ok!(>::hold(asset, &TestId::Foo, &dest, 60)); + assert_eq!(>::balance(asset, &dest), 40); + assert_eq!(>::total_balance_on_hold(asset, &dest), 60); assert_eq!( - >::total_balance_on_hold(asset, &receiver), - 60 - ); - assert_eq!( - >::balance_on_hold(asset, &TestId::Foo, &receiver), + >::balance_on_hold(asset, &TestId::Foo, &dest), 60 ); assert_eq!( - >::balance_on_hold(asset, &TestId::Foo, &receiver), + >::balance_on_hold(asset, &TestId::Foo, &dest), 60 ); assert_ok!(>::release( asset, &TestId::Foo, - &receiver, + &dest, 30, Exact )); assert_eq!( - >::balance_on_hold(asset, &TestId::Foo, &receiver), - 30 - ); - assert_eq!( - >::total_balance_on_hold(asset, &receiver), + >::balance_on_hold(asset, &TestId::Foo, &dest), 30 ); + assert_eq!(>::total_balance_on_hold(asset, &dest), 30); assert_ok!(>::release( asset, &TestId::Foo, - &receiver, + &dest, 30, Exact )); assert_eq!( - >::balance_on_hold(asset, &TestId::Foo, &receiver), + >::balance_on_hold(asset, &TestId::Foo, &dest), 0 ); + assert_eq!(>::total_balance_on_hold(asset, &dest), 0); + let holds = Holds::::get(&dest, asset); + assert_eq!(holds.len(), 0); + }); +} + +#[test] +fn transfer_and_hold_works() { + new_test_ext().execute_with(|| { + let asset = 0; + let admin = 1; + let source = 2; // account with own deposit + let dest = 3; // account with own deposit + assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset, admin, true, 1)); + + Balances::make_free_balance_be(&admin, 100); + Balances::make_free_balance_be(&source, 100); + + assert_eq!(>::balance(asset, &source), 0); + assert_ok!(Assets::mint(RuntimeOrigin::signed(1), asset, source, 100)); + + assert_eq!(>::balance(asset, &source), 100); + + assert_ok!(>::transfer_and_hold( + asset, + &TestId::Foo, + &source, + &dest, + 60, + Exact, + Protect, + Polite + )); + + assert_eq!(>::balance(asset, &source), 40); assert_eq!( - >::total_balance_on_hold(asset, &receiver), - 0 + >::balance_on_hold(asset, &TestId::Foo, &dest), + 60 ); - let holds = Holds::::get(&receiver, asset); - assert_eq!(holds.len(), 0); + assert_eq!(>::total_balance_on_hold(asset, &dest), 60); + + assert_ok!(>::release( + asset, + &TestId::Foo, + &dest, + 20, + Exact + )); + assert_eq!( + >::balance_on_hold(asset, &TestId::Foo, &dest), + 40 + ); + assert_eq!(>::balance(asset, &dest), 20); }); } From 60e5147d306eedbbde42d3b71b83e13a24a3f8ab Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Fri, 4 Aug 2023 18:34:36 +0200 Subject: [PATCH 5/6] fmt --- bin/node/runtime/src/lib.rs | 28 +++++++++--------- frame/asset-conversion/src/lib.rs | 4 ++- frame/asset-conversion/src/tests.rs | 6 +++- frame/assets/src/functions.rs | 46 ++++++++++++++--------------- frame/assets/src/impl_fungibles.rs | 11 +++---- frame/assets/src/lib.rs | 2 +- frame/assets/src/types.rs | 4 +-- 7 files changed, 54 insertions(+), 47 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4f921da4ac5df..874f40e9f951b 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -311,21 +311,21 @@ impl InstanceFilter for ProxyType { ProxyType::Any => true, ProxyType::NonTransfer => !matches!( c, - RuntimeCall::Balances(..) - | RuntimeCall::Assets(..) - | RuntimeCall::Uniques(..) - | RuntimeCall::Nfts(..) - | RuntimeCall::Vesting(pallet_vesting::Call::vested_transfer { .. }) - | RuntimeCall::Indices(pallet_indices::Call::transfer { .. }) + RuntimeCall::Balances(..) | + RuntimeCall::Assets(..) | + RuntimeCall::Uniques(..) | + RuntimeCall::Nfts(..) | + RuntimeCall::Vesting(pallet_vesting::Call::vested_transfer { .. }) | + RuntimeCall::Indices(pallet_indices::Call::transfer { .. }) ), ProxyType::Governance => matches!( c, - RuntimeCall::Democracy(..) - | RuntimeCall::Council(..) - | RuntimeCall::Society(..) - | RuntimeCall::TechnicalCommittee(..) - | RuntimeCall::Elections(..) - | RuntimeCall::Treasury(..) + RuntimeCall::Democracy(..) | + RuntimeCall::Council(..) | + RuntimeCall::Society(..) | + RuntimeCall::TechnicalCommittee(..) | + RuntimeCall::Elections(..) | + RuntimeCall::Treasury(..) ), ProxyType::Staking => { matches!(c, RuntimeCall::Staking(..) | RuntimeCall::FastUnstake(..)) @@ -693,8 +693,8 @@ impl Get> for OffchainRandomBalancing { max => { let seed = sp_io::offchain::random_seed(); let random = ::decode(&mut TrailingZeroInput::new(&seed)) - .expect("input is padded with zeroes; qed") - % max.saturating_add(1); + .expect("input is padded with zeroes; qed") % + max.saturating_add(1); random as usize }, }; diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index f9aeeace11fe7..8d266a20492a2 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -1194,7 +1194,9 @@ pub mod pallet { () ); } else { - let MultiAssetIdConversionResult::Converted(asset_id) = T::MultiAssetIdConverter::try_convert(asset) else { + let MultiAssetIdConversionResult::Converted(asset_id) = + T::MultiAssetIdConverter::try_convert(asset) + else { return Err(()) }; let minimal = T::Assets::minimum_balance(asset_id); diff --git a/frame/asset-conversion/src/tests.rs b/frame/asset-conversion/src/tests.rs index 80faf5363b011..450a074ec3675 100644 --- a/frame/asset-conversion/src/tests.rs +++ b/frame/asset-conversion/src/tests.rs @@ -66,7 +66,11 @@ fn pool_assets() -> Vec { fn create_tokens(owner: u128, tokens: Vec>) { for token_id in tokens { - let MultiAssetIdConversionResult::Converted(asset_id) = NativeOrAssetIdConverter::try_convert(&token_id) else { unreachable!("invalid token") }; + let MultiAssetIdConversionResult::Converted(asset_id) = + NativeOrAssetIdConverter::try_convert(&token_id) + else { + unreachable!("invalid token") + }; assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset_id, owner, false, 1)); } } diff --git a/frame/assets/src/functions.rs b/frame/assets/src/functions.rs index e0d7313285217..543a69b93b1bf 100644 --- a/frame/assets/src/functions.rs +++ b/frame/assets/src/functions.rs @@ -86,7 +86,7 @@ impl, I: 'static> Pallet { // allow accidental usage of all consumer references which could cause grief. if !frame_system::Pallet::::can_inc_consumer(who) { frame_system::Pallet::::dec_consumers(who); - return Err(Error::::UnavailableConsumer.into()); + return Err(Error::::UnavailableConsumer.into()) } ExistenceReason::Consumer }; @@ -134,25 +134,25 @@ impl, I: 'static> Pallet { }; if increase_supply && details.supply.checked_add(&amount).is_none() { - return DepositConsequence::Overflow; + return DepositConsequence::Overflow } if let Some(account) = Account::::get(id, who) { if account.status.is_blocked() { - return DepositConsequence::Blocked; + return DepositConsequence::Blocked } if account.balance.checked_add(&amount).is_none() { - return DepositConsequence::Overflow; + return DepositConsequence::Overflow } } else { if amount < details.min_balance { - return DepositConsequence::BelowMinimum; + return DepositConsequence::BelowMinimum } if !details.is_sufficient && !frame_system::Pallet::::can_accrue_consumers(who, 2) { - return DepositConsequence::CannotCreate; + return DepositConsequence::CannotCreate } if details.is_sufficient && details.sufficients.checked_add(1).is_none() { - return DepositConsequence::Overflow; + return DepositConsequence::Overflow } } @@ -172,20 +172,20 @@ impl, I: 'static> Pallet { None => return UnknownAsset, }; if details.supply.checked_sub(&amount).is_none() { - return Underflow; + return Underflow } if details.status == AssetStatus::Frozen { - return Frozen; + return Frozen } if amount.is_zero() { - return Success; + return Success } let account = match Account::::get(&id, who) { Some(a) => a, None => return BalanceLow, }; if account.status.is_frozen() { - return Frozen; + return Frozen } if let Some(rest) = account.balance.checked_sub(&amount) { if let Some(frozen) = T::Freezer::frozen_balance(id.clone(), who) { @@ -269,7 +269,7 @@ impl, I: 'static> Pallet { Ok(dust) => actual.saturating_add(dust), //< guaranteed by reducible_balance Err(e) => { debug_assert!(false, "passed from reducible_balance; qed"); - return Err(e); + return Err(e) }, }; @@ -362,7 +362,7 @@ impl, I: 'static> Pallet { debug_assert!(false, "refund did not result in dead account?!"); // deposit may have been refunded, need to update `Account` Account::::insert(id, &who, account); - return Ok(()); + return Ok(()) } Asset::::insert(&id, details); // Executing a hook here is safe, since it is not in a `mutate`. @@ -393,12 +393,12 @@ impl, I: 'static> Pallet { debug_assert!(false, "refund did not result in dead account?!"); // deposit may have been refunded, need to update `Account` Account::::insert(&id, &who, account); - return Ok(()); + return Ok(()) } Asset::::insert(&id, details); // Executing a hook here is safe, since it is not in a `mutate`. T::Freezer::died(id, &who); - return Ok(()); + return Ok(()) } /// Increases the asset `id` balance of `beneficiary` by `amount`. @@ -443,7 +443,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult, ) -> DispatchResult { if amount.is_zero() { - return Ok(()); + return Ok(()) } Self::can_increase(id.clone(), beneficiary, amount, true).into_result()?; @@ -530,7 +530,7 @@ impl, I: 'static> Pallet { ) -> DispatchResult, ) -> Result { if amount.is_zero() { - return Ok(amount); + return Ok(amount) } let details = Asset::::get(&id).ok_or(Error::::Unknown)?; @@ -553,7 +553,7 @@ impl, I: 'static> Pallet { debug_assert!(account.balance.is_zero(), "checked in prep; qed"); target_died = Some(Self::dead_account(target, details, &account.reason, false)); if let Some(Remove) = target_died { - return Ok(()); + return Ok(()) } }; *maybe_account = Some(account); @@ -606,7 +606,7 @@ impl, I: 'static> Pallet { ) -> Result<(T::Balance, Option), DispatchError> { // Early exit if no-op. if amount.is_zero() { - return Ok((amount, None)); + return Ok((amount, None)) } let details = Asset::::get(&id).ok_or(Error::::Unknown)?; ensure!(details.status == AssetStatus::Live, Error::::AssetNotLive); @@ -629,7 +629,7 @@ impl, I: 'static> Pallet { // Skip if source == dest if source == dest { - return Ok(()); + return Ok(()) } // Burn any dust if needed. @@ -674,7 +674,7 @@ impl, I: 'static> Pallet { Some(Self::dead_account(source, details, &source_account.reason, false)); if let Some(Remove) = source_died { Account::::remove(&id, &source); - return Ok(()); + return Ok(()) } } Account::::insert(&id, &source, &source_account); @@ -778,7 +778,7 @@ impl, I: 'static> Pallet { defensive!("destroy did not result in dead account?!"); } if i + 1 >= (max_items as usize) { - break; + break } } remaining_accounts = details.accounts; @@ -819,7 +819,7 @@ impl, I: 'static> Pallet { removed_approvals = removed_approvals.saturating_add(1); details.approvals = details.approvals.saturating_sub(1); if removed_approvals >= max_items { - break; + break } } Self::deposit_event(Event::ApprovalsDestroyed { diff --git a/frame/assets/src/impl_fungibles.rs b/frame/assets/src/impl_fungibles.rs index a19bfcf57c944..cb733c0ea5c95 100644 --- a/frame/assets/src/impl_fungibles.rs +++ b/frame/assets/src/impl_fungibles.rs @@ -334,7 +334,7 @@ impl, I: 'static> fungibles::InspectHold for Pallet::get(asset).map(|x| x.min_balance).unwrap(); if free.saturating_sub(total_hold) < ed { - return total_hold.saturating_sub(ed); + return total_hold.saturating_sub(ed) } total_hold } @@ -348,15 +348,15 @@ impl, I: 'static> fungibles::InspectHold for Pallet::get(asset.clone()).unwrap(); let holds = Holds::::get(who, asset); if !holds.is_full() && asset_details.is_sufficient == true { - return true; + return true } if frame_system::Pallet::::providers(who) == 0 { - return false; + return false } if holds.is_full() && !holds.iter().any(|x| &x.id == reason) { - return false; + return false } true } @@ -403,7 +403,8 @@ impl, I: 'static> fungibles::UnbalancedHold for Palle Account::::insert(&asset, &who, new_account); } - // Here the balance pallet calls try_mutate_account that calculates then dust. We should do something similar. + // Here the balance pallet calls try_mutate_account that calculates then dust. We should do + // something similar. Holds::::insert(who, asset, holds); Ok(()) } diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index dd72bf20d6f56..5e026476d82bc 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -1092,7 +1092,7 @@ pub mod pallet { ensure!(details.status == AssetStatus::Live, Error::::LiveAsset); ensure!(origin == details.owner, Error::::NoPermission); if details.owner == owner { - return Ok(()); + return Ok(()) } let metadata_deposit = Metadata::::get(&id).deposit; diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index 2a2b236c344bc..7348c60fef7e7 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -129,7 +129,7 @@ where { pub(crate) fn take_deposit(&mut self) -> Option { if !matches!(self, ExistenceReason::DepositHeld(_)) { - return None; + return None } if let ExistenceReason::DepositHeld(deposit) = sp_std::mem::replace(self, ExistenceReason::DepositRefunded) @@ -142,7 +142,7 @@ where pub(crate) fn take_deposit_from(&mut self) -> Option<(AccountId, Balance)> { if !matches!(self, ExistenceReason::DepositFrom(..)) { - return None; + return None } if let ExistenceReason::DepositFrom(depositor, deposit) = sp_std::mem::replace(self, ExistenceReason::DepositRefunded) From 26c3601308e8743b909af0ed27cf00599db99131 Mon Sep 17 00:00:00 2001 From: Nicolas Fernandez Date: Thu, 10 Aug 2023 12:25:55 +0200 Subject: [PATCH 6/6] Fixing other mock files --- frame/asset-conversion/src/mock.rs | 27 +++++++++++++++++-- frame/nft-fractionalization/src/mock.rs | 25 +++++++++++++++-- .../asset-conversion-tx-payment/src/mock.rs | 25 ++++++++++++++++- .../asset-tx-payment/src/mock.rs | 21 +++++++++++++++ 4 files changed, 93 insertions(+), 5 deletions(-) diff --git a/frame/asset-conversion/src/mock.rs b/frame/asset-conversion/src/mock.rs index 7fe81b814047d..0be453a8b8728 100644 --- a/frame/asset-conversion/src/mock.rs +++ b/frame/asset-conversion/src/mock.rs @@ -19,7 +19,7 @@ use super::*; use crate as pallet_asset_conversion; - +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ construct_runtime, instances::{Instance1, Instance2}, @@ -32,7 +32,7 @@ use sp_arithmetic::Permill; use sp_core::H256; use sp_runtime::{ traits::{AccountIdConversion, BlakeTwo256, IdentityLookup}, - BuildStorage, + BuildStorage, RuntimeDebug, }; type Block = frame_system::mocking::MockBlock; @@ -90,6 +90,25 @@ impl pallet_balances::Config for Test { type MaxHolds = (); } +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum TestId { + Foo, + Bar, + Baz, +} + impl pallet_assets::Config for Test { type RuntimeEvent = RuntimeEvent; type Balance = u128; @@ -109,6 +128,8 @@ impl pallet_assets::Config for Test { type Extra = (); type WeightInfo = (); type CallbackHandle = (); + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); } @@ -134,6 +155,8 @@ impl pallet_assets::Config for Test { type Extra = (); type WeightInfo = (); type CallbackHandle = (); + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); } diff --git a/frame/nft-fractionalization/src/mock.rs b/frame/nft-fractionalization/src/mock.rs index 6565adaf6fc7e..f7ab1bfeaabab 100644 --- a/frame/nft-fractionalization/src/mock.rs +++ b/frame/nft-fractionalization/src/mock.rs @@ -19,11 +19,11 @@ use super::*; use crate as pallet_nft_fractionalization; - +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ construct_runtime, parameter_types, traits::{AsEnsureOriginWithArg, ConstU32, ConstU64}, - BoundedVec, PalletId, + BoundedVec, PalletId, RuntimeDebug, }; use frame_system::EnsureSigned; use pallet_nfts::PalletFeatures; @@ -38,6 +38,25 @@ type Signature = MultiSignature; type AccountPublic = ::Signer; type AccountId = ::AccountId; +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum TestId { + Foo, + Bar, + Baz, +} + // Configure a mock runtime to test the pallet. construct_runtime!( pub enum Test @@ -110,6 +129,8 @@ impl pallet_assets::Config for Test { type Extra = (); type CallbackHandle = (); type WeightInfo = (); + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); } diff --git a/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index bfbe8b4178cee..2a110bddf98e4 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -34,13 +34,32 @@ use pallet_transaction_payment::CurrencyAdapter; use sp_core::H256; use sp_runtime::{ traits::{AccountIdConversion, BlakeTwo256, IdentityLookup, SaturatedConversion}, - Permill, + Permill, RuntimeDebug, }; type Block = frame_system::mocking::MockBlock; type Balance = u64; type AccountId = u64; +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum TestId { + Foo, + Bar, + Baz, +} + frame_support::construct_runtime!( pub enum Runtime { @@ -191,6 +210,8 @@ impl pallet_assets::Config for Runtime { type CallbackHandle = (); type WeightInfo = (); type RemoveItemsLimit = ConstU32<1000>; + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); } @@ -215,6 +236,8 @@ impl pallet_assets::Config for Runtime { type Extra = (); type WeightInfo = (); type CallbackHandle = (); + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); } diff --git a/frame/transaction-payment/asset-tx-payment/src/mock.rs b/frame/transaction-payment/asset-tx-payment/src/mock.rs index b8d7b523ca258..aae8cc67b3ad7 100644 --- a/frame/transaction-payment/asset-tx-payment/src/mock.rs +++ b/frame/transaction-payment/asset-tx-payment/src/mock.rs @@ -35,6 +35,25 @@ type Block = frame_system::mocking::MockBlock; type Balance = u64; type AccountId = u64; +#[derive( + Encode, + Decode, + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + MaxEncodedLen, + scale_info::TypeInfo, + RuntimeDebug, +)] +pub enum TestId { + Foo, + Bar, + Baz, +} + frame_support::construct_runtime!( pub struct Runtime { System: system, @@ -164,6 +183,8 @@ impl pallet_assets::Config for Runtime { type CallbackHandle = (); type WeightInfo = (); type RemoveItemsLimit = ConstU32<1000>; + type MaxHolds = ConstU32<500>; + type RuntimeHoldReason = TestId; pallet_assets::runtime_benchmarks_enabled! { type BenchmarkHelper = (); }