fix(web): invalid-dispute-vote-reveal #2134
Conversation
WalkthroughAdds a fallback in getSaltAndChoice to use a default “Refuse To Arbitrate” candidate when Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Reveal UI
participant Func as getSaltAndChoice
participant Answers as answers[]
UI->>Func: call getSaltAndChoice(commit, answers, ...)
alt answers is empty
Func->>Func: candidates = [{ id: "0x0", title: "Refuse To Arbitrate", description: "Refuse To Arbitrate" }]
note right of Func #DDEBF7: fallback candidate inserted
else answers not empty
Func->>Func: candidates = answers
end
Func->>Func: reduce over candidates -> compute salt & compare innerCommit
Func-->>UI: return { salt, choice }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
✅ 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-university 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-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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 (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)
72-78: Guard against undefined result and reset isSending on early return.getSaltAndChoice can return undefined; destructuring will throw and isSending stays true.
@@ - const { salt, choice } = isUndefined(storedSaltAndChoice) - ? await getSaltAndChoice(signingAccount, generateSigningAccount, saltKey, disputeDetails?.answers ?? [], commit) - : JSON.parse(storedSaltAndChoice); - if (isUndefined(choice)) return; + const result = isUndefined(storedSaltAndChoice) + ? await getSaltAndChoice( + signingAccount, + generateSigningAccount, + saltKey, + disputeDetails?.answers ?? [], + commit + ) + : JSON.parse(storedSaltAndChoice); + if (!result || isUndefined(result.choice) || (typeof result.choice === "bigint" && result.choice < 0n)) { + setIsSending(false); + return; + } + const { salt, choice } = result;
🧹 Nitpick comments (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)
150-166: Tiny perf/readability nit: precompute salt as BigInt once.@@ - const salt = keccak256(rawSalt); + const salt = keccak256(rawSalt); + const saltBig = BigInt(salt); @@ - const innerCommit = keccak256(encodePacked(["uint256", "uint256"], [BigInt(answer.id), BigInt(salt)])); + const innerCommit = keccak256(encodePacked(["uint256", "uint256"], [BigInt(answer.id), saltBig]));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx(1 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-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (2)
152-156: Intent LGTM: sensible fallback for invalid disputes.
152-155: Verify Answer.id format with SDK definitions
Inspect the local kleros-sdk workspace’s package.json for its version and review the SDK’s Answer.id declaration (e.g., in kleros-sdk/src/types/Answer.ts or the generated .d.ts files) to confirm whether IDs are decimal strings (“0”, “1”, …) or hex strings (“0x0”, “0x1”, …) and adjust the injected “0x0” accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (2)
158-167: Prevent sending an invalid choice (-1) when commit doesn’t match any candidate.Current code can return -1 and proceed to cast, causing an invalid uint256 or a failed tx. Return an undefined choice when not found so the caller short-circuits safely.
- const { choice } = candidates.reduce<{ found: boolean; choice: bigint }>( + const { found, choice } = candidates.reduce<{ found: boolean; choice?: bigint }>( (acc, answer) => { if (acc.found) return acc; const innerCommit = keccak256(encodePacked(["uint256", "uint256"], [BigInt(answer.id), BigInt(salt)])); if (innerCommit === commit) { - return { found: true, choice: BigInt(answer.id) }; + return { found: true, choice: BigInt(answer.id) }; } else return acc; }, - { found: false, choice: BigInt(-1) } + { found: false, choice: undefined } ); - return { salt, choice }; + if (!found) return { salt, choice: undefined }; + return { salt, choice };
72-91: Always clear isSending; move it to a finally block.If choice is undefined, handleReveal returns early and leaves isSending stuck true.
const handleReveal = useCallback(async () => { - setIsSending(true); - const { salt, choice } = isUndefined(storedSaltAndChoice) - ? await getSaltAndChoice(signingAccount, generateSigningAccount, saltKey, disputeDetails?.answers ?? [], commit) - : JSON.parse(storedSaltAndChoice); - if (isUndefined(choice)) return; + setIsSending(true); + try { + const { salt, choice } = isUndefined(storedSaltAndChoice) + ? await getSaltAndChoice(signingAccount, generateSigningAccount, saltKey, disputeDetails?.answers ?? [], commit) + : JSON.parse(storedSaltAndChoice); + if (isUndefined(choice)) return; const simulate = isGated ? simulateDisputeKitGatedCastVote : simulateDisputeKitClassicCastVote; const { request } = await catchShortMessage( simulate(wagmiConfig, { args: [parsedDisputeID, parsedVoteIDs, BigInt(choice), BigInt(salt), justification], }) ); if (request && walletClient && publicClient) { await wrapWithToast(async () => await walletClient.writeContract(request), publicClient).then(({ status }) => { setIsOpen(status); }); } - setIsSending(false); + } finally { + setIsSending(false); + } }, [
♻️ Duplicate comments (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)
152-157: Good fix: derive local candidates without mutating input.Using a local candidates list avoids side effects on React state/query caches and matches prior feedback.
🧹 Nitpick comments (3)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (3)
152-152: Fix the comment and acronym.The code no longer mutates answers, and the common acronym is “RTA”.
- // when dispute is invalid, just add RFA to the answers array + // Dispute invalid: derive a local candidates list starting with “Refuse To Arbitrate” (RTA); do not mutate `answers`.
154-155: Drop optional chaining; answers is non-optional here.Minor cleanup; improves type narrowing.
- answers?.length > 0 + answers.length > 0
156-156: Mark fallback as reserved (if supported) and keep the extra field at runtime.Helps downstream logic/UI distinguish the special option; safe even with the assertion.
- : [{ id: "0x0", title: "Refuse To Arbitrate", description: "Refuse To Arbitrate" } as Answer]; + : [{ id: "0x0", title: "Refuse To Arbitrate", description: "Refuse To Arbitrate", reserved: true } as Answer];If the SDK’s Answer type doesn’t include reserved?: boolean, keep the extra field (runtime only) or omit it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)
web/src/context/NewDisputeContext.tsx (1)
Answer(13-18)
⏰ 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-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-university
- 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
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
tractorss
dismissed stale reviews from kemuru and coderabbitai[bot]
via
6bea072
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/wagmi.config.ts (1)
62-65: Guard suffix stripping to avoid accidental empty-suffix slice.Defensive check ensures we only strip when the suffix is present and actually at the end of the name.
- if (type === ArbitratorTypes.university) { - nameWithoutSuffix = name.slice(0, -artifactSuffix.length); - // console.debug(`Using ${nameWithoutSuffix} instead of ${name}`); - } + if (type === ArbitratorTypes.university && artifactSuffix && name.endsWith(artifactSuffix)) { + nameWithoutSuffix = name.slice(0, -artifactSuffix.length); + // console.debug(`Using ${nameWithoutSuffix} instead of ${name}`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/wagmi.config.ts(2 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: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/wagmi.config.ts (1)
20-23: ArbitratorTypes.neo is present and deployments have no “Neo”-suffixed artifacts Verified thatArbitratorTypes.neoexists in the enum and no*Neo.jsonfiles are in the deployments directories, so the suffixless handling is correct.
fix(web): invalid-dispute-vote-reveal
3 participants
PR-Codex overview
This PR focuses on updating the handling of answers in the
Reveal.tsxfile to accommodate invalid disputes and modifying thereadArtifactsfunction inwagmi.config.tsto include a new condition for artifact suffixes based onArbitratorTypes.Detailed summary
Reveal.tsx, added logic to initializecandidateswith a default answer ifanswersis empty.reducefunction to usecandidatesinstead ofanswers.wagmi.config.ts, updated artifact suffix logic to includeArbitratorTypes.neo.ifstatement from checking forvanillatouniversity.Summary by CodeRabbit
Bug Fixes
Chores