-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
|
/// i.e. The game type should indicate the security model. | ||
/// @return gameType_ The type of proof system being used. | ||
function gameType() public view returns (GameType) { | ||
// TODO: Retrive and return the game type. |
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 new GameTypes.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 would GameTypes.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:
GameType internal constant OP_SUCCINCT = GameType.wrap(3);
here, we could temporary do:
return GameType.wrap(3);
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.
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.
impl IDisputeGame for OPSuccinctL2OutputOracle
Additionals notes:
Initializable on OPSuccinctL2OutputOracle clashes with IInitializable on IDisputeGame (they both define initialize()), so I renamed OPSuccinctL2OutputOracle::initialize() to initializeParams()
I don't see an update to OPSuccinctL2OutputOracle
? Did you incorporate that in this PR? You should make sure that you update the scripts if so.
Can you also add a new folder inside of the book/
for experimental/beta features and document how users can use the OptimismPortalV2
features. I'm planning to go through the docs and repo later this week. It's important that this feature/deployment for it is documented well.
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.
LGTM, once the documentation nits + DGF contract link are added
* :add * build on branch * update * add * add * add * add * add * add * add * add * add * feat: update elf * add * add * add * add * add * add * fix * add * add * add * add
* ci: add ci for elf check * chore(elf): update elfs
Adding
OPSuccinctDisputeGame
andOPSuccinctDisputeGameFactory
contracts to integrateOPSuccinctL2OutputOracle
toOptimismPortalV2
.OPSuccinctDisputeGame
implementsIDisputeGame
and is a thin wrapper aroundOPSuccinctL2OutputOracle
. It also implementsCWIA
and is cloned each time a new game is created. I didn't implementIDisputeGame
onOPSuccinctL2OutputOracle
as it doesn't make sense to clone it each time and is more expensive.The proposer has been updated to call
OPSuccinctDisputeGameFactory.create()
each time a newoutputRoot
is proposed, ifDGF_ADDRESS
is set.