Conversation
…n rollup-core, rollup-full-node, and ovm, and creating a l1-to-l2-messaging package to handle the l1 message listener microservice
karlfloersch
left a comment
There was a problem hiding this comment.
Overall it looks good! And wowza that was a lot of files updated!
I left a couple questions which I was curious about & with a better understanding there can take another look at the couple of brand new files
| @@ -0,0 +1,5 @@ | |||
| # L1 To L2 Message Passing | |||
There was a problem hiding this comment.
Should this be called the State Synchronization Station? Or maybe just State Synchronizer? I need to look into the rest of this codebase but I imagine that pulling messages from L1 and ingesting them will also have to manage things like timestamps, or are you thinking we have that in another micro-service?
There was a problem hiding this comment.
That could be another service, but it if will be coupled at all with L1 -> L2 messages, then it should be done here. Maybe L1ToL2 StateSynchronizer? Still want to be as specific as possible.
There was a problem hiding this comment.
Ah one thing is that we may want to use the StateSynchronizer for verifiers in addition to just using it for the Sequencer. For instance, a verifier simply downloads all txs off of L1, bundles the txs in a similar way that we ingest L1->L2 txs
| ): Promise<string> { | ||
| // TODO: change / append to calldata to add sender? | ||
|
|
||
| const tx = { |
There was a problem hiding this comment.
Does this transaction also need to include a L1 transaction sender?
There was a problem hiding this comment.
Also of course at some point we will need to either add timestamp logic in this class or we will need to put it in another service.
There was a problem hiding this comment.
Yes, so that's something I held off on because all the info is here, but there are a few routes we could go in how we send that data.
- Do we pass it as calldata and call a special EM function?
- If so, do we call that directly?
- Do we have an L1 to L2 Transaction receiver?
Either way, it seems like that should be passed as calldata to something and that something should check to make sure the L2 tx sender is the authorized L1 To L2 Tx sender address.
There was a problem hiding this comment.
Ah! So I like the idea of in Geth having some EOA that is authenticated to send transactions that include an L1TransactionSender because it would be a pretty simple check to make in Geth that transactions that include an l1MsgSender ( https://github.com/ethereum-optimism/optimism-monorepo/blob/master/packages/ovm/src/contracts/ExecutionManager.sol#L165 ) are signed by someone we trust, otherwise throw out the tx. So that seems easy.
What seems quite a bit harder is timestamp management... I don't know how to include that information as well... I mean it could be the case that we parse the transaction even further and add custom logic which updates the timestamp based on what the tx wants the timestamp to be... that is a possibility. hmm
There was a problem hiding this comment.
Ok yeah it seems reasonable to modify Geth's transaction ingestion to accept these weird transactions which:
- Set the timestamp
- Optionally execute some calldata on behalf of an L1MessageSender
It's a little weird because we might have transactions which don't even produce blocks because all they do is update the timestamp... but might still be the easiest way to implement this without adding another endpoint.
| @@ -0,0 +1,59 @@ | |||
| { | |||
| "name": "@eth-optimism/l1-to-l2-messaging", | |||
| "version": "0.0.1-alpha.25", | |||
There was a problem hiding this comment.
No big deal but noticed this is starting at alpha 25
There was a problem hiding this comment.
Yeah, trying to keep it consistent with everything else since all other packages are, but that's bound to diverge at some point 🤷
| * @param fullnodeHandler The handler to use for the provider's send function. | ||
| * @return The provider. | ||
| */ | ||
| export const createProviderForHandler = ( |
There was a problem hiding this comment.
Kinda unrelated to some of the other stuff in this PR but is this function used anymore? I think it might be totally unused & can be deleted.
There was a problem hiding this comment.
Looks like it's used in 3 different tests, all in rollup-full-node. Can move it to be a test util if you'd like.
* Reverts an accidental breaking merge * Tiny lint
…mism#147) * add docs * fix string * fix string * fix string * dn vi cmd * fix ordering * more info * more changes * more changes * fix string * bash code listings * empty space * ls bash cmd * separate docs folder and installation guide * fix cmds * configuration.md setup * fix strings * more info * config related info * dump eots config * config related attrs for eotsd * fix strings * fix strings * remove redundant info * operations.md * fix bash listings * proper numbering * add notes * more commands * more notes * more details * fix links * fix readme * fix * fix * more info * fix comments * fix headings * fix links * add more info * change to level 4 heading * Update README.md Co-authored-by: Vitalis Salis <VitSalis@gmail.com> * Update README.md Co-authored-by: Vitalis Salis <VitSalis@gmail.com> * Update README.md Co-authored-by: Vitalis Salis <VitSalis@gmail.com> * Update README.md Co-authored-by: Vitalis Salis <VitSalis@gmail.com> * Update docs/configuration.md Co-authored-by: Vitalis Salis <VitSalis@gmail.com> * merge installation md in root readme * remove install.md as the contents are merged * remove sed cmds * fix hyperlinks * fix hyperlink --------- Co-authored-by: Vitalis Salis <VitSalis@gmail.com>
…ox-2 fix: merge conflicts in lockbox
### Description Updates the arbitrary `BaseFeeParams` to bound to types. ### Context See [discussion here](alloy-rs/op-alloy#144 (comment))
Description
Creates executable micro-service for handling L1-to-L2 Tx subscription, processing, and submission to L2.
Metadata
Fixes
Contributing Agreement