-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
SuperSystem: Add Emitter Contract - Basic Message Passing #11956
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Semgrep found 14
Inputs to functions must be prepended with an underscore ( |
protolambda
reviewed
Sep 17, 2024
protolambda
reviewed
Sep 17, 2024
This was referenced Sep 18, 2024
protolambda
approved these changes
Sep 20, 2024
samlaf
pushed a commit
to samlaf/optimism
that referenced
this pull request
Nov 10, 2024
…ptimism#11956) * Add Emitter Contract * Emitter Contract and Bindings * AddL2RPC without Stopping * Rename testdata folder to contracts * update generate.sh * update solidity and gen script
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has two components
Emitter Contract in
op-e2e/interop/contracts
Now there is a directory where custom contracts can be used for message passing or other test behaviors. At the moment there is only one:
emit.go
. The Emitter contract simply takes the caller's data and emits it as a log event. This allows us to generate content on the Supervisor, and we can also use this contract to test the receiving side.To update the binding,
generate.sh
does a forge build, extracts the bin and abi, andabigen
creates a go binding.Custom Contracts for L2s are currently stored in a
map[string]interface{}
to store arbitrary contract bindings in the future. This one is keyed as "emitter".The Emitter Contract adds two functions to the
SuperSystem
interface. I'd like to avoid scope-creeping the interface, but for now this provides good abstraction for tests to use this contract:AddL2RPC without Restarting Supervisor
As soon as the log emissions started, I found another bug in the Supervisor. This one is related to file closing -- in
AddL2RPC
, weStop
the backend before adding the L2 and then weStart
it back up. However, in underlying calls, a file object is closed which never gets opened again.As a symptom, log emissions for any chain except the most recently added would fail because the underlying file was closed to writing!
I could have tired to make the
logDB
re-openable, but the real solution was just to make it possible to add new L2s without stopping the existing monitors. It was easier than I expected, the main change required was that the code used to add L2s needed to know whether or not to start the chain-monitor too.Testing
As soon as log emission worked, the closed-file issue started. By implementing these changes, I see each Chain Monitor start exactly once, and I see them process (or attempt to process) chain updates.
fixes: #11959