diff --git a/integration-tests/src/non_native_fee.rs b/integration-tests/src/non_native_fee.rs index 3f307bc03..c4b9134a9 100644 --- a/integration-tests/src/non_native_fee.rs +++ b/integration-tests/src/non_native_fee.rs @@ -1,9 +1,10 @@ #![cfg(test)] use crate::polkadot_test_net::*; +use frame_support::sp_runtime::codec::Encode; use frame_support::{ assert_ok, - dispatch::DispatchInfo, + dispatch::{DispatchInfo, GetDispatchInfo}, sp_runtime::{traits::DispatchTransaction, FixedU128, Permill}, weights::Weight, }; @@ -170,6 +171,160 @@ fn set_currency_should_work_in_batch_transaction_when_first_tx() { }); } +#[test] +fn set_currency_should_work_in_dispatch_with_extra_gas() { + // Regression test for https://github.com/galacticcouncil/hydration-node/issues/1296 + // dispatch_with_extra_gas wrapping set_currency should charge the fee in the NEW currency, + // not in the previously stored account currency. + TestNet::reset(); + + Hydra::execute_with(|| { + use frame_support::traits::OnInitialize; + hydradx_runtime::MultiTransactionPayment::on_initialize(1); + + // BOB starts with no explicit fee currency (defaults to HDX). + assert_eq!(MultiTransactionPayment::get_currency(AccountId::from(BOB)), None); + + let set_currency_call = hydradx_runtime::RuntimeCall::MultiTransactionPayment( + pallet_transaction_multi_payment::Call::set_currency { currency: BTC }, + ); + + let call = hydradx_runtime::RuntimeCall::Dispatcher(pallet_dispatcher::Call::dispatch_with_extra_gas { + call: Box::new(set_currency_call), + extra_gas: 0, + }); + + let info = call.get_dispatch_info(); + let len = call.encoded_size(); + + let bob_hdx_before = hydradx_runtime::Balances::free_balance(AccountId::from(BOB)); + let bob_btc_before = hydradx_runtime::Tokens::free_balance(BTC, &AccountId::from(BOB)); + + assert_ok!( + pallet_transaction_payment::ChargeTransactionPayment::::from(0) + .validate_and_prepare(Some(AccountId::from(BOB)).into(), &call, &info, len, 0,) + ); + + let bob_hdx_after = hydradx_runtime::Balances::free_balance(AccountId::from(BOB)); + let bob_btc_after = hydradx_runtime::Tokens::free_balance(BTC, &AccountId::from(BOB)); + + assert!( + bob_btc_after < bob_btc_before, + "BTC balance should decrease — fee must be charged in the new currency" + ); + assert_eq!(bob_hdx_after, bob_hdx_before, "HDX must not be charged"); + }); +} + +#[test] +fn set_currency_should_work_in_dispatch_with_extra_gas_for_evm_account() { + // Regression test for https://github.com/galacticcouncil/hydration-node/issues/1293 + // EVM accounts calling dispatch_with_extra_gas { set_currency } should charge the fee + // in the NEW currency, not in WETH (the default EVM account fee currency). + TestNet::reset(); + + Hydra::execute_with(|| { + use frame_support::traits::OnInitialize; + hydradx_runtime::MultiTransactionPayment::on_initialize(1); + + let evm_acc = evm_account(); + + // Fund the EVM account with WETH (default EVM fee currency) and DAI + assert_ok!(hydradx_runtime::Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + evm_acc.clone(), + WETH, + 1_000_000_000_000_000_000i128, + )); + assert_ok!(hydradx_runtime::Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + evm_acc.clone(), + DAI, + 1_000_000_000_000_000_000i128, + )); + + assert_eq!( + MultiTransactionPayment::get_currency(evm_acc.clone()), + Some(WETH), + "EVM account should have WETH set as default fee currency" + ); + + let set_currency_call = hydradx_runtime::RuntimeCall::MultiTransactionPayment( + pallet_transaction_multi_payment::Call::set_currency { currency: DAI }, + ); + + let call = hydradx_runtime::RuntimeCall::Dispatcher(pallet_dispatcher::Call::dispatch_with_extra_gas { + call: Box::new(set_currency_call), + extra_gas: 0, + }); + + let info = call.get_dispatch_info(); + let len = call.encoded_size(); + + let weth_before = hydradx_runtime::Tokens::free_balance(WETH, &evm_acc); + let dai_before = hydradx_runtime::Tokens::free_balance(DAI, &evm_acc); + + assert_ok!( + pallet_transaction_payment::ChargeTransactionPayment::::from(0) + .validate_and_prepare(Some(evm_acc.clone()).into(), &call, &info, len, 0,) + ); + + let weth_after = hydradx_runtime::Tokens::free_balance(WETH, &evm_acc); + let dai_after = hydradx_runtime::Tokens::free_balance(DAI, &evm_acc); + + assert!( + dai_after < dai_before, + "DAI balance should decrease — fee must be charged in the new currency" + ); + assert_eq!(weth_after, weth_before, "WETH must not be charged"); + }); +} + +#[test] +fn set_currency_should_not_work_in_dispatch_with_extra_gas_when_not_direct_inner_call() { + // set_currency must only be recognised when it is the direct inner call of + // dispatch_with_extra_gas, not when it is nested deeper (e.g. inside a batch inside the + // dispatcher). In that case the fee should fall back to the previously stored currency. + TestNet::reset(); + + Hydra::execute_with(|| { + use frame_support::traits::OnInitialize; + hydradx_runtime::MultiTransactionPayment::on_initialize(1); + + let set_currency_call = hydradx_runtime::RuntimeCall::MultiTransactionPayment( + pallet_transaction_multi_payment::Call::set_currency { currency: BTC }, + ); + + // Wrap set_currency inside a batch, then wrap that inside dispatch_with_extra_gas. + // The resolver only looks one level deep, so BTC should NOT be picked up. + let batch = hydradx_runtime::RuntimeCall::Utility(pallet_utility::Call::batch { + calls: vec![set_currency_call], + }); + + let call = hydradx_runtime::RuntimeCall::Dispatcher(pallet_dispatcher::Call::dispatch_with_extra_gas { + call: Box::new(batch), + extra_gas: 0, + }); + + let info = call.get_dispatch_info(); + let len = call.encoded_size(); + + let bob_btc_before = hydradx_runtime::Tokens::free_balance(BTC, &AccountId::from(BOB)); + + assert_ok!( + pallet_transaction_payment::ChargeTransactionPayment::::from(0) + .validate_and_prepare(Some(AccountId::from(BOB)).into(), &call, &info, len, 0,) + ); + + let bob_btc_after = hydradx_runtime::Tokens::free_balance(BTC, &AccountId::from(BOB)); + + assert_eq!( + bob_btc_after, bob_btc_before, + "BTC must not be charged when set_currency is nested deeper than the direct inner call" + ); + }); +} + #[test] fn set_currency_should_not_work_in_batch_transaction_when_not_first_tx() { TestNet::reset(); diff --git a/pallets/transaction-multi-payment/Cargo.toml b/pallets/transaction-multi-payment/Cargo.toml index 93a72537b..883545864 100644 --- a/pallets/transaction-multi-payment/Cargo.toml +++ b/pallets/transaction-multi-payment/Cargo.toml @@ -36,7 +36,7 @@ pallet-utility = { workspace = true } pallet-evm = { workspace = true, optional = true } [dev-dependencies] -pallet-currencies = { workspace = true } +pallet-currencies = { workspace = true, features = ["std"] } orml-tokens = { workspace = true, features = ["std"] } pallet-balances = { workspace = true, features = ["std"] } pallet-evm-accounts = { workspace = true, features = ["std"] } @@ -58,6 +58,9 @@ std = [ "scale-info/std", "pallet-evm/std", "primitives/std", + "pallet-utility/std", + "pallet-xyk/std", + "hydra-dx-math/std", ] try-runtime = ["frame-support/try-runtime"] evm = ["pallet-evm"] diff --git a/pallets/transaction-multi-payment/src/lib.rs b/pallets/transaction-multi-payment/src/lib.rs index 7175c2396..59b728f09 100644 --- a/pallets/transaction-multi-payment/src/lib.rs +++ b/pallets/transaction-multi-payment/src/lib.rs @@ -414,7 +414,7 @@ pub mod pallet { let encoded = data.clone(); let mut encoded_extrinsic = encoded.as_slice(); - let maybe_call: Result<::RuntimeCall, _> = + let maybe_call: Result = DecodeLimit::decode_all_with_depth_limit(32, &mut encoded_extrinsic); let currency = if let Ok(call) = maybe_call { @@ -484,7 +484,7 @@ pub mod pallet { let encoded = data.clone(); let mut encoded_extrinsic = encoded.as_slice(); - let maybe_call: Result<::RuntimeCall, _> = + let maybe_call: Result = DecodeLimit::decode_all_with_depth_limit(32, &mut encoded_extrinsic); let currency = if let Ok(call) = maybe_call { @@ -586,7 +586,7 @@ impl Pallet { where BalanceOf: FixedPointOperand, { - if let Some(price) = Self::price(currency) { + if let Some(price) = as NativePriceOracle, Price>>::price(currency) { Some(price) } else { // If not loaded in on_init, let's try first the spot price provider again @@ -635,14 +635,12 @@ pub struct TransferFees(PhantomData<(T, MC, DF, FR, WF)>); impl OnChargeTransaction for TransferFees where - T: Config + pallet_utility::Config, + T: Config, MC: MultiCurrency<::AccountId>, AssetIdOf: Into, MC::Balance: FixedPointOperand, FR: Get, DF: DepositFee, - ::RuntimeCall: IsSubType> + IsSubType>, - ::RuntimeCall: IsSubType>, BalanceOf: FixedPointOperand, BalanceOf: From, WF: WithdrawFuseControl, @@ -655,8 +653,8 @@ where /// Note: The `fee` already includes the `tip`. fn withdraw_fee( who: &T::AccountId, - call: &::RuntimeCall, - _info: &DispatchInfoOf<::RuntimeCall>, + call: &T::RuntimeCall, + _info: &DispatchInfoOf, fee: Self::Balance, _tip: Self::Balance, ) -> Result { @@ -727,8 +725,8 @@ where /// Note: The `fee` already includes the `tip`. fn correct_and_deposit_fee( who: &T::AccountId, - _dispatch_info: &DispatchInfoOf<::RuntimeCall>, - _post_info: &PostDispatchInfoOf<::RuntimeCall>, + _dispatch_info: &DispatchInfoOf, + _post_info: &PostDispatchInfoOf, corrected_fee: Self::Balance, tip: Self::Balance, already_withdrawn: Self::LiquidityInfo, @@ -779,8 +777,8 @@ where fn can_withdraw_fee( who: &T::AccountId, - call: &::RuntimeCall, - _info: &DispatchInfoOf<::RuntimeCall>, + call: &T::RuntimeCall, + _info: &DispatchInfoOf, fee: Self::Balance, _tip: Self::Balance, ) -> Result<(), TransactionValidityError> { @@ -817,32 +815,14 @@ where } } -impl TransferFees +impl TransferFees where - T: Config + pallet_utility::Config, - MC: MultiCurrency<::AccountId>, + MC: MultiCurrency, AssetIdOf: Into, - ::RuntimeCall: IsSubType> + IsSubType>, - ::RuntimeCall: IsSubType>, WF: WithdrawFuseControl, { - fn resolve_currency_from_call(who: &T::AccountId, call: &::RuntimeCall) -> AssetIdOf { - if let Some(Call::set_currency { currency }) = call.is_sub_type() { - *currency - } else if let Some(pallet_utility::pallet::Call::batch { calls }) - | Some(pallet_utility::pallet::Call::batch_all { calls }) - | Some(pallet_utility::pallet::Call::force_batch { calls }) = call.is_sub_type() - { - match calls.first() { - Some(first_call) => match first_call.is_sub_type() { - Some(Call::set_currency { currency }) => *currency, - _ => Pallet::::account_currency(who), - }, - None => Pallet::::account_currency(who), - } - } else { - Pallet::::account_currency(who) - } + fn resolve_currency_from_call(who: &T::AccountId, call: &T::RuntimeCall) -> AssetIdOf { + T::TryCallCurrency::try_convert(call).unwrap_or_else(|_| Pallet::::account_currency(who)) } } @@ -929,11 +909,38 @@ impl AccountFeeCurrency for Pallet { } } -pub struct NoCallCurrency(PhantomData); -impl TryConvert<&::RuntimeCall, AssetIdOf> for NoCallCurrency { +/// Test-only implementation of `TryCallCurrency` for unit tests. +/// Handles `set_currency` and batch patterns, but not `dispatch_with_extra_gas` +/// (which requires `pallet_dispatcher` dependency). +pub struct TestCallCurrency(PhantomData); +impl TryConvert<&::RuntimeCall, AssetIdOf> + for TestCallCurrency +where + ::RuntimeCall: IsSubType> + IsSubType>, + ::RuntimeCall: IsSubType>, +{ fn try_convert( call: &::RuntimeCall, ) -> Result, &::RuntimeCall> { + // Handle direct set_currency calls + if let Some(Call::set_currency { currency }) = call.is_sub_type() { + return Ok(*currency); + } + + // Handle batch/batch_all/force_batch with set_currency as first call + if let Some(pallet_utility::pallet::Call::batch { calls }) + | Some(pallet_utility::pallet::Call::batch_all { calls }) + | Some(pallet_utility::pallet::Call::force_batch { calls }) = call.is_sub_type() + { + match calls.first() { + Some(first_call) => match first_call.is_sub_type() { + Some(Call::set_currency { currency }) => return Ok(*currency), + _ => {} + }, + _ => {} + } + } + Err(call) } } diff --git a/pallets/transaction-multi-payment/src/mock.rs b/pallets/transaction-multi-payment/src/mock.rs index 65d67ca1b..dbc78e45a 100644 --- a/pallets/transaction-multi-payment/src/mock.rs +++ b/pallets/transaction-multi-payment/src/mock.rs @@ -176,7 +176,7 @@ impl Config for Test { type EvmAssetId = EvmAssetId; type InspectEvmAccounts = EVMAccounts; type EvmPermit = PermitDispatchHandler; - type TryCallCurrency<'a> = NoCallCurrency; + type TryCallCurrency<'a> = TestCallCurrency; type SwappablePaymentAssetSupport = MockedInsufficientAssetSupport; }