diff --git a/prdoc/pr_6506.prdoc b/prdoc/pr_6506.prdoc new file mode 100644 index 000000000000..7c6164a9959a --- /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 0fe616782903..107a995761f4 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::::zero()) - } else { - Debt::::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::::zero()) + } else { + Debt::::zero() + }; // merge the imbalance caused by paying the fees and refunding parts of it again. let adjusted_paid: Credit = paid .offset(refund_imbalance) diff --git a/substrate/frame/transaction-payment/src/tests.rs b/substrate/frame/transaction-payment/src/tests.rs index bc0efd2d64a3..679cfd67e6b4 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!(>::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(), + }; + + >::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; + + 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 = >::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!(>::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))