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.
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
feat: migrate OPSuccinctL2OutputOracle to OptimismPortalV2 #277
feat: migrate OPSuccinctL2OutputOracle to OptimismPortalV2 #277
Changes from 16 commits
20bd110
1f28ebe
ca1eaa9
8418b7c
cbcf629
e386e4e
4bd230a
f1dc0e8
0cb9d04
c8e529e
3c9601a
f238d22
b6fd11f
34b3ef1
8f6ba7c
3158d35
a90dd34
4a317f9
0a8b80e
67b93ed
a3e4484
eb54fad
a684b65
344c464
8827da9
6e89ac2
9ce9ec8
abbc052
9725fe5
b9e9072
5652806
903d0b0
bfc7438
23a53d7
64de2f0
6693663
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Does this still need to be added?
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 set it to
GameTypes.CANNON
for now, and I discussed with OP Labs, they aren't opposed to adding a newGameTypes.OP_SUCCINCT
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.
What is the use of
GAME_TYPE
? To see the security model? When wouldGameTypes.OP_SUCCINCT
be upstreamed?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.
GameTypes are used in DisputeGameFactory to store multiple games in a mapping, with the game type as key.
We don't have a clear view on when GameTypes.OP_SUCCINCT will be upstreamed, but if for instance we add:
here, we could temporary do:
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.
Sounds good, just add a comment that GameType.wrap(3) will eventually be upstreamed to be OP Succinct. Add a TODO that this
GameType
needs to be changed in the future, and link to the corresponding Optimism contracts PR.