-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Adds L2 Batch Creator #175
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
Conversation
…s from getTransaction and getBlock requests and adding test
…s from getTransaction and getBlock requests and adding test
… other future scheduled tasks)
…ckBatchSubmitter into L2NodeService
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.
Awesome! This LGTM! And it seems very clean as well! Great separation between batch creation, submission, and pulling data from L1/L2 into rdb!
Left a comment on blockBatches vs blockOfBatches lol forgive me but other than that I didn't have much to say. Oh and there's a sanity check test that could be added as well
| * @inheritDoc | ||
| */ | ||
| public async handleBlockBatches(blockBatches: BlockBatches): Promise<void> { | ||
| public async sendBlockBatches(blockBatches: BlockBatches): Promise<void> { |
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 really have struggled to find things to complain about in this PR! Lol everything seems good! That said, I will say that I still do like the name BlockOfBatches here because once again I had to remind myself what this object was
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 know we said we'd update it later but figure that because this PR touches them again might be a reasonable time to make the change if it's not super hard?
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.
Yeah, I think a rename is in order since we will no longer be sending all batches for an L1 block together, and each one will be a single batch. This requires a change on the go side as well, so just left it for now.
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.
Created a ticket https://optimists.atlassian.net/browse/YAS-499
| `1 L1 matching batche should have been attempted!` | ||
| ) | ||
| }) | ||
| }) |
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.
👍 solid! I guess one thing we could add is a test that actually does produce a batch
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.
Yep! That'll have to be on the submission side of things though, as all this does is stage the batch for submission in the DB.
| l1Timestamp: 1, | ||
| l1BlockNumber: 1, | ||
| l1TxHash: keccak256FromUtf8('tx hash'), | ||
| queueOrigin: 1, |
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.
Dope!
* Add nonreentrant to ExecutionManager.run() * Cheaper check using uninit'd tx context * Remove extraneous import
c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1
c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1
c9dbc5f58 Test fix & fmt fcf735497 ExternalHeaderMode rebase f09d4e5ff Add for wind/unwind support to tbc (ethereum-optimism#159) 5c764a0cd popm: exclude tx outputs that would be dust (ethereum-optimism#186) 1f168f225 popm/wasm: fix dispatch params for {add,remove}EventListener (ethereum-optimism#181) 383611feb popm/wasm: add 'minerStatus' method (ethereum-optimism#178) 6a94b4c0b popm/wasm: rename KeyResult fields and add bitcoinScriptHash (ethereum-optimism#177) 9c4cea583 popm/wasm: add events and clean up globals (ethereum-optimism#175) a2372394d popm: improve UTXO selection when creating Bitcoin transaction (ethereum-optimism#173) 939813cac popm/wasm: fix ethereum address when generating and parsing keys (ethereum-optimism#174) 9245995ef Updated localnet to use more recent commits of optimism and op-geth (ethereum-optimism#139) 159fbc5f5 ci: improve runtime of high usage CI workflows (ethereum-optimism#163) 60a3489db popm/wasm: add 'bitcoinAddressToScriptHash' method (ethereum-optimism#169) fe9752a5a popm/wasm: add 'parseKey' method (ethereum-optimism#168) 65aa0caef ci: add registry-url to setup-node in release (ethereum-optimism#167) 8922ce805 popm/wasm: add new @hemilabs/pop-miner NPM package (ethereum-optimism#162) 67166046b scripts: add release script (ethereum-optimism#164) eea96059b ci: add labeler action (ethereum-optimism#165) git-subtree-dir: heminetwork git-subtree-split: c9dbc5f58a7f997fa4b3af0d765a2967ed3462d1
* binary release for op-node * add a comment * add workflow for op-challenger
Description
Adds L2 Batch Creator that creates batches from txs run in L2.
Note: This is distinct from L1 Batch Creator, which will create batches out of Rollup Transactions pulled from L1 tx calldata.
Metadata
Fixes
Contributing Agreement