From 940966e46148b303e246b352339f5ec29319cbae Mon Sep 17 00:00:00 2001 From: "paritytech-cmd-bot-polkadot-sdk[bot]" <179002856+paritytech-cmd-bot-polkadot-sdk[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:26:09 +0100 Subject: [PATCH] [stable2409] Backport #6506 (#6658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport #6506 into `stable2409` from Dinonard. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> --------- Co-authored-by: Dino PaÄandi <3002868+Dinonard@users.noreply.github.com> Co-authored-by: EgorPopelyaev <egor@parity.io> --- prdoc/pr_6506.prdoc | 10 +++++ .../frame/transaction-payment/src/payment.rs | 17 +++++---- .../frame/transaction-payment/src/tests.rs | 37 +++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 prdoc/pr_6506.prdoc diff --git a/prdoc/pr_6506.prdoc b/prdoc/pr_6506.prdoc new file mode 100644 index 00000000000..7c6164a9959 --- /dev/null +++ b/prdoc/pr_6506.prdoc @@ -0,0 +1,10 @@ +title: Zero refund check for FungibleAdapter +doc: +- audience: Runtime User + description: |- + `FungibleAdapter` will now check if the _refund amount_ is zero before calling deposit & emitting an event. + + Fixes https://github.com/paritytech/polkadot-sdk/issues/6469. +crates: +- name: pallet-transaction-payment + bump: patch diff --git a/substrate/frame/transaction-payment/src/payment.rs b/substrate/frame/transaction-payment/src/payment.rs index 0fe61678290..107a995761f 100644 --- a/substrate/frame/transaction-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/src/payment.rs @@ -121,14 +121,15 @@ where if let Some(paid) = already_withdrawn { // Calculate how much refund we should return let refund_amount = paid.peek().saturating_sub(corrected_fee); - // refund to the the account that paid the fees if it exists. otherwise, don't refind - // anything. - let refund_imbalance = if F::total_balance(who) > F::Balance::zero() { - F::deposit(who, refund_amount, Precision::BestEffort) - .unwrap_or_else(|_| Debt::<T::AccountId, F>::zero()) - } else { - Debt::<T::AccountId, F>::zero() - }; + // Refund to the the account that paid the fees if it exists & refund is non-zero. + // Otherwise, don't refund anything. + let refund_imbalance = + if refund_amount > Zero::zero() && F::total_balance(who) > F::Balance::zero() { + F::deposit(who, refund_amount, Precision::BestEffort) + .unwrap_or_else(|_| Debt::<T::AccountId, F>::zero()) + } else { + Debt::<T::AccountId, F>::zero() + }; // merge the imbalance caused by paying the fees and refunding parts of it again. let adjusted_paid: Credit<T::AccountId, F> = paid .offset(refund_imbalance) diff --git a/substrate/frame/transaction-payment/src/tests.rs b/substrate/frame/transaction-payment/src/tests.rs index 35d5322a6f3..ae4e136ee58 100644 --- a/substrate/frame/transaction-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/src/tests.rs @@ -841,3 +841,40 @@ fn genesis_default_works() { assert_eq!(NextFeeMultiplier::<Runtime>::get(), Multiplier::saturating_from_integer(1)); }); } + +#[test] +fn fungible_adapter_no_zero_refund_action() { + type FungibleAdapterT = payment::FungibleAdapter<Balances, DealWithFees>; + + ExtBuilder::default().balance_factor(10).build().execute_with(|| { + System::set_block_number(10); + + let dummy_acc = 1; + let (actual_fee, no_tip) = (10, 0); + let already_paid = <FungibleAdapterT as OnChargeTransaction<Runtime>>::withdraw_fee( + &dummy_acc, + CALL, + &CALL.get_dispatch_info(), + actual_fee, + no_tip, + ).expect("Account must have enough funds."); + + // Correction action with no expected side effect. + assert!(<FungibleAdapterT as OnChargeTransaction<Runtime>>::correct_and_deposit_fee( + &dummy_acc, + &CALL.get_dispatch_info(), + &default_post_info(), + actual_fee, + no_tip, + already_paid, + ).is_ok()); + + // Ensure no zero amount deposit event is emitted. + let events = System::events(); + assert!(!events + .iter() + .any(|record| matches!(record.event, RuntimeEvent::Balances(pallet_balances::Event::Deposit { amount, .. }) if amount.is_zero())), + "No zero amount deposit amount event should be emitted.", + ); + }); +} -- GitLab