fix origin of erc20 xtokens transfer

modules/currencies/src/lib.rs

@@ -364,12 +364,34 @@ impl<T: Config> MultiCurrency<T::AccountId> for Pallet<T> {
 			return Ok(());
 		}
 		match currency_id {
-			CurrencyId::Erc20(_) => <Self as MultiCurrency<T::AccountId>>::transfer(
-				currency_id,
-				&T::AddressMapping::get_account_id(&T::Erc20HoldingAccount::get()),
-				who,
-				amount,
-			),
+			CurrencyId::Erc20(contract) => {
+				// deposit from erc20 holding account to receiver(who). in xcm case which receive erc20 from sibling
+				// parachain, we choose receiver to charge storage fee. we must make sure receiver has enough native
+				// token to charge storage fee.
+				let sender = T::Erc20HoldingAccount::get();
+				let from = T::AddressMapping::get_account_id(&sender);
+				ensure!(
+					!Self::free_balance(currency_id, &from).is_zero(),
+					Error::<T>::DepositFailed
+				);
+				let receiver = T::AddressMapping::get_or_create_evm_address(who);
+				T::EVMBridge::transfer(
+					InvokeContext {
+						contract,
+						sender,
+						origin: receiver,
+					},
+					receiver,
+					amount,
+				)?;
+				Self::deposit_event(Event::Transferred {
+					currency_id,
+					from,
+					to: who.clone(),
+					amount,
+				});
+				Ok(())
+			}
 			id if id == T::GetNativeCurrencyId::get() => T::NativeCurrency::deposit(who, amount),
 			_ => T::MultiCurrency::deposit(currency_id, who, amount),
 		}
@@ -381,12 +403,30 @@ impl<T: Config> MultiCurrency<T::AccountId> for Pallet<T> {
 		}
 
 		match currency_id {
-			CurrencyId::Erc20(_) => <Self as MultiCurrency<T::AccountId>>::transfer(
-				currency_id,
-				who,
-				&T::AddressMapping::get_account_id(&T::Erc20HoldingAccount::get()),
-				amount,
-			),
+			CurrencyId::Erc20(contract) => {
+				// withdraw from sender(who) to erc20 holding account. in xcm case which receive erc20 from sibling
+				// parachain, sender is sibling parachain sovereign account. As the origin here is used to charge
+				// storage fee, we must make sure sibling parachain sovereign account has enough native token to
+				// charge storage fee.
+				let receiver = T::Erc20HoldingAccount::get();
+				let sender = T::AddressMapping::get_evm_address(who).ok_or(Error::<T>::EvmAccountNotFound)?;
+				T::EVMBridge::transfer(
+					InvokeContext {
+						contract,
+						sender,
+						origin: sender,
+					},
+					receiver,
+					amount,
+				)?;
+				Self::deposit_event(Event::Transferred {
+					currency_id,
+					from: who.clone(),
+					to: T::AddressMapping::get_account_id(&receiver),
+					amount,
+				});
+				Ok(())
+			}
 			id if id == T::GetNativeCurrencyId::get() => T::NativeCurrency::withdraw(who, amount),
 			_ => T::MultiCurrency::withdraw(currency_id, who, amount),
 		}

modules/currencies/src/tests.rs

@@ -1330,9 +1330,11 @@ fn fungible_mutate_trait_should_work() {
 				&alice(),
 				0
 			));
+			// mint_into will deposit erc20 holding account to recipient.
+			// but here erc20 holding account don't have enough balance.
 			assert_noop!(
 				<Currencies as fungibles::Mutate<_>>::mint_into(CurrencyId::Erc20(erc20_address()), &alice(), 1),
-				Error::<Runtime>::RealOriginNotFound
+				Error::<Runtime>::DepositFailed
 			);
 
 			assert_eq!(<AdaptedBasicCurrency as fungible::Inspect<_>>::total_issuance(), 101000);

runtime/integration-tests/src/relaychain/erc20.rs

@@ -103,47 +103,65 @@ fn erc20_transfer_between_sibling() {
 		));
 	});
 
+	let initial_native_amount = 1_000_000_000_000u128;
+	let storage_fee = 6_400_000_000u128;
+
 	Karura::execute_with(|| {
 		let alith = MockAddressMapping::get_account_id(&alice_evm_addr());
+		let total_erc20 = 100_000_000_000_000_000_000_000u128;
+		let transfer_amount = 10 * dollar(NATIVE_CURRENCY);
+
+		// used to deploy contracts
 		assert_ok!(Currencies::deposit(
 			NATIVE_CURRENCY,
 			&alice(),
 			1_000_000 * dollar(NATIVE_CURRENCY)
 		));
+		// when transfer erc20 cross chain, the origin `alith` is used to charge storage
 		assert_ok!(Currencies::deposit(
 			NATIVE_CURRENCY,
 			&alith.clone(),
-			1_000_000 * dollar(NATIVE_CURRENCY)
+			initial_native_amount
+		));
+		// when withdraw sibling parachain account, the origin `sibling_reserve_account` is used to charge
+		// storage
+		assert_ok!(Currencies::deposit(
+			NATIVE_CURRENCY,
+			&sibling_reserve_account(),
+			initial_native_amount
+		));
+		// when deposit to recipient, the origin is recipient `BOB`, and is used to charge storage.
+		assert_ok!(Currencies::deposit(
+			NATIVE_CURRENCY,
+			&AccountId::from(BOB),
+			initial_native_amount
+		));
+		// when xcm finished, deposit to treasury account, the origin is `treasury account`, and is used to
+		// charge storage.
+		assert_ok!(Currencies::deposit(
+			NATIVE_CURRENCY,
+			&KaruraTreasuryAccount::get(),
+			initial_native_amount
 		));
 
 		deploy_erc20_contracts();
 
-		// Erc20 claim account
-		assert_ok!(EvmAccounts::claim_account(
-			Origin::signed(AccountId::from(ALICE)),
-			EvmAccounts::eth_address(&alice_key()),
-			EvmAccounts::eth_sign(&alice_key(), &AccountId::from(ALICE))
-		));
-
+		// `transfer` invoked by `TransferReserveAsset` xcm instruction need to passing origin check.
+		// In frontend/js, when issue xtokens extrinsic, it have `EvmSetOrigin` SignedExtra to `set_origin`.
+		// In testcase, we're manual invoke `set_origin` here. because in erc20 xtokens transfer,
+		// the `from` or `to` is not erc20 holding account. so we need make sure origin exists.
 		<EVM as EVMTrait<AccountId>>::set_origin(alith.clone());
 
-		// use Currencies `transfer` dispatch call to transfer erc20 token to bob.
-		assert_ok!(Currencies::transfer(
-			Origin::signed(alith),
-			MultiAddress::Id(AccountId::from(CHARLIE)),
-			CurrencyId::Erc20(erc20_address_0()),
-			1_000_000_000_000_000
-		));
 		assert_eq!(
-			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &AccountId::from(CHARLIE)),
-			1_000_000_000_000_000
+			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &alith),
+			total_erc20
 		);
 
 		// transfer erc20 token to Sibling
 		assert_ok!(XTokens::transfer(
-			Origin::signed(CHARLIE.into()),
+			Origin::signed(alith.clone()),
 			CurrencyId::Erc20(erc20_address_0()),
-			10_000_000_000_000,
+			transfer_amount,
 			Box::new(
 				MultiLocation::new(
 					1,
@@ -160,14 +178,24 @@ fn erc20_transfer_between_sibling() {
 			1_000_000_000,
 		));
 
+		// using native token to charge storage fee
 		assert_eq!(
-			990_000_000_000_000,
-			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &AccountId::from(CHARLIE))
+			initial_native_amount - storage_fee,
+			Currencies::free_balance(NATIVE_CURRENCY, &alith)
 		);
 		assert_eq!(
-			10_000_000_000_000,
+			total_erc20 - transfer_amount,
+			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &alith)
+		);
+		assert_eq!(
+			transfer_amount,
 			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &sibling_reserve_account())
 		);
+		// initial_native_amount + ed
+		assert_eq!(
+			1_100_000_000_000,
+			Currencies::free_balance(NATIVE_CURRENCY, &KaruraTreasuryAccount::get())
+		);
 
 		System::reset_events();
 	});
@@ -213,30 +241,49 @@ fn erc20_transfer_between_sibling() {
 			5_000_000_000_000,
 			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &sibling_reserve_account())
 		);
-		assert_eq!(
-			6_400_000_000,
-			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &KaruraTreasuryAccount::get())
-		);
 		assert_eq!(
 			4_993_600_000_000,
 			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &AccountId::from(BOB))
 		);
+		assert_eq!(
+			6_400_000_000,
+			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &KaruraTreasuryAccount::get())
+		);
 		assert_eq!(
 			0,
 			Currencies::free_balance(CurrencyId::Erc20(erc20_address_0()), &erc20_holding_account)
 		);
+		// withdraw erc20 need charge storage fee
+		assert_eq!(
+			initial_native_amount - storage_fee,
+			Currencies::free_balance(NATIVE_CURRENCY, &sibling_reserve_account())
+		);
+		// deposit erc20 need charge storage fee
+		assert_eq!(
+			initial_native_amount - storage_fee,
+			Currencies::free_balance(NATIVE_CURRENCY, &AccountId::from(BOB))
+		);
+		// deposit reserve and unreserve storage fee, so the native token not changed.
+		assert_eq!(
+			1_100_000_000_000,
+			Currencies::free_balance(NATIVE_CURRENCY, &KaruraTreasuryAccount::get())
+		);
+
+		// withdraw operation transfer from sibling parachain account to erc20 holding account
 		System::assert_has_event(Event::Currencies(module_currencies::Event::Transferred {
 			currency_id: CurrencyId::Erc20(erc20_address_0()),
 			from: sibling_reserve_account(),
 			to: erc20_holding_account.clone(),
 			amount: 5_000_000_000_000,
 		}));
+		// deposit operation transfer from erc20 holding account to recipient
 		System::assert_has_event(Event::Currencies(module_currencies::Event::Transferred {
 			currency_id: CurrencyId::Erc20(erc20_address_0()),
 			from: erc20_holding_account.clone(),
 			to: AccountId::from(BOB),
 			amount: 4_993_600_000_000,
 		}));
+		// TakeRevenue deposit from erc20 holding account to treasury account
 		System::assert_has_event(Event::Currencies(module_currencies::Event::Transferred {
 			currency_id: CurrencyId::Erc20(erc20_address_0()),
 			from: erc20_holding_account,