-
Notifications
You must be signed in to change notification settings - Fork 698
Fix expected surplus in sequencer #4038
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
base: master
Are you sure you want to change the base?
Conversation
❌ 4 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
64e9da2 to
6cb625f
Compare
| } else { | ||
| // l1GasPrice can be zero because of roundings, hence backlogCost is calculated separately | ||
| backlogFee := big.NewInt(backlogCallDataUnits) | ||
| backlogFee.Mul(backlogFee, blobFeePerByte) |
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.
I don't have enough context to understand this line change. Could you please add a brief description?
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.
now I look at the entire computation and not sure I get it.
can you add a comment with the full computation for computing backlog cost here?
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.
Sure! We want to capture two values: l1GasPrice and backlogCost. This is the code we had initially:
blobFeePerByte, _ := s.parentChain.BlobFeePerByte(ctx, header)
blobFeePerByte.Mul(blobFeePerByte, blobTxBlobGasPerBlob)
blobFeePerByte.Div(blobFeePerByte, usableBytesInBlob)
l1GasPrice = blobFeePerByte.Int64() / 16
backlogCost = (backlogCallDataUnits * blobFeePerByte.Int64()) / 16
This means:
l1GasPrice = (blobFeePerByte * blobTxBlobGasPerBlob) / (usableBytesInBlob * 16)
backlogCost = backlogCallDataUnits * (blobFeePerByte * blobTxBlobGasPerBlob) / (usableBytesInBlob * 16)
The problem here was that we divide with usableBytesInBlob too early and because of rounding, the value of blobFeePerByte becomes zero. Then even if we multiply with backlogCallDataUnits, it will still stay zero. The solution here was to multiply with backlogCallDataUnits before we divide. The problem here was that I forgot a line that multiplied with blobFeePerByte (the intention was to use it there as, well, this is why I had even extracted it outside of the if else).
tsahee
left a comment
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.
question
| } else { | ||
| // l1GasPrice can be zero because of roundings, hence backlogCost is calculated separately | ||
| backlogFee := big.NewInt(backlogCallDataUnits) | ||
| backlogFee.Mul(backlogFee, blobFeePerByte) |
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.
now I look at the entire computation and not sure I get it.
can you add a comment with the full computation for computing backlog cost here?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4038 +/- ##
==========================================
- Coverage 33.35% 33.14% -0.21%
==========================================
Files 453 453
Lines 55536 55537 +1
==========================================
- Hits 18524 18409 -115
- Misses 33774 33922 +148
+ Partials 3238 3206 -32 |
No description provided.