Adding L1 blocknumber to OVM context#222
Conversation
This reverts commit 3ecebe8.
ben-chain
changed the title
ben-chain
changed the title
| uint public cumulativeNumElements; | ||
| bytes32[] public batches; | ||
| uint public lastOVMTimestamp; | ||
| uint public lastOVMBlocknumber; |
There was a problem hiding this comment.
Any reason not to capitalize Number?
There was a problem hiding this comment.
Ugh yeah you're right, I did lowercases all over the place but that's non consistent with rest of the repo, will update!
| txsCalldata, | ||
| timestamp | ||
| timestamp, | ||
| blocknumber |
There was a problem hiding this comment.
Will there need to be a corresponding change in log-handlers.ts for this or any other changes in this PR or does this not change the data structure at all?
There was a problem hiding this comment.
Hmmm, I did leave the handlers alone. Mainly this was because the handlers are already ignoring the l2 timestamp, so I figured we had a separate ticket for it. Talking with @karlfloersch about it right now, will make a ticket to do this if it's not already represented.
| require( | ||
| _timestamp <= safetyQueue.peekTimestamp(), | ||
| "Must process older SafetyQueue batches first to enforce OVM timestamp monotonicity" | ||
| ); | ||
|
|
||
| require( | ||
| _blocknumber <= safetyQueue.peekBlocknumber(), | ||
| "Must process older SafetyQueue batches first to enforce OVM blocknumber monotonicity" | ||
| ); |
There was a problem hiding this comment.
Can this be consistently be DoS'd by queueing txs with an old block number / timestamp? Could be useful to also have some sort of force inclusion period enforced on the queues.
There was a problem hiding this comment.
The block number and timestamp of the queues are set as the block.number and block.timestamp at the time they are rolled up, so not sure what you mean by "old". Generally this logic is an exact duplicate of the existing timestamp logic, so it shouldn't fundamentally change any ordering with the sequencer--though it does need they mean to track/update the block number alongside the existing timestamp.
In general, people can spam the queues and force the sequencer to break up batches--have some thoughts on this I'll share separately.
* initial * get .calls test working * add assertions to withdrawal tests * add missing finalizeDeposit tests * clean up tests * clean up crosschain library * further clean up tests * clean up L1 Gateway, fix bug * further clean up contracts * quick save * quick save * Add unsupported to fix compiler bug, need to remove this * fix constructor inputs * build fixes * improve test case names * l1 gateway testing WIP * add comment on init * Add withdraw tests * temp only * All tests implemented, 2 still failing, needs polish * All tests implemented, 2 still failing, needs polish * add failing test for finalizeWithdrawAndCall * Add univ2erc20, broken build tho * get contracts building * clean up tests and interface * updates * move to uniswap ERC20 * more updates, fix gateway tests with uniswap * finish deploy config? * Add ETH deposit gateway (OVM_L1ETHGateway) (#225) * OVM_L1ETHGateway almost working * OVM_L1ETHGateway tests all passing * clean up unused stuff * lower gaslimit * lower gas further * typo * break out helper * resolve messenger * update deploy config * renaming * lint * update tests to use AM * restructure fs layout * clean unused envar * remove interface version bound * remove todo * various minor cleanups * add safemath to contract account * update naming conventions * fix test config and test name * cleanup for consistency * fix eth send test * remove .only * clean up, add deposit gas limit * lint * add gas config and tests * fix indent Co-authored-by: ben <ben@bens-MacBook-Pro.local> Co-authored-by: Maurelian <maurelian@protonmail.ch> Co-authored-by: Matt Masurka <m.masurka@gmail.com>
### Description Re-work the `op-alloy-consensus` docs.
Closes #164 --------- Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
3 participants
Description
This PR addresses YAS 432, adding L1 block number to the OVM.
Questions
Metadata
Fixes
Contributing Agreement