From fb7075d4ad0cd64c19fe71f3416c3de79b10ca33 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, 4 Dec 2024 10:50:45 +0100 Subject: [PATCH] [stable2407] Backport #6506 (#6657) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport #6506 into `stable2407` 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: Egor_P <egor@parity.io> --- prdoc/pr_6506.prdoc | 10 +++ .../frame/transaction-payment/src/payment.rs | 17 ++-- .../frame/transaction-payment/src/tests.rs | 77 +++++++++++++++++++ 3 files changed, 96 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 bc0efd2d64a..679cfd67e6b 100644 --- a/substrate/frame/transaction-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/src/tests.rs @@ -841,3 +841,80 @@ fn genesis_default_works() { assert_eq!(<NextFeeMultiplier<Runtime>>::get(), Multiplier::saturating_from_integer(1)); }); } +<<<<<<< HEAD +======= + +#[test] +fn no_fee_and_no_weight_for_other_origins() { + ExtBuilder::default().build().execute_with(|| { + let ext = Ext::from(0); + + let mut info = CALL.get_dispatch_info(); + info.extension_weight = ext.weight(CALL); + + // Ensure we test the refund. + assert!(info.extension_weight != Weight::zero()); + + let len = CALL.encoded_size(); + + let origin = frame_system::RawOrigin::Root.into(); + let (pre, origin) = ext.validate_and_prepare(origin, CALL, &info, len, 0).unwrap(); + + assert!(origin.as_system_ref().unwrap().is_root()); + + let pd_res = Ok(()); + let mut post_info = frame_support::dispatch::PostDispatchInfo { + actual_weight: Some(info.total_weight()), + pays_fee: Default::default(), + }; + + <Ext as TransactionExtension<RuntimeCall>>::post_dispatch( + pre, + &info, + &mut post_info, + len, + &pd_res, + ) + .unwrap(); + + assert_eq!(post_info.actual_weight, Some(info.call_weight)); + }) +} + +#[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.", + ); + }); +} +>>>>>>> f520adb (Zero refund check for FungibleAdapter (#6506)) -- GitLab