-
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
feat(ctb): Add new move type to FDG for OR counters #10438
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #10438 +/- ##
============================================
- Coverage 42.31% 29.22% -13.09%
============================================
Files 73 31 -42
Lines 4838 2898 -1940
Branches 766 614 -152
============================================
- Hits 2047 847 -1200
+ Misses 2682 1976 -706
+ Partials 109 75 -34
Flags with carried forward coverage won't be shown. Click here to find out more. |
cd7f015
to
6b88ec6
Compare
WalkthroughWalkthroughThis update involves minor adjustments to gas costs, semantic version updates in the Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files ignored due to path filters (1)
Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Out of diff range and nitpick comments (5)
packages/contracts-bedrock/src/dispute/lib/Errors.sol (5)
48-48
: Ensure the error message forInvalidClaim
is descriptive enough to aid debugging.
108-108
: Ensure the error message forInvalidOutputRootProof
is descriptive enough to aid debugging.
111-111
: Ensure the error message forInvalidHeaderRLP
is descriptive enough to aid debugging.
115-115
: Ensure the error message forBlockNumberMatches
is descriptive enough to aid debugging.
118-118
: Ensure the error message forL2BlockNumberChallenged
is descriptive enough to aid debugging.
6b88ec6
to
03a27e7
Compare
03a27e7
to
8c3051c
Compare
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.
This is good. Sorry to ask you to do this, but can we avoid touching claimData
or introducing new claims?
The changes to resolution should be minimal; just need to add a check asserting that the root's subgame cannot be resolved if the L2 block number is challenged.
8c3051c
to
95099d2
Compare
ed8a0ae
to
83462ea
Compare
Adds a potential new move type to the `FaultDisputeGame` that allows for a participant to reveal the preimage of the claimed output root to display to the dispute game that the claimed L2 block number does not match up with the block number that the block header within the output root commits to. The root output can be challenged with the new special move type iff: 1. The passed `OutputRootProof` hashes to equal the claimed output root. 1. The passed Header RLP hashes to equal the block hash within the `OutputRootProof` above. 1. The claimed block number in the dispute game does not equal the block number that the output root commits to. If there is a successful challenge with the new move type, that claim itself is inserted as a special case counter. In `resolveClaim`, the contract will always consider the creator of the L2 block challenge the winner of the bond. Notably, this only applies for the root claim subgame.
* feat(ctb): Transition `RLPReader` to 4byte errors * semver
83462ea
to
a8e6e2c
Compare
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.
Actionable comments posted: 0
Out of diff range and nitpick comments (2)
packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol (2)
8-8
: Consider removing unused imports to clean up the code.
14-16
: Ensure that all imported libraries are used within the file to avoid unnecessary dependencies.
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.
Looking good, leaving one comment. Personally I would've preferred not to make the RLP changes but they've been made already so whatever. We need to justify any diff that's being made on top of audited code when we go to gov vote, prefer to avoid this sort of stuff if we can.
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.
Looks great to me! Nice unit tests as 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.
Excellent!
* feat(ctb): Add new move type to FDG for OR counters Adds a potential new move type to the `FaultDisputeGame` that allows for a participant to reveal the preimage of the claimed output root to display to the dispute game that the claimed L2 block number does not match up with the block number that the block header within the output root commits to. The root output can be challenged with the new special move type iff: 1. The passed `OutputRootProof` hashes to equal the claimed output root. 1. The passed Header RLP hashes to equal the block hash within the `OutputRootProof` above. 1. The claimed block number in the dispute game does not equal the block number that the output root commits to. If there is a successful challenge with the new move type, that claim itself is inserted as a special case counter. In `resolveClaim`, the contract will always consider the creator of the L2 block challenge the winner of the bond. Notably, this only applies for the root claim subgame. * feat(ctb): Transition `RLPReader` to 4byte errors (#10439) * feat(ctb): Transition `RLPReader` to 4byte errors * semver * update summary * portal semver * extra checks
Overview
Adds a potential new move type to the
FaultDisputeGame
that allows for a participant to reveal the preimage of the claimed output root to display to the dispute game that the claimed L2 block number does not match up with the block number that the block header within the output root commits to.Semantics
The root output can be challenged with the new special move type iff:
OutputRootProof
hashes to equal the claimed output root.OutputRootProof
above.If there is a successful challenge with the new move type, that claim itself is inserted as a special case counter. In
resolveClaim
, the contract will always consider the creator of the L2 block challenge the winner of the bond. Notably, this only applies for the root claim subgame.