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(op-dispute-mon): L2BlockNumberChallenged Support #10451
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughWalkthroughThe recent updates across various components focus on integrating a new boolean field, Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (23)
Files skipped from review as they are similar to previous changes (8)
Additional comments not posted (22)
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 (
|
a4a64c1
to
835921f
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: 3
8a3c79f
to
67e0edf
Compare
dispute-mon shouldn't trust the contract for l2 block disputes to be robust against contract exploits. |
This is correct but is a different follow up metric to this pr - the dispute mon should extract the block number challenge from the game and then forecast the result properly. Then it should also have a metric to validate the l2 block number challenge and yell loudly. Though, the way our contracts are set up now - if an invalid l2 block number challenge succeeds, the game will resolve incorrectly regardless as I understand it. |
feat(op-dispute-mon): query for the l2 block number through the game metadata call fix(op-dispute-mon): query for the l2 block number through the game metadata call fix(op-dispute-mon): query for the l2 block number through the game metadata call
5404c20
to
b837cc4
Compare
Gotcha. I just realized that dispute-mon already checks the claimed L2 block number against the claimed output. That is the check I wanted to ensure was happening. |
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!
You were correct - we were able to do this easily since @ajsutton's recent PRs added this agreement hydration to the enriched game object with nice concurrency 😄 |
Implemented my suggested changes in #10483 |
Reviewed, I do like the change to pull the status out into a separate metric, though I don't think we should leave the incorrect forecast ambiguous and explicitly create an alert if |
* op-dispute-mon: Consider l2 block number challenged when forecasting but don't make it a new game status. Add a separate metric to report the number of successful L2 block number challenges. * op-challenger: Remove unused blockNumChallenged field from list-games info struct * op-dispute-mon: Consider l2 block challenger in expected credits. (#10484) * feat(op-dispute-mon): Update L2 Challenges Metrics (#10492) * fix: change the l2 challenges to tha gauge vec * fix: consistent metric labels --------- Co-authored-by: refcell <abigger87@gmail.com>
Description
Updates the
op-dispute-mon
to support L2 Block Number challenging. On game extraction, the dispute monitor now queries theFaultDisputeGame
contract if the L2 Block Number has been challenged and hydrates a new field in theEnrichedGameData
that downstream monitor components can read. TheForecast
component has been updated as well to record games asAgreeChallengerWins
if the game's L2 Block Number has been challenged successfully, ignoring all other claim status.Metadata
Fixes https://github.com/ethereum-optimism/client-pod/issues/853