Release #2012
Release #2012
Conversation
…res upgrading KlerosCore
chore(web): style and button changes
The isCurrentRound field in the Round entity was not being set to false for the previous round when a new round was created due to an appeal. This commit modifies the handleAppealDecision handler in KlerosCore.ts to: 1. Load the round that was current before the appeal. 2. Set its isCurrentRound field to false and save it. 3. Proceed to create the new round, which will have isCurrentRound set to true by the createRoundFromRoundInfo function. A new test suite, kleros-core.test.ts, was added with a specific test case to verify this behavior. The test ensures that after an appeal: - The previous round's isCurrentRound is false. - The new round's isCurrentRound is true. - The dispute's currentRound and currentRoundIndex are correctly updated. - Court statistics are updated.
The isCurrentRound field in the Round entity was not being set to false for the previous round when a new round was created due to an appeal. This commit modifies the handleAppealDecision handler in KlerosCore.ts to: 1. Load the round that was current before the appeal. 2. Set its isCurrentRound field to false and save it. 3. Proceed to create the new round, which will have isCurrentRound set to true by the createRoundFromRoundInfo function. Newly added test files (kleros-core.test.ts and kleros-core-utils.ts) were removed from this commit due to feedback regarding outdated testing tooling. The core logic fix remains.
…-time chore: timeline clarify remaining time
fix(subgraph): handle-batched-disputes-request-events
…in-file-viewer fix: bug in color loading text in file viewer
Fix: Update isCurrentRound for previous round on appeal
## Walkthrough
This update introduces new external view functions to the `KlerosCoreBase` contract for enhanced dispute and court data querying. The `draw` function was modified to return the number of jurors drawn. The keeper bot script was improved to avoid unnecessary draw calls. The subgraph was enhanced for better round state management and precise event log matching. In the frontend, it refactors case and juror listing buttons, updates related UI components for improved layout and clarity, and adjusts text strings for better user experience. Contract versions and initialization functions were also updated.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|
| contracts/src/arbitration/KlerosCoreBase.sol | Modified `draw` to return number of jurors drawn; added four external view functions: `getRoundInfo`, `getPnkAtStakePerJuror`, `getNumberOfRounds`, `isSupported`. |
| contracts/scripts/keeperBot.ts | Added pre-check simulation in `drawJurors` to avoid unnecessary draw calls when no jurors are drawn; introduced `MAX_DRAW_CALLS_WITHOUT_JURORS` constant. |
| contracts/src/arbitration/KlerosCore.sol, contracts/src/arbitration/KlerosCoreNeo.sol | Updated contract version strings from 0.9.3/0.8.0 to 0.9.4; renamed `initialize4()` to `initialize5()` with updated reinitializer versions. |
| subgraph/core/src/KlerosCore.ts | Improved round state management in `handleAppealDecision` to mark previous round as not current. |
| subgraph/core/src/entities/Dispute.ts | Enhanced event log matching to ensure correct event log is identified by dispute ID. |
| subgraph/package.json | Bumped package version from 0.15.2 to 0.15.4. |
| web/README.md | Removed a trailing period in the "Pre-Requisites" section. |
| web/src/components/AllCasesButton.tsx | Removed the `AllCasesButton` React component. |
| web/src/components/FileViewer/index.tsx | Added CSS rule to style loading elements with secondary text color. |
| web/src/components/LatestCases.tsx | Replaced `AllCasesButton` with `SeeAllCasesButton`, removed `courtName` prop, updated layout with flex container. |
| web/src/components/SeeAllCasesButton.tsx | Added new `SeeAllCasesButton` React component for case navigation. |
| web/src/components/SeeAllJurorsButton.tsx | Added new `SeeAllJurorsButton` React component for juror navigation. |
| web/src/pages/Cases/CaseDetails/Timeline.tsx | Updated countdown text wording in `AppealBanner` for clarity. |
| web/src/pages/Courts/CourtDetails/index.tsx | Simplified `LatestCases` usage by removing `courtName` prop. |
| web/src/pages/Home/TopJurors/index.tsx | Replaced `JurorsLeaderboardButton` with `SeeAllJurorsButton`, updated layout and removed redundant button. |
| web/src/components/DisputePreview/Policies.tsx | Added `disputeId` query parameter from route params to attachment links. |
| web/src/components/EvidenceCard.tsx | Added `disputeId` query parameter from route params to attachment link. |
| web/src/pages/AttachmentDisplay/Header.tsx | Changed back navigation to context-sensitive routing based on `title` query param. |
| contracts/scripts/find-initializer-versions.sh | Added new script to find initializer versions from proxy deployment logs across multiple networks. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant WebApp
participant KlerosCoreBase (Contract)
User->>WebApp: Clicks "See all" cases/jurors button
WebApp->>KlerosCoreBase (Contract): Calls new view function (e.g., getRoundInfo)
KlerosCoreBase (Contract)-->>WebApp: Returns requested data
WebApp-->>User: Displays updated case/juror listPossibly related PRs
Suggested labels
Suggested reviewers
Poem
|
✅ 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? →
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/src/components/SeeAllJurorsButton.tsx (1)
6-14: LGTM! Consider making the component more reusable.The implementation is clean and follows React best practices. The component correctly uses existing styled components for consistency.
Consider making the component more reusable by accepting optional props:
-const SeeAllJurorsButton: React.FC = () => { +interface ISeeAllJurorsButton { + text?: string; + to?: string; +} + +const SeeAllJurorsButton: React.FC<ISeeAllJurorsButton> = ({ + text = "See all", + to = "/jurors/1/desc/all" +}) => { return ( - <InternalLink to={"/jurors/1/desc/all"}> + <InternalLink to={to}> <BlueIconTextButtonContainer> - <label>See all</label> + <label>{text}</label> </BlueIconTextButtonContainer> </InternalLink> ); };web/src/components/SeeAllCasesButton.tsx (1)
19-19: Minor: Inconsistent label wrapping compared to SeeAllJurorsButton.The label text is placed directly inside
BlueIconTextButtonContainerwithout a<label>tag, whileSeeAllJurorsButtonuses<label>See all</label>.For consistency, consider wrapping the text in a label tag:
- <BlueIconTextButtonContainer>{labelText}</BlueIconTextButtonContainer> + <BlueIconTextButtonContainer> + <label>{labelText}</label> + </BlueIconTextButtonContainer>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
contracts/src/arbitration/KlerosCoreBase.sol(1 hunks)subgraph/core/src/KlerosCore.ts(1 hunks)subgraph/core/src/entities/Dispute.ts(1 hunks)subgraph/package.json(1 hunks)web/README.md(1 hunks)web/src/components/AllCasesButton.tsx(0 hunks)web/src/components/FileViewer/index.tsx(1 hunks)web/src/components/LatestCases.tsx(2 hunks)web/src/components/SeeAllCasesButton.tsx(1 hunks)web/src/components/SeeAllJurorsButton.tsx(1 hunks)web/src/pages/Cases/CaseDetails/Timeline.tsx(1 hunks)web/src/pages/Courts/CourtDetails/index.tsx(1 hunks)web/src/pages/Home/TopJurors/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/AllCasesButton.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
web/src/pages/Courts/CourtDetails/index.tsx (1)
web/src/utils/getDescriptiveCourtName.ts (1)
getDescriptiveCourtName(1-5)
web/src/pages/Cases/CaseDetails/Timeline.tsx (1)
web/src/utils/date.ts (1)
secondsToDayHourMinute(10-16)
web/src/components/SeeAllJurorsButton.tsx (2)
web/src/components/InternalLink.tsx (1)
InternalLink(4-8)web/src/components/BlueIconTextButtonContainer.tsx (1)
BlueIconTextButtonContainer(4-32)
web/src/components/SeeAllCasesButton.tsx (2)
web/src/components/InternalLink.tsx (1)
InternalLink(4-8)web/src/components/BlueIconTextButtonContainer.tsx (1)
BlueIconTextButtonContainer(4-32)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: contracts-testing
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: Dependabot
🔇 Additional comments (15)
web/src/components/FileViewer/index.tsx (1)
28-30: LGTM! Good styling improvement for loading states.The CSS rule appropriately targets loading elements and applies consistent theming. This enhances the user experience by providing visual feedback during document loading.
web/src/components/SeeAllCasesButton.tsx (1)
12-22:✅ Verification successful
LGTM! Well-structured component with proper filtering logic.
The implementation correctly handles optional court filtering and uses the
encodeURIFilterutility appropriately. The conditional filter creation and link construction follow good practices.Verify that the
encodeURIFilterutility correctly handles empty objects and that the path structure/cases/display/1/desc/${filter}matches the expected routing pattern:
🏁 Script executed:
#!/bin/bash # Description: Verify encodeURIFilter utility usage and routing patterns # Search for encodeURIFilter usage patterns rg "encodeURIFilter" -A 2 -B 2 # Search for similar routing patterns in cases rg "/cases/display/" -A 1 -B 1Length of output: 6441
SeeAllCasesButton:
encodeURIFilterand routing pattern verified
encodeURIFilter({})returns"all"(perweb/src/utils/uri.ts), matching other “all” routes.- The
/cases/display/1/desc/${encodedFilter}path aligns with existing usages inSubmitBatchDisputesButton.tsxand the header navigation.No further changes needed—LGTM.
web/src/pages/Home/TopJurors/index.tsx (4)
18-18: Good margin adjustment for improved spacing.The increased top margin provides better visual separation and improves the overall layout.
21-27: Excellent layout improvement with flex container.The new
TitleAndButtonContainercreates a clean horizontal layout that aligns the title and button properly. The gap spacing of 12px provides good visual separation.
29-32: Good margin cleanup for better layout control.Removing the bottom margin from the Title component allows the container to control spacing more consistently.
61-64: Excellent UI consistency improvement.The new layout with
TitleAndButtonContainerandSeeAllJurorsButtoncreates a consistent pattern that aligns with similar components throughout the application. This improves the overall user experience.subgraph/package.json (1)
3-3: LGTM! Version bump aligns with release process.The version increment from "0.15.2" to "0.15.4" appears to correspond with the enhancements to the subgraph's core functionality and contract interface extensions mentioned in the AI summary.
web/README.md (1)
26-26: LGTM! Minor formatting improvement.Removing the trailing period improves consistency in the documentation formatting.
web/src/pages/Courts/CourtDetails/index.tsx (1)
152-152: LGTM! Cleaner prop interface for LatestCases.The refactoring simplifies the component usage by explicitly passing
titleandfiltersprops instead of spreadingcourtName. This makes the interface more explicit and maintainable, while preserving the same functionality.web/src/pages/Cases/CaseDetails/Timeline.tsx (1)
86-89: LGTM! Text improvements enhance clarity.The updated countdown message text is more concise and user-friendly. The changes from "left until...can be funded" to "remaining to fund..." provide clearer, more direct messaging without affecting the underlying logic.
subgraph/core/src/KlerosCore.ts (1)
218-230: Excellent state management improvement for round transitions.The addition of explicit round state management properly handles the transition from current round to previous round when an appeal occurs. This ensures:
- The previous round is correctly marked as
isCurrentRound = false- The dispute's
currentRoundproperty is updated to point to the new round- Proper state consistency is maintained throughout the appeal process
This change enhances data integrity and prevents potential issues with stale round states.
web/src/components/LatestCases.tsx (3)
14-14: Component import updated correctly.The change from
AllCasesButtontoSeeAllCasesButtonaligns with the component refactoring mentioned in the summary.
20-31: Improved layout with flex container design.The new
TitleAndButtonContainerwith flex layout provides better organization by placing the title and button side-by-side with appropriate spacing. The removal ofmargin-bottomfrom the title works well with the new flex layout structure.
47-57: Clean interface simplification and layout implementation.The removal of the
courtNameprop simplifies the component interface, and the new layout structure withTitleAndButtonContainercreates a more organized presentation of the title and action button.subgraph/core/src/entities/Dispute.ts (1)
60-89: Excellent enhancement to event log validation.The updated logic significantly improves event identification accuracy by:
- Proper dispute ID validation: The addition of
coreDisputeIdcomparison ensures that only events related to the specific dispute are processed- Robust event filtering: Separate loops for
DisputeRequestSignatureandCrossChainDisputeIncomingSignatureevents with appropriate topic index validation- Enhanced data integrity: The
decodedId.equals(coreDisputeId)check prevents processing of unrelated events that might have the same signatureThe implementation correctly handles the topic indexing (topics[2] for DisputeRequest, topics[3] for CrossChainDisputeIncoming) and includes proper null checks for decoded values. This change significantly reduces the risk of incorrect event processing and improves the subgraph's data accuracy.
| function isSupported(uint96 _courtID, uint256 _disputeKitID) external view returns (bool) { | ||
| return courts[_courtID].supportedDisputeKits[_disputeKitID]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bounds checking for court ID.
The function should validate the _courtID parameter before accessing the courts array.
Apply this fix:
function isSupported(uint96 _courtID, uint256 _disputeKitID) external view returns (bool) {
+ if (_courtID >= courts.length) revert InvalidCourtID();
return courts[_courtID].supportedDisputeKits[_disputeKitID];
}Add the corresponding custom error declaration:
error InvalidCourtID();🤖 Prompt for AI Agents
In contracts/src/arbitration/KlerosCoreBase.sol around lines 1012 to 1014, the
function isSupported accesses the courts mapping with _courtID without
validating if the court ID is valid. To fix this, add a check at the start of
the function to verify that _courtID is within the valid range of courts. If it
is not, revert with the custom error InvalidCourtID(). Also, declare the custom
error InvalidCourtID() in the contract as specified.
| /// @dev Gets the round info for a specified dispute and round. | ||
| /// @dev This function must not be called from a non-view function because it returns a dynamic array which might be very large, theoretically exceeding the block gas limit. | ||
| /// @param _disputeID The ID of the dispute. | ||
| /// @param _round The round to get the info for. | ||
| /// @return round The round info. | ||
| function getRoundInfo(uint256 _disputeID, uint256 _round) external view returns (Round memory) { | ||
| return disputes[_disputeID].rounds[_round]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bounds checking for array access.
The function directly accesses disputes[_disputeID].rounds[_round] without validating that the indices are within bounds. This could cause reverts with unhelpful error messages when called with invalid parameters.
Add input validation to provide better error handling:
function getRoundInfo(uint256 _disputeID, uint256 _round) external view returns (Round memory) {
+ if (_disputeID >= disputes.length) revert InvalidDisputeID();
+ if (_round >= disputes[_disputeID].rounds.length) revert InvalidRoundNumber();
return disputes[_disputeID].rounds[_round];
}You'll need to add the corresponding custom error declarations:
error InvalidDisputeID();
error InvalidRoundNumber();🤖 Prompt for AI Agents
In contracts/src/arbitration/KlerosCoreBase.sol around lines 984 to 991, the
function getRoundInfo accesses disputes[_disputeID].rounds[_round] without
checking if _disputeID and _round are valid indices, which can cause unhelpful
reverts. Fix this by adding input validation: first check if _disputeID is less
than the length of the disputes array and revert with a custom error
InvalidDisputeID() if not; then check if _round is less than the length of
disputes[_disputeID].rounds and revert with InvalidRoundNumber() if invalid.
Declare these custom errors in the contract as specified.
| function getPnkAtStakePerJuror(uint256 _disputeID, uint256 _round) external view returns (uint256) { | ||
| return disputes[_disputeID].rounds[_round].pnkAtStakePerJuror; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bounds checking for array access.
Similar to getRoundInfo, this function lacks input validation for _disputeID and _round parameters.
Apply this fix:
function getPnkAtStakePerJuror(uint256 _disputeID, uint256 _round) external view returns (uint256) {
+ if (_disputeID >= disputes.length) revert InvalidDisputeID();
+ if (_round >= disputes[_disputeID].rounds.length) revert InvalidRoundNumber();
return disputes[_disputeID].rounds[_round].pnkAtStakePerJuror;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getPnkAtStakePerJuror(uint256 _disputeID, uint256 _round) external view returns (uint256) { | |
| return disputes[_disputeID].rounds[_round].pnkAtStakePerJuror; | |
| } | |
| function getPnkAtStakePerJuror(uint256 _disputeID, uint256 _round) external view returns (uint256) { | |
| if (_disputeID >= disputes.length) revert InvalidDisputeID(); | |
| if (_round >= disputes[_disputeID].rounds.length) revert InvalidRoundNumber(); | |
| return disputes[_disputeID].rounds[_round].pnkAtStakePerJuror; | |
| } |
🤖 Prompt for AI Agents
In contracts/src/arbitration/KlerosCoreBase.sol around lines 997 to 999, the
function getPnkAtStakePerJuror accesses the disputes and rounds arrays without
validating that _disputeID and _round are within valid bounds. Add checks to
ensure _disputeID is less than the length of disputes and _round is less than
the length of rounds for the specified dispute before accessing the arrays to
prevent out-of-bounds errors.
| function getNumberOfRounds(uint256 _disputeID) external view returns (uint256) { | ||
| return disputes[_disputeID].rounds.length; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bounds checking for dispute ID.
The function should validate the _disputeID parameter before accessing the disputes array.
Apply this fix:
function getNumberOfRounds(uint256 _disputeID) external view returns (uint256) {
+ if (_disputeID >= disputes.length) revert InvalidDisputeID();
return disputes[_disputeID].rounds.length;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function getNumberOfRounds(uint256 _disputeID) external view returns (uint256) { | |
| return disputes[_disputeID].rounds.length; | |
| } | |
| function getNumberOfRounds(uint256 _disputeID) external view returns (uint256) { | |
| if (_disputeID >= disputes.length) revert InvalidDisputeID(); | |
| return disputes[_disputeID].rounds.length; | |
| } |
🤖 Prompt for AI Agents
In contracts/src/arbitration/KlerosCoreBase.sol around lines 1004 to 1006, the
function getNumberOfRounds lacks validation for the _disputeID parameter before
accessing the disputes array. Add a check to ensure _disputeID is within valid
bounds (e.g., less than the length of the disputes array) and revert or handle
the error if it is out of range to prevent invalid access.
✅ 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-neo-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. |
fix: avoid unnecessary calls draw() when no juror is available
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/scripts/keeperBot.ts (1)
252-264: Review the simulation logic for potential edge cases.The simulation logic is a good optimization to avoid unnecessary transactions, but there are some considerations:
Simulation accuracy: The simulation uses
iterations * MAX_DRAW_CALLS_WITHOUT_JURORS(10x multiplier), which may be overly conservative. If the simulation with 10x iterations shows no jurors, it doesn't guarantee that the actual smaller iteration count wouldn't draw some jurors.Gas estimation concerns: The static call uses
HIGH_GAS_LIMIT, which is appropriate for simulation but ensure this doesn't cause issues if the actual draw needs different gas limits.Consider adding a comment explaining the simulation logic:
+ // Simulate drawing with higher iterations to check if any jurors would be available + // If simulation shows no jurors in 10x iterations, skip the actual draw to save gas const simulatedIterations = iterations * MAX_DRAW_CALLS_WITHOUT_JURORS;Also consider logging the actual vs simulated iteration counts for better debugging:
logger.debug( - `Draw: ${extraJurors} jurors available in the next ${simulatedIterations} iterations for dispute ${dispute.id}` + `Draw: ${extraJurors} jurors available in the next ${simulatedIterations} iterations (actual: ${iterations}) for dispute ${dispute.id}` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
contracts/deploy/upgrade-all.ts(1 hunks)contracts/scripts/keeperBot.ts(2 hunks)contracts/src/arbitration/KlerosCore.sol(2 hunks)contracts/src/arbitration/KlerosCoreBase.sol(3 hunks)contracts/src/arbitration/KlerosCoreNeo.sol(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- contracts/src/arbitration/KlerosCore.sol
- contracts/src/arbitration/KlerosCoreNeo.sol
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/src/arbitration/KlerosCoreBase.sol
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
- GitHub Check: Scorecards analysis
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
🔇 Additional comments (2)
contracts/scripts/keeperBot.ts (1)
10-10: LGTM! Good constant naming and reasonable value.The constant
MAX_DRAW_CALLS_WITHOUT_JURORSis well-named and the value of 10 provides a reasonable simulation multiplier for checking juror availability.contracts/deploy/upgrade-all.ts (1)
84-84:✅ Verification successful
Verify the initializer function exists in the contract implementation.
The change from
initialize4toinitialize5appears correct based on the contract updates mentioned in the summary. However, it's critical to ensure this matches the actual contract implementation.Please run the following script to verify that the
initialize5function exists in the core contract implementation:
🏁 Script executed:
#!/bin/bash # Description: Verify that the initialize5 function exists in the core contract implementations # Search for initialize5 function in KlerosCore contracts echo "Searching for initialize5 function in core contracts..." ast-grep --pattern 'function initialize5($$$) $$$' # Also search for the reinitializer(5) annotation to confirm version echo "Searching for reinitializer(5) annotation..." rg -A 2 -B 2 "reinitializer\(5\)" # Check if initialize4 still exists (should not in the new version) echo "Checking if initialize4 still exists (should be removed)..." ast-grep --pattern 'function initialize4($$$) $$$'Length of output: 66507
Initializer ‘initialize5’ confirmed and upgrade call is correct
– We’ve verified that both core implementations include:
• contracts/src/arbitration/KlerosCore.sol →function initialize5() external reinitializer(5) { … }
• contracts/src/arbitration/KlerosCoreNeo.sol →function initialize5() external reinitializer(5) { … }
– The legacyinitialize4remains in KlerosCore.sol for backward‐compatibility and should not be removed.
– The callawait upgrade(core, "initialize5", []);incontracts/deploy/upgrade-all.tsnow matches the implementation.All aligned—approving this change.
fix: allow return in case evidence was opened in new tab
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
web/src/components/EvidenceCard.tsx (1)
263-263: Consider URL encoding the dispute ID parameter.The dispute ID is directly interpolated into the URL string. If the ID contains special characters, this could break the URL structure.
- <StyledInternalLink to={`/attachment/?disputeId=${id}&title=${"Evidence File"}&url=${getIpfsUrl(fileURI)}`}> + <StyledInternalLink to={`/attachment/?disputeId=${encodeURIComponent(id || '')}&title=${encodeURIComponent("Evidence File")}&url=${encodeURIComponent(getIpfsUrl(fileURI))}`}>web/src/components/DisputePreview/Policies.tsx (1)
78-80: Apply consistent URL encoding for all parameters.For consistency with the suggested improvements in EvidenceCard, consider URL encoding all parameters in the attachment URLs.
- <StyledInternalLink - to={`/attachment/?disputeId=${id}&title=${"Case Policy"}&url=${getIpfsUrl(attachment.uri)}`} - > + <StyledInternalLink + to={`/attachment/?disputeId=${encodeURIComponent(id || '')}&title=${encodeURIComponent("Case Policy")}&url=${encodeURIComponent(getIpfsUrl(attachment.uri))}`} + > ... - <StyledInternalLink - to={`/attachment/?disputeId=${id}&title=${"Dispute Policy"}&url=${getIpfsUrl(disputePolicyURI)}`} - > + <StyledInternalLink + to={`/attachment/?disputeId=${encodeURIComponent(id || '')}&title=${encodeURIComponent("Dispute Policy")}&url=${encodeURIComponent(getIpfsUrl(disputePolicyURI))}`} + >Also applies to: 86-88
web/src/pages/AttachmentDisplay/Header.tsx (1)
73-83: Well-structured conditional navigation logic.The navigation logic comprehensively handles different attachment types and provides a sensible fallback. The string matching approach is simple and effective for the current use case.
Consider using an enum or constants for the attachment titles to avoid magic strings and improve maintainability:
const ATTACHMENT_TITLES = { EVIDENCE_FILE: "Evidence File", CASE_POLICY: "Case Policy", DISPUTE_POLICY: "Dispute Policy", POLICY_FILE: "Policy File" } as const; const handleReturn = () => { if (attachmentTitle === ATTACHMENT_TITLES.EVIDENCE_FILE) { navigate(`/cases/${disputeId}/evidence`); } else if (attachmentTitle === ATTACHMENT_TITLES.CASE_POLICY || attachmentTitle === ATTACHMENT_TITLES.DISPUTE_POLICY) { navigate(`/cases/${disputeId}/overview`); } else if (attachmentTitle === ATTACHMENT_TITLES.POLICY_FILE) { navigate(`/resolver/policy`); } else { navigate("/"); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/components/DisputePreview/Policies.tsx(2 hunks)web/src/components/EvidenceCard.tsx(3 hunks)web/src/pages/AttachmentDisplay/Header.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: SonarCloud
- GitHub Check: Scorecards analysis
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/src/pages/AttachmentDisplay/Header.tsx (1)
4-4: Excellent use of useSearchParams for query parameter handling.The switch from
useNavigate(-1)to conditional navigation based on query parameters is a good improvement that provides more predictable navigation behavior.Also applies to: 69-71
| @@ -3,6 +3,7 @@ import styled, { css } from "styled-components"; | |||
|
|
|||
| import Identicon from "react-identicons"; | |||
| import ReactMarkdown from "react-markdown"; | |||
| import { useParams } from "react-router-dom"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding null safety for the route parameter.
The useParams hook can return undefined for route parameters. Consider adding type safety and null checking to prevent potential runtime errors.
-import { useParams } from "react-router-dom";
+import { useParams } from "react-router-dom";
...
- const { id } = useParams();
+ const { id } = useParams<{ id: string }>();
+
+ if (!id) {
+ console.warn("Dispute ID not found in route parameters");
+ }Also applies to: 228-228
🤖 Prompt for AI Agents
In web/src/components/EvidenceCard.tsx at line 6 and also at line 228, the
useParams hook is used without null safety, which can cause runtime errors if
the route parameter is undefined. Add type annotations to define the expected
parameters and include null or undefined checks before using the parameter
values. This ensures safe access and prevents potential crashes due to missing
route parameters.
| @@ -1,6 +1,8 @@ | |||
| import React from "react"; | |||
| import styled, { css } from "styled-components"; | |||
|
|
|||
| import { useParams } from "react-router-dom"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent null safety pattern.
Similar to EvidenceCard, consider adding type safety and null checking for the route parameter to maintain consistency across components.
-import { useParams } from "react-router-dom";
+import { useParams } from "react-router-dom";
...
- const { id } = useParams();
+ const { id } = useParams<{ id: string }>();Also applies to: 72-72
🤖 Prompt for AI Agents
In web/src/components/DisputePreview/Policies.tsx at line 4 and also line 72,
the useParams hook usage lacks consistent null safety and type checking for the
route parameter. Update the code to explicitly type the parameters expected from
useParams and add null or undefined checks before using these parameters to
ensure type safety and prevent runtime errors, following the pattern used in
EvidenceCard for consistency.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
contracts/scripts/find-initializer-versions.sh (3)
10-12: Handle missing proxy files gracefully.If no
*_Proxy.jsonfiles exist, the glob will remain literal, causing errors. Consider enablingnullglobor skipping non-existent paths:+# Treat unmatched globs as empty +shopt -s nullglob for c in arbitrum arbitrumSepolia arbitrumSepoliaDevnet; do echo "$c" - for f in "$SCRIPT_DIR"/../deployments/"$c"/*_Proxy.json; do + for f in "$SCRIPT_DIR/../deployments/$c"/*_Proxy.json; do address=$(jq -r .address "$f")
16-16: Improve readability by naming the event signature and using--event.Embedding the raw signature in the call hampers maintainability. Extract it to a variable and switch to the
--eventflag.- results=$(cast logs --from-block "$block" --to-block latest 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2 --address "$address" --rpc-url "${rpcUrls[$c]}" --json | jq -r .[].data) + EVENT_SIG="0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2" + results=$( + cast logs \ + --event "$EVENT_SIG" \ + --from-block "$block" \ + --to-block latest \ + --address "$address" \ + --rpc-url "${rpcUrls[$c]}" \ + --json \ + | jq -r '.[].data' + )
17-19: Use a safe loop to handle whitespace and large datasets.Iterating over
$resultscan break on spaces or newlines. Switch to awhile readto correctly process each entry.- for result in $results; do - cast --to-dec "$result" - done + printf '%s\n' "$results" | while read -r hexData; do + cast --to-dec "$hexData" + done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/scripts/find-initializer-versions.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: contracts-testing
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: Scorecards analysis
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
|
PR-Codex overview
This PR focuses on updating components, enhancing functionality, and fixing issues in the Kleros project, including version upgrades, UI improvements, and contract adjustments.
Detailed summary
versioninsubgraph/package.jsonfrom"0.15.2"to"0.15.4".initialize4toinitialize5incontracts/deploy/upgrade-all.ts.web/src/pages/Courts/CourtDetails/index.tsx.SeeAllJurorsButtonandSeeAllCasesButtoncomponents.KlerosCoreandKlerosCoreNeocontracts' version to"0.9.4".contracts/scripts/keeperBot.ts.web/src/pages/AttachmentDisplay/Header.tsx.disputeIdhandling in various components for better routing.contracts/src/arbitration/KlerosCoreBase.solfor dispute management.Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores