Fix/outbox resolution #432
Conversation
✅ Deploy Preview for veascan ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds a FailedResolution(uint256) event to three outbox contracts. Updates startVerification to require no active challenger and to emit Verified only on success; otherwise emit FailedResolution. Updates resolveDisputedClaim to emit Verified only on successful resolution and FailedResolution on failure; removes prior unconditional Verified emissions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Outbox as Outbox Contract
participant Storage as Claim Storage
rect rgb(245,245,255)
note over User,Outbox: startVerification
User->>Outbox: startVerification(epoch, claim)
Outbox->>Storage: load Claim(epoch)
Outbox-->>User: require(claim.challenger == 0x0)
alt verification succeeds
Outbox->>Storage: update claim status
Outbox-->>User: emit Verified(epoch)
else verification fails
Outbox-->>User: emit FailedResolution(epoch)
end
end
sequenceDiagram
autonumber
participant Resolver as Resolver (any caller)
participant Outbox as Outbox Contract
participant Storage as Claim Storage
rect rgb(245,255,245)
note over Resolver,Outbox: resolveDisputedClaim
Resolver->>Outbox: resolveDisputedClaim(epoch)
Outbox->>Storage: read claim/roots
alt claim/roots validate
Outbox->>Storage: update honest party / finalize
Outbox-->>Resolver: emit Verified(epoch)
else validation fails or no claim
Outbox-->>Resolver: emit FailedResolution(epoch)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol (1)
287-302: Emit Verified event on failure branch to unblock relayers
Relayers filtering only on Verified(_) will miss on-chain stateRoot updates when the claim hash mismatches. Addemit Verified(_epoch);in theelseblock beforeemit FailedResolution(_epoch):} else { - emit FailedResolution(_epoch); + emit Verified(_epoch); + emit FailedResolution(_epoch); }
🧹 Nitpick comments (2)
contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol (1)
82-85: Index the epoch in FailedResolution for parity with VerifiedVerified(_epoch) is indexed in this contract; FailedResolution(_epoch) should be indexed too for consistent filtering and off-chain querying.
- event FailedResolution(uint256 _epoch); + event FailedResolution(uint256 indexed _epoch);contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol (1)
84-87: Event addition LGTM; consider indexing _epoch (optional)FailedResolution is fine. If you want uniform log filtering, consider indexing _epoch here and (optionally) in Verified for this file in a follow-up, but this would change event signatures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
contracts/src/arbitrumToEth/VeaOutboxArbToEth.sol(3 hunks)contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol(3 hunks)contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mani99brar
PR: kleros/vea#430
File: contracts/src/interfaces/inboxes/IVeaInbox.sol:0-0
Timestamp: 2025-08-27T07:02:45.825Z
Learning: The sendMessage signature simplification in IVeaInbox.sol and related call site updates across relay-cli and test files were fixed in a separate PR by mani99brar.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Redirect rules - veascan
- GitHub Check: Header rules - veascan
- GitHub Check: test
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript)
- GitHub Check: Pages changed - veascan
🔇 Additional comments (5)
contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol (1)
271-287: Emit Verified on claimless resolution and verify off-chain consumers
State is updated unconditionally when L1 resolution succeeds, but Verified is only emitted if a claim exists; indexers/relayers relying solely on Verified may miss finalization for epochs without claims. To avoid liveness breaks, emit Verified in the else branch and confirm in your off-chain relayer repo that this change poses no regressions.} else { - emit FailedResolution(_epoch); + emit Verified(_epoch); + emit FailedResolution(_epoch); }contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol (1)
227-227: Good guard against starting verification on challenged claimsPrevents starting verification when a challenger exists; aligns with verifySnapshot’s guard.
contracts/src/arbitrumToEth/VeaOutboxArbToEth.sol (3)
88-91: Event addition LGTMFailedResolution improves observability of unsuccessful resolution paths.
281-282: Good: block startVerification when already challengedPrevents inconsistent verification start on challenged claims; consistent with challenge/verify flow.
343-358: Ensure Verified event is emitted on state finalization
stateRoot/latestVerifiedEpoch are updated unconditionally, but Verified is only emitted when the claim matches. Off-chain relayers relying on Verified may never see the finalization. If suppressing Verified here is intentional, confirm that all consumers also handle FailedResolution or poll the on-chain state directly.
FailedResolutionevent, emitted when claim sent from Inbox does not match with claim on Outbox during a dispute resolution.Verifiedevent to emit only when the dispute is resolved inresolveDisputeForClaim().startVerification()to ensure that the claim can not be updated after it's been challenged.