fix: compare gas prices properly #3771
Draft
+42
−13
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a bug in the gas price comparison logic, where a larger gas price would return
false
for being larger, when compared to a smaller gas price:In this example, a user sent a transaction with the following gas pricing data:
10000
5ugnot
This returned
false
, when being compared to a gas price ofGas Wanted: 1000, Gas Fee: 1ugnot
🤷♂️The issue was that the function cross-multiplied mismatched fields, comparing fee-per-gas ratios instead of comparing total fees (gas x price), which led to an incorrect result when gas values differ. In this example, it would compare the ratios
0.00005
and0.001
, and returnfalse
:How our entire testing suite did not catch a bug like this is beyond me, but here we are.EDIT:
I've figured it out.
Our
std.Fee
is very misleading.The amount the user puts in the
tx.Fee
is actually the total amount of whatever the user is willing to pay for the entire transaction, not just a single unit of gas. The effective gas price is the ratio of the maximum gas wanted and the maximum fee (for the entire tx).