Adding the juror's total stake for all courts in the StakeSet event back in #1935
Conversation
WalkthroughThis pull request updates the event logging functionality in staking-related contracts by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant Juror
participant Contract
participant EventLogger
Juror->>Contract: setStake(stake, courtID, currentStake)
Contract->>EventLogger: Emit StakeSet(juror, courtID, stake, totalStaked)
alt Delayed Stake Scenario
Contract->>EventLogger: Emit StakeDelayedNotTransferred / StakeDelayedAlreadyTransferredDeposited / StakeDelayedAlreadyTransferredWithdrawn
end
alt Stake Locking
Contract->>EventLogger: Emit StakeLocked(juror, amount, unlockState)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (14)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
🧰 Additional context used🧬 Code Definitions (2)subgraph/core/tests/sortition-module.test.ts (1)
subgraph/core-university/src/SortitionModule.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🔇 Additional comments (19)
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the StakeSet event in the Sortition modules by adding an additional parameter that represents the juror's total stake across all courts.
- Updated StakeSet event arguments in tests to include the total staked amount.
- Adjusted test cases in both staking.ts and staking-neo.ts to account for the new parameter.
Reviewed Changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| contracts/test/arbitration/staking.ts | Updated test expectations for StakeSet event with the new total stake. |
| contracts/test/arbitration/staking-neo.ts | Modified test assertions and totalStaked validations to include the new parameter. |
Files not reviewed (3)
- contracts/src/arbitration/SortitionModuleBase.sol: Language not supported
- contracts/src/arbitration/university/SortitionModuleUniversity.sol: Language not supported
- contracts/test/foundry/KlerosCore.t.sol: Language not supported
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the StakeSet event in the staking contracts by including an additional parameter representing the total staked amount across all courts, and updates the related tests and documentation accordingly.
- Updated the StakeSet event signature to include the new total staked parameter.
- Adjusted test cases in both staking.ts and staking-neo.ts to validate the new event parameter.
- Added detailed documentation for multiple staking-related events.
Reviewed Changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| contracts/test/arbitration/staking.ts | Updated test expectations for StakeSet to include the new parameter. |
| contracts/test/arbitration/staking-neo.ts | Adjusted test cases to pass the new total staked parameter correctly. |
Files not reviewed (3)
- contracts/src/arbitration/SortitionModuleBase.sol: Language not supported
- contracts/src/arbitration/university/SortitionModuleUniversity.sol: Language not supported
- contracts/test/foundry/KlerosCore.t.sol: Language not supported
Comments suppressed due to low confidence (1)
contracts/test/arbitration/staking-neo.ts:706
- The use of 'await sortition' with .to.emit is inconsistent with other tests where the contract instance is passed directly. Consider removing 'await' for consistency.
.to.emit(await sortition, "StakeSet")
jaybuidl
dismissed stale reviews from unknownunknown1, kemuru, and coderabbitai[bot]
via
4554947
…yedAlreadyTransferredDeposited
jaybuidl
force-pushed
the
feat/add-stakeset-total
branch
from
4554947 to
c399c03
Compare
…s-calculations chore: revert stakeset handling in this subgraph and add new event parameter
|
Code Climate has analyzed commit ba85c09 and detected 17 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
jaybuidl
added
Type: Enhancement ✨
Package: Subgraph
|
PR-Codex overview
This PR updates the
StakeSetevent to include an additional parameter,amountAllCourts, and modifies related code to handle this change. It also refines event names and updates the version of the package.Detailed summary
StakeSetevent to includeamountAllCourts.createStakeSetEventfunction to accept the new parameter.handleStakeSetto utilize the new parameter.StakeDelayedAlreadyTransferredtoStakeDelayedAlreadyTransferredDeposited.0.12.0to0.13.0.Summary by CodeRabbit
New Features
Tests