fix: only destroy funds on refund if collateral is BelowMinimum

pallets/pallet-bonded-coins/src/lib.rs

@@ -64,7 +64,7 @@ pub mod pallet {
 				Create as CreateFungibles, Destroy as DestroyFungibles, Inspect as InspectFungibles,
 				Mutate as MutateFungibles,
 			},
-			tokens::{Fortitude, Precision as WithdrawalPrecision, Preservation, Provenance},
+			tokens::{DepositConsequence, Fortitude, Precision as WithdrawalPrecision, Preservation, Provenance},
 			AccountTouch,
 		},
 		Hashable, Parameter,
@@ -1119,8 +1119,7 @@ pub mod pallet {
 
 			if amount.is_zero()
 				|| T::Collaterals::can_deposit(pool_details.collateral.clone(), &who, amount, Provenance::Extant)
-					.into_result()
-					.is_err()
+					== DepositConsequence::BelowMinimum
 			{
 				// Funds are burnt but the collateral received is not sufficient to be deposited
 				// to the account. This is tolerated as otherwise we could have edge cases where

pallets/pallet-bonded-coins/src/mock.rs

@@ -58,6 +58,7 @@ pub mod runtime {
 		weights::constants::RocksDbWeight,
 	};
 	use frame_system::{EnsureRoot, EnsureSigned};
+	use pallet_assets::FrozenBalance;
 	use sp_core::U256;
 	use sp_runtime::{
 		traits::{BlakeTwo256, IdentifyAccount, IdentityLookup, Verify},
@@ -69,7 +70,7 @@ pub mod runtime {
 		self as pallet_bonded_coins,
 		traits::NextAssetIds,
 		types::{Locks, PoolStatus},
-		Config, DepositBalanceOf, FungiblesAssetIdOf, PoolDetailsOf,
+		AccountIdOf, Config, DepositBalanceOf, FungiblesAssetIdOf, FungiblesBalanceOf, PoolDetailsOf,
 	};
 
 	pub type Hash = sp_core::H256;
@@ -190,6 +191,28 @@ pub mod runtime {
 		}
 	}
 
+	/// Store freezes for the assets pallet.
+	#[storage_alias]
+	pub type Freezes<Assets: PalletInfoAccess> = StorageDoubleMap<
+		Assets,
+		Blake2_128Concat,
+		FungiblesAssetIdOf<Test>,
+		Blake2_128Concat,
+		AccountIdOf<Test>,
+		FungiblesBalanceOf<Test>,
+		OptionQuery,
+	>;
+
+	pub struct FreezesHook;
+
+	impl FrozenBalance<AssetId, AccountId, Balance> for FreezesHook {
+		fn died(_asset: AssetId, _who: &AccountId) {}
+
+		fn frozen_balance(asset: AssetId, who: &AccountId) -> Option<Balance> {
+			Freezes::<Assets>::get(asset, who)
+		}
+	}
+
 	frame_support::construct_runtime!(
 		pub enum Test
 		{
@@ -279,7 +302,7 @@ pub mod runtime {
 		type Currency = Balances;
 		type Extra = ();
 		type ForceOrigin = EnsureRoot<AccountId>;
-		type Freezer = ();
+		type Freezer = FreezesHook;
 		type MetadataDepositBase = ConstU128<0>;
 		type MetadataDepositPerByte = ConstU128<0>;
 		type RemoveItemsLimit = ConstU32<5>;
@@ -355,6 +378,7 @@ pub mod runtime {
 		//  pool_id, PoolDetails
 		pools: Vec<(AccountId, PoolDetailsOf<Test>)>,
 		collaterals: Vec<AssetId>,
+		freezes: Vec<(AssetId, AccountId, Balance)>,
 	}
 
 	impl ExtBuilder {
@@ -378,6 +402,11 @@ pub mod runtime {
 			self
 		}
 
+		pub(crate) fn with_freezes(mut self, freezes: Vec<(AssetId, AccountId, Balance)>) -> Self {
+			self.freezes = freezes;
+			self
+		}
+
 		pub(crate) fn build(self) -> sp_io::TestExternalities {
 			let mut storage = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
 
@@ -448,6 +477,10 @@ pub mod runtime {
 				});
 
 				NextAssetId::<BondingPallet>::set(next_asset_id);
+
+				self.freezes.iter().for_each(|(asset_id, account, amount)| {
+					Freezes::<Assets>::set(asset_id, account, Some(*amount));
+				});
 			});
 
 			ext

pallets/pallet-bonded-coins/src/tests/transactions/refund_account.rs

@@ -17,9 +17,9 @@
 // If you feel like getting in touch with us, you can do so at info@botlabs.org
 use frame_support::{
 	assert_err, assert_ok,
-	traits::fungibles::{Create, Inspect, Mutate},
+	traits::fungibles::{roles::Inspect as InspectRole, Create, Inspect, Mutate},
 };
-use frame_system::{pallet_prelude::OriginFor, RawOrigin};
+use frame_system::{pallet_prelude::OriginFor, ConsumerLimits, RawOrigin};
 use sp_runtime::TokenError;
 
 use crate::{
@@ -266,6 +266,112 @@ fn refund_below_min_balance() {
 		});
 }
 
+#[test]
+fn refund_account_fails_when_account_blocked() {
+	let pool_details = generate_pool_details(
+		vec![DEFAULT_BONDED_CURRENCY_ID],
+		get_linear_bonding_curve(),
+		false,
+		Some(PoolStatus::Refunding),
+		Some(ACCOUNT_00),
+		None,
+		None,
+		None,
+	);
+	let pool_id: AccountIdOf<Test> = calculate_pool_id(&[DEFAULT_BONDED_CURRENCY_ID]);
+
+	ExtBuilder::default()
+		.with_native_balances(vec![(ACCOUNT_01, ONE_HUNDRED_KILT)])
+		.with_pools(vec![(pool_id.clone(), pool_details)])
+		.with_collaterals(vec![DEFAULT_COLLATERAL_CURRENCY_ID])
+		.with_bonded_balance(vec![
+			(DEFAULT_COLLATERAL_CURRENCY_ID, pool_id.clone(), ONE_HUNDRED_KILT),
+			(DEFAULT_COLLATERAL_CURRENCY_ID, ACCOUNT_01, ONE_HUNDRED_KILT),
+			(DEFAULT_BONDED_CURRENCY_ID, ACCOUNT_01, ONE_HUNDRED_KILT),
+		])
+		.build_and_execute_with_sanity_tests(|| {
+			let origin: OriginFor<Test> = RawOrigin::Signed(ACCOUNT_01).into();
+
+			Assets::block(
+				RawOrigin::Signed(Assets::owner(DEFAULT_COLLATERAL_CURRENCY_ID).unwrap()).into(),
+				DEFAULT_COLLATERAL_CURRENCY_ID,
+				ACCOUNT_01,
+			)
+			.expect("Failed to block account for test");
+
+			// Ensure the refund_account call fails due to failing can_deposit check
+			assert_err!(
+				BondingPallet::refund_account(origin, pool_id.clone(), ACCOUNT_01, 0, 1),
+				TokenError::Blocked
+			);
+		});
+}
+
+#[test]
+fn refund_account_fails_if_account_cannot_be_created() {
+	let pool_details = generate_pool_details(
+		vec![DEFAULT_BONDED_CURRENCY_ID],
+		get_linear_bonding_curve(),
+		false,
+		Some(PoolStatus::Refunding),
+		Some(ACCOUNT_00),
+		None,
+		None,
+		None,
+	);
+	let pool_id: AccountIdOf<Test> = calculate_pool_id(&[DEFAULT_BONDED_CURRENCY_ID]);
+
+	// get MaxConsumers value
+	let max_consumers: usize = <Test as frame_system::Config>::MaxConsumers::max_consumers()
+		.try_into()
+		.expect("");
+	// Collateral must be non-sufficient for these tests to work, so we'll create a
+	// new collateral asset in the test
+	let collateral_id = DEFAULT_BONDED_CURRENCY_ID + 1;
+
+	ExtBuilder::default()
+		.with_native_balances(vec![
+			(ACCOUNT_01, ONE_HUNDRED_KILT),
+			(pool_id.clone(), ONE_HUNDRED_KILT),
+		])
+		.with_pools(vec![(pool_id.clone(), pool_details)])
+		.with_collaterals(vec![DEFAULT_COLLATERAL_CURRENCY_ID])
+		.with_bonded_balance(vec![
+			(DEFAULT_COLLATERAL_CURRENCY_ID, pool_id.clone(), ONE_HUNDRED_KILT),
+			(DEFAULT_BONDED_CURRENCY_ID, ACCOUNT_01, ONE_HUNDRED_KILT),
+		])
+		// Freeze some funds for ACCOUNT_01 so refund can only be partial
+		.with_freezes(vec![(DEFAULT_BONDED_CURRENCY_ID, ACCOUNT_01, 100_000)])
+		.build_and_execute_with_sanity_tests(|| {
+			// switch pool's collateral to a non-sufficient one
+			assert_ok!(<Assets as Create<_>>::create(collateral_id, ACCOUNT_00, false, 1));
+			Pools::<Test>::mutate(&pool_id, |details| details.as_mut().unwrap().collateral = collateral_id);
+			// make sure pool account holds collateral - this should work because it also
+			// holds the (sufficient) original collateral
+			assert_ok!(Assets::mint_into(collateral_id, &pool_id, ONE_HUNDRED_KILT));
+			// create non-sufficient assets to increase ACCOUNT_01 consumers count
+			// the assets pallet requires at least two references to be available for
+			// creating an account, but creates only one, meaning we create max_consumers -
+			// 2 currencies (resulting in max_consumers - 1 consumers because we created one
+			// previously)
+			for i in 1..max_consumers - 1 {
+				let i_u32: u32 = i.try_into().expect("Failed to convert to u32");
+				let asset_id = collateral_id + i_u32;
+				assert_ok!(<Assets as Create<_>>::create(asset_id, ACCOUNT_00, false, 1));
+				assert_ok!(Assets::mint_into(asset_id, &ACCOUNT_01, ONE_HUNDRED_KILT));
+			}
+
+			let origin: OriginFor<Test> = RawOrigin::Signed(ACCOUNT_01).into();
+
+			// Collateral is non-sufficient and thus would create additional consumer
+			// references on ACCOUNT_01, which is too many
+			assert_err!(
+				BondingPallet::refund_account(origin, pool_id.clone(), ACCOUNT_01, 0, 1),
+				TokenError::CannotCreate
+			);
+		});
+}
+
 #[test]
 fn refund_account_fails_when_pool_not_refunding() {
 	let pool_details = generate_pool_details(