change(pallet-assets-freezer): apply requested changes

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

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<T: Config<I>, I: 'static> FrozenBalance<AssetIdOf<T, I>, AccountIdOf<T>, AssetBalanceOf<T, I>>
@@ -37,8 +37,9 @@ impl<T: Config<I>, I: 'static> FrozenBalance<AssetIdOf<T, I>, AccountIdOf<T>, 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<T: Config<I>, I: 'static> Inspect<AccountIdOf<T>> for Pallet<T, I> {
 	type AssetId = AssetIdOf<T, I>;
 	type Balance = AssetBalanceOf<T, I>;

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<T>,
 		AssetBalanceOf<T, I>,
 	>;
-}
 
-use frame_support::pallet_prelude::DispatchResult;
-use sp_runtime::{
-	traits::{Saturating, Zero},
-	BoundedSlice,
-};
+	#[pallet::hooks]
+	impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
+		#[cfg(feature = "try-runtime")]
+		fn try_state(_: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
+			Self::do_try_state()
+		}
+	}
+}
 
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	fn update_freezes(
@@ -151,6 +158,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		FrozenBalances::<T, I>::set(asset.clone(), who, Some(after_frozen));
 		if freezes.is_empty() {
 			Freezes::<T, I>::remove(asset.clone(), who);
+			FrozenBalances::<T, I>::remove(asset.clone(), who);
 		} else {
 			Freezes::<T, I>::insert(asset.clone(), who, freezes);
 		}
@@ -163,4 +171,21 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		}
 		Ok(())
 	}
+
+	#[cfg(any(test, feature = "try-runtime"))]
+	fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
+		for (asset, who, _) in FrozenBalances::<T, I>::iter() {
+			let max_frozen_amount = Freezes::<T, I>::get(asset.clone(), who.clone())
+				.into_iter()
+				.reduce(IdAmount::<T::RuntimeFreezeReason, AssetBalanceOf<T, I>>::max)
+				.map(|l| l.amount);
+
+			ensure!(
+				FrozenBalances::<T, I>::get(asset, who) == max_frozen_amount,
+				"The `FrozenAmount` is not equal to the maximum amount in `Freezes` for (`asset`, `who`)"
+			);
+		}
+
+		Ok(())
+	}
 }

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<Test>;
 
@@ -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

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::<Test>::set(
-		ASSET_ID,
-		WHO,
-		BoundedVec::truncate_from(vec![IdAmount { id: DummyFreezeReason::Governance, amount: 1 }]),
-	);
-	FrozenBalances::<Test>::insert(ASSET_ID, WHO, 1);
+fn test_set_freeze(id: DummyFreezeReason, amount: Balance) {
+	let mut freezes = Freezes::<Test>::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,30 +55,28 @@ 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::<Test>::insert(ASSET_ID, WHO, 3);
 		assert_eq!(AssetsFreezer::frozen_balance(ASSET_ID, &WHO), Some(3u64));
 	});
 }
 
 #[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::<Test>::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::<Test>::AssetFrozen { asset_id: ASSET_ID, who: WHO, amount: 10 }.into(),

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<T, I> = <pallet_assets::Pallet<T, I> as Inspect<AccountIdOf<T>>>::AssetId;
 pub type AssetBalanceOf<T, I> = <pallet_assets::Pallet<T, I> as Inspect<AccountIdOf<T>>>::Balance;
 pub type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
 
 /// An identifier and balance.
-#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, TypeInfo)]
-pub struct IdAmount<Id, Balance> {
+#[derive(
+	Encode, Decode, Clone, PartialEq, Eq, PartialOrd, Ord, RuntimeDebug, MaxEncodedLen, TypeInfo,
+)]
+pub struct IdAmount<Id: Ord, Balance: Ord> {
 	/// An identifier for this item.
 	pub id: Id,
 	/// Some amount for this item.