From a22e502567d3a624067277167cfeff41e20e5f0b Mon Sep 17 00:00:00 2001 From: Khuzama Shahid Date: Thu, 26 Feb 2026 15:48:47 +0500 Subject: [PATCH 1/3] Refactor currency resolution logic in TransferFees to simplify handling of set_currency calls --- integration-tests/src/non_native_fee.rs | 163 ++++++++++++++++++- pallets/transaction-multi-payment/src/lib.rs | 21 +-- 2 files changed, 164 insertions(+), 20 deletions(-) diff --git a/integration-tests/src/non_native_fee.rs b/integration-tests/src/non_native_fee.rs index 3f307bc036..96427288d0 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,166 @@ 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)); + + // Fee must be charged in BTC (the new currency declared inside dispatch_with_extra_gas). + assert!( + bob_btc_after < bob_btc_before, + "BTC balance should decrease — fee must be charged in the new currency" + ); + // HDX must not be touched. + 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, + )); + + // EVM accounts have WETH automatically set as their fee currency on account creation + 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); + + // Fee must be charged in DAI (the new currency), not WETH (EVM default). + assert!( + dai_after < dai_before, + "DAI balance should decrease — fee must be charged in the new currency" + ); + // WETH must not be touched. + 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)); + + // BTC must not be charged — the resolver did not reach the nested set_currency. + 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/src/lib.rs b/pallets/transaction-multi-payment/src/lib.rs index 081ff7e459..439023af17 100644 --- a/pallets/transaction-multi-payment/src/lib.rs +++ b/pallets/transaction-multi-payment/src/lib.rs @@ -821,30 +821,13 @@ where impl TransferFees where - T: Config + pallet_utility::Config, + T: Config, MC: MultiCurrency<::AccountId>, 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) - } + T::TryCallCurrency::try_convert(call).unwrap_or_else(|_| Pallet::::account_currency(who)) } } From 958353667a6f74fd9272afa91daa949214b5cb29 Mon Sep 17 00:00:00 2001 From: Khuzama Shahid Date: Fri, 27 Feb 2026 14:41:17 +0500 Subject: [PATCH 2/3] Implement TestCallCurrency for unit tests to handle set_currency calls --- pallets/transaction-multi-payment/src/lib.rs | 31 +++++++++++++++++-- pallets/transaction-multi-payment/src/mock.rs | 2 +- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/pallets/transaction-multi-payment/src/lib.rs b/pallets/transaction-multi-payment/src/lib.rs index 439023af17..ec21e87bf9 100644 --- a/pallets/transaction-multi-payment/src/lib.rs +++ b/pallets/transaction-multi-payment/src/lib.rs @@ -914,11 +914,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 4125017b51..8f5bc0d01f 100644 --- a/pallets/transaction-multi-payment/src/mock.rs +++ b/pallets/transaction-multi-payment/src/mock.rs @@ -177,7 +177,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; } From 71dd4abcab9a693210d657d6010f23de03527afe Mon Sep 17 00:00:00 2001 From: Khuzama Shahid Date: Thu, 5 Mar 2026 13:58:22 +0500 Subject: [PATCH 3/3] Refactor currency handling in transaction multi-payment pallet for improved clarity and consistency --- integration-tests/src/non_native_fee.rs | 6 ---- pallets/transaction-multi-payment/Cargo.toml | 5 +++- pallets/transaction-multi-payment/src/lib.rs | 29 +++++++++----------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/integration-tests/src/non_native_fee.rs b/integration-tests/src/non_native_fee.rs index 96427288d0..c4b9134a96 100644 --- a/integration-tests/src/non_native_fee.rs +++ b/integration-tests/src/non_native_fee.rs @@ -208,12 +208,10 @@ fn set_currency_should_work_in_dispatch_with_extra_gas() { 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)); - // Fee must be charged in BTC (the new currency declared inside dispatch_with_extra_gas). assert!( bob_btc_after < bob_btc_before, "BTC balance should decrease — fee must be charged in the new currency" ); - // HDX must not be touched. assert_eq!(bob_hdx_after, bob_hdx_before, "HDX must not be charged"); }); } @@ -245,7 +243,6 @@ fn set_currency_should_work_in_dispatch_with_extra_gas_for_evm_account() { 1_000_000_000_000_000_000i128, )); - // EVM accounts have WETH automatically set as their fee currency on account creation assert_eq!( MultiTransactionPayment::get_currency(evm_acc.clone()), Some(WETH), @@ -275,12 +272,10 @@ fn set_currency_should_work_in_dispatch_with_extra_gas_for_evm_account() { let weth_after = hydradx_runtime::Tokens::free_balance(WETH, &evm_acc); let dai_after = hydradx_runtime::Tokens::free_balance(DAI, &evm_acc); - // Fee must be charged in DAI (the new currency), not WETH (EVM default). assert!( dai_after < dai_before, "DAI balance should decrease — fee must be charged in the new currency" ); - // WETH must not be touched. assert_eq!(weth_after, weth_before, "WETH must not be charged"); }); } @@ -323,7 +318,6 @@ fn set_currency_should_not_work_in_dispatch_with_extra_gas_when_not_direct_inner let bob_btc_after = hydradx_runtime::Tokens::free_balance(BTC, &AccountId::from(BOB)); - // BTC must not be charged — the resolver did not reach the nested set_currency. assert_eq!( bob_btc_after, bob_btc_before, "BTC must not be charged when set_currency is nested deeper than the direct inner call" diff --git a/pallets/transaction-multi-payment/Cargo.toml b/pallets/transaction-multi-payment/Cargo.toml index 0bb6520a29..6beb0e2498 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 ec21e87bf9..383bdd7592 100644 --- a/pallets/transaction-multi-payment/src/lib.rs +++ b/pallets/transaction-multi-payment/src/lib.rs @@ -416,7 +416,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 { @@ -486,7 +486,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 { @@ -588,7 +588,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 @@ -637,14 +637,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, @@ -657,8 +655,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 { @@ -729,8 +727,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, @@ -781,8 +779,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> { @@ -819,14 +817,13 @@ where } } -impl TransferFees +impl TransferFees where - T: Config, - MC: MultiCurrency<::AccountId>, + MC: MultiCurrency, AssetIdOf: Into, WF: WithdrawFuseControl, { - fn resolve_currency_from_call(who: &T::AccountId, call: &::RuntimeCall) -> AssetIdOf { + fn resolve_currency_from_call(who: &T::AccountId, call: &T::RuntimeCall) -> AssetIdOf { T::TryCallCurrency::try_convert(call).unwrap_or_else(|_| Pallet::::account_currency(who)) } }