feat(kleros-sdk): extra-evidences-schema #2210
Conversation
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ 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? →
|
WalkthroughAdds transaction-hash and structured-evidence schemas to the SDK, exposes TxHashSchema tests, and updates the web UI to accept and display optional evidence fields and arbitrable-provided extra evidences using populated dispute data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![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 (3)
kleros-sdk/test/disputeDetailsSchema.test.ts (2)
67-80: Test suite implementation looks good.The test logic correctly validates both success and failure cases with appropriate error message checking.
Minor naming observation: The test suite is named "txHashSchema" (camelCase) while the imported schema is "TxHashSchema" (PascalCase). Consider using "TxHashSchema" for consistency, though this is a minor style point.
Optional: Align test suite naming with import
- describe("txHashSchema", () => { + describe("TxHashSchema", () => {
131-133: Consider adding tests for the extended DisputeDetailsSchema.With the addition of the
extraEvidencesfield toDisputeDetailsSchema, this would be a good opportunity to implement tests for the full schema including the new evidence validation.Would you like me to help generate test cases for the
disputeDetailsSchemaincluding validation of the newextraEvidencesfield with various evidence objects?kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (1)
26-28: Transaction hash validation is correct.The schema properly validates Ethereum transaction hashes (32 bytes = 64 hex characters + "0x" prefix = 66 total length).
The implementation reuses
isHexIdand adds a length check, which is functionally correct and maintains consistency with existing patterns in the codebase.Optional: Performance micro-optimization
If you prefer a single-pass validation, you could create a dedicated regex:
export const isTxHash = (str: string): boolean => /^0x[a-fA-F0-9]{64}$/.test(str); export const TxHashSchema = z.string().refine(isTxHash, { message: "Provided transaction hash is invalid.", });However, the current approach is perfectly acceptable and maintains consistency with the existing
isHexIdutility.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.tskleros-sdk/test/disputeDetailsSchema.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
kleros-sdk/test/disputeDetailsSchema.test.ts (1)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (1)
TxHashSchema(26-28)
⏰ 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
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- 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
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
🔇 Additional comments (4)
kleros-sdk/test/disputeDetailsSchema.test.ts (2)
6-6: LGTM!The import is correctly added and matches the export from the implementation file.
33-48: Comprehensive test data!The test data effectively covers the validation requirements with proper edge cases including length variations, prefix issues, and invalid hex characters.
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (2)
60-70: Well-structured evidence schema following ERC-1497.The implementation correctly follows the ERC-1497 Evidence Standard for core fields while adding appropriate court UI-specific extensions. The use of existing schemas (
TxHashSchema,ethAddressOrEnsNameSchema) for validation ensures consistency across the codebase.
91-91: Good use of default value for array field.The
extraEvidencesfield is properly defined as an array ofEvidenceSchemawith a sensible default of an empty array. This defensive approach prevents undefined/null issues in consuming code.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (2)
98-98: Consider handling error and loading states.While the hook internally handles undefined parameters safely (as per learnings), consider destructuring and handling
errorandisLoadingstates fromusePopulatedDisputeDatato provide better user feedback when fetching arbitrable evidences fails or is in progress.🔎 Suggested enhancement
- const { data: disputeData } = usePopulatedDisputeData(id, arbitrable); + const { data: disputeData, error, isLoading } = usePopulatedDisputeData(id, arbitrable);Then add appropriate UI feedback based on these states.
134-140: Use unique identifier for key instead of array index.Using
indexas the key can cause issues if the array order changes. If the evidence items have a unique identifier (e.g.,id,transactionHash, or a combination), use that instead.🔎 Suggested improvement
- {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }, index) => ( + {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }) => ( <EvidenceCard - key={index} + key={transactionHash || `${sender}-${timestamp}`} evidence="" {...{ sender, timestamp, transactionHash, name, description, fileURI }} />Adjust the key based on which fields are guaranteed to be unique and present.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/EvidenceCard.tsxweb/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-10-14T15:31:27.821Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79
Timestamp: 2024-10-14T15:31:27.821Z
Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsxweb/src/components/EvidenceCard.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-09T13:39:15.086Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0
Timestamp: 2025-05-09T13:39:15.086Z
Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-09-09T13:33:46.896Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 2117
File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66
Timestamp: 2025-09-09T13:33:46.896Z
Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:23:39.325Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:98-103
Timestamp: 2024-10-09T10:23:39.325Z
Learning: In `SelectArbitrable.tsx` of the web-devtools project (React/TypeScript), direct DOM manipulation using `child.click()` is acceptable when considered reasonably safe.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧬 Code graph analysis (2)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
web/src/hooks/useSpamEvidence.ts (1)
useSpamEvidence(33-43)web/src/hooks/queries/usePopulatedDisputeData.ts (1)
usePopulatedDisputeData(30-84)web/src/components/Divider.tsx (1)
Divider(3-10)
web/src/components/EvidenceCard.tsx (2)
web/src/utils/index.ts (1)
isUndefined(5-6)web/src/utils/date.ts (1)
formatDate(23-37)
⏰ 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: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
🔇 Additional comments (6)
web/src/components/EvidenceCard.tsx (3)
188-192: LGTM! Interface changes support arbitrable evidences.Making
sender,index, andtransactionHashoptional appropriately supports the new arbitrable evidence flow where these fields may not be available.
215-215: LGTM! Rendering guards appropriately handle optional fields.The conditional rendering logic correctly uses
isUndefinedchecks to prevent displaying incomplete UI elements when optional props are missing.Also applies to: 230-239
207-209: EnsuregetTxnExplorerLinkreturnsundefinedfor empty hash input to properly guard against invalid URLs.The function returns
${getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url}/tx/${hash}, which always produces a truthy string—even whenhashis empty. This means the guard at line 235 (isUndefined(transactionExplorerLink)) won't prevent rendering an incomplete URL likehttps://arbitrum.io/tx/. Either modifygetTxnExplorerLinkto returnundefinedfor empty strings, or add an explicit check before calling it.web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
88-91: LGTM! Interface appropriately extends Evidence component.The
IEvidenceinterface with an optionalarbitrableaddress prop correctly enables the component to display arbitrable-provided evidences while maintaining backward compatibility.
119-119: LGTM! Safe extraction of arbitrable evidences.The optional chaining appropriately handles cases where
disputeDatais undefined orextraEvidencesis not present.
134-134: Field alignment is correct. All destructured fields (name,description,fileURI,sender,timestamp,transactionHash) are defined in EvidenceSchema and match their expected types. The optionalfileTypeExtensionfield is not used in this component, which is intentional.
PR-Codex overview
This PR introduces a new
TxHashSchemafor validating transaction hashes, adds anEvidenceSchemafor handling evidence data, and updates theEvidenceCardcomponent to accommodate optional fields. It also includes tests for the transaction hash validation.Detailed summary
TxHashSchemato validate transaction hashes.EvidenceSchemato define evidence structure.DisputeDetailsSchemato includeextraEvidences.IEvidenceCardinterface to make fields optional.EvidenceCardcomponent to handle optional fields and improve rendering.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.