-
Notifications
You must be signed in to change notification settings - Fork 6
feat: transact gas limits #56
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
/// @param gas - The gas limit for the transaction. | ||
/// @param maxFeePerGas - The maximum fee per gas for the transaction (per EIP-1559). | ||
/// @custom:emits Transact indicating the transaction to mine on the rollup. | ||
function enterTransact( |
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.
note: moving transact
to Zenith && splitting Passage from Zenith means that EOAs can no longer atomically enter
+ transact
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.
this seems bad
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.
the only ways around this would be
- keep Passage & Zenith deployed at the same address (as we discussed, makes potential migration path worse)
- call
Passage.enter
as an external contract (i find this to be undesirable but it could be done)
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.
call Passage.enter as an external contract (i find this to be undesirable but it could be done)
seems fine
@@ -4,6 +4,9 @@ pragma solidity ^0.8.24; | |||
import {Passage} from "./Passage.sol"; | |||
|
|||
contract Zenith is Passage { | |||
/// @notice Each `transact` call cannot use more than 1/5 of the global `transact` gasLimit for the block. | |||
uint256 public constant PER_TRANSACT_GAS_LIMIT = 5; |
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.
still need to align on parameterization of this constant
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.
MAX_TRANSACTS_PER_BLOCK
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.
well, it wouldn't be max transacts per block right
if this is set at 5, you could still have 10 transacts in the block if they use less than the limit (which i think we expect)
but yeah.. we should rename this var
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.
when I said "need to align on parameterization" above I meant what should we set this as - 5? 3? 2? i think not more than 5.
mapping(uint256 => uint256) public lastSubmittedAtBlock; | ||
|
||
/// @notice The gasLimit of the last submitted block for a given rollup chainId. | ||
/// @dev NOTE that this can change mid-block, causing some `transact` to have a different limit than others. |
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.
starting at this WIP code to figure the best way to fix this (or if it's necessary)
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.
consider the following set of host chain transactions within a single block:
[transact, transact, submitBlock, transact]
The first two transact
s will have the previous RU block's gasLimit; the last transact
will have the current RU block's gasLimit
i wonder if this could cause any weird or unexpected behavior?
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.
that's not necessarily the case, you'd just have to store the last block's limit the first time a transact occurs. bears more discussion tho
a0a2165
to
174f8bf
Compare
174f8bf
to
eab5ed6
Compare
closing in favor of #61 |
No description provided.