No evidence submission by the contract #86
Conversation
✅ Deploy Preview for curate-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughEvidenceModule references were removed; evidence is now passed in event string fields (RequestSubmitted, RequestChallenged). Contracts, factory/deploy, subgraph, and frontend updated to emit, index, and display requester/challenger evidence strings instead of using an evidence module. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User (wallet)
participant C as CurateV2 contract
participant A as Arbitrator
participant S as Subgraph/Indexer
rect rgb(240,248,255)
note over C: New flow — evidence carried in events (string)
U->>C: submitItem(..., evidence:string)
C-->>U: emit RequestSubmitted(itemID, requestID, evidence)
S->>S: index RequestSubmitted → store requesterEvidence
U->>C: challengeRequest(..., evidence:string)
C-->>U: emit RequestChallenged(itemID, requestID, evidence)
S->>S: index RequestChallenged → store challengerEvidence
C->>A: createDispute(..., arbitratorExtraData)
A-->>C: ruling(...)
end
sequenceDiagram
autonumber
participant U as User
participant C as CurateV2
participant EM as EvidenceModule
participant A as Arbitrator
rect rgb(253,246,227)
note over C,EM: Prior flow (removed)
U->>C: submit/challenge(..., evidence)
C->>EM: submitEvidence(itemID, requestID, evidence)
EM-->>C: ack
C->>A: createDispute(...)
A-->>C: ruling(...)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus review on:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/components/ActionButton/Modal/EvidenceUpload.tsx (1)
23-30: Missing dependency in useEffect causes stale evidence state.The
useEffectusesfileTypeExtensionin line 28 but doesn't include it in the dependency array on line 30. This means changes tofileTypeExtensionwon't trigger the effect, potentially leaving theevidencestate with stale data.🔎 Proposed fix
useEffect(() => { setEvidence({ name: title, description, fileURI, fileTypeExtension, }); - }, [title, description, fileURI]); + }, [title, description, fileURI, fileTypeExtension]);subgraph/src/Curate.ts (1)
267-310: Fix incorrect getDisputeID() method call on line 305.The
try_requestsDisputeDatafunction returns a tuple with outputs accessible as properties, not methods. Based on the contract ABI, the first output is a uint256 nameddisputeID. Line 305 must be changed from:request.disputeID = disputeInfo.value.getDisputeID();to:
request.disputeID = disputeInfo.value.disputeID;The return type from the generated bindings provides named property access to tuple elements, not getter methods.
🧹 Nitpick comments (10)
web/src/context/SubmitListContext.tsx (1)
140-145: Add missing dependency to useEffect.The
useEffectcleanup logic is missingresetListDatain its dependency array. WhileresetListDatais defined in the component and callssetListDataandsetListMetadata(which are stable references fromuseLocalStorage), it's better to either:
- Include
resetListDatain the dependency array, or- Memoize
resetListDatawithuseCallbackto ensure it's stable🔎 Proposed fix using useCallback
+import React, { createContext, useState, useContext, useMemo, useEffect, useCallback } from "react"; const [progress, setProgress] = useState<ListProgress>(ListProgress.Start); const location = useLocation(); - const resetListData = () => { + const resetListData = useCallback(() => { setListData(initialListData); setListMetadata(initialListMetadata); - }; + }, [setListData, setListMetadata]); useEffect(() => { // Cleanup function to clear local storage when user leaves the route if (location.pathname.includes("/submit-list") || location.pathname.includes("/attachment")) return; resetListData(); - }, [location.pathname]); + }, [location.pathname, resetListData]);contracts/.env.example (1)
5-5: Consider adding a blank line at the end of the file.It's a common convention to end files with a newline character for better POSIX compatibility and cleaner diffs.
contracts/scripts/dotenv.sh (1)
12-15: Consider using safer variable access in the Node.js script.The direct use of
process.env.$varKeyat line 14 relies on string interpolation that could fail with special characters. Consider using bracket notation for more robust access:🔎 Proposed fix
node -e " require('dotenv').config({ path: '"$SCRIPT_DIR"/../.env' }) - console.log(process.env.$varKey) + console.log(process.env['$varKey']) "Note: The quotes in line 13 flagged by Shellcheck work correctly in this context, but the variable access could be made more robust.
web/src/utils/getFileExtension.ts (1)
5-5: Consider the "jpeg" vs "jpg" extension distinction.The
FileExtensiontype includes both "jpg" and "jpeg", butgetExtensionFromMagicBytesonly returns "jpg" for JPEG files. This is acceptable since they're equivalent, but consider whether you want to normalize all JPEG extensions to "jpg" or preserve the user's original choice from the filename.contracts/deploy/00-curate-v2.ts (1)
28-28: Document the extraData change.The
extraDatahex string has been expanded. Consider adding a comment explaining what the additional data represents, especially since this appears related to the evidence module removal changes.web/src/utils/parseEvidence.ts (2)
3-11: Add explicit return type annotation.The function should explicitly declare its return type for better type safety and documentation.
🔎 Proposed fix
+import { Evidence, evidenceSchema } from "src/types/Evidence"; -import { evidenceSchema } from "src/types/Evidence"; -export const parseEvidence = (stringifiedEvidence: string) => { +export const parseEvidence = (stringifiedEvidence: string): Evidence | undefined => { try { const parsedJson = JSON.parse(stringifiedEvidence); return evidenceSchema.parse(parsedJson); } catch (error) { console.log(`Unable to parse evidence: ${stringifiedEvidence}`); return; } };
8-8: Use console.error and consider sanitizing logged evidence.Two suggestions:
- Use
console.errorinstead ofconsole.logfor error cases- Consider whether logging the full
stringifiedEvidenceis appropriate, as it could contain sensitive user data or be very large. Consider logging just an excerpt or hash instead.🔎 Proposed improvements
} catch (error) { - console.log(`Unable to parse evidence: ${stringifiedEvidence}`); + const preview = stringifiedEvidence.length > 100 + ? stringifiedEvidence.slice(0, 100) + '...' + : stringifiedEvidence; + console.error(`Unable to parse evidence: ${preview}`, error); return; }templates/index.ts (1)
17-27: extraEvidences array structure is correct.The Mustache template logic correctly handles conditional rendering of evidence items and the separating comma. The comma on line 22 only appears when both
requesterEvidenceandchallengerEvidenceexist, ensuring valid JSON array syntax.💡 Optional: Add clarifying comment
Consider adding a comment to explain the conditional comma logic:
"extraEvidences": [ {{#requesterEvidence}} {{{requesterEvidence}}} {{/requesterEvidence}} + {{! Render comma separator only if both evidences exist }} {{#requesterEvidence}}{{#challengerEvidence}},{{/challengerEvidence}}{{/requesterEvidence}} {{#challengerEvidence}} {{{challengerEvidence}}} {{/challengerEvidence}} ],contracts/src/CurateV2.sol (2)
386-473: LGTM: Evidence correctly emitted in removal and challenge flows.Both
removeItemandchallengeRequestcorrectly accept and emit evidence through their respective events. The use ofcalldatafor the evidence string parameter is gas-efficient.Note on gas implications: Large evidence strings will increase transaction costs. Consider documenting recommended evidence size limits or best practices for evidence submission (e.g., storing large files off-chain and including only the hash/URI in the evidence string).
351-384: Consider accepting evidence parameter in addItem for consistency.The function correctly emits
RequestSubmittedwith an empty string for evidence. However,removeItemaccepts an_evidenceparameter (line 389) whileaddItemdoes not (line 351), creating an asymmetry where users can provide justification for removal requests but not for registration requests. Consider whether the interface should allow evidence for both operation types.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (35)
contracts/.env.examplecontracts/.gitignorecontracts/README.mdcontracts/README.md.templatecontracts/deploy/00-curate-v2.tscontracts/deployments/arbitrumSepoliaDevnet/CurateFactory.jsoncontracts/deployments/arbitrumSepoliaDevnet/CurateV2.jsoncontracts/deployments/arbitrumSepoliaDevnet/CurateView.jsoncontracts/hardhat.config.tscontracts/package.jsoncontracts/scripts/dotenv.shcontracts/scripts/verifyProxies.shcontracts/src/CurateFactory.solcontracts/src/CurateV2.solsubgraph/package.jsonsubgraph/schema.graphqlsubgraph/src/Curate.tssubgraph/src/entities/Request.tssubgraph/subgraph.yamltemplates/index.tsweb/src/components/ActionButton/Modal/ChallengeItemModal.tsxweb/src/components/ActionButton/Modal/EvidenceUpload.tsxweb/src/components/HistoryDisplay/Party/JustificationDetails.tsxweb/src/components/HistoryDisplay/Party/JustificationModal.tsxweb/src/components/HistoryDisplay/Party/index.tsxweb/src/components/HistoryDisplay/index.tsxweb/src/consts/arbitration.tsweb/src/context/SubmitListContext.tsxweb/src/hooks/queries/useRequestsQuery.tsweb/src/pages/SubmitList/NavigationButtons/SubmitListButton.tsxweb/src/types/Evidence.tsweb/src/utils/getFileExtension.tsweb/src/utils/parseEvidence.tsweb/src/utils/submitListUtils.tsweb/wagmi.config.ts
💤 Files with no reviewable changes (3)
- web/src/utils/submitListUtils.ts
- contracts/README.md.template
- web/src/pages/SubmitList/NavigationButtons/SubmitListButton.tsx
✅ Files skipped from review due to trivial changes (1)
- subgraph/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-04T13:42:03.737Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/components/ActionButton/Modal/EvidenceUpload.tsx:74-82
Timestamp: 2024-11-04T13:42:03.737Z
Learning: In `web/src/components/ActionButton/Modal/EvidenceUpload.tsx`, the `uploadFile` function already manages error handling and logging, so additional error handling after calling it is unnecessary.
Applied to files:
web/src/components/HistoryDisplay/Party/JustificationModal.tsxweb/src/components/ActionButton/Modal/EvidenceUpload.tsxweb/src/components/ActionButton/Modal/ChallengeItemModal.tsx
🧬 Code graph analysis (8)
web/src/components/HistoryDisplay/Party/index.tsx (1)
web/src/types/Evidence.ts (1)
Evidence(11-11)
web/src/utils/parseEvidence.ts (1)
web/src/types/Evidence.ts (1)
evidenceSchema(4-9)
web/src/components/HistoryDisplay/index.tsx (1)
web/src/utils/parseEvidence.ts (1)
parseEvidence(3-11)
web/src/components/HistoryDisplay/Party/JustificationDetails.tsx (1)
web/src/types/Evidence.ts (1)
Evidence(11-11)
subgraph/src/Curate.ts (2)
subgraph/src/utils.ts (2)
getExtendedStatus(40-50)getStatus(32-35)subgraph/src/entities/User.ts (1)
ensureUser(3-11)
web/src/components/HistoryDisplay/Party/JustificationModal.tsx (1)
web/src/types/Evidence.ts (1)
Evidence(11-11)
web/src/components/ActionButton/Modal/EvidenceUpload.tsx (2)
web/src/utils/wrapWithToast.ts (3)
infoToast(15-15)successToast(16-16)errorToast(17-17)web/src/utils/getFileExtension.ts (1)
getFileExtension(120-122)
contracts/deploy/00-curate-v2.ts (1)
templates/index.ts (3)
registrationTemplate(31-49)dataMappings(71-114)removalTemplate(51-69)
🪛 dotenv-linter (4.0.0)
contracts/.env.example
[warning] 2-2: [UnorderedKey] The MAINNET_PRIVATE_KEY key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The INFURA_API_KEY key should go before the MAINNET_PRIVATE_KEY key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The ETHERSCAN_API_KEY key should go before the INFURA_API_KEY key
(UnorderedKey)
[warning] 5-5: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 5-5: [UnorderedKey] The GNOSISSCAN_API_KEY key should go before the INFURA_API_KEY key
(UnorderedKey)
🪛 Shellcheck (0.11.0)
contracts/scripts/dotenv.sh
[warning] 13-13: The surrounding quotes actually unquote this. Remove or escape them.
(SC2027)
contracts/scripts/verifyProxies.sh
[warning] 13-13: Iterating over ls output is fragile. Use globs.
(SC2045)
⏰ 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). (3)
- GitHub Check: Redirect rules - curate-v2
- GitHub Check: Header rules - curate-v2
- GitHub Check: Pages changed - curate-v2
🔇 Additional comments (40)
web/src/context/SubmitListContext.tsx (3)
7-21: Breaking change to IList interface is correctly implemented.The removal of the
evidenceModuleproperty from theIListinterface aligns with the PR's goal to eliminate the evidence module from arbitration parameters. TypeScript's type system will catch any consumers still expecting this property.
72-81: LGTM!The removal of
evidenceModulefrominitialListDatais consistent with theIListinterface changes. Existing local storage data with the old field will be safely ignored, and the route-based cleanup effect (lines 140-145) helps clear stale data.
4-4: The removal of theEVIDENCE_MODULEimport is safe and complete.The verification confirms there are no remaining references to
evidenceModuleorEVIDENCE_MODULEin the web codebase, and the constant is not defined inconsts/arbitration.ts. This import removal aligns with the PR objective of removing the evidenceModule property from the IList interface.contracts/README.md (1)
19-21: LGTM!The deployment address updates align with the new contract deployments.
contracts/.gitignore (1)
187-188: LGTM!The negation patterns correctly ensure
.env.exampleis tracked while other.env*files remain ignored.contracts/hardhat.config.ts (1)
70-71: Remove concern — Etherscan API v2 supports Arbitrum Sepolia with unified API key.Etherscan API v2 officially supports Arbitrum Sepolia Testnet, and API subscriptions are migrated to Etherscan API V2, which provides access to 50+ EVM chains with a single API key. The
ETHERSCAN_API_KEYis the correct configuration variable for Arbitrum Sepolia verification. The switch from Arbiscan to the unified Etherscan v2 API endpoint is the intended approach and requires no additional verification.web/src/consts/arbitration.ts (1)
1-7: Removal of EVIDENCE_MODULE is complete with no orphaned references.The extraction of
EVIDENCE_MODULEfrom this file has been successfully completed. A codebase search confirms no remaining references exist in TypeScript/TSX files.web/wagmi.config.ts (1)
93-99: Verify no code uses the removed EvidenceModule hooks.No references to EvidenceModule, useEvidenceModule, or evidenceModule were found in the web/src/ codebase. The removal is clean and complete.
web/src/hooks/queries/useRequestsQuery.ts (1)
34-35: Subgraph schema and UI components properly support the new evidence fields.The addition of
requesterEvidenceandchallengerEvidenceis verified:
- The subgraph schema (./subgraph/schema.graphql) includes both fields in the Request entity
- The RequestDetails fragment at lines 34-35 correctly requests these fields
- UI components (HistoryDisplay) use parseEvidence() utility to process and display evidence through the JustificationModal
contracts/scripts/verifyProxies.sh (3)
24-28: LGTM: API key validation added.Good defensive practice to validate the API key before proceeding with verification calls.
5-5: The v2 API configuration is correct.The script properly uses
https://api.etherscan.io/v2/apiwith thechainidquery parameter (lowercase). The v2 API endpoint fully supports proxy contract verification across multiple chains.
30-30: ChainId values are correct.42161 is the correct chainId for Arbitrum One (mainnet), and 421614 is the correct chainId for Arbitrum Sepolia.
web/src/utils/getFileExtension.ts (1)
65-104: LGTM: Magic bytes detection is correctly implemented.The magic byte signatures for PDF, PNG, JPEG, WEBP, and GIF are accurate and properly checked.
contracts/deploy/00-curate-v2.ts (3)
96-97: Confirm using the same dataMappings for both registration and removal is intentional.Both
registrationTemplateParametersandremovalTemplateParametersreceive the samedataMappingsconstant. While this may be correct if the data mapping logic is identical for both operations, please verify this is the intended behavior and that the dataMappings properly handle both request types.
88-88: Good documentation added.The comment clarifying this is the "main curate" deployment improves code readability.
65-79: Initialize signature matches the contract interface.The
initializecall correctly passesregistrationTemplateParametersandremovalTemplateParametersasstring[2]tuples containing the template and dataMappings. This aligns with theTemplateRegistryParamsstruct definition in CurateV2.sol and the updated contract interface.web/src/types/Evidence.ts (1)
1-11: LGTM: Well-structured Evidence type with runtime validation.The Zod schema provides both type safety and runtime validation for evidence data. The structure aligns with the ERC-1497 standard referenced in the comment, and the optional fields (fileURI, fileTypeExtension) appropriately reflect that evidence can be text-only or file-based.
subgraph/schema.graphql (2)
173-178: LGTM: Evidence fields properly added to Request entity.The
requesterEvidenceandchallengerEvidencefields are correctly typed as optional strings with clear documentation. This aligns with the PR's goal of embedding evidence directly in events.
160-205: Address schema breaking changes correctly in subgraph migration plan.The schema has legitimate breaking changes, but the claim about removing
externalDisputeIDis incorrect—this field never existed in the schema. Actual breaking changes include:
requester: Bytes!→requester: User!(type change)challenger: Bytes!→challenger: User(type change)- Removed derived fields:
roundsandevidenceGroupVerify the subgraph deployment plan accounts for these field type conversions and removed fields, and confirm all existing queries are updated accordingly.
Likely an incorrect or invalid review comment.
subgraph/src/entities/Request.ts (2)
34-34: The RequestSubmitted event signature includes the _evidence parameter.The event is defined as
event RequestSubmitted(bytes32 indexed _itemID, uint256 _requestID, string _evidence)in CurateV2.sol, confirming the code change is aligned with the contract's event signature. The field assignment on line 34 correctly stores the evidence string from the event.
8-45: The review comment references a non-existent field. The actual schema definesdisputeID(notexternalDisputeID), which is properly tracked: it remains unset when the request is created increateRequestFromEvent(correct, since disputes don't exist yet) and is correctly populated later inhandleRequestChallengedviarequest.disputeID = disputeInfo.value.getDisputeID(). No issue present.Likely an incorrect or invalid review comment.
contracts/package.json (1)
89-89: Verify that @kleros/kleros-v2-contracts v2.0.0-rc.1 breaking changes are properly handled in the codebase.v2.0.0-rc.1 is a release candidate published a month ago. Breaking changes were introduced ahead of internal reviews, and while Court V2 contracts are approaching audit completion, this RC version is not yet stable for production. Kleros 2.0 maintains backward compatibility, with migration support available from the team, but verify that all contract interactions align with the v2 API.
subgraph/subgraph.yaml (3)
14-16: Deployment configuration updated.The factory address and start block have been updated to reflect a new deployment on Arbitrum Sepolia testnet.
52-52: RequestSubmitted event signature extended with evidence parameter.The event signature now includes a
stringparameter for evidence, aligning with the contract changes to embed evidence directly in events rather than using a separate evidence module.
62-63: Event renamed and signature updated.The event has been renamed from
DisputeRequesttoRequestChallengedand now includes astring _evidenceparameter, consistent with the new evidence handling approach.web/src/components/HistoryDisplay/Party/index.tsx (1)
7-13: Evidence type properly integrated.The component now accepts an optional
justificationprop of typeEvidence, enabling structured evidence display in the justification modal.web/src/components/ActionButton/Modal/ChallengeItemModal.tsx (1)
9-9: Import refactored to use centralized Evidence type.The Evidence type is now imported from a dedicated types module, improving code organization and maintainability.
Also applies to: 20-20
web/src/components/ActionButton/Modal/EvidenceUpload.tsx (1)
32-53: Improved async error handling and concurrent file operations.The refactor to async/await with
Promise.allimproves readability and enables concurrent upload and extension detection. Error handling now provides specific user feedback via toasts.web/src/components/HistoryDisplay/Party/JustificationDetails.tsx (1)
6-8: Type updated to use centralized Evidence definition.The component now uses the
Evidencetype from the centralized types module, replacing the previousJustificationalias. This improves consistency across the codebase.web/src/components/HistoryDisplay/index.tsx (1)
70-75: Evidence parsing properly integrated into history display.The
parseEvidenceutility is used consistently to transform evidence strings into structuredEvidenceobjects for the justification display. Error handling is implicit through the optional return type, which safely results in undefined justification when parsing fails.[approve_code_challenges]
Also applies to: 96-102, 109-114
templates/index.ts (1)
71-100: DataMappings correctly updated for new evidence fields.The GraphQL query, seek paths, and populate targets have been properly extended to include
requesterEvidenceandchallengerEvidence, aligning with the subgraph schema changes. The variable renaming fromexternalDisputeIDtodisputeIDmatches the updated data model.contracts/src/CurateFactory.sol (2)
11-11: LGTM: Import statement correctly updated.The import has been appropriately simplified to remove the EvidenceModule reference, aligning with the PR's objective to remove evidence module dependencies.
62-91: Deploy function correctly updated to remove evidence module.The function signature and initialization call have been properly updated to align with the removal of the evidence module from CurateV2. All external callers of the
deployfunction, including the deployment script atcontracts/deploy/00-curate-v2.ts, have been updated to match the new signature. No remaining references toevidenceModuleexist in the codebase.web/src/components/HistoryDisplay/Party/JustificationModal.tsx (3)
1-13: LGTM: Imports correctly updated for simplified component.The import changes properly reflect the refactoring from a data-fetching component to a prop-driven one. The Evidence type is correctly imported from the centralized type definition.
15-22: LGTM: Component interface correctly updated.The addition of the optional
justificationprop to the interface and component signature is appropriate. The optional nature of the prop elegantly handles cases where no justification is provided.
23-47: LGTM: Simplified rendering logic improves component clarity.The refactored rendering logic makes the component more focused and maintainable by moving data-fetching responsibility upstream. The conditional rendering based on the
justificationprop is clean and handles both scenarios appropriately.contracts/src/CurateV2.sol (4)
68-71: LGTM: ArbitrationParams struct correctly simplified.The removal of the
evidenceModulefield from the ArbitrationParams struct aligns with the PR's architectural change to emit evidence directly in events rather than through a separate module.
171-214: LGTM: Initialize function correctly updated.The
initializefunction has been properly updated to remove the_evidenceModuleparameter and construct ArbitrationParams with only the arbitrator and arbitratorExtraData fields. This is consistent with the struct changes and overall architectural direction.
308-315: LGTM: Arbitration params change function updated correctly.The
changeArbitrationParamsfunction has been appropriately updated to remove the evidence module parameter and construct ArbitrationParams with the simplified structure.
133-140: LGTM: Event signatures correctly updated with evidence parameters.The
RequestSubmittedandRequestChallengedevents properly emit evidence, and the subgraph handlers correctly capture and store evidence data.createRequestFromEventstoresrequesterEvidencewhilehandleRequestChallengedstoreschallengerEvidence, both using the_evidenceparameter from the events. The schema includes the necessary fields to persist evidence for both request types.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/utils/getFileExtension.ts (1)
59-98: Consider adding error handling for file read failures.
file.slice(0, 12).arrayBuffer()can throw if the file is unreadable (e.g., permissions changed, file deleted). While letting errors propagate is acceptable, catching them and returning null would make this function more resilient.🔎 Proposed enhancement to handle read errors
export async function getExtensionFromMagicBytes(file: File): Promise<string | null> { + try { const buffer = await file.slice(0, 12).arrayBuffer(); const bytes = new Uint8Array(buffer); // PDF: %PDF if (bytes[0] === 0x25 && bytes[1] === 0x50 && bytes[2] === 0x44 && bytes[3] === 0x46) { return "pdf"; } // PNG if (bytes[0] === 0x89 && bytes[1] === 0x50 && bytes[2] === 0x4e && bytes[3] === 0x47) { return "png"; } // JPEG if (bytes[0] === 0xff && bytes[1] === 0xd8) { return "jpg"; } // WEBP (RIFF....WEBP) if ( bytes[0] === 0x52 && // R bytes[1] === 0x49 && // I bytes[2] === 0x46 && // F bytes[3] === 0x46 && // F bytes[8] === 0x57 && // W bytes[9] === 0x45 && // E bytes[10] === 0x42 && // B bytes[11] === 0x50 // P ) { return "webp"; } // GIF if (bytes[0] === 0x47 && bytes[1] === 0x49 && bytes[2] === 0x46) { return "gif"; } return null; + } catch { + return null; + } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/utils/getFileExtension.ts
⏰ 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). (3)
- GitHub Check: Redirect rules - curate-v2
- GitHub Check: Header rules - curate-v2
- GitHub Check: Pages changed - curate-v2
🔇 Additional comments (3)
web/src/utils/getFileExtension.ts (3)
5-31: LGTM! Clean edge-case handling.The MIME mapping is concise and the filename parser correctly handles dotfiles, missing extensions, and normalizes to lowercase.
42-45: LGTM!Straightforward MIME type lookup with proper null handling.
100-116: No changes needed; the implementation is correct.The priority order (filename → MIME → magic bytes) is intentional, not a security flaw. The numbered list describes a fallback strategy, not contradictory claims. Magic bytes are listed last as the most trustworthy fallback, checked only when faster methods return null.
More importantly,
fileTypeExtensionis purely metadata stored with the evidence—it's not used for file validation or security decisions. The actual upload validation occurs server-side viauploadFile(file, Roles.Evidence), which handles MIME type/content validation independently. Renamingmalware.exetoevidence.pdfdoes not bypass this validation.The current implementation correctly optimizes for performance while maintaining security through server-side validation.
Likely an incorrect or invalid review comment.
PR-Codex overview
This PR focuses on enhancing the evidence handling and dispute management in the Kleros Curate system. It updates the evidence structure, modifies event emissions, and adjusts the project configuration for improved functionality and integration.
Detailed summary
.gitignoreto include.env.example..env.example.versioninsubgraph/package.jsonfrom0.2.5to0.3.5.evidenceModulefrom various components and contracts.requesterEvidenceandchallengerEvidencein GraphQL fragments and schemas.RequestSubmittedandRequestChallengedto include evidence.EvidenceUpload.tsx.parseEvidence.ts.Summary by CodeRabbit
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.