Dispute kit can force an early court jump and override the next round nbVotes #2110
Conversation
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
WalkthroughCentralizes court and dispute-kit jump logic in KlerosCoreBase with new internal helpers, updates appeal flow and cost calculations to use dispute-kit’s getNbVotesAfterAppeal, adds two IDisputeKit methods with DisputeKitClassicBase implementations, and lowers Hardhat Solidity optimizer runs from 10000 to 2000. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant Core as KlerosCoreBase
participant Court as Court (current)
participant DK as DisputeKit (current)
participant PC as Court (parent)
participant NewDK as DisputeKit (resolved)
U->>Core: appeal(disputeID)
Core->>Court: load court params
Core->>DK: earlyCourtJump(disputeID)
DK-->>Core: jumpSignal (bool)
Core->>Core: _isCourtJumping(round, court, disputeID)
alt Jump required
Core->>PC: climb to parent court
Core->>PC: check DK support
Core->>NewDK: choose DK (fallback to Classic if unsupported)
Core-->>U: emit CourtJump(newCourtID, newDisputeKitID)
else No jump
Note right of Core: remain in current court & DK
end
Core->>NewDK: getNbVotesAfterAppeal(prevDK, round.nbVotes)
NewDK-->>Core: nbVotesAfterAppeal
Core->>Core: compute appealCost with nbVotesAfterAppeal and court fees
Core-->>U: create next round, set extraRound.disputeKitID
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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). (10)
✨ 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 (
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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
🧹 Nitpick comments (7)
contracts/hardhat.config.ts (1)
34-34: Lowering optimizer runs to 2000: verify gas impact and consider making it configurable.Reducing runs from 10000 to 2000 generally favors smaller bytecode over runtime gas. Given frequent on-chain interactions for Core/DK, this could increase users’ per-call gas. Suggest: parametrize via env so CI/prod can still use higher runs, while local/dev can use lower runs.
Apply this diff to make it configurable without code churn:
- runs: 2000, + runs: parseInt(process.env.SOLC_RUNS ?? "2000"),Suggested checks:
- Compare gas of critical paths (createDispute, draw, passPeriod, appeal, execute) with SOLC_RUNS=10000 vs 2000 on a small scenario set.
- Ensure no contract exceeds size limits after this change on your largest court graph.
contracts/src/arbitration/interfaces/IDisputeKit.sol (1)
116-120: New earlyCourtJump hook: good extension; document semantics wrt DK-jump vs court-jump.The naming is clear; consider clarifying in the NatSpec that this hook only influences the court-jump decision. Whether a DK-jump happens is still determined by parent-court support (Core-side). No code change needed.
contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol (2)
630-635: Make the override virtual to allow Classic derivatives to customize early jump.If a downstream “Classic-like” DK wants to enable early jumps based on extraData or custom logic, marking this as virtual provides extension flexibility at no cost.
- function earlyCourtJump(uint256 /* _coreDisputeID */) external pure override returns (bool) { + function earlyCourtJump(uint256 /* _coreDisputeID */) external pure virtual override returns (bool) { return false; }
636-642: Same here: make vote-scaling policy overrideable by Classic derivatives.This keeps ClassicBase extensible if a sub-variant wants a different growth rule (e.g., cap or non-linear).
- function getNbVotesAfterAppeal(uint256 _currentNbVotes) external pure override returns (uint256) { + function getNbVotesAfterAppeal(uint256 _currentNbVotes) external pure virtual override returns (uint256) { return (_currentNbVotes * 2) + 1; }contracts/src/arbitration/KlerosCoreBase.sol (3)
645-651: Event round index for CourtJump: confirm whether it should reference “from” or “to” round.You push extraRound before emitting CourtJump and pass
dispute.rounds.length - 1as_roundID(the new round). If analytics/indexers expect the jump event to reference the round being concluded (old round), this will be off by one after the push.If the intended semantics are “the round that starts after the jump,” keep as-is but document it. If the intent is “the round that triggered the jump,” consider:
- emit CourtJump(_disputeID, dispute.rounds.length - 1, dispute.courtID, newCourtID); + // Emit index of the round that triggered the jump (previous last round). + emit CourtJump(_disputeID, dispute.rounds.length - 2, dispute.courtID, newCourtID);Either way, aligning the event’s
_roundIDsemantics with docs/tests will save downstream headaches.
924-939: appealCost: correct usage of DK-defined nbVotesAfterAppeal; simplify branching.Logic is sound and now respects DK vote-growth policy and possible early court-jumps. You can reduce duplication to one multiplication site for readability.
- (, uint256 newDisputeKitID, bool courtJump, ) = _getCourtAndDisputeKitJumps(dispute, round, court, _disputeID); - - uint256 nbVotesAfterAppeal = disputeKits[newDisputeKitID].getNbVotesAfterAppeal(round.nbVotes); - - if (courtJump) { - // Jump to parent court. - if (dispute.courtID == GENERAL_COURT) { - // TODO: Handle the forking when appealed in General court. - cost = NON_PAYABLE_AMOUNT; // Get the cost of the parent court. - } else { - cost = courts[court.parent].feeForJuror * nbVotesAfterAppeal; - } - } else { - // Stay in current court. - cost = court.feeForJuror * nbVotesAfterAppeal; - } + (, uint256 newDisputeKitID, bool courtJump, ) = _getCourtAndDisputeKitJumps(dispute, round, court, _disputeID); + uint256 nbVotesAfterAppeal = disputeKits[newDisputeKitID].getNbVotesAfterAppeal(round.nbVotes); + if (courtJump && dispute.courtID == GENERAL_COURT) { + // TODO: Handle the forking when appealed in General court. + return NON_PAYABLE_AMOUNT; + } + Court storage feeCourt = courtJump ? courts[court.parent] : court; + cost = feeCourt.feeForJuror * nbVotesAfterAppeal;Note: The NON_PAYABLE_AMOUNT pattern will cause downstream DK math to revert on overflow (expected), effectively blocking funding in General Court — just ensure tests assert this path.
1047-1081: New helpers centralize jump decisions: good. Consider minor doc tweaks and boundary tests.
- The >= comparison matches the comment in Court (“The appeal after the one that reaches this number of jurors will go to the parent”) — boundary is correct.
- Please add in-line NatSpec to
_isCourtJumpingto state that the early jump is DK-signaled and may occur even if juror threshold not met.- Add tests for:
- nbVotes == jurorsForCourtJump triggers jump on next appeal.
- nbVotes == jurorsForCourtJump - 1 with earlyCourtJump=false does not jump.
- earlyCourtJump=true causes jump irrespective of threshold.
- isDisputeKitJumping returns false when parent supports current DK, true otherwise.
I can draft Hardhat tests covering these scenarios if helpful.
📜 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 (4)
contracts/hardhat.config.ts(1 hunks)contracts/src/arbitration/KlerosCoreBase.sol(4 hunks)contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol(1 hunks)contracts/src/arbitration/interfaces/IDisputeKit.sol(1 hunks)
⏰ 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). (11)
- GitHub Check: Analyze (javascript)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: contracts-testing
🔇 Additional comments (2)
contracts/src/arbitration/interfaces/IDisputeKit.sol (1)
121-125: Breaking IDisputeKit change verified – all internal DKs updated
- contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol implements both
•earlyCourtJump(...)
•getNbVotesAfterAppeal(...)- No other
contract … is IDisputeKitdeclarations are missing these methods.Please ensure any downstream packages or external DK implementations are also updated to include these interface methods to avoid compilation breaks.
contracts/src/arbitration/KlerosCoreBase.sol (1)
1027-1033: isDisputeKitJumping now delegates to _isCourtJumping: LGTM.This correctly distinguishes “court jump needed?” from “DK switch needed?” and preserves the existing parent-support check.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
contracts/src/arbitration/interfaces/IDisputeKit.sol (1)
116-120: Clarify “early court jump” trigger semantics and tighten NatSpecThe name/docs can be read as “is the dispute currently jumping,” whereas Core likely queries this to decide “should the next appeal transition jump to parent.” Make the trigger point explicit so DKs implement deterministically.
Apply this doc-only diff for precision:
- /// @dev Returns true if the dispute is jumping to a parent court. - /// @param _coreDisputeID The ID of the dispute in Kleros Core, not in the Dispute Kit. - /// @return Whether the dispute is jumping to a parent court or not. + /// @dev Indicates whether this Dispute Kit requests Kleros Core to jump to the parent court + /// on the next round/appeal transition of the given dispute. + /// @param _coreDisputeID The ID of the dispute in Kleros Core (not in the Dispute Kit). + /// @return shouldJump True if Core must perform a parent-court jump at the next appeal transition. + /// @notice Must be a pure/read-only decision based on the dispute’s state; no storage writes. + /// @dev Implementations SHOULD be deterministic and not depend on block.timestamp or randomness.Optionally consider renaming to
shouldEarlyCourtJumpfor self-documenting intent. Low priority since this is a draft and call sites are confined to Core.Please confirm
KlerosCoreBasecalls this exactly once per appeal transition (before computing costs/nbVotes) to avoid inconsistent cost derivations when the value could change mid-flow.
📜 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/arbitration/KlerosCoreBase.sol(4 hunks)contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol(1 hunks)contracts/src/arbitration/interfaces/IDisputeKit.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol
- contracts/src/arbitration/KlerosCoreBase.sol
⏰ 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). (15)
- GitHub Check: SonarCloud
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
jaybuidl
force-pushed
the
feat/modular-jump-and-nbvotes
branch
from
285c04b to
057957e
Compare
|
PR-Codex overview
This PR focuses on enhancing the dispute resolution process in the Kleros system by introducing new functionalities to manage court jumps and vote counting after appeals, alongside updates to the
hardhat.config.tsand theCHANGELOG.md.Detailed summary
optimizerruns from10000to2000inhardhat.config.ts.DisputeKitClassicBase.solfor early court jumps and vote counting after appeals.IDisputeKit.solto include new functions for court jumps and vote management.KlerosCoreBase.solto utilize new helper functions.getNbVotesAfterAppealto calculate votes based on appeal status.CHANGELOG.mdto reflect new features and fixes.Summary by CodeRabbit
New Features
Refactor
Chores
Documentation