Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zero refund check for FungibleAdapter #6506

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions prdoc/pr_6506.prdoc
Original file line number Diff line number Diff line change
@@ -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
17 changes: 9 additions & 8 deletions substrate/frame/transaction-payment/src/payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,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)
Expand Down
37 changes: 37 additions & 0 deletions substrate/frame/transaction-payment/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,3 +877,40 @@ fn no_fee_and_no_weight_for_other_origins() {
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.",
);
});
}
Loading