Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Incorrect ordering of fees and tips in Transaction Payment #8854

Closed
xx-labs opened this issue May 19, 2021 · 1 comment · Fixed by #8860
Closed

Incorrect ordering of fees and tips in Transaction Payment #8854

xx-labs opened this issue May 19, 2021 · 1 comment · Fixed by #8860
Labels
U2-some_time_soon Issue is worth doing soon.

Comments

@xx-labs
Copy link

xx-labs commented May 19, 2021

It appears that the ordering of transaction fees and tips is incorrect when passing to the on_unbalanceds function:

// Call someone else to handle the imbalance (fee and tip separately)
let imbalances = adjusted_paid.split(tip);
OU::on_unbalanceds(Some(imbalances.0).into_iter().chain(Some(imbalances.1)));

The split function returns the split amount first, and the remainder second

fn split(self, amount: T::Balance) -> (Self, Self) {
let first = self.0.min(amount);
let second = self.0 - first;
mem::forget(self);
(Self(first), Self(second))
}

This means that what is passed to on_unbalanceds is [tip, fees] instead of [fees, tip] as the comments specify.

This implementation of DealWithFees that splits fees between block producer and Treasury illustrates the issue, as the comments clearly specify that the first item is believed to be the fees.

pub struct DealWithFees;
impl OnUnbalanced<NegativeImbalance> for DealWithFees {
fn on_unbalanceds<B>(mut fees_then_tips: impl Iterator<Item=NegativeImbalance>) {
if let Some(fees) = fees_then_tips.next() {
// for fees, 80% to treasury, 20% to author
let mut split = fees.ration(80, 20);
if let Some(tips) = fees_then_tips.next() {
// for tips, if any, 80% to treasury, 20% to author (though this can be anything)
tips.ration_merge_into(80, 20, &mut split);
}
Treasury::on_unbalanced(split.0);
Author::on_unbalanced(split.1);
}
}
}

This bug can be found in the Polkadot Mainnet.
Here is an example of a transfer without tips:
https://polkadot.js.org/apps/#/explorer/query/0x0218a64f98f808d654f9113cd1feeb70d66e1fb02742b2ee43df33d3b88922e7
The totality of the transaction fee is deposited in the block producer's account.

Here is an example of a transfer with a tip:
https://polkadot.js.org/apps/#/explorer/query/0xf0d74cc96794b6ef3ff3836ff115bff18fefb9132790ae033d5d5014a24cb102
The totality of the transaction fee + 20% of the tip is deposited in the block producer's account.
80% of the tip is deposited into the Treasury account.

@github-actions github-actions bot added the J2-unconfirmed Issue might be valid, but it’s not yet known. label May 19, 2021
@gui1117 gui1117 added I3-bug The node fails to follow expected behavior. U2-some_time_soon Issue is worth doing soon. and removed J2-unconfirmed Issue might be valid, but it’s not yet known. I3-bug The node fails to follow expected behavior. labels May 19, 2021
@gui1117
Copy link
Contributor

gui1117 commented May 19, 2021

yes your diagnostic is right. This PR should fix it #8860

jordy25519 added a commit to cennznet/cennznet that referenced this issue Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
U2-some_time_soon Issue is worth doing soon.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant