-
Notifications
You must be signed in to change notification settings - Fork 791
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
fix(list-totalcost): Add l1Cost
instead of transaction cost
#324
base: optimism
Are you sure you want to change the base?
Conversation
…calculating `l.totalcost`.
does it require a hard fork ? |
No, I don't believe so. It only affects wheter a transaction enters a mempool or not. Since |
@tynes When you have a moment, could you please review this PR? I believe it's affecting the UX, as some of the transactions could be falsely rejected when entering the mempool. |
@sebastianst, @geoknee, pinging you both as @tynes seems to be occupied. |
@@ -344,6 +344,10 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64, l1CostFn txpool.L1Co | |||
l.totalcost.Add(l.totalcost, cost) | |||
if l1CostFn != nil { | |||
if l1Cost := l1CostFn(tx.RollupCostData()); l1Cost != nil { // add rollup cost | |||
cost, overflow := uint256.FromBig(l1Cost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be modifying line 351 to be l.totalcost.Add(l.totalcost, l1Cost)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this doesn't work because typeOf(l1Cost) = *big.Int
and typeOf(l1.totalcost) = *uint256.Int
. Consequently, I referred to lines 340-344 to implement the l1Cost
cast from *big.Int
to *uint256.Int
.
The variable cost
in this fix holds the intermediate value of l1Cost
as *uint256.Int
, allowing it to be added to l1.totalcost
on line 351.
Description
Previously,
l.totalcost
was calculated incorrectly. The transactioncost
was added twice, and thel1Cost
was left out.This PR fixes the sum calculation so that it is correctly calculated as
tx.Cost() + l1Cost
.