From f59c9eada2f6de523c8d029afc712a7a0146e1c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Wed, 3 Apr 2024 10:49:30 -0500 Subject: [PATCH] change(pallet-assets-freezer): apply requested changes --- substrate/frame/assets-freezer/Cargo.toml | 1 - substrate/frame/assets-freezer/README.md | 29 ------------- substrate/frame/assets-freezer/src/impls.rs | 7 +-- substrate/frame/assets-freezer/src/lib.rs | 47 ++++++++++++++++----- substrate/frame/assets-freezer/src/mock.rs | 11 ++--- substrate/frame/assets-freezer/src/tests.rs | 44 ++++++++++--------- substrate/frame/assets-freezer/src/types.rs | 8 ++-- 7 files changed, 74 insertions(+), 73 deletions(-) delete mode 100644 substrate/frame/assets-freezer/README.md diff --git a/substrate/frame/assets-freezer/Cargo.toml b/substrate/frame/assets-freezer/Cargo.toml index f5234443c7f62..0d2f087925d65 100644 --- a/substrate/frame/assets-freezer/Cargo.toml +++ b/substrate/frame/assets-freezer/Cargo.toml @@ -7,7 +7,6 @@ license = "MIT-0" homepage = "https://substrate.io" repository.workspace = true description = "Provides freezing features to `pallet-assets`" -readme = "README.md" [lints] workspace = true diff --git a/substrate/frame/assets-freezer/README.md b/substrate/frame/assets-freezer/README.md deleted file mode 100644 index bc9cb9d3eb4d4..0000000000000 --- a/substrate/frame/assets-freezer/README.md +++ /dev/null @@ -1,29 +0,0 @@ -# Assets Freezer Pallet - -A pallet capable of freezing fungibles from `pallet-assets`. - -> Made with *Substrate*, for *Polkadot*. - -[![github]](https://github.com/paritytech/polkadot-sdk/tree/master/substrate/frame/examples/basic) -[![polkadot]](https://polkadot.network) - -[polkadot]: https://img.shields.io/badge/polkadot-E6007A?style=for-the-badge&logo=polkadot&logoColor=white -[github]: https://img.shields.io/badge/github-8da0cb?style=for-the-badge&labelColor=555555&logo=github - -## Pallet API - -See the [`pallet`] module for more information about the interfaces this pallet exposes, -including its configuration trait, dispatchables, storage items, events and errors. - -## Overview - -This pallet provides the following functionality: - -- Pallet hooks that implement custom logic to let `pallet-assets` know whether an balance is - frozen for an account on a given asset (see: [`pallet_assets::types::FrozenBalance`][docs:frozen_balance]). -- An implementation of the fungibles [inspect][docs:inspect_freeze] and the [mutation][docs:mutate_freeze] - APIs for freezes. - -[docs:frozen_balance]: https://docs.rs/pallet-assets/latest/pallet_assets/trait.FrozenBalance.html -[docs:inspect_freeze]: https://docs.rs/frame-support/latest/frame_support/traits/tokens/fungibles/index.html#reexport.InspectFreeze -[docs:mutate_freeze]: https://docs.rs/frame-support/latest/frame_support/traits/tokens/fungibles/index.html#reexport.MutateFreeze diff --git a/substrate/frame/assets-freezer/src/impls.rs b/substrate/frame/assets-freezer/src/impls.rs index 71ba40703e455..ba4b2d83f9a6c 100644 --- a/substrate/frame/assets-freezer/src/impls.rs +++ b/substrate/frame/assets-freezer/src/impls.rs @@ -17,8 +17,8 @@ use super::*; -use pallet_assets::FrozenBalance; use frame_support::traits::fungibles::{Inspect, InspectFreeze, MutateFreeze}; +use pallet_assets::FrozenBalance; use sp_runtime::traits::{Get, Zero}; impl, I: 'static> FrozenBalance, AccountIdOf, AssetBalanceOf> @@ -37,8 +37,9 @@ impl, I: 'static> FrozenBalance, AccountIdOf, As } } -// Implement fungibles::Inspect as it is required. To do so, we'll re-export -// all of `pallet-assets`' implementation of the same trait. +/// Implement [`frame_support::traits::fungibles::Inspect`] as it is bound by +/// [`frame_support::traits::fungibles::InspectFreeze`] and [`frame_support::traits::fungibles::MutateFreeze`]. +/// To do so, we'll re-export all of `pallet-assets` implementation of the same trait. impl, I: 'static> Inspect> for Pallet { type AssetId = AssetIdOf; type Balance = AssetBalanceOf; diff --git a/substrate/frame/assets-freezer/src/lib.rs b/substrate/frame/assets-freezer/src/lib.rs index 7711f4a525095..3eb8c9e9b8923 100644 --- a/substrate/frame/assets-freezer/src/lib.rs +++ b/substrate/frame/assets-freezer/src/lib.rs @@ -38,14 +38,18 @@ //! //! - Pallet hooks that implement custom logic to let `pallet-assets` know whether an balance is //! frozen for an account on a given asset (see: [`pallet_assets::FrozenBalance`]). -//! - An implementation of the fungibles [inspect][docs:inspect_freeze] and the [mutation][docs:mutate_freeze] -//! APIs for freezes. -//! -//! [docs:inspect_freeze]: `frame_support::traits::fungibles::InspectFreeze` -//! [docs:mutate_freeze]: `frame_support::traits::fungibles::MutateFreeze` +//! - An implementation of the fungibles [inspect][`frame_support::traits::fungibles::InspectFreeze`] +//! and the [mutation][`frame_support::traits::fungibles::InspectFreeze`] APIs for freezes. #![cfg_attr(not(feature = "std"), no_std)] +use frame_support::{ensure, pallet_prelude::DispatchResult}; +use frame_system::pallet_prelude::BlockNumberFor; +use sp_runtime::{ + traits::{Saturating, Zero}, + BoundedSlice, +}; + pub use pallet::*; #[cfg(test)] @@ -72,6 +76,7 @@ pub mod pallet { + FullCodec + TypeInfo + PartialEq + + Ord + MaxEncodedLen + Clone + Debug @@ -126,13 +131,15 @@ pub mod pallet { AccountIdOf, AssetBalanceOf, >; -} -use frame_support::pallet_prelude::DispatchResult; -use sp_runtime::{ - traits::{Saturating, Zero}, - BoundedSlice, -}; + #[pallet::hooks] + impl, I: 'static> Hooks> for Pallet { + #[cfg(feature = "try-runtime")] + fn try_state(_: BlockNumberFor) -> Result<(), sp_runtime::TryRuntimeError> { + Self::do_try_state() + } + } +} impl, I: 'static> Pallet { fn update_freezes( @@ -151,6 +158,7 @@ impl, I: 'static> Pallet { FrozenBalances::::set(asset.clone(), who, Some(after_frozen)); if freezes.is_empty() { Freezes::::remove(asset.clone(), who); + FrozenBalances::::remove(asset.clone(), who); } else { Freezes::::insert(asset.clone(), who, freezes); } @@ -163,4 +171,21 @@ impl, I: 'static> Pallet { } Ok(()) } + + #[cfg(any(test, feature = "try-runtime"))] + fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + for (asset, who, _) in FrozenBalances::::iter() { + let max_frozen_amount = Freezes::::get(asset.clone(), who.clone()) + .into_iter() + .reduce(IdAmount::>::max) + .map(|l| l.amount); + + ensure!( + FrozenBalances::::get(asset, who) == max_frozen_amount, + "The `FrozenAmount` is not equal to the maximum amount in `Freezes` for (`asset`, `who`)" + ); + } + + Ok(()) + } } diff --git a/substrate/frame/assets-freezer/src/mock.rs b/substrate/frame/assets-freezer/src/mock.rs index 7fefcd1abc4ba..03cab98109e8b 100644 --- a/substrate/frame/assets-freezer/src/mock.rs +++ b/substrate/frame/assets-freezer/src/mock.rs @@ -17,6 +17,7 @@ //! Tests mock for `pallet-assets-freezer`. +use crate as pallet_assets_freezer; pub use crate::*; use codec::{Compact, Decode, Encode, MaxEncodedLen}; use frame_support::{ @@ -29,10 +30,9 @@ use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, BuildStorage, }; -use crate as pallet_assets_freezer; pub type AccountId = u64; -type Balance = u64; +pub type Balance = u64; pub type AssetId = u32; type Block = frame_system::mocking::MockBlock; @@ -41,9 +41,8 @@ frame_support::construct_runtime!( { System: frame_system, Assets: pallet_assets, - Balances: pallet_balances, - // Assets: pallet_assets, AssetsFreezer: pallet_assets_freezer, + Balances: pallet_balances, } ); @@ -130,7 +129,7 @@ impl Config for Test { type MaxFreezes = ConstU32<2>; } -pub fn new_test_ext() -> sp_io::TestExternalities { +pub fn new_test_ext(execute: impl FnOnce()) -> sp_io::TestExternalities { let t = RuntimeGenesisConfig { assets: pallet_assets::GenesisConfig { assets: vec![(1, 0, true, 1)], @@ -145,6 +144,8 @@ pub fn new_test_ext() -> sp_io::TestExternalities { let mut ext: sp_io::TestExternalities = t.into(); ext.execute_with(|| { System::set_block_number(1); + execute(); + frame_support::assert_ok!(AssetsFreezer::do_try_state()); }); ext diff --git a/substrate/frame/assets-freezer/src/tests.rs b/substrate/frame/assets-freezer/src/tests.rs index 7a6a5ddfc2b57..9b16cb034aaa9 100644 --- a/substrate/frame/assets-freezer/src/tests.rs +++ b/substrate/frame/assets-freezer/src/tests.rs @@ -18,7 +18,6 @@ //! Tests for pallet-assets-freezer. use crate::mock::*; -use sp_runtime::BoundedVec; use frame_support::{ assert_ok, @@ -29,19 +28,24 @@ use pallet_assets::FrozenBalance; const WHO: AccountId = 1; const ASSET_ID: AssetId = 1; -fn basic_freeze() { - Freezes::::set( - ASSET_ID, - WHO, - BoundedVec::truncate_from(vec![IdAmount { id: DummyFreezeReason::Governance, amount: 1 }]), - ); - FrozenBalances::::insert(ASSET_ID, WHO, 1); +fn test_set_freeze(id: DummyFreezeReason, amount: Balance) { + let mut freezes = Freezes::::get(ASSET_ID, WHO); + + if let Some(i) = freezes.iter_mut().find(|l| l.id == id) { + i.amount = amount; + } else { + freezes + .try_push(IdAmount { id, amount }) + .expect("freeze is added without exceeding bounds; qed"); + } + + assert_ok!(AssetsFreezer::update_freezes(ASSET_ID, &WHO, freezes.as_bounded_slice())); } #[test] fn it_works_returning_balance_frozen() { - new_test_ext().execute_with(|| { - basic_freeze(); + new_test_ext(|| { + test_set_freeze(DummyFreezeReason::Governance, 1); assert_eq!( AssetsFreezer::balance_frozen(ASSET_ID, &DummyFreezeReason::Governance, &WHO), 1u64 @@ -51,9 +55,10 @@ fn it_works_returning_balance_frozen() { #[test] fn it_works_returning_frozen_balances() { - new_test_ext().execute_with(|| { - basic_freeze(); + new_test_ext(|| { + test_set_freeze(DummyFreezeReason::Governance, 1); assert_eq!(AssetsFreezer::frozen_balance(ASSET_ID, &WHO), Some(1u64)); + test_set_freeze(DummyFreezeReason::Staking, 3); FrozenBalances::::insert(ASSET_ID, WHO, 3); assert_eq!(AssetsFreezer::frozen_balance(ASSET_ID, &WHO), Some(3u64)); }); @@ -61,20 +66,17 @@ fn it_works_returning_frozen_balances() { #[test] fn it_works_returning_can_freeze() { - new_test_ext().execute_with(|| { - basic_freeze(); + new_test_ext(|| { + test_set_freeze(DummyFreezeReason::Governance, 1); assert!(AssetsFreezer::can_freeze(ASSET_ID, &DummyFreezeReason::Staking, &WHO)); - Freezes::::mutate(&ASSET_ID, &WHO, |f| { - f.try_push(IdAmount { id: DummyFreezeReason::Staking, amount: 1 }) - .expect("current freezes size is less than max freezes; qed"); - }); + test_set_freeze(DummyFreezeReason::Staking, 1); assert!(!AssetsFreezer::can_freeze(ASSET_ID, &DummyFreezeReason::Other, &WHO)); }); } #[test] fn set_freeze_works() { - new_test_ext().execute_with(|| { + new_test_ext(|| { assert_ok!(AssetsFreezer::set_freeze(ASSET_ID, &DummyFreezeReason::Governance, &WHO, 10)); assert_eq!( AssetsFreezer::reducible_balance( @@ -106,7 +108,7 @@ fn set_freeze_works() { #[test] fn extend_freeze_works() { - new_test_ext().execute_with(|| { + new_test_ext(|| { assert_ok!(AssetsFreezer::set_freeze(ASSET_ID, &DummyFreezeReason::Governance, &WHO, 10)); assert_ok!(AssetsFreezer::extend_freeze(ASSET_ID, &DummyFreezeReason::Governance, &WHO, 8)); System::assert_last_event( @@ -144,7 +146,7 @@ fn extend_freeze_works() { #[test] fn thaw_works() { - new_test_ext().execute_with(|| { + new_test_ext(|| { assert_ok!(AssetsFreezer::set_freeze(ASSET_ID, &DummyFreezeReason::Governance, &WHO, 10)); System::assert_has_event( Event::::AssetFrozen { asset_id: ASSET_ID, who: WHO, amount: 10 }.into(), diff --git a/substrate/frame/assets-freezer/src/types.rs b/substrate/frame/assets-freezer/src/types.rs index a53f07e97a51d..29e8252384b76 100644 --- a/substrate/frame/assets-freezer/src/types.rs +++ b/substrate/frame/assets-freezer/src/types.rs @@ -15,16 +15,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::traits::fungibles::Inspect; use frame_support::pallet_prelude::{Decode, Encode, MaxEncodedLen, RuntimeDebug, TypeInfo}; +use frame_support::traits::fungibles::Inspect; pub type AssetIdOf = as Inspect>>::AssetId; pub type AssetBalanceOf = as Inspect>>::Balance; pub type AccountIdOf = ::AccountId; /// An identifier and balance. -#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, TypeInfo)] -pub struct IdAmount { +#[derive( + Encode, Decode, Clone, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug, MaxEncodedLen, TypeInfo, +)] +pub struct IdAmount { /// An identifier for this item. pub id: Id, /// Some amount for this item.