Evidence event extra _arbitrable parameter
#2167
Conversation
jaybuidl
added
Type: Enhancement ✨
Compatibility: ABI change 🗯
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ 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. |
WalkthroughAdds an explicit arbitrable address parameter to evidence submission functions and updates corresponding event signatures and emissions across EvidenceModule, ModeratedEvidenceModule, and IEvidence. Updates tests to reflect new function signatures and reordered/augmented event fields and indexing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant EM as EvidenceModule
participant E as Event Log
U->>EM: submitEvidence(_arbitrable, _externalDisputeID, _evidence)
EM-->>E: emit Evidence(_arbitrable, _externalDisputeID, msg.sender, _evidence)
sequenceDiagram
autonumber
participant U as User
participant MEM as ModeratedEvidenceModule
participant ARB as IArbitratorV2
participant E as Event Log
U->>MEM: submitEvidence(_arbitrable, _evidenceGroupID, _evidence)
MEM->>ARB: (internal checks/refs)
MEM-->>E: emit ModeratedEvidence(ARBitrator, _arbitrable, _evidenceGroupID, msg.sender, _evidence)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
![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 (4)
contracts/src/arbitration/evidence/EvidenceModule.sol (2)
61-67: Fix Natspec wording and optionally guard against zero arbitrable
- Natspec for _externalDisputeID mentions “evidence group ID”. Replace with “external dispute ID” for consistency.
- Optional: reject address(0) to avoid meaningless logs.
Apply:
- /// @param _externalDisputeID Unique identifier for this dispute outside Kleros. It's the submitter responsibility to submit the right evidence group ID. + /// @param _externalDisputeID Unique identifier for this dispute outside Kleros. It's the submitter's responsibility to submit the correct external dispute ID. function submitEvidence(address _arbitrable, uint256 _externalDisputeID, string calldata _evidence) external { - emit Evidence(_arbitrable, _externalDisputeID, msg.sender, _evidence); + require(_arbitrable != address(0), "Invalid arbitrable"); + emit Evidence(_arbitrable, _externalDisputeID, msg.sender, _evidence); }
5-5: Remove unused importIArbitratorV2 is not used in this module.
-import "../interfaces/IArbitratorV2.sol";contracts/src/arbitration/evidence/ModeratedEvidenceModule.sol (1)
73-85: Align naming: use _evidenceGroupID in event (docs + identifier)Function param is _evidenceGroupID but the event uses _externalDisputeID. Rename the event identifier and docs for clarity (types unchanged; ABI unaffected).
- /// @param _externalDisputeID Unique identifier for this dispute outside Kleros. It's the submitter responsibility to submit the right evidence group ID. + /// @param _evidenceGroupID Unique identifier of the evidence group this evidence belongs to. event ModeratedEvidence( IArbitratorV2 indexed _arbitrator, address indexed _arbitrable, - uint256 indexed _externalDisputeID, + uint256 indexed _evidenceGroupID, address _party, string _evidence );contracts/test/evidence/index.ts (1)
151-167: Add standalone test for EvidenceModule’s Evidence event
AllsubmitEvidencecall sites have been updated; add a test that deploysEvidenceModuleand asserts itsEvidence(arbitrable, externalDisputeID, party, evidence)event to guard against regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/src/arbitration/evidence/EvidenceModule.sol(1 hunks)contracts/src/arbitration/evidence/ModeratedEvidenceModule.sol(3 hunks)contracts/src/arbitration/interfaces/IEvidence.sol(1 hunks)contracts/test/evidence/index.ts(5 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-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
- GitHub Check: Pages changed - kleros-v2-testnet
🔇 Additional comments (2)
contracts/src/arbitration/evidence/ModeratedEvidenceModule.sol (1)
195-199: Submit + emit changes look correctSignature extended with _arbitrable and emit order matches event.
Also applies to: 220-221
contracts/src/arbitration/interfaces/IEvidence.sol (1)
8-18: No naming inconsistencies detectedOnly
_externalDisputeIDappears in the codebase; there are no_evidenceGroupIDreferences, so no identifier renames or doc updates are needed.Likely an incorrect or invalid review comment.
|
Not relevant anymore following #2168 |
PR-Codex overview
This PR focuses on enhancing the
submitEvidencefunctionality in the Kleros arbitration system by adding an_arbitrableaddress parameter to related functions and events. This improves clarity and ensures that evidence submissions are correctly associated with their respective arbitrable contracts.Detailed summary
_arbitrableparameter tosubmitEvidenceinEvidenceModule.solandModeratedEvidenceModule.sol.Evidenceevent inIEvidence.solto include_arbitrable.ModeratedEvidenceevent inModeratedEvidenceModule.solto include_arbitrable.index.tsto reflect the new_arbitrableparameter in evidence submission.Summary by CodeRabbit