Merge pull request #1358 from galacticcouncil/fix/dust-account-whitelist

fix: dust account

Author: mrq1911

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "runtime-integration-tests"
-version = "1.68.0"
+version = "1.69.0"
 description = "Integration tests"
 authors = ["GalacticCouncil"]
 edition = "2021"

integration-tests/Cargo.toml

@@ -1372,15 +1372,15 @@ fn check_atoken_transfer_with_rounding_error() {
 		assert_eq!(Currencies::free_balance(ADOT, &BOB.into()), first_transfer_amount);
 
 		let adot_contract = HydraErc20Mapping::asset_address(ADOT);
+		assert_ok!(EVMAccounts::bind_evm_address(RuntimeOrigin::signed(ALICE.into())));
+		let alice_dot_balance_before = Currencies::free_balance(DOT, &ALICE.into());
 		assert_ok!(AaveTradeExecutor::<hydradx_runtime::Runtime>::withdraw_all_to(
 			adot_contract,
 			&BOB.into(),
 			&ALICE.into()
 		));
-		assert_eq!(
-			Currencies::free_balance(ADOT, &ALICE.into()),
-			alice_balance + first_transfer_amount
-		);
+		assert_eq!(Currencies::free_balance(ADOT, &BOB.into()), 0);
+		assert!(Currencies::free_balance(DOT, &ALICE.into()) > alice_dot_balance_before);
 	})
 }
 

integration-tests/src/aave_router.rs

@@ -386,6 +386,9 @@ mod atoken_dust {
 			assert_ok!(EVMAccounts::bind_evm_address(hydradx_runtime::RuntimeOrigin::signed(
 				ALICE.into()
 			)));
+			assert_ok!(EVMAccounts::bind_evm_address(hydradx_runtime::RuntimeOrigin::signed(
+				Treasury::account_id()
+			)));
 
 			let alice_dot_balance_before = 8999999999999998;
 			assert_eq!(
@@ -407,14 +410,23 @@ mod atoken_dust {
 				0
 			);
 
+			let treasury_dot_balance_before =
+				Currencies::free_balance(crate::aave_router::DOT, &Treasury::account_id());
+
 			assert_ok!(Duster::dust_account(
 				hydradx_runtime::RuntimeOrigin::signed(ALICE.into()),
 				ALICE.into(),
 				ADOT,
 			));
 
 			assert_eq!(Currencies::free_balance(ADOT, &ALICE.into()), 0);
-			assert_eq!(Currencies::free_balance(ADOT, &Treasury::account_id()), 1);
+			// Treasury receives underlying DOT, not ADOT, as Treasury doesnt need AToken
+			assert_eq!(Currencies::free_balance(ADOT, &Treasury::account_id()), 0);
+			assert_eq!(
+				Currencies::free_balance(crate::aave_router::DOT, &Treasury::account_id()),
+				treasury_dot_balance_before + 1
+			);
+
 			//Alice DOT (adot underlying asset) balance should remain the same after dusting
 			assert_eq!(
 				Currencies::free_balance(crate::aave_router::DOT, &ALICE.into()),
@@ -423,6 +435,60 @@ mod atoken_dust {
 		});
 	}
 
+	#[test]
+	fn dust_account_should_work_for_unbound_evm_account_when_atoken_balance_below_ed() {
+		TestNet::reset();
+
+		crate::aave_router::with_atoken(|| {
+			let ed = 10000;
+			set_ed(ADOT, ed);
+			//Needed because withdraw_all_to sends the assets to treasury which should be a known bounded account
+			assert_ok!(EVMAccounts::bind_evm_address(hydradx_runtime::RuntimeOrigin::signed(
+				Treasury::account_id()
+			)));
+
+			let alice_dot_balance_before = Currencies::free_balance(crate::aave_router::DOT, &ALICE.into());
+			assert_eq!(Currencies::free_balance(ADOT, &ALICE.into()), START_BALANCE);
+
+			// Make account fall below ED
+			assert_ok!(Currencies::transfer(
+				hydradx_runtime::RuntimeOrigin::signed(ALICE.into()),
+				BOB.into(),
+				ADOT,
+				START_BALANCE - 1
+			));
+
+			assert_eq!(
+				hydradx_runtime::Currencies::free_balance(ADOT, &Treasury::account_id()),
+				0
+			);
+
+			let treasury_dot_balance_before =
+				Currencies::free_balance(crate::aave_router::DOT, &Treasury::account_id());
+
+			// Act
+			assert_ok!(Duster::dust_account(
+				hydradx_runtime::RuntimeOrigin::signed(ALICE.into()),
+				ALICE.into(),
+				ADOT,
+			));
+
+			assert_eq!(Currencies::free_balance(ADOT, &ALICE.into()), 0);
+			// Treasury receives underlying DOT, not ADOT
+			assert_eq!(Currencies::free_balance(ADOT, &Treasury::account_id()), 0);
+			assert_eq!(
+				Currencies::free_balance(crate::aave_router::DOT, &Treasury::account_id()),
+				treasury_dot_balance_before + 1
+			);
+
+			// Alice DOT (underlying asset) balance should remain the same after dusting
+			assert_eq!(
+				Currencies::free_balance(crate::aave_router::DOT, &ALICE.into()),
+				alice_dot_balance_before
+			);
+		});
+	}
+
 	#[test]
 	fn no_dusting_when_atoken_balance_above_ed() {
 		TestNet::reset();
@@ -456,6 +522,11 @@ mod atoken_dust {
 		let ed_range = 1_u128..(START_BALANCE - 1);
 
 		crate::aave_router::with_atoken_rollback(|| {
+			//Needed because withdraw_all_to sends the assets to treasury which should be a known bounded account
+			assert_ok!(EVMAccounts::bind_evm_address(hydradx_runtime::RuntimeOrigin::signed(
+				Treasury::account_id()
+			)));
+
 			// We run prop test this way to use the same state of the chain for all run without loading the snapshot again in every run
 			let mut runner = TestRunner::new(Config {
 				cases: successfull_cases,

integration-tests/src/dust.rs

@@ -1,6 +1,6 @@
 [package]
 name = "pallet-duster"
-version = "3.6.0"
+version = "3.7.0"
 description = "Account duster"
 authors = ["GalacticCouncil"]
 edition = "2021"
@@ -36,7 +36,6 @@ frame-system = { workspace = true }
 frame-benchmarking = { workspace = true, optional = true }
 
 [dev-dependencies]
-lazy_static = { workspace = true }
 sp-io = { workspace = true }
 sp-core = { workspace = true }
 pallet-balances = { workspace = true }
@@ -63,6 +62,7 @@ std = [
     "scale-info/std",
     "orml-tokens/std",
     "hydradx-traits/std",
-    "pallet-balances/std"
+    "pallet-balances/std",
+    "pallet-currencies/std"
 ]
 try-runtime = ["frame-support/try-runtime"]

pallets/duster/Cargo.toml

@@ -164,7 +164,8 @@ pub mod pallet {
 		/// IF account balance is < min. existential deposit of given currency, and account is allowed to
 		/// be dusted, the remaining balance is transferred to treasury account.
 		///
-		/// In case of AToken, we perform an erc20 dust, which does a wihtdraw all then supply atoken on behalf of the dust receiver
+		/// In case of AToken, we perform an erc20 dust, which does a wihtdraw all to the treasury account
+		/// Note that in this case, the treasury will just receive the underlying token, not the atoken variant.
 		///
 		/// The transaction fee is returned back in case of successful dusting.
 		///
@@ -194,10 +195,7 @@ pub mod pallet {
 			let dust_dest_account = T::TreasuryAccountId::get();
 
 			if T::Erc20Support::is_atoken(currency_id) {
-				//Temporarily adding the account to whitelist to prevent ED error when AToken is withdrawn from contract
-				Self::add_account(&account)?;
 				T::Erc20Support::on_dust(&account, &dust_dest_account, currency_id)?;
-				Self::remove_account(&account)?;
 			} else {
 				Self::transfer_dust(&account, &dust_dest_account, currency_id, dust)?;
 			}

pallets/duster/src/lib.rs

@@ -19,12 +19,12 @@ use pallet_currencies::fungibles::FungibleCurrencies;
 use primitives::EvmAddress;
 use sp_runtime::{
 	traits::{BlakeTwo256, IdentityLookup},
-	BuildStorage,
+	AccountId32, BuildStorage,
 };
 use sp_std::cell::RefCell;
 use sp_std::vec::Vec;
 
-type AccountId = u64;
+type AccountId = AccountId32;
 pub type AssetId = u32;
 type Balance = u128;
 type Amount = i128;
@@ -35,19 +35,17 @@ pub const ATOKEN_ED: u128 = 1000u128;
 
 type Block = frame_system::mocking::MockBlock<Test>;
 
-lazy_static::lazy_static! {
-pub static ref ALICE: AccountId = 100;
-pub static ref BOB: AccountId = 200;
-pub static ref DUSTER: AccountId = 300;
-pub static ref TREASURY: AccountId = 400;
-}
+pub const ALICE: AccountId = AccountId32::new([1u8; 32]);
+pub const BOB: AccountId = AccountId32::new([2u8; 32]);
+pub const DUSTER: AccountId = AccountId32::new([3u8; 32]);
+pub const TREASURY: AccountId = AccountId32::new([4u8; 32]);
 
 thread_local! {
 	pub static ATOKEN_IDS: RefCell<Vec<AssetId>> = const { RefCell::new(vec![]) };
 }
 
 parameter_types! {
-	pub TreasuryAccount: AccountId = *TREASURY;
+	pub TreasuryAccount: AccountId = TREASURY;
 }
 
 frame_support::construct_runtime!(
@@ -76,13 +74,13 @@ parameter_types! {
 }
 
 thread_local! {
-	pub static KILLED: RefCell<Vec<u64>> = const { RefCell::new(vec![]) };
+	pub static KILLED: RefCell<Vec<AccountId32>> = const { RefCell::new(vec![]) };
 }
 
 pub struct RecordKilled;
-impl OnKilledAccount<u64> for RecordKilled {
-	fn on_killed_account(who: &u64) {
-		KILLED.with(|r| r.borrow_mut().push(*who))
+impl OnKilledAccount<AccountId32> for RecordKilled {
+	fn on_killed_account(who: &AccountId32) {
+		KILLED.with(|r| r.borrow_mut().push(who.clone()))
 	}
 }
 
@@ -97,7 +95,7 @@ impl system::Config for Test {
 	type Block = Block;
 	type Hash = H256;
 	type Hashing = BlakeTwo256;
-	type AccountId = u64;
+	type AccountId = AccountId32;
 	type Lookup = IdentityLookup<Self::AccountId>;
 	type RuntimeEvent = RuntimeEvent;
 	type BlockHashCount = BlockHashCount;
@@ -190,8 +188,8 @@ impl Erc20OnDust<AccountId, AssetId> for ATokenDusterMock {
 		let balance = Tokens::free_balance(currency_id, account);
 		if balance < ATOKEN_ED {
 			Tokens::transfer(
-				RuntimeOrigin::signed(*account),
-				*dust_dest_account,
+				RuntimeOrigin::signed(account.clone()),
+				dust_dest_account.clone(),
 				currency_id,
 				balance,
 			)?;
@@ -221,7 +219,7 @@ impl pallet_currencies::Config for Test {
 	type NativeCurrency = BasicCurrencyAdapter<Test, Balances, Amount, u32>;
 	type Erc20Currency = MockErc20Currency<Test>;
 	type BoundErc20 = MockBoundErc20<Test>;
-	type ReserveAccount = ();
+	type ReserveAccount = TreasuryAccount;
 	type GetNativeCurrencyId = NativeCurrencyId;
 	type RegistryInspect = MockBoundErc20<Test>;
 	type WeightInfo = ();
@@ -252,7 +250,7 @@ impl Default for ExtBuilder {
 	fn default() -> Self {
 		Self {
 			endowed_accounts: vec![],
-			native_balances: vec![(*TREASURY, 1_000_000)],
+			native_balances: vec![(TREASURY, 1_000_000)],
 		}
 	}
 }
@@ -284,7 +282,7 @@ impl ExtBuilder {
 		.unwrap();
 
 		duster::GenesisConfig::<Test> {
-			account_whitelist: vec![*TREASURY],
+			account_whitelist: vec![TREASURY],
 		}
 		.assimilate_storage(&mut t)
 		.unwrap();