-
Notifications
You must be signed in to change notification settings - Fork 322
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
feat: sum transaction fees and pay on l1 #6522
Changes from all commits
5dec00b
73ee90e
96e6bbd
f40fe09
0214f8c
780f3af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,9 +95,9 @@ contract Rollup is IRollup { | |
header.globalVariables.blockNumber, header.contentCommitment.outHash, l2ToL1TreeHeight | ||
); | ||
|
||
// pay the coinbase 1 gas token if it is not empty | ||
if (header.globalVariables.coinbase != address(0)) { | ||
GAS_TOKEN.transfer(address(header.globalVariables.coinbase), 1); | ||
// pay the coinbase 1 gas token if it is not empty and header.totalFees is not zero | ||
if (header.globalVariables.coinbase != address(0) && header.totalFees > 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we pay if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and there are comments around the codebase saying that coinbase should not be allowed to be undefined when we are in production. Not all the tests set coinbase yet. So I added #6539 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool! The rest looks good to me 😃 |
||
GAS_TOKEN.transfer(address(header.globalVariables.coinbase), header.totalFees); | ||
} | ||
|
||
emit L2BlockProcessed(header.globalVariables.blockNumber); | ||
|
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.
@Maddiaa0 I was confused about why over in #6457 some fields were bumped by two, so just wanted your eyes to make sure this doesn't need a similar operation. If it does, could you explain why?