-
Notifications
You must be signed in to change notification settings - Fork 50
Autostake the token rewards in the court where the stake originates from #2165
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
Conversation
❌ Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →
|
✅ 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-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughIntroduces a local Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Core as KlerosCore
participant Draws as Draws/DisputeData
participant Court as SortitionModule/Court
participant Token as Token
Caller->>Core: executeRewards(_params,...)
activate Core
Note over Core: repartition = _params.repartition % _params.numberOfVotesInRound
Core->>Draws: getDrawnJuror(repartition)
Draws-->>Core: jurorAddress
Core->>Draws: getCourtIdForJuror(repartition)
Draws-->>Core: courtId
Core->>Core: reward = getDegreeOfCoherenceReward(repartition,...)
alt stake via setStakeReward available
Core->>Court: setStakeReward(jurorAddress, courtId, reward)
Court-->>Core: success / fail
alt success
Note right of Core: reward staked in courtId
else fail
Core->>Token: transfer(jurorAddress, reward)
Token-->>Core: ok
end
else no staking
Core->>Token: transfer(jurorAddress, reward)
Token-->>Core: ok
end
deactivate Core
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
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
🧹 Nitpick comments (1)
contracts/src/arbitration/KlerosCore.sol (1)
972-1011
: LGTM! Rewards are now correctly staked in the juror's origin court.The changes correctly implement the PR objective by:
- Computing a local
repartition
variable to map the second-pass iteration index back to the juror array index- Using this
repartition
consistently to resolve the correct juror's address and origin court- Staking rewards in
penalizedInCourtID
(the court where the juror's stake originated) rather than the current dispute courtThis is a significant improvement because jurors who were drawn from a subcourt will now have their rewards auto-staked in that subcourt, maintaining their staking position and avoiding the need for manual restaking.
The fallback to direct transfer (lines 1011-1013) provides appropriate error handling.
Optional: Consider renaming
repartition
tojurorIndex
for clarity.While the current implementation is correct, the variable name
repartition
might be confusing since it represents a juror index rather than a repartition action. Consider:- uint256 repartition = _params.repartition % _params.numberOfVotesInRound; + uint256 jurorIndex = _params.repartition % _params.numberOfVotesInRound;And update all references accordingly. This would make the code more immediately understandable.
Additionally, the empty line at 1009 could be removed to maintain consistency with the surrounding code style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/src/arbitration/KlerosCore.sol
(3 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). (14)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
cd70d8f
to
bc906e5
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
🧹 Nitpick comments (1)
contracts/src/arbitration/KlerosCore.sol (1)
972-978
: Consider adding a comment to explain the repartition calculation.The
repartition
variable maps the reward iteration index back to the vote index range. This is a key aspect of the two-pass execution model (penalties first, then rewards), and a brief inline comment would help future maintainers understand this logic.For example:
+ // Map reward iteration index back to vote index (rewards are processed in second pass: i >= numberOfVotesInRound) uint256 repartition = _params.repartition % _params.numberOfVotesInRound;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/src/arbitration/KlerosCore.sol
(3 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). (14)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: hardhat-tests
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Analyze (javascript)
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
🔇 Additional comments (2)
contracts/src/arbitration/KlerosCore.sol (2)
1008-1014
: LGTM! Correctly implements autostaking in the origin court.The change correctly implements the PR objective by staking rewards in the court where the juror's stake originates (
rewardedInCourtID
fromround.drawnJurorFromCourtIDs[repartition]
) rather than the dispute's court. This aligns with the penalty handling in_executePenalties
(line 923) and ensures consistency. The fallback to direct transfer if staking fails (line 1012) provides good defensive behavior.Based on learnings.
1008-1008
: No action needed: arrays are updated together
drawnJurors
anddrawnJurorFromCourtIDs
are only ever pushed side-by-side (KlerosCore.sol 759–760; KlerosCoreUniversity.sol 608–609) and never modified independently, so their lengths always match.
PR-Codex overview
This PR focuses on improving the handling of juror coherence and rewards in the
KlerosCore
contract by refining how juror accounts are accessed and how rewards are processed based on therepartition
parameter.Detailed summary
repartition
to simplify calculations based on_params.repartition
.account
to userepartition
instead of the previous modulo operation.rewardedInCourtID
is determined, usingrepartition
for retrieving the court ID.setStakeReward
method call to use the newrewardedInCourtID
.Summary by CodeRabbit
Bug Fixes
Reliability