Optimism Dispute Game Support #45
Conversation
| return l2out.outputRoot; | ||
| } | ||
|
|
||
| function getDisputeGameOutput( |
There was a problem hiding this comment.
Are there any reference you used so that I can verify?
There was a problem hiding this comment.
There was a problem hiding this comment.
How did you manage to test each failure scenario like GameChallenged, GameTooEarly, DeprecatedProof or are they even possible to test?
op-verifier/contracts/OPVerifier.sol
Outdated
| revert GameTypeMismatch(index, respectedGameType, gameType); | ||
| } | ||
|
|
||
| // Revert if the game is challenged |
There was a problem hiding this comment.
Can we revert even if there's a challenge initiated? Eg, only allow unchallenged or finalized games?
There was a problem hiding this comment.
That will make it vulnerable to malicious challengers that try to challenge random game?
|
Merged and pushed the new version Using OPOutputLookup contract I have built at https://github.com/Opti-domains/dispute-game-lookup/blob/main/src/OPOutputLookup.sol to do the calculation and advanced search in a single call. So, the lowest latency one can achieve. Pending unit test on https://github.com/Opti-domains/dispute-game-lookup However, it works manual testing: https://app.ens.domains/disputegame.eth?tab=records (On Sepolia testnet) |
…op-fault-proof-new
…op-fault-proof-new
|
Sorry for the delay, I have been busy for these weeks! Helper contracts and libraries have been merged into the repo and a deployment script to deterministically deploy OPOutputLookup have been added. Some trick has been implemented to be able to import Current contract and gateway (with app variable moved into a local variable) has been successfully tested on current implementation. https://app.ens.domains/disputegame.eth?tab=records (Sepolia)
|
op-gateway/src/OPProofService.ts
Outdated
| const block = await this.opOutputLookup.getOPProvableBlock( | ||
| await this.optimismPortal.getAddress(), | ||
| this.minAge, | ||
| 1000000000 |
There was a problem hiding this comment.
What parameter is this? Can you leave a comment on it?
There was a problem hiding this comment.
It's Unlimited maximum age to have the gateway fetch output root regardless of maximum age parameter as it will be verified on the contract anyway. Commented
| ); | ||
| } | ||
|
|
||
| // Revert if the game is challenged |
There was a problem hiding this comment.
We should also reject games that have an open challenge, not just games that have already been successfully challenged. Otherwise an attacker can keep using an invalid state during the whole challenge process.
There was a problem hiding this comment.
That would make it delay a few days at least if we check for the IN_PROGRESS status. I haven't seen a function to directly check for an open challenge.
There was a problem hiding this comment.
Can be seen at https://sepolia.etherscan.io/address/0x99b0e2a750Cd985A3707a8C792BB0ADb28d148b4#readContract
That try to implement this one
It go back 4 days in OP sepolia testnet
There was a problem hiding this comment.
I'm a bit confused by this. Reading the OP code, it looks like the assumption is that every dispute game in fact corresponds to an active dispute. Are dispute games automatically created for each new block committed, or are they only created when someone challenges a block?
There was a problem hiding this comment.
Dispute games are created for every state root proposal and the successful resolution of a game is what updates the anchor state in the AnchorStateRegistry.
In principle anyone can create a game (and Optimism do have confirmed production test canaries trying to break things with invalid root claims) but the game has to resolve in favour of the proposed state for the anchor to be updated.
There was a problem hiding this comment.
It may be possible to detect that the length of claimData is greater than 1, which means someone has tried to challenge, but I'm not 100% sure.
There was a problem hiding this comment.
Implemented open challenge detection with these checks
function _isGameChallenging(
IChallengingDisputeGame proxy
) internal view returns (bool) {
// If the game is finalized, we ignore the challenging status
if (proxy.status() == GameStatus.DEFENDER_WINS) return false;
// If claimData length is greater than 1, the game is challenging
if (proxy.claimDataLen() > 1) {
return true;
}
// If root claim is countered by someone then the game is challenging
(, address counteredBy, , , , , ) = proxy.claimData(0);
if (counteredBy != address(0)) return true;
return false;
}And fixed interpolation search revert on case that lower bound has moved beyond the max timestamp
There was a problem hiding this comment.
Looks good! Is interpolation search working fine now then? Some tests (and benchmarks) are all that's missing to merge this, I think.
Thank you again for all your hard work on this PR.
There was a problem hiding this comment.
Test cases have been implemented in my dispute-game-lookup repository. I am using Foundry's fuzz testing combined with mainnet forking to identify and fix edge cases, including integer underflow and rounding errors.
The test cases can be found at the following links:
The files in the dispute-game-lookup repo and this repo differ only in the import section, as shown in these diffs:
There was a problem hiding this comment.
For benchmarking, it take about 3.3s for each record processed in parallel on chrome network profiling
https://app.ens.domains/disputegame.eth (On Sepolia)
| ); | ||
| } | ||
|
|
||
| // Revert if the game is challenged |
There was a problem hiding this comment.
I'm a bit confused by this. Reading the OP code, it looks like the assumption is that every dispute game in fact corresponds to an active dispute. Are dispute games automatically created for each new block committed, or are they only created when someone challenges a block?
|
Hey, just wondering what's required here to get this into |
|
This version is working, but there is still room for improvement, such as adding an anchor or backup feature for dispute game types. |
An improved approach to #36
In this version, there's no need to split the code into two different files. We first try to find the index using the dispute game. If the dispute game is unavailable, we will fall back to using L2OutputOracle.
The function that needs L2OutputOracle has been replaced with OptimismPortalProxy, which is the core of the OP Stack. OptimismPortalProxy can derive both L2OutputOracle and DisputeGameFactory.
This implementation has been tested and works well.