Skip to content

Conversation

@tractorss
Copy link
Contributor

@tractorss tractorss commented Jan 6, 2026

PR-Codex overview

This PR focuses on enhancing the voting and commitment functionalities within a decentralized application, integrating new features for handling votes, commits, and reveals, while also improving local development setups and testing capabilities.

Detailed summary

  • Added mining configuration in hardhat.config.ts.
  • Introduced getVoteKey function in helpers/storage/key.ts.
  • Implemented VoteContext, CommitContext, and RevealContext interfaces.
  • Created hashVote and hashJustification utility functions.
  • Added new builders for voting, committing, and revealing actions.
  • Enhanced local deployment configurations and scripts.
  • Updated local environment variables for testing.
  • Introduced new commit data storage functions.
  • Improved error handling and validation in various hooks.
  • Added tests for key functionalities like hashVote and getVoteKey.
  • Refactored components to utilize new hooks and context structures.

The following files were skipped due to too many changes: web/src/context/Web3Provider.tsx, web/src/actions/reveal/builders/gated.builder.test.ts, web/src/utils/crypto/generateSalt.test.ts, web/src/actions/commit/builders/gated.builder.test.ts, web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx, web/src/actions/reveal/builders/shutter.builder.test.ts, web/src/actions/commit/builders/classic.builder.test.ts, web/src/actions/reveal/helpers/bruteForceChoice.test.ts, web/src/actions/helpers/storage/storage.test.ts, web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx, web/src/actions/reveal/builders/classic.builder.test.ts, web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx, web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx, web/src/actions/vote/builders/classic.builder.test.ts, web/src/actions/commit/builders/shutter.builder.test.ts, web/src/actions/reveal/resolveRevealInputs.test.ts, yarn.lock, contracts/deployments/hardhat.viem.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • Documentation

    • Expanded local deployment guide: populate devnet courts, regenerate bindings after contract/ABI changes, Docker Desktop note, build-before-start, redeploy steps, restart web server, updated deployment artifact target, and optional local Blockscout setup.
  • New Features

    • First-class local Hardhat support with faster local mining and local token for dev.
    • Unified cast-based commit/reveal/vote flows with wallet-backed hooks, local storage and crypto helpers.
    • Local Blockscout explorer workflow.
  • Chores

    • Env, scripts and config updates to streamline local development.
  • Tests

    • Extensive unit and integration tests for commit/reveal/vote flows and helpers.

✏️ Tip: You can customize this high-level summary in your review settings.

@netlify
Copy link

netlify bot commented Jan 6, 2026

Deploy Preview for kleros-v2-neo failed. Why did it fail? →

Name Link
🔨 Latest commit 4109286
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-neo/deploys/6970f128d49e9c0008f1d0bc

@netlify
Copy link

netlify bot commented Jan 6, 2026

Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →

Name Link
🔨 Latest commit 4109286
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-testnet-devtools/deploys/6970f1289fcc310008c643b5

@netlify
Copy link

netlify bot commented Jan 6, 2026

Deploy Preview for kleros-v2-testnet ready!

Name Link
🔨 Latest commit 4109286
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-testnet/deploys/6970f1289386b100084dc26c
😎 Deploy Preview https://deploy-preview-2212--kleros-v2-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

Walkthrough

Adds local Hardhat support and developer workflows, conditional local token deployment, async/merged wagmi artifact loading, Hardhat auto-mining, new typed commit/reveal/vote builder & execute frameworks with storage and crypto helpers, hook-driven UI flows replacing per-component signing, local Blockscout instructions, and many tests/tooling updates.

Changes

Cohort / File(s) Summary
Local detection & utils
contracts/deploy/utils/index.ts
Add isLocalhost(network) predicate (name "localhost" or chainId 31337).
Deployment scripts & tokens
contracts/deploy/00-home-chain-arbitration.ts, contracts/deploy/00-rng-chainlink.ts, contracts/deploy/utils/deployTokens.ts, contracts/package.json
Use isLocalhost in devnet checks; shorten RNG fallback on localhost; deploy PinakionV2Local for localhost; add populate:local and include Resolver tag in scripts.
Local ERC20
contracts/src/token/mock/PinakionV2Local.sol
New PinakionV2Local contract with mint/recover and allowance overrides for local use.
Hardhat network config
contracts/hardhat.config.ts
Enable hardhat auto-mining with 5000ms interval.
Contracts wagmi config
contracts/wagmi.config.hardhat.ts
Replace static config with async getConfig, read/merge artifacts across networks, include IHomeGateway ABI.
Web wagmi / artifact loading
web/wagmi.config.ts
Add hardhat chain/artifact awareness; include hardhat artifacts in deployment resolution and adjust artifact loader.
Web local integration & env
web/.env.local.public, web/src/consts/index.ts, web/src/consts/processEnvConsts.ts, web/src/consts/chains.ts, web/src/context/Web3Provider.tsx, web/src/utils/getGraphqlUrl.ts
Add isLocalDeployment and HARDHAT_NODE_RPC; route transports to hardhat when local; include hardhat in chain maps and subgraph lookups; update envs to point subgraphs to localhost.
Commit framework
web/src/actions/helpers/builder.ts, web/src/actions/commit/*, web/src/actions/commit/builders/*
New typed action builder abstraction, commit param/context types, multiple commit builders (classic, gated, shutter, gatedShutter), dispatcher and executeCommit.
Reveal framework
web/src/actions/reveal/*, web/src/actions/reveal/builders/*, web/src/actions/reveal/resolveRevealInputs.ts, web/src/actions/reveal/helpers/*
New reveal types/context/builders, buildRevealTxn/executeReveal, resolve logic to reconstruct reveal inputs (salt/choice) and brute-force helper.
Vote framework
web/src/actions/vote/*, web/src/actions/vote/builders/*
New vote types, builders, dispatcher, and executeVote.
Storage helpers & keys
web/src/actions/helpers/storage/*
LocalStorage helpers for commit data, key generator (getVoteKey), and storage types.
Crypto utilities
web/src/utils/crypto/{generateSalt,hashVote,hashJustification}.ts
New signing and hashing helpers for salts, votes, and justifications.
Hooks: cast/reveal/vote & queries
web/src/hooks/useCastCommit.tsx, web/src/hooks/useRevealVote.ts, web/src/hooks/useVote.ts, web/src/hooks/queries/useDrawQuery.ts
New hooks wiring centralized commit/reveal/vote flows, query key normalization and cache invalidation.
UI commit/reveal pages
web/src/pages/Cases/CaseDetails/Voting/*
Replace per-component signing/storage with centralized hooks/builders; remove refetch props; update commit typing and component props to hook-driven flows.
Graph node & subgraph scripts
services/graph-node/docker-compose.yml, subgraph/scripts/all.sh
Add GRAPH_ALLOW_NON_DETERMINISTIC_FULLTEXT_SEARCH env var; simplify postgres command; remove core-university from subgraph loop.
Blockscout & README
README.md
Add optional local Blockscout setup and update local deployment steps (populate:local, viem binding regen, web build/start order, Docker Desktop note).
Web tooling & tests
web/package.json, web/tsconfig.json, web/vitest.config.ts, web/src/test/*, many *.test.ts
Add actions alias, tsconfig mapping, vitest setup, localStorage test mock and many unit/integration tests for builders/helpers/crypto.
Utility types
web/src/utils/types.ts
Add DistributiveOmit and PartialBy utility types.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UI as "Commit Component"
  participant Hook as "useCastCommit"
  participant Builder as "buildCommitTxn"
  participant Exec as "executeCommit"
  participant Wallet as "walletClient"
  participant Chain as "Blockchain"

  User->>UI: click "Commit"
  UI->>Hook: call castCommit(params)
  Hook->>Builder: buildCommitTxn(params, context)
  Builder-->>Hook: txParameters
  Hook->>Exec: executeCommit(txParameters, context)
  Exec->>Wallet: walletClient.writeContract(tx)
  Wallet->>Chain: submit transaction
  Chain-->>Wallet: tx mined
  Wallet-->>Exec: tx result (txHash)
  Exec-->>Hook: return txHash/status
  Hook-->>UI: update UI, invalidate queries
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

Type: Feature🗿

Suggested reviewers

  • alcercu
  • kemuru

Poem

🐰 I hopped through localhost with bindings bright,
I minted PNK and routed RPCs by night,
I built commits, revealed with careful art,
Tests in my burrow now pass — that's a start,
Carrots and code, dev meadow feels right. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feat/testing implementation' is vague and generic, using non-descriptive terms that don't convey the main changes of this large, multi-faceted changeset. Use a more specific title that summarizes the primary change, e.g., 'Add commit/reveal action builders and testing infrastructure' or 'Implement voting actions framework with tests'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI Agents
In @contracts/deploy/utils/deployTokens.ts:
- Around line 24-25: Fix the typo in the inline comment inside deployTokens.ts:
change "ERC20contract" to "ERC20 contract" in the comment that explains swapping
the local ERC20 implementation to one with increaseAllowance (refer to the
comment containing "increaseAllowance" in deployTokens.ts).

In @contracts/src/token/mock/PinakionV2Local.sol:
- Around line 25-34: The recoverTokens function incorrectly wraps
SafeERC20.safeTransfer (which returns void) in a require and uses .send for ETH;
remove the require and call the safeTransfer library call directly (e.g.,
token.safeTransfer(owner(), balance) or SafeERC20.safeTransfer(token, owner(),
balance) depending on imports), and replace
payable(owner()).send(address(this).balance) with a call pattern that bubbles
errors (e.g., (bool success, ) = payable(owner()).call{value:
address(this).balance}(""); require(success, "ETH transfer failed");) so
failures revert correctly; keep references to recoverTokens, token,
safeTransfer, and owner() when making the edits.

In @README.md:
- Around line 223-239: Fix the malformed link fragment and heading level:
replace the fragment text "#####shell-1---local-rpc-with-contracts-deployed"
used in the [hardhat node] link with a proper fragment starting with a single
hash (e.g., "#shell-1---local-rpc-with-contracts-deployed"), and change the
"#### Step 2: Start the Docker compose stack" heading to "### Step 2: Start the
Docker compose stack" so the heading hierarchy flows from "##" to "###"; also
ensure the localhost URL is written as <http://localhost> for consistency.

In @web/src/consts/processEnvConsts.ts:
- Line 23: The file exports an unused legacy function isLocalDeployment that
reads process.env.REACT_APP_DEPLOYMENT and duplicates behavior already
implemented in index.ts; either delete web/src/consts/processEnvConsts.ts
entirely, or move/merge the isLocalDeployment export into
web/src/consts/index.ts replacing process.env.REACT_APP_DEPLOYMENT with
import.meta.env.REACT_APP_DEPLOYMENT to match Vite usage, and remove any
duplicate exports so only the index.ts implementation remains.
🧹 Nitpick comments (5)
contracts/src/token/mock/PinakionV2Local.sol (1)

36-51: Add a comment explaining why increaseAllowance and decreaseAllowance are manually implemented.

These functions were removed from OpenZeppelin ERC20 in v5 as non-standard helpers, but your contract reimplements them. Adding a comment clarifying the rationale (compatibility with existing code, backwards compatibility, etc.) will help future maintainers understand the intent.

contracts/deploy/utils/index.ts (1)

31-32: Consider clarifying the comment.

The comment mentions the network name is "hardhat" when deployed while starting the node, but the code checks for network.name === "localhost". While the chainId check correctly handles both cases (covering "hardhat" name via the chainId), the comment could be clearer.

🔎 Suggested comment improvement
-// when deployed while starting node, the network name is "hardhat", the common factor for determining local node is chainId
+// The network name can be "localhost" or "hardhat" (when deployed while starting the node).
+// ChainId 31337 is the common factor for reliably detecting local Hardhat nodes.
 export const isLocalhost = (network: Network) => network.name === "localhost" || network.config.chainId === 31337;
contracts/deploy/utils/deployTokens.ts (1)

26-27: Consider extracting the repeated condition.

The condition ticker === "PNK" && isLocalhost(hre.network) is repeated on consecutive lines. While this works correctly, extracting it to a variable would improve maintainability.

🔎 Suggested refactor
-  const contractName = ticker === "PNK" && isLocalhost(hre.network) ? "PinakionV2Local" : "TestERC20";
-  const args = ticker === "PNK" && isLocalhost(hre.network) ? [] : [ticker, ticker];
+  const isPNKLocalDeployment = ticker === "PNK" && isLocalhost(hre.network);
+  const contractName = isPNKLocalDeployment ? "PinakionV2Local" : "TestERC20";
+  const args = isPNKLocalDeployment ? [] : [ticker, ticker];
web/src/context/Web3Provider.tsx (1)

53-56: Potential undefined access in defaultTransport websocket fallback.

chain.rpcUrls.default?.webSocket?.[0] may be undefined for chains that don't provide a default WebSocket URL. While webSocket(undefined) is handled gracefully by viem (it becomes a no-op in the fallback), consider making the fallback more explicit or adding a comment explaining this behavior.

🔎 Optional: Make undefined handling explicit
 const defaultTransport = (chain: AppKitNetwork) =>
-  fallback([http(chain.rpcUrls.default?.http?.[0]), webSocket(chain.rpcUrls.default?.webSocket?.[0])]);
+  fallback([
+    http(chain.rpcUrls.default?.http?.[0]),
+    // WebSocket may not be available for all chains; fallback handles undefined gracefully
+    ...(chain.rpcUrls.default?.webSocket?.[0] ? [webSocket(chain.rpcUrls.default.webSocket[0])] : []),
+  ]);
contracts/wagmi.config.hardhat.ts (1)

2-2: Same import assertion syntax note as web/wagmi.config.ts.

Biome flags the assert { type: "json" } syntax. Consider updating to with { type: "json" } for consistency if your TypeScript/bundler versions support import attributes.

🔎 Proposed update to import attributes syntax
-import IHomeGateway from "./artifacts/src/gateway/interfaces/IHomeGateway.sol/IHomeGateway.json" assert { type: "json" };
+import IHomeGateway from "./artifacts/src/gateway/interfaces/IHomeGateway.sol/IHomeGateway.json" with { type: "json" };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 46da5a2 and 79dfe04.

📒 Files selected for processing (19)
  • README.md
  • contracts/deploy/00-home-chain-arbitration.ts
  • contracts/deploy/00-rng-chainlink.ts
  • contracts/deploy/utils/deployTokens.ts
  • contracts/deploy/utils/index.ts
  • contracts/deployments/hardhat.viem.ts
  • contracts/hardhat.config.ts
  • contracts/package.json
  • contracts/src/token/mock/PinakionV2Local.sol
  • contracts/wagmi.config.hardhat.ts
  • services/graph-node/docker-compose.yml
  • subgraph/scripts/all.sh
  • web/.env.local.public
  • web/src/consts/chains.ts
  • web/src/consts/index.ts
  • web/src/consts/processEnvConsts.ts
  • web/src/context/Web3Provider.tsx
  • web/src/utils/getGraphqlUrl.ts
  • web/wagmi.config.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-11-19T05:31:48.701Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1744
File: web/src/hooks/useGenesisBlock.ts:9-31
Timestamp: 2024-11-19T05:31:48.701Z
Learning: In `useGenesisBlock.ts`, within the `useEffect` hook, the conditions (`isKlerosUniversity`, `isKlerosNeo`, `isTestnetDeployment`) are mutually exclusive, so multiple imports won't execute simultaneously, and race conditions are not a concern.

Applied to files:

  • contracts/deploy/00-home-chain-arbitration.ts
  • contracts/deploy/utils/index.ts
  • web/src/consts/processEnvConsts.ts
  • web/src/consts/index.ts
  • contracts/deploy/00-rng-chainlink.ts
📚 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/utils/getGraphqlUrl.ts
📚 Learning: 2024-10-15T16:18:32.543Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1687
File: web/src/context/AtlasProvider.tsx:225-244
Timestamp: 2024-10-15T16:18:32.543Z
Learning: In `web/src/context/AtlasProvider.tsx`, the `atlasUri` variable comes from environment variables and does not change, so it does not need to be included in dependency arrays.

Applied to files:

  • web/.env.local.public
  • web/src/consts/index.ts
📚 Learning: 2024-10-22T09:38:20.093Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: kleros-sdk/src/dataMappings/utils/actionTypes.ts:1-1
Timestamp: 2024-10-22T09:38:20.093Z
Learning: In the TypeScript file `kleros-sdk/src/dataMappings/utils/actionTypes.ts`, the `Abi` type is parsed later in the action functions, so importing `Abi` from `viem` in this file is unnecessary.

Applied to files:

  • web/wagmi.config.ts
📚 Learning: 2025-09-03T22:48:32.972Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 0
File: :0-0
Timestamp: 2025-09-03T22:48:32.972Z
Learning: In the Kleros v2 codebase, the team prioritizes gas optimization over strict CEI pattern compliance when dealing with trusted contracts. For penalty execution logic, they prefer batching storage writes (`round.pnkPenalties`) rather than updating incrementally after each penalty calculation to save gas costs, as the risk is extremely low between trusted contracts.

Applied to files:

  • contracts/src/token/mock/PinakionV2Local.sol
🧬 Code graph analysis (6)
contracts/deploy/00-home-chain-arbitration.ts (1)
contracts/deploy/utils/index.ts (2)
  • isDevnet (30-30)
  • isLocalhost (32-32)
web/src/consts/processEnvConsts.ts (1)
web/src/consts/index.ts (1)
  • isLocalDeployment (33-33)
web/src/consts/index.ts (1)
web/src/consts/processEnvConsts.ts (1)
  • isLocalDeployment (23-23)
contracts/deploy/00-rng-chainlink.ts (1)
contracts/deploy/utils/index.ts (1)
  • isLocalhost (32-32)
web/src/context/Web3Provider.tsx (3)
contracts/deploy/utils/index.ts (1)
  • isLocalhost (32-32)
web/src/consts/index.ts (2)
  • isLocalDeployment (33-33)
  • HARDHAT_NODE_RPC (53-53)
web/src/consts/processEnvConsts.ts (1)
  • isLocalDeployment (23-23)
web/src/consts/chains.ts (2)
web/src/consts/processEnvConsts.ts (3)
  • DEFAULT_CHAIN (32-32)
  • isLocalDeployment (23-23)
  • isProductionDeployment (22-22)
web/src/consts/index.ts (2)
  • isLocalDeployment (33-33)
  • isProductionDeployment (31-31)
🪛 Biome (2.1.2)
web/wagmi.config.ts

[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none

An explicit or implicit semicolon is expected here...

...Which is required to end this statement

(parse)


[error] 12-12: Expected a semicolon or an implicit semicolon after a statement, but found none

An explicit or implicit semicolon is expected here...

...Which is required to end this statement

(parse)

contracts/wagmi.config.hardhat.ts

[error] 2-2: Expected a semicolon or an implicit semicolon after a statement, but found none

An explicit or implicit semicolon is expected here...

...Which is required to end this statement

(parse)


[error] 2-2: Expected a semicolon or an implicit semicolon after a statement, but found none

An explicit or implicit semicolon is expected here...

...Which is required to end this statement

(parse)

🪛 dotenv-linter (4.0.0)
web/.env.local.public

[warning] 3-3: [UnorderedKey] The REACT_APP_CORE_SUBGRAPH key should go before the REACT_APP_DEPLOYMENT key

(UnorderedKey)

🪛 markdownlint-cli2 (0.18.1)
README.md

113-113: Dollar signs used before commands without showing output

(MD014, commands-show-output)


225-225: Link fragments should be valid

(MD051, link-fragments)


227-227: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4

(MD001, heading-increment)


239-239: Bare URL used

(MD034, no-bare-urls)

⏰ 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). (16)
  • 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-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet-devtools
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet-devtools
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: Analyze (javascript)
  • GitHub Check: hardhat-tests
  • GitHub Check: SonarCloud
  • GitHub Check: Mend Security Check
🔇 Additional comments (27)
contracts/hardhat.config.ts (1)

60-63: LGTM!

The mining configuration with auto: true and a 5-second interval provides predictable block times for local development and testing. This is a standard pattern for simulating realistic block production while maintaining fast feedback loops.

services/graph-node/docker-compose.yml (2)

26-26: Acknowledge local-only configuration for non-deterministic fulltext search.

Enabling GRAPH_ALLOW_NON_DETERMINISTIC_FULLTEXT_SEARCH is appropriate for local development but should never be used in production as it can cause inconsistent query results across Graph Node instances. This is acceptable since this docker-compose is for local development only.


38-38: LGTM!

The postgres command reformatting is a non-functional change that improves readability.

contracts/package.json (2)

63-65: LGTM!

Adding the Resolver tag to both start-local and deploy-local scripts ensures consistent local deployment of resolver contracts alongside arbitration components.


70-70: LGTM!

The populate:local script provides a convenient way to seed local development with courts and policy registry data from v2_devnet, streamlining the local setup workflow.

web/.env.local.public (1)

2-5: LGTM!

The environment variable updates correctly configure the web app for local development, pointing to local Graph Node endpoints and setting the deployment mode to localhost. This aligns with the broader local development infrastructure changes in this PR.

subgraph/scripts/all.sh (1)

13-13: The removal of core-university from the batch loop is intentional and safe.

The core-university subgraph remains fully functional with dedicated npm scripts in package.json (lines 24-32), including update, codegen, build, test, deploy, and rebuild commands. The change to all.sh is a deliberate scope reduction for the batch automation script, allowing developers to run core and drt subgraphs together while invoking core-university commands individually when needed. This does not break any workflows.

contracts/deploy/00-rng-chainlink.ts (2)

3-3: LGTM!

The addition of isLocalhost to the import aligns with the local development support introduced across the deployment scripts.


79-79: LGTM!

The conditional timeout logic is well-designed: 10 seconds for local development (appropriate for the 5-second Hardhat mining interval) and 30 minutes for production networks.

web/src/utils/getGraphqlUrl.ts (2)

1-1: LGTM!

The addition of hardhat to the wagmi/chains import enables local development support.


13-14: LGTM!

The hardhat chain mapping follows the same pattern as other chains, using an environment variable with an appropriate fallback message.

contracts/deploy/00-home-chain-arbitration.ts (2)

6-6: LGTM!

The addition of isLocalhost to the imports enables localhost detection for devnet-equivalent behavior.


50-52: LGTM!

Treating localhost as devnet-equivalent for timing constants is appropriate. The shorter minStakingTime (180s vs 1800s) and maxFreezingTime (600s vs 1800s) enable faster testing cycles during local development.

contracts/deploy/utils/deployTokens.ts (2)

4-4: LGTM!

The addition of isLocalhost to the imports follows the existing pattern.


28-33: LGTM!

The conditional contract deployment correctly uses PinakionV2Local for PNK on localhost and TestERC20 otherwise, with appropriate constructor arguments for each case.

web/src/consts/chains.ts (2)

14-18: LGTM - Local deployment chain selection is correctly prioritized.

The ternary chain correctly checks isLocalDeployment() first, then falls back to the production/testnet logic. This ensures local development uses Hardhat while preserving existing behavior for other environments.


21-27: The code correctly handles the chain/transport mismatch for local deployments. While QUERY_CHAINS includes gnosis/gnosisChiado and mainnet regardless of deployment type, the createAppKit configuration uses allowUnsupportedChain: true (line 110 of Web3Provider.tsx) which gracefully allows chains without corresponding transports. Additionally, mainnet is included in the localhost transport for ENS resolution. Since gnosis/gnosisChiado are read-only query chains not needed during local development, no changes are required.

web/src/consts/index.ts (2)

52-53: LGTM - Standard Hardhat RPC endpoint.

The HARDHAT_NODE_RPC constant uses the default Hardhat network address. This is the standard endpoint for local Hardhat nodes.


33-33: The isLocalDeployment function in index.ts is the only active version—processEnvConsts.ts is not imported anywhere in the codebase.

The concern about duplication is partially valid, but processEnvConsts.ts (using process.env) appears to be unused or legacy code. Only the import.meta.env version in index.ts is actively imported and used by chains.ts and Web3Provider.tsx. Consider clarifying whether processEnvConsts.ts should be maintained or removed.

Likely an incorrect or invalid review comment.

web/src/context/Web3Provider.tsx (2)

67-71: LGTM - Local transport correctly includes mainnet for ENS resolution.

The local transport configuration properly includes both the Hardhat node and mainnet (via Alchemy) to support ENS resolution during local development.


98-108: LGTM - SDK and AppKit configuration consistently updated for local deployment.

Both configureSDK and createAppKit use the same isLocalhost check to select the appropriate chain and transport, maintaining consistency across the Web3 stack.

README.md (1)

110-123: LGTM - Clear instructions for local data population and artifact generation.

The new section clearly documents the required steps after local deployment, including the court population and wagmi/viem bindings regeneration.

web/wagmi.config.ts (3)

92-96: LGTM - Localhost deployment case correctly configured.

The new localhost case properly sets viemNetwork to "hardhat", hardhatNetwork to "localhost", and uses hardhatViem for the arbitrator contracts, maintaining consistency with the artifact directory structure.


20-20: Good design: Optional hardhatChainName parameter enables flexible artifact resolution.

The readArtifacts function now accepts an optional hardhatChainName to decouple the viem chain name from the Hardhat deployment directory name. This elegantly handles cases like localhost/hardhat mapping.

Also applies to: 51-51


12-16: The assert { type: "json" } syntax is the standard ES2022 import assertions syntax and is fully supported by TypeScript 5.6.3 with module set to ESNext. This same syntax is used consistently across 7 files in the project (web/wagmi.config.ts, web-devtools/wagmi.config.ts, web/scripts/gitInfo.js, and multiple wagmi config files). While TypeScript 5.5+ nominally prefers with { type: "json" } for forward compatibility, assert remains valid and functional. The suggestion to update only this one file creates an inconsistency with the rest of the codebase and is not a functional requirement.

contracts/wagmi.config.hardhat.ts (2)

5-27: LGTM - Async config with multi-network artifact discovery.

The async getConfig function properly:

  1. Reads artifacts from localhost, gnosisChiado, and sepolia networks
  2. Logs discovered artifacts for debugging visibility
  3. Merges artifacts across networks using the helper
  4. Includes IHomeGateway in the final output

The comment on line 10 about network renaming breaking other scripts is a useful note for future maintainers.


3-3: No action required. The wagmiHelpers module exists at contracts/scripts/wagmiHelpers.ts and properly exports all three required functions: getAbi (line 11), readArtifacts (line 15), and merge (line 45).

@jaybuidl jaybuidl added Type: Toolchain ⚒️ Build tools configuration, CI/CD Package: Web Court web frontend Package: Contracts Court smart contracts Package: Subgraph Court subgraph labels Jan 9, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In @web/src/actions/commit/helpers/index.ts:
- Around line 1-4: encodeShutterMessage currently uses choice.toString() but
relies on implicit conversion for salt; update the function
(encodeShutterMessage) to call salt.toString() as well when building the string
with SEPARATOR and justification to keep both bigint parameters handled
consistently.

In @web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx:
- Around line 37-44: The call to castCommit passes currentRoundIndex which can
be undefined (derived from disputeData?.dispute?.currentRoundIndex); guard it
before calling castCommit by returning early or showing an error/loading state
when currentRoundIndex is undefined, e.g., check if currentRoundIndex == null
and abort the commit flow (do not call castCommit) or provide a valid default,
then only call castCommit with the validated roundIndex and
setIsOpen(res.status) on success.

In @web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx:
- Around line 55-64: The call to castCommit uses currentRoundIndex which can be
undefined; before invoking castCommit in Commit.tsx check that currentRoundIndex
is a valid number (e.g., !== undefined/null) and bail out or show an error if
not set so you never pass undefined as roundIndex; update the code path that
currently calls castCommit({ ..., roundIndex: currentRoundIndex, ... }) to guard
on currentRoundIndex and only call castCommit when it is defined, referencing
the same symbols (castCommit, currentRoundIndex,
DisputeKits.GatedShutter/DisputeKits.Shutter, setIsOpen) so the UI handles the
missing round index safely.
🧹 Nitpick comments (9)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)

5-5: Unused import.

useLocalStorage is imported but no longer used after the refactor to direct localStorage.getItem().

-import { useLocalStorage } from "react-use";
web/src/utils/crypto/generateSalt.ts (1)

15-16: Unnecessary optional chaining after validation.

The optional chaining signingAccount?.signMessage on line 15 is redundant since signingAccount.signMessage is already validated to exist on line 11. This is a minor inconsistency.

♻️ Suggested cleanup
-  const signature = await signingAccount?.signMessage({ message });
+  const signature = await signingAccount.signMessage({ message });
web/src/hooks/useCastCommit.tsx (1)

67-67: Hardcoded query key may become out of sync.

The query key ["useDrawQuery"] is hardcoded. If the actual query definition uses a different key or includes additional parameters, this invalidation won't work as expected. Consider centralizing query keys to avoid mismatches.

web/src/actions/commit/builders/index.ts (1)

14-23: Consider improving type safety in the builder registry.

The any type in the Record and as never cast work correctly at runtime due to the discriminated union, but they bypass compile-time type checking. This is a common pattern for dispatch tables, but worth noting.

If stricter typing is desired later, an overloaded function signature could enforce the relationship between params.type and the expected param type.

web/src/actions/commit/builders/gated.builder.ts (1)

8-22: Consider validating chain support before address lookup.

The address lookup disputeKitGatedAddress[chain.id] will return undefined for unsupported chains, which would cause a transaction failure. While the UI likely restricts to supported chains, a defensive check could provide a clearer error message.

♻️ Optional defensive check
 export const gatedCommitBuilder: CommitBuilder<GatedCommitParams, typeof disputeKitGatedAbi> = {
   build: async (params, context) => {
     const { disputeId, voteIds, choice, salt } = params;
     const { chain, account } = context;

+    const address = disputeKitGatedAddress[chain.id];
+    if (!address) {
+      throw new Error(`DisputeKitGated not deployed on chain ${chain.id}`);
+    }
+
     const commit = hashVote(choice, salt);
     return {
       account,
-      address: disputeKitGatedAddress[chain.id],
+      address,
       abi: disputeKitGatedAbi,
       functionName: "castCommit",
       args: [disputeId, voteIds, commit],
       chain,
     };
   },
 };
web/src/actions/commit/builders/gatedShutter.builder.ts (1)

27-28: Redundant BigInt() conversion on salt.

According to BaseCommitParams in params.ts, salt is already typed as bigint. The BigInt(salt) calls are unnecessary and could be simplified.

Suggested simplification
-    const choiceCommit = hashVote(choice, BigInt(salt));
-    const justificationCommit = hashJustification(BigInt(salt), justification);
+    const choiceCommit = hashVote(choice, salt);
+    const justificationCommit = hashJustification(salt, justification);
web/src/actions/commit/builders/shutter.builder.ts (2)

13-38: Significant code duplication with gatedShutter.builder.ts.

This builder shares nearly identical logic with gatedShutter.builder.ts (env validation, message encoding, encryption, hashing). Consider extracting shared Shutter-specific logic into a helper function to reduce duplication.


27-28: Redundant BigInt() conversion on salt.

Same issue as gatedShutter.builder.ts - salt is already bigint per BaseCommitParams.

Suggested simplification
-    const choiceCommit = hashVote(choice, BigInt(salt));
-    const justificationCommit = hashJustification(BigInt(salt), justification);
+    const choiceCommit = hashVote(choice, salt);
+    const justificationCommit = hashJustification(salt, justification);
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (1)

37-39: Duplicate data fetching for dispute details.

The component receives dispute as a prop (line 25) but also calls useDisputeDetailsQuery(id) (line 37) just to get currentRoundIndex. Consider using dispute?.currentRoundIndex from the prop instead to avoid redundant network requests.

Suggested change
-  const { data: disputeData } = useDisputeDetailsQuery(id);
-
-  const currentRoundIndex = disputeData?.dispute?.currentRoundIndex;
+  const currentRoundIndex = dispute?.currentRoundIndex;
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79dfe04 and d1e6518.

📒 Files selected for processing (22)
  • web/package.json
  • web/src/actions/commit/builders/baseBuilder.ts
  • web/src/actions/commit/builders/classic.builder.ts
  • web/src/actions/commit/builders/gated.builder.ts
  • web/src/actions/commit/builders/gatedShutter.builder.ts
  • web/src/actions/commit/builders/index.ts
  • web/src/actions/commit/builders/shutter.builder.ts
  • web/src/actions/commit/context.ts
  • web/src/actions/commit/execute.ts
  • web/src/actions/commit/helpers/index.ts
  • web/src/actions/commit/params.ts
  • web/src/hooks/queries/useDrawQuery.ts
  • web/src/hooks/useCastCommit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
  • web/src/utils/crypto/generateSalt.ts
  • web/src/utils/crypto/hashJustification.ts
  • web/src/utils/crypto/hashVote.ts
  • web/src/utils/crypto/shutter.ts
  • web/tsconfig.json
🧰 Additional context used
🧠 Learnings (12)
📚 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/hooks/queries/useDrawQuery.ts
  • web/src/pages/Cases/CaseDetails/Voting/Classic/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/hooks/queries/useDrawQuery.ts
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
📚 Learning: 2025-09-30T17:18:12.895Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 2145
File: contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol:277-286
Timestamp: 2025-09-30T17:18:12.895Z
Learning: In DisputeKitClassicBase.sol's castCommit function, jurors are allowed to re-submit commits during the commit period. The implementation uses a commitCount variable to track only first-time commits (where commit == bytes32(0)) so that totalCommitted is not incremented when a juror updates their existing commit.

Applied to files:

  • web/src/actions/commit/execute.ts
  • web/src/actions/commit/builders/gated.builder.ts
  • web/src/hooks/useCastCommit.tsx
  • web/src/actions/commit/params.ts
  • web/src/actions/commit/context.ts
  • web/src/actions/commit/builders/classic.builder.ts
  • web/src/actions/commit/builders/shutter.builder.ts
  • web/src/actions/commit/builders/index.ts
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
📚 Learning: 2024-10-28T05:55:12.728Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1716
File: web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx:29-42
Timestamp: 2024-10-28T05:55:12.728Z
Learning: In the `CustomContextInputs` component located at `web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx`, the `DisputeRequestParams` array is used to exclude certain variables from the custom input since they are already provided in a preceding component. Therefore, converting it to a type is unnecessary.

Applied to files:

  • web/src/actions/commit/params.ts
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
📚 Learning: 2024-12-16T17:17:32.359Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1794
File: web/src/hooks/useStarredCases.tsx:13-18
Timestamp: 2024-12-16T17:17:32.359Z
Learning: In `useStarredCases.tsx`, when handling the `starredCases` Map from local storage, direct mutation is acceptable to prevent the overhead of copying, provided it doesn't adversely affect React's render cycle.

Applied to files:

  • web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.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/Voting/Classic/Reveal.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
📚 Learning: 2024-12-09T12:36:59.441Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1775
File: web/src/pages/Courts/CourtDetails/StakePanel/StakeWithdrawButton.tsx:0-0
Timestamp: 2024-12-09T12:36:59.441Z
Learning: In the `StakeWithdrawButton` component, the transaction flow logic is tightly linked to component updates, so extracting it into a custom hook does not provide significant benefits.

Applied to files:

  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
📚 Learning: 2024-11-19T05:31:48.701Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1744
File: web/src/hooks/useGenesisBlock.ts:9-31
Timestamp: 2024-11-19T05:31:48.701Z
Learning: In `useGenesisBlock.ts`, within the `useEffect` hook, the conditions (`isKlerosUniversity`, `isKlerosNeo`, `isTestnetDeployment`) are mutually exclusive, so multiple imports won't execute simultaneously, and race conditions are not a concern.

Applied to files:

  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.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/Voting/Shutter/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
📚 Learning: 2025-12-17T14:55:22.988Z
Learnt from: kemuru
Repo: kleros/kleros-v2 PR: 1871
File: web/src/pages/Profile/Votes/index.tsx:69-75
Timestamp: 2025-12-17T14:55:22.988Z
Learning: In `web/src/pages/Profile/Votes/index.tsx`, fetching up to 1000 draws client-side is a temporary solution until the subgraph supports fetching `totalVotes` directly. This is acknowledged technical debt that will be addressed when the subgraph capability becomes available.

Applied to files:

  • web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
📚 Learning: 2024-06-27T10:11:54.861Z
Learnt from: nikhilverma360
Repo: kleros/kleros-v2 PR: 1632
File: web/src/components/DisputeView/DisputeInfo/DisputeInfoList.tsx:37-42
Timestamp: 2024-06-27T10:11:54.861Z
Learning: `useMemo` is used in `DisputeInfoList` to optimize the rendering of `FieldItems` based on changes in `fieldItems`, ensuring that the mapping and truncation operation are only performed when necessary.

Applied to files:

  • web/src/pages/Cases/CaseDetails/Voting/Classic/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/Voting/Classic/index.tsx
🧬 Code graph analysis (11)
web/src/actions/commit/execute.ts (3)
web/src/actions/commit/params.ts (1)
  • CommitParams (32-32)
web/src/actions/commit/context.ts (1)
  • CommitContext (3-8)
web/src/actions/commit/builders/index.ts (1)
  • buildCommitTxn (21-23)
web/src/actions/commit/builders/gated.builder.ts (3)
web/src/actions/commit/builders/baseBuilder.ts (1)
  • CommitBuilder (14-16)
web/src/actions/commit/params.ts (1)
  • GatedCommitParams (22-24)
web/src/utils/crypto/hashVote.ts (1)
  • hashVote (11-23)
web/src/hooks/useCastCommit.tsx (3)
web/src/actions/commit/params.ts (5)
  • ClassicCommitParams (12-14)
  • ShutterCommitParams (16-20)
  • GatedCommitParams (22-24)
  • GatedShutterCommitParams (26-30)
  • CommitParams (32-32)
web/src/utils/crypto/generateSalt.ts (1)
  • generateSalt (10-19)
web/src/actions/commit/execute.ts (1)
  • executeCommit (9-13)
web/src/actions/commit/params.ts (1)
web/src/consts/disputeFeature.ts (1)
  • DisputeKits (32-32)
web/src/actions/commit/builders/classic.builder.ts (3)
web/src/actions/commit/builders/baseBuilder.ts (1)
  • CommitBuilder (14-16)
web/src/actions/commit/params.ts (1)
  • ClassicCommitParams (12-14)
web/src/utils/crypto/hashVote.ts (1)
  • hashVote (11-23)
web/src/actions/commit/builders/shutter.builder.ts (6)
web/src/actions/commit/builders/baseBuilder.ts (1)
  • CommitBuilder (14-16)
web/src/actions/commit/params.ts (1)
  • ShutterCommitParams (16-20)
web/src/actions/commit/helpers/index.ts (1)
  • encodeShutterMessage (2-4)
web/src/utils/crypto/shutter.ts (1)
  • encrypt (151-175)
web/src/utils/crypto/hashVote.ts (1)
  • hashVote (11-23)
web/src/utils/crypto/hashJustification.ts (1)
  • hashJustification (3-15)
web/src/actions/commit/builders/gatedShutter.builder.ts (6)
web/src/actions/commit/builders/baseBuilder.ts (1)
  • CommitBuilder (14-16)
web/src/actions/commit/params.ts (1)
  • GatedShutterCommitParams (26-30)
web/src/actions/commit/helpers/index.ts (1)
  • encodeShutterMessage (2-4)
web/src/utils/crypto/shutter.ts (1)
  • encrypt (151-175)
web/src/utils/crypto/hashVote.ts (1)
  • hashVote (11-23)
web/src/utils/crypto/hashJustification.ts (1)
  • hashJustification (3-15)
web/src/actions/commit/builders/baseBuilder.ts (2)
web/src/actions/commit/params.ts (1)
  • CommitParams (32-32)
web/src/actions/commit/context.ts (1)
  • CommitContext (3-8)
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (4)
web/src/hooks/queries/useDisputeDetailsQuery.ts (1)
  • useDisputeDetailsQuery (46-62)
web/src/pages/Cases/CaseDetails/Timeline.tsx (1)
  • getDeadline (145-156)
web/src/hooks/useCastCommit.tsx (1)
  • useCastCommit (25-73)
web/src/consts/disputeFeature.ts (1)
  • DisputeKits (32-32)
web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx (2)
web/src/hooks/queries/useDrawQuery.ts (1)
  • useDrawQuery (23-37)
web/src/hooks/useVotingContext.tsx (1)
  • useVotingContext (134-134)
web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx (3)
web/src/hooks/queries/useDisputeDetailsQuery.ts (1)
  • useDisputeDetailsQuery (46-62)
web/src/hooks/useCastCommit.tsx (1)
  • useCastCommit (25-73)
web/src/consts/disputeFeature.ts (1)
  • DisputeKits (32-32)
⏰ 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). (16)
  • 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-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet-devtools
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet-devtools
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: hardhat-tests
  • GitHub Check: SonarCloud
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Mend Security Check
🔇 Additional comments (18)
web/package.json (1)

13-13: LGTM!

The new actions alias is consistent with the existing alias pattern and aligns with the newly introduced src/actions modules.

web/tsconfig.json (1)

15-17: LGTM!

The actions* path mapping correctly mirrors the package.json alias and follows the established pattern for other path aliases in this configuration.

web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)

70-76: LGTM on the localStorage change.

Using localStorage.getItem() directly during reveal is appropriate since you only need a one-time read of the stored salt/choice at execution time, rather than maintaining reactive state via the hook.

web/src/hooks/queries/useDrawQuery.ts (1)

28-28: QueryKey is correctly structured for React Query cache management.

The array format ["useDrawQuery", address, disputeID, roundID] is the proper approach, enabling React Query to properly compare and invalidate queries based on individual dependency parameters. The cache invalidation call in useCastCommit.tsx correctly references the new key name, so no additional changes are needed.

web/src/utils/crypto/hashVote.ts (1)

11-22: Consider edge case: empty string justification.

The condition justification === undefined means an empty string "" will be treated as a valid justification and hashed. If an empty justification should be treated the same as no justification, consider using a truthy check instead.

If this is intentional (empty string is a valid justification distinct from none), then the implementation is correct.

web/src/utils/crypto/hashJustification.ts (1)

3-15: LGTM!

The implementation correctly computes a deterministic justification hash using viem's cryptographic utilities. The ABI encoding structure is well-documented with inline comments.

web/src/actions/commit/context.ts (1)

3-8: LGTM!

The CommitContext interface is well-structured and provides a clean abstraction for the commit workflow. The types from viem are appropriately used.

web/src/hooks/useCastCommit.tsx (2)

25-72: Overall implementation looks good.

The hook properly orchestrates the commit workflow with appropriate validation, salt generation, localStorage persistence, and query invalidation. The use of useMutation from react-query is appropriate for this side-effect-heavy operation.


29-29: Missing validation for publicClient before use.

publicClient is retrieved on line 29 but used with a non-null assertion on line 61 without prior validation. If publicClient is undefined (e.g., during initial render or network switching), this will cause a runtime error.

🐛 Proposed fix
       if (isUndefined(walletClient)) {
         throw new Error("WalletClient not defined. Is the wallet connected?");
       }
+
+      if (isUndefined(publicClient)) {
+        throw new Error("PublicClient not defined. Is the wallet connected?");
+      }

Then on line 61:

-      const result = await wrapWithToast(executeTxn, publicClient!);
+      const result = await wrapWithToast(executeTxn, publicClient);

Also applies to: 61-61

⛔ Skipped due to learnings
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 2145
File: contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol:277-286
Timestamp: 2025-09-30T17:18:12.895Z
Learning: In DisputeKitClassicBase.sol's castCommit function, jurors are allowed to re-submit commits during the commit period. The implementation uses a commitCount variable to track only first-time commits (where commit == bytes32(0)) so that totalCommitted is not incremented when a juror updates their existing commit.
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`.
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: kleros-sdk/src/sdk.ts:1-3
Timestamp: 2024-10-22T10:23:15.789Z
Learning: In `kleros-sdk/src/sdk.ts`, the `PublicClient` type is used and should not be flagged as unused.
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1744
File: web/src/hooks/queries/useHomePageBlockQuery.ts:71-71
Timestamp: 2024-11-19T05:29:56.238Z
Learning: In `web/src/hooks/queries/useHomePageBlockQuery.ts`, the non-null assertions on `blockNumber!` and `genesisBlock!` within `queryFn` are safe because `isEnabled` ensures that `queryFn` only runs when either `blockNumber` or `genesisBlock` is defined.
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.
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1775
File: web/src/pages/Courts/CourtDetails/StakePanel/StakeWithdrawButton.tsx:0-0
Timestamp: 2024-12-09T12:36:59.441Z
Learning: In the `StakeWithdrawButton` component, the transaction flow logic is tightly linked to component updates, so extracting it into a custom hook does not provide significant benefits.
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: kleros-sdk/src/sdk.ts:13-17
Timestamp: 2024-10-22T10:07:21.327Z
Learning: In the SDK, `getPublicClient()` handles the scenario where `publicClient` is undefined by throwing `SdkNotConfiguredError`, so additional error handling in functions that call `getPublicClient()` is not necessary.
web/src/actions/commit/execute.ts (1)

9-13: LGTM!

Clean separation of transaction building and execution. The function correctly delegates to the centralized builder dispatch and executes via the wallet client. The return type (Hash) is properly inferred from writeContract.

web/src/actions/commit/builders/baseBuilder.ts (1)

1-16: LGTM - Well-designed builder interface.

The generic interface cleanly constrains builder implementations while maintaining type safety with viem's WriteContractParameters. This provides a solid foundation for the commit builder pattern.

web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx (1)

25-31: LGTM - Simplified data flow with proper cache invalidation.

The removal of refetch from the useDrawQuery destructuring and Commit props is a clean refactor. Query cache invalidation after commit is correctly handled in the useCastCommit mutation's onSuccess callback via queryClient.invalidateQueries({ queryKey: ["useDrawQuery"] }), which is the idiomatic React Query pattern.

web/src/actions/commit/builders/classic.builder.ts (1)

8-23: LGTM!

Clean implementation following the builder pattern. The hashVote call correctly uses salt directly without redundant conversion.

web/src/actions/commit/params.ts (2)

12-32: LGTM!

Well-structured discriminated union types. The pattern enables type-safe dispatch to specific builders based on the type field.


1-10: No issues found. DisputeKits is correctly defined as an enum in src/consts/index.ts with the expected members (Classic, Shutter, Gated, GatedShutter), and params.ts properly imports and uses it as a type discriminator.

web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx (1)

26-47: LGTM on the refactored commit flow.

The simplification using useCastCommit is a good improvement over the previous multi-step wallet interaction. The dependency array is complete.

web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (1)

49-76: LGTM on callback structure.

The decryption delay calculation with the 5-minute buffer is correctly implemented. The dependency array is complete.

web/src/actions/commit/builders/gatedShutter.builder.ts (1)

30-37: Add explicit chain support validation in all commit builders.

The address lookups (disputeKitGatedShutterAddress[chain.id], etc.) may return undefined if the chain is unsupported. While wagmi likely prevents unsupported chains from reaching this code, add an explicit guard for defensive programming, matching the pattern used in useDisputeKitAddresses.ts:

if (!(chain.id in disputeKitGatedShutterAddress)) {
  throw new Error(`Unsupported chain: ${chain.id}`);
}

This applies to all four builders: classic.builder.ts, shutter.builder.ts, gated.builder.ts, and gatedShutter.builder.ts.

⛔ Skipped due to learnings
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1762
File: web/src/utils/parseWagmiError.ts:10-17
Timestamp: 2024-11-29T06:23:15.955Z
Learning: In the `web/src/utils/parseWagmiError.ts` file and throughout the codebase, prefer using optional chaining to handle `undefined` or `null` values, including optional arrays, without adding explicit existence or length checks.
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

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/Shutter/index.tsx (1)

26-37: Ensure commit is present before rendering Reveal.

The commit value from useVotingContext is optional and derived from drawData?.draws?.[0]?.vote?.commit (useVotingContext.tsx:112), which can be undefined if data is still loading. Since shouldShowReveal only checks id && isVotingPeriod without validating commit, the component may render Reveal with an undefined value. While the type assertion masks this at compile time, Reveal will fail at runtime when the undefined commit is passed to revealVote.

✅ Suggested fix
-  const shouldShowReveal = id && isVotingPeriod;
+  const shouldShowReveal = id && isVotingPeriod && !!commit;
🤖 Fix all issues with AI agents
In `@web/src/actions/helpers/storage/index.ts`:
- Around line 3-11: Wrap all direct localStorage access in try/catch to prevent
exceptions from bubbling up: in storeCommitData (and the analogous functions
that read/remove commit data) perform JSON.stringify and localStorage.setItem
inside a try block, catch errors, optionally log the error (e.g., via
console.error) and fail gracefully (no throw) or return a safe fallback value;
ensure you preserve the StoredCommitData shape conversion (salt/choice
toString()) before attempting storage and do not change the function signatures.

In `@web/src/actions/reveal/builders/classic.builder.ts`:
- Around line 8-18: The build function in classic.builder.ts currently uses
disputeKitClassicAddress[chain.id] without guarding undefined; update the build
implementation to validate that disputeKitClassicAddress[chain.id] exists (e.g.,
const address = disputeKitClassicAddress[chain.id]) and if it is undefined throw
a clear error (e.g., "Unsupported chain id: {chain.id} for
disputeKitClassicAddress") before returning the transaction object so castVote
never gets built with an undefined address; ensure references to build,
ClassicRevealParams, disputeKitClassicAddress, and chain.id are used to locate
the fix.

In `@web/src/actions/reveal/resolveRevealInputs.ts`:
- Around line 20-23: The current logic always sets justification from params
then returns { ...params, ...stored, justification }, which discards
stored.justification when stored exists; change the justification resolution to
prefer stored.justification first, then params.justification, then empty string
(e.g., compute justification = stored?.justification ?? params.justification ??
""), and use that justification in the return path so resolveRevealInputs
preserves an existing stored justification.

In `@web/src/actions/vote/builders/index.ts`:
- Around line 9-18: The builders map and buildVoteTxn call use
builders[params.type] but VoteParams is currently only ClassicVoteParams (type:
DisputeKits.Classic), causing a type mismatch; fix by either (A) expanding
VoteParams to a union that includes GatedVoteParams, ShutterVoteParams, and
GatedShutterVoteParams (these can alias ClassicVoteParams if signatures are
identical) so params.type can be any DisputeKits and builders stays
Record<DisputeKits, VoteBuilder>, or (B) narrow builders to only include
[DisputeKits.Classic] and type it accordingly so buildVoteTxn uses a builders
keyed by Classic only; update the VoteParams/ClassicVoteParams and builders
declarations and ensure buildVoteTxn's params.type typing aligns with the chosen
approach (referencing VoteParams, ClassicVoteParams, builders, and
buildVoteTxn).

In `@web/src/hooks/useRevealVote.ts`:
- Around line 27-48: The code uses publicClient from usePublicClient() without
guarding it before calling wrapWithToast; add a check like the existing guards
for walletClient/account/chain in the mutationFn to throw a clear error if
publicClient is undefined, and ensure wrapWithToast is called only with a
validated publicClient (remove or avoid the non-null assertion publicClient!);
this touches the mutationFn in useRevealVote where executeTxn is wrapped with
wrapWithToast and should reference usePublicClient/publicClient.

In `@web/src/hooks/useVote.ts`:
- Around line 16-35: The code calls wrapWithToast(publicClient!) without
guarding publicClient; add a defensive check in useVote's mutationFn (same area
that checks account/chain/walletClient) to throw an Error like "Public client
not defined. Is the wallet initialized?" when usePublicClient() returns
undefined, and then pass publicClient (no non-null assertion) into
wrapWithToast; locate the logic around executeVote, wrapWithToast, and the
useMutation mutationFn to implement this change.

In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx`:
- Around line 60-83: Guard against currentRoundIndex being undefined before
calling revealVote: in the handleReveal callback (Reveal.tsx) check that
currentRoundIndex is not undefined and return early (or show a validation) if it
is, so you do not call revealVote with an undefined round; reference the
revealVote call and the currentRoundIndex variable to locate the change. Also
disable the related Reveal button when currentRoundIndex is undefined (matching
the pattern used in Commit.tsx) so the user cannot trigger the action before
dispute data loads. Ensure the revealVote call still passes the same
params/context when currentRoundIndex is valid.

In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Vote.tsx`:
- Around line 36-47: handleVote currently calls
BigInt(disputeData?.dispute?.currentRoundIndex) which can be BigInt(undefined)
while disputeData is still loading; update handleVote to guard the salt
computation by checking disputeData?.dispute?.currentRoundIndex is not
null/undefined before calling BigInt (or return early/disable voting when it's
missing), i.e. compute a local safeSalt only when currentRoundIndex is defined
and pass that to vote (or bail out if undefined) in the handleVote callback to
avoid calling BigInt on undefined.

In `@web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx`:
- Around line 31-63: Guard the reveal action until currentRoundIndex is
available: disable the Reveal button when currentRoundIndex is undefined (e.g.,
change Button disabled to disabled={isPending || currentRoundIndex ===
undefined}), and add a defensive check at the start of handleReveal to return
early if currentRoundIndex is undefined to avoid calling revealVote with an
invalid roundIndex; reference currentRoundIndex, handleReveal, revealVote,
parsedDisputeID, parsedVoteIDs, justification, isGated and the Button component
when making the changes.
♻️ Duplicate comments (2)
web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx (1)

37-48: Guard against undefined currentRoundIndex before calling castCommit.

currentRoundIndex (line 31) can be undefined when disputeData hasn't loaded. Passing undefined to castCommit will propagate to getVoteKey() and salt generation, likely causing runtime issues.

Suggested fix
   const handleCommit = useCallback(
     async (choice: bigint) => {
+      if (currentRoundIndex === undefined) {
+        return;
+      }
       castCommit({
         type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
         disputeId: parsedDisputeID,
         choice,
         voteIds: parsedVoteIDs,
         roundIndex: currentRoundIndex,
       });
     },
     [castCommit, parsedDisputeID, currentRoundIndex, parsedVoteIDs, isGated]
   );
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (1)

51-65: Guard commit when round index is unavailable.

currentRoundIndex can be undefined before the dispute data loads, which makes the commit call invalid. Add a guard before invoking castCommit.

🐛 Suggested guard
   const handleCommit = useCallback(
     async (choice: bigint) => {
       /* an extra 300 seconds (5 minutes) of decryptionDelay is enforced after Commit period is over
       to avoid premature decryption and voting attacks if no one passes the Commit period quickly */
       const decryptionDelay = (countdownToVotingPeriod ?? 0) + 300;
 
+      if (currentRoundIndex === undefined) {
+        console.error("Round index not available");
+        return;
+      }
       castCommit({
         type: isGated ? DisputeKits.GatedShutter : DisputeKits.Shutter,
         disputeId: parsedDisputeID,
         choice,
         voteIds: parsedVoteIDs,
         roundIndex: currentRoundIndex,
         justification,
         decryptionDelay,
       });
     },
     [justification, parsedVoteIDs, parsedDisputeID, countdownToVotingPeriod, isGated, castCommit, currentRoundIndex]
   );
🧹 Nitpick comments (3)
web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx (1)

33-42: Unsafe type assertion on commit.

Casting commit as \0x${string}`bypasses TypeScript's type checking. Ifcommitisundefinedor an empty string fromuseVotingContext()`, this assertion masks the issue until runtime.

Since Reveal.tsx already handles isUndefined(commit) in rendering, consider passing commit with its actual type and letting the child component handle the typing:

Safer approach
       <Reveal
         {...{
           arbitrable,
           setIsOpen,
           voteIDs,
           isRevealPeriod: !isCommitPeriod,
           isGated,
-          commit: commit as `0x${string}`,
+          commit,
         }}
       />

Then update IReveal interface in Reveal.tsx to accept commit?: \0x${string}`` and handle the undefined case explicitly.

web/src/actions/helpers/storage/key.ts (1)

1-3: Make array-to-string conversion explicit for clarity.

The voteIds array is implicitly converted to a comma-separated string via template literal interpolation. While this works and produces consistent keys within a session (React Query caches the query results), the implicit toString() behavior is non-obvious to readers.

Suggested improvement
 export const getVoteKey = (disputeId: bigint, roundIndex: number, voteIds: bigint[]) => {
-  return `dispute-${disputeId}-round-${roundIndex}-voteids-${voteIds}`;
+  return `dispute-${disputeId}-round-${roundIndex}-voteids-${voteIds.join(",")}`;
 };
web/src/actions/commit/builders/gatedShutter.builder.ts (1)

23-35: Normalize justification before hashing/encrypting.

If justification is optional, passing undefined into encodeShutterMessage/hashJustification can produce inconsistent commits or runtime errors. Consider normalizing to an empty string and storing that.

♻️ Suggested tweak
-    const { disputeId, voteIds, choice, salt, decryptionDelay, justification, roundIndex } = params;
+    const { disputeId, voteIds, choice, salt, decryptionDelay, justification, roundIndex } = params;
+    const normalizedJustification = justification ?? "";
@@
-    storeCommitData(key, { choice, salt, justification });
+    storeCommitData(key, { choice, salt, justification: normalizedJustification });
@@
-    const encodedMessage = encodeShutterMessage(choice, salt, justification);
+    const encodedMessage = encodeShutterMessage(choice, salt, normalizedJustification);
@@
-    const justificationCommit = hashJustification(BigInt(salt), justification);
+    const justificationCommit = hashJustification(BigInt(salt), normalizedJustification);

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@web/package.json`:
- Around line 28-30: The package.json currently pins an incorrect Vitest version
(a non-stable v4) referenced by the test scripts ("test", "test:watch",
"test:coverage") and the vitest dependency; update the vitest dependency to a
stable release (e.g., "^3.2.4") instead of any v4 beta, keep other test deps
(jsdom ^27.4.0) as-is, then run the package manager to update lockfiles
(npm/yarn/pnpm install) so the lockfile reflects the stable version.

In `@web/src/actions/commit/builders/shutter.builder.test.ts`:
- Around line 2-41: The test uses vi.stubEnv("REACT_APP_SHUTTER_API", ...) which
persists across suites; add an afterAll cleanup that calls vi.unstubAllEnvs() to
restore environment state after the tests in this file run (use the afterAll
hook in shutter.builder.test.ts alongside the existing mocks so vi.stubEnv is
undone); reference vi.stubEnv and call vi.unstubAllEnvs() inside afterAll to
prevent env leakage to other tests.

In `@web/src/actions/reveal/helpers/bruteForceChoice.test.ts`:
- Around line 85-107: Update the typo in the test case title: change the it
block whose description reads "iterates answers in the given orderh" to
"iterates answers in the given order" (the test around bruteForceChoice using
the answers array and createCommit should keep its logic unchanged; only rename
the test description string).

In `@web/src/actions/reveal/helpers/bruteForceChoice.ts`:
- Around line 11-23: The bruteForceChoice helper always calls hashVote without
the justification, so commits that included a justification cannot be recovered;
update bruteForceChoice to accept an optional justification parameter and pass
it through to hashVote when computing innerCommit, and then update callers (for
example resolveRevealInputs) to forward the justification (or undefined when
none) into bruteForceChoice so both justified and non-justified commits can be
matched.

In `@web/src/actions/reveal/resolveRevealInputs.test.ts`:
- Around line 325-355: The test fails because module-level vi.mock of
generateSalt returns undefined for this integration case; update the test to
restore the real implementation of generateSalt for this spec so BigInt(salt)
receives a real value: inside the "reconstructs choice and salt end-to-end"
test, replace the mocked generateSalt with the actual implementation (use
vi.importActual or retrieve the real generateSalt and set the mockGenerateSalt
to call-through) before calling generateSalt, then proceed to call
resolveRevealInputs and assert result.choice and result.salt as before.

In `@web/src/utils/crypto/generateSalt.test.ts`:
- Around line 37-51: The tests in generateSalt.test.ts use invalid hex
signatures (e.g., "0xconst123", "0xsig111") which cause viem's keccak256 to
throw; update the test vectors used in createMockAccount within the tests (both
the deterministic test using mockSignature and the differing-signature test
cases at the later lines) to use valid lowercase hex string literals (only 0-9
and a-f after 0x) of appropriate length so keccak256 accepts them; ensure all
other occurrences referenced (createMockAccount, mockAccount1, mockAccount2, and
any signatures around lines 79-81) are replaced consistently with valid hex
values.
🧹 Nitpick comments (9)
web/src/utils/crypto/hashVote.test.ts (1)

173-178: Consider adding a symmetric test for negative salt.

Since salt is also encoded as uint256, a negative salt should throw IntegerOutOfRangeError just like negative choice. Adding this test would provide symmetric coverage.

💡 Suggested addition
     it("should throw on negative choice", () => {
       const choice = -1n;
       const salt = 123456789n;

       expect(() => hashVote(choice, salt)).toThrow(IntegerOutOfRangeError);
     });
+
+    it("should throw on negative salt", () => {
+      const choice = 1n;
+      const salt = -1n;
+
+      expect(() => hashVote(choice, salt)).toThrow(IntegerOutOfRangeError);
+    });
   });
 });
web/src/actions/commit/builders/classic.builder.test.ts (1)

88-104: Prefer getVoteKey over a hardcoded key string.
This keeps the test aligned with the production key format if it changes.

♻️ Proposed refactor
-import { storeCommitData } from "actions/helpers/storage";
+import { storeCommitData, getVoteKey } from "actions/helpers/storage";
@@
-      expect(storeCommitData).toHaveBeenCalledWith("dispute-100-round-2-voteids-5,6", {
+      const expectedKey = getVoteKey(params.disputeId, params.roundIndex, params.voteIds);
+      expect(storeCommitData).toHaveBeenCalledWith(expectedKey, {
         choice: 1n,
         salt: 777n,
       });
web/src/actions/vote/builders/classic.builder.test.ts (2)

13-18: Mock address is malformed.

The mock address 0xVOTEADDRESS12345678901234567890123456 is invalid:

  1. Contains non-hex characters (O, T, D).
  2. Only 37 characters after 0x instead of the required 40.

While this works because of the type assertion, using a valid address improves test robustness and avoids potential issues with stricter validation.

Suggested fix
 vi.mock("hooks/contracts/generated", () => ({
   disputeKitClassicAbi: [{ name: "castVote", type: "function" }],
   disputeKitClassicAddress: {
-    421614: "0xVOTEADDRESS12345678901234567890123456" as const,
+    421614: "0x1234567890123456789012345678901234567890" as const,
   },
 }));

206-226: Test name doesn't match behavior.

The test is named "comparison with reveal builder" but doesn't actually import or compare with the reveal builder's output. It only verifies the structure of the vote builder independently.

Consider either:

  1. Renaming the describe block to better reflect what it tests (e.g., "args structure validation")
  2. Actually importing and comparing with the reveal builder if cross-builder consistency is the intent
web/src/actions/commit/builders/gated.builder.test.ts (2)

18-23: Use a valid Ethereum address format in mock.

The mock address 0xGATED1234567890123456789012345678901234 is not a valid Ethereum address:

  1. Contains 'G' which is not valid hexadecimal (only 0-9 and a-f are valid)
  2. Has 39 hex characters instead of the required 40

While this won't break the current tests, it could cause issues if viem performs address validation, and it makes the tests less realistic.

Suggested fix
 vi.mock("hooks/contracts/generated", () => ({
   disputeKitGatedAbi: [{ name: "castCommit", type: "function" }],
   disputeKitGatedAddress: {
-    421614: "0xGATED1234567890123456789012345678901234" as const,
+    421614: "0x1234567890abcdef1234567890abcdef12345678" as const,
   },
 }));

Then update the expectation on line 56:

-        address: "0xGATED1234567890123456789012345678901234",
+        address: "0x1234567890abcdef1234567890abcdef12345678",

77-95: Consider mocking getVoteKey for better test isolation.

This test hardcodes the expected key format "dispute-100-round-2-voteids-5,6", creating an implicit dependency on getVoteKey's implementation. If getVoteKey changes its format, this test will fail even though gatedCommitBuilder itself remains correct.

Options:

  1. Mock getVoteKey to return a predictable value (better isolation)
  2. Import and use getVoteKey to compute the expected key (more maintainable)
  3. Keep as-is if intentional integration testing (document the intent)
Option 2: Use getVoteKey for expected value
+import { getVoteKey } from "actions/helpers/storage/key";
+
 // In the test:
+      const expectedKey = getVoteKey(params.disputeId, params.roundIndex, params.voteIds);
       expect(storeCommitData).toHaveBeenCalledTimes(1);
-      expect(storeCommitData).toHaveBeenCalledWith("dispute-100-round-2-voteids-5,6", {
+      expect(storeCommitData).toHaveBeenCalledWith(expectedKey, {
         choice: 1n,
         salt: 777n,
       });
web/src/actions/helpers/storage/key.test.ts (1)

5-65: Good test coverage, consider adding edge case for empty voteIds.

The test suite comprehensively covers key formatting, large values, determinism, and uniqueness. Consider adding a test for an empty voteIds array to document the expected behavior in that edge case.

Optional: Add edge case test
+  it("should handle empty voteIds array", () => {
+    const result = getVoteKey(1n, 0, []);
+
+    expect(result).toBe("dispute-1-round-0-voteids-");
+  });
web/src/test/fakes/shutter.ts (1)

18-27: Deterministic test fake is well-designed.

The fake produces verifiable, deterministic outputs using keccak256 hashing. The TODO comment appropriately notes the need to validate against the real API.

Consider extracting the shared hash computation to reduce duplication between fakeEncrypt and verifyFakeEncryptOutput.

Optional: Extract shared computation
+function computeShutterOutputs(message: string, decryptionDelay: number): ShutterEncryptResult {
+  const identity = keccak256(encodePacked(["string", "uint256"], [message, BigInt(decryptionDelay)]));
+  const encryptedCommitment = keccak256(encodePacked(["bytes32", "string"], [identity, "encrypted"]));
+  return { encryptedCommitment, identity };
+}
+
 export function fakeEncrypt(message: string, decryptionDelay: number): ShutterEncryptResult {
-  const identity = keccak256(encodePacked(["string", "uint256"], [message, BigInt(decryptionDelay)]));
-  const encryptedCommitment = keccak256(encodePacked(["bytes32", "string"], [identity, "encrypted"]));
-
-  return {
-    encryptedCommitment,
-    identity,
-  };
+  return computeShutterOutputs(message, decryptionDelay);
 }

 export function verifyFakeEncryptOutput(
   message: string,
   decryptionDelay: number,
   actualIdentity: `0x${string}`,
   actualEncrypted: `0x${string}`
 ): boolean {
-  const expectedIdentity = keccak256(encodePacked(["string", "uint256"], [message, BigInt(decryptionDelay)]));
-  const expectedEncrypted = keccak256(encodePacked(["bytes32", "string"], [expectedIdentity, "encrypted"]));
-
-  return actualIdentity === expectedIdentity && actualEncrypted === expectedEncrypted;
+  const expected = computeShutterOutputs(message, decryptionDelay);
+  return actualIdentity === expected.identity && actualEncrypted === expected.encryptedCommitment;
 }

Also applies to: 33-43

web/src/actions/commit/builders/shutter.builder.test.ts (1)

130-135: Use getVoteKey to keep the test aligned with key formatting.

This avoids duplicating the string format and protects the test if key formatting changes.

♻️ Suggested update
-import { storeCommitData } from "actions/helpers/storage";
+import { storeCommitData, getVoteKey } from "actions/helpers/storage";
@@
-      expect(storeCommitData).toHaveBeenCalledWith("dispute-100-round-2-voteids-5,6", {
+      const expectedKey = getVoteKey(params.disputeId, params.roundIndex, params.voteIds);
+      expect(storeCommitData).toHaveBeenCalledWith(expectedKey, {
         choice: 1n,
         salt: 777n,
         justification: "Stored justification",
       });

@tractorss tractorss marked this pull request as ready for review January 21, 2026 15:30
@tractorss tractorss requested review from a team and jaybuidl as code owners January 21, 2026 15:30
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@web/package.json`:
- Around line 28-30: The test:coverage script is missing a Vitest coverage
provider; add a coverage provider package (preferably `@vitest/coverage-v8` or
`@vitest/coverage-istanbul`) to devDependencies and install it, then ensure Vitest
picks it up (either via the default provider detection or by setting
coverage.provider in your Vitest config like vitest.config.*); this will allow
the "test:coverage" script to produce coverage reports.

In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx`:
- Around line 34-52: The handler is using mutateAsync (from useCastCommit) but
never awaiting it, causing misleading async behavior and premature UI state
changes; replace the destructured mutateAsync with mutate (i.e., const { mutate:
castCommit } = useCastCommit(...)) and call castCommit(...) from handleCommit,
and remove the unnecessary async keyword from handleCommit so the function is
synchronous and relies on the hook's onSuccess/onError callbacks; references:
useCastCommit, mutateAsync → mutate, castCommit, and handleCommit.
♻️ Duplicate comments (4)
README.md (1)

239-239: Wrap bare URL in angle brackets.

Use <http://localhost> to satisfy markdownlint and keep link formatting consistent.

web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)

100-107: Button should also be disabled when currentRoundIndex is undefined.

The early return guard in handleReveal prevents the reveal action when currentRoundIndex is undefined, but the button remains clickable. From a UX perspective, clicking the button would do nothing, which could confuse users. Adding the check to the disabled state provides proper feedback.

♻️ Suggested fix
             <StyledButton
               variant="secondary"
               text={t("buttons.justify_and_reveal")}
-              disabled={isPending || isUndefined(disputeDetails)}
+              disabled={isPending || isUndefined(disputeDetails) || isUndefined(currentRoundIndex)}
               isLoading={isPending}
               onClick={handleReveal}
             />
web/src/actions/helpers/storage/index.ts (1)

18-37: localStorage.getItem and removeItem calls are still unguarded.

The try/catch was added to storeCommitData, but restoreCommitData and removeCommitData still have unguarded localStorage calls that can throw when storage is unavailable (private browsing, blocked, quota exceeded).

🛠️ Proposed fix
 export function restoreCommitData(key: string): CommitData | undefined {
-  const raw = localStorage.getItem(key);
-  if (!raw) return undefined;
-
   try {
+    const raw = localStorage.getItem(key);
+    if (!raw) return undefined;
+
     const storedData = JSON.parse(raw) as StoredCommitData;
 
     return {
       salt: BigInt(storedData.salt),
       choice: BigInt(storedData.choice),
       justification: storedData.justification,
     };
   } catch {
     return undefined;
   }
 }
 
 export function removeCommitData(key: string): void {
-  localStorage.removeItem(key);
+  try {
+    localStorage.removeItem(key);
+  } catch {
+    // Storage unavailable - no-op
+  }
 }
web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx (1)

48-71: Disable reveal while currentRoundIndex is unknown.
Line 49 already guards, but Line 70 still allows a click that no-ops. Please disable the button until the round index is available.

🐛 Suggested guard
-      <Button text={t("buttons.reveal_your_vote")} onClick={handleReveal} disabled={isPending} isLoading={isPending} />
+      <Button
+        text={t("buttons.reveal_your_vote")}
+        onClick={handleReveal}
+        disabled={isPending || isUndefined(currentRoundIndex)}
+        isLoading={isPending}
+      />
🧹 Nitpick comments (4)
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1)

62-78: Consider awaiting revealVote for consistent async handling.

The revealVote mutation is not awaited, so handleReveal returns immediately while the mutation runs in the background. While useRevealVote handles errors via toast internally, awaiting ensures the callback completes with the mutation and enables potential future error handling at the call site.

♻️ Suggested change
   const handleReveal = useCallback(async () => {
     if (isUndefined(currentRoundIndex)) {
       return;
     }
-    revealVote({
+    await revealVote({
       params: {
         disputeId: parsedDisputeID,
         voteIds: parsedVoteIDs,
         justification,
         roundIndex: Number(currentRoundIndex),
         type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
       },
       context: {
         commit,
         answers: disputeDetails?.answers,
       },
     });
   }, [
web/src/actions/reveal/helpers/bruteForceChoice.test.ts (1)

96-107: Consider clarifying the test description.

The test name "iterates answers in the given order" is slightly misleading. The test actually verifies that bruteForceChoice finds the correct choice regardless of the answer array's ordering (IDs 5, 1 instead of sequential 0, 1, 2...). A clearer name might be "finds matching choice regardless of answer array ordering" or "handles non-sequential answer IDs".

✏️ Suggested rename
-    it("iterates answers in the given order", async () => {
+    it("finds matching choice regardless of answer array ordering", async () => {
web/src/actions/reveal/resolveRevealInputs.integration.test.ts (1)

6-9: Consider consistent path alias usage.

The imports use different path alias patterns (utils/crypto/... vs src/consts). If the project has established conventions for path aliases, consider aligning these for consistency.

web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx (1)

44-66: Avoid mutateAsync without awaiting to prevent unhandled rejections.
Line 45 uses mutateAsync but the promise isn’t awaited/caught. Prefer mutate for fire‑and‑forget, or await with a try/catch.

♻️ Suggested adjustment (use mutate)
-  const { mutateAsync: revealVote, isPending } = useRevealVote(() => {
+  const { mutate: revealVote, isPending } = useRevealVote(() => {
     setIsOpen(true);
   });
-  const handleReveal = useCallback(async () => {
+  const handleReveal = useCallback(() => {
     if (isUndefined(currentRoundIndex)) {
       return;
     }

     revealVote({

Comment on lines +28 to +30
"test": "vitest run",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing coverage provider for test:coverage script.

The test:coverage script requires a coverage provider package to function. Vitest needs either @vitest/coverage-v8 (recommended, uses V8's built-in coverage) or @vitest/coverage-istanbul to generate coverage reports.

🔧 Proposed fix

Add the coverage provider to devDependencies:

+    "@vitest/coverage-v8": "^4.0.17",
     "jsdom": "^27.4.0",
🤖 Prompt for AI Agents
In `@web/package.json` around lines 28 - 30, The test:coverage script is missing a
Vitest coverage provider; add a coverage provider package (preferably
`@vitest/coverage-v8` or `@vitest/coverage-istanbul`) to devDependencies and install
it, then ensure Vitest picks it up (either via the default provider detection or
by setting coverage.provider in your Vitest config like vitest.config.*); this
will allow the "test:coverage" script to produce coverage reports.

Comment on lines +34 to +52
const { mutateAsync: castCommit } = useCastCommit(() => {
setIsOpen(true);
});

const handleCommit = useCallback(
async (choice: bigint) => {
const message = { message: saltKey };
const rawSalt = !isUndefined(signingAccount)
? await signingAccount.signMessage(message)
: await (async () => {
const account = await generateSigningAccount();
return await account?.signMessage(message);
})();

if (isUndefined(rawSalt)) return;

const salt = keccak256(rawSalt);
setSalt(JSON.stringify({ salt, choice: choice.toString() }));
const commit = keccak256(encodePacked(["uint256", "uint256"], [BigInt(choice), BigInt(salt)]));

const simulate = isGated ? simulateDisputeKitGatedCastCommit : simulateDisputeKitClassicCastCommit;
if (isUndefined(currentRoundIndex)) {
return;
}

const { request } = await simulate(wagmiConfig, {
args: [parsedDisputeID, parsedVoteIDs, commit],
castCommit({
type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
disputeId: parsedDisputeID,
choice,
voteIds: parsedVoteIDs,
roundIndex: Number(currentRoundIndex),
});
if (walletClient && publicClient) {
await wrapWithToast(async () => await walletClient.writeContract(request), publicClient).then(({ status }) => {
setIsOpen(status);
});
}
refetch();
},
[
wagmiConfig,
saltKey,
setSalt,
parsedVoteIDs,
parsedDisputeID,
publicClient,
setIsOpen,
walletClient,
generateSigningAccount,
signingAccount,
refetch,
isGated,
]
[castCommit, parsedDisputeID, currentRoundIndex, parsedVoteIDs, isGated]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's explore the file structure and find the relevant files
find web -type f -name "*.tsx" | grep -E "(Commit|CastCommit)" | head -20

Repository: kleros/kleros-v2

Length of output: 385


🏁 Script executed:

# Let's read the Commit.tsx file to see the full context
cat -n web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx

Repository: kleros/kleros-v2

Length of output: 2200


🏁 Script executed:

# Find the useCastCommit hook definition
fd -e ts -e tsx -type f | xargs rg -l "useCastCommit" | head -10

Repository: kleros/kleros-v2

Length of output: 395


🏁 Script executed:

# Read the useCastCommit hook to understand its implementation
cat -n web/src/hooks/useCastCommit.tsx

Repository: kleros/kleros-v2

Length of output: 3072


🏁 Script executed:

# Also check the Shutter/Commit.tsx to see if it uses the hook differently
cat -n web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx

Repository: kleros/kleros-v2

Length of output: 3197


🏁 Script executed:

# Let's check how handleCommit is used in OptionsContainer
cat -n web/src/pages/Cases/CaseDetails/Voting/Classic/OptionsContainer.tsx

Repository: kleros/kleros-v2

Length of output: 159


🏁 Script executed:

# Also check if there are any other usages of these Commit components
rg -A5 -B5 "handleSelection" web/src/pages/Cases/CaseDetails/Voting/ | head -60

Repository: kleros/kleros-v2

Length of output: 4846


🏁 Script executed:

# Let's check the full OptionsContainer.tsx to see how it uses handleSelection
cat -n web/src/pages/Cases/CaseDetails/Voting/OptionsContainer.tsx

Repository: kleros/kleros-v2

Length of output: 4904


🏁 Script executed:

# Check if there's a TypeScript error or if the type mismatch is handled
rg -A20 "interface IOptions" web/src/pages/Cases/CaseDetails/Voting/OptionsContainer.tsx

Repository: kleros/kleros-v2

Length of output: 878


Use mutate instead of mutateAsync since the promise isn't awaited.

mutateAsync returns a promise that's never awaited, making the async handler misleading. The error handling is already in place via the onError callback, but this pattern creates a timing mismatch where the UI loading state resets before the mutation completes. Prefer mutate:

Suggested adjustment
-  const { mutateAsync: castCommit } = useCastCommit(() => {
+  const { mutate: castCommit } = useCastCommit(() => {
     setIsOpen(true);
   });

   const handleCommit = useCallback(
-    async (choice: bigint) => {
+    (choice: bigint) => {
       if (isUndefined(currentRoundIndex)) {
         return;
       }

       castCommit({
         type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
         disputeId: parsedDisputeID,
         choice,
         voteIds: parsedVoteIDs,
         roundIndex: Number(currentRoundIndex),
       });
     },
     [castCommit, parsedDisputeID, currentRoundIndex, parsedVoteIDs, isGated]
   );
📝 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.

Suggested change
const { mutateAsync: castCommit } = useCastCommit(() => {
setIsOpen(true);
});
const handleCommit = useCallback(
async (choice: bigint) => {
const message = { message: saltKey };
const rawSalt = !isUndefined(signingAccount)
? await signingAccount.signMessage(message)
: await (async () => {
const account = await generateSigningAccount();
return await account?.signMessage(message);
})();
if (isUndefined(rawSalt)) return;
const salt = keccak256(rawSalt);
setSalt(JSON.stringify({ salt, choice: choice.toString() }));
const commit = keccak256(encodePacked(["uint256", "uint256"], [BigInt(choice), BigInt(salt)]));
const simulate = isGated ? simulateDisputeKitGatedCastCommit : simulateDisputeKitClassicCastCommit;
if (isUndefined(currentRoundIndex)) {
return;
}
const { request } = await simulate(wagmiConfig, {
args: [parsedDisputeID, parsedVoteIDs, commit],
castCommit({
type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
disputeId: parsedDisputeID,
choice,
voteIds: parsedVoteIDs,
roundIndex: Number(currentRoundIndex),
});
if (walletClient && publicClient) {
await wrapWithToast(async () => await walletClient.writeContract(request), publicClient).then(({ status }) => {
setIsOpen(status);
});
}
refetch();
},
[
wagmiConfig,
saltKey,
setSalt,
parsedVoteIDs,
parsedDisputeID,
publicClient,
setIsOpen,
walletClient,
generateSigningAccount,
signingAccount,
refetch,
isGated,
]
[castCommit, parsedDisputeID, currentRoundIndex, parsedVoteIDs, isGated]
const { mutate: castCommit } = useCastCommit(() => {
setIsOpen(true);
});
const handleCommit = useCallback(
(choice: bigint) => {
if (isUndefined(currentRoundIndex)) {
return;
}
castCommit({
type: isGated ? DisputeKits.Gated : DisputeKits.Classic,
disputeId: parsedDisputeID,
choice,
voteIds: parsedVoteIDs,
roundIndex: Number(currentRoundIndex),
});
},
[castCommit, parsedDisputeID, currentRoundIndex, parsedVoteIDs, isGated]
🤖 Prompt for AI Agents
In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx` around lines 34 -
52, The handler is using mutateAsync (from useCastCommit) but never awaiting it,
causing misleading async behavior and premature UI state changes; replace the
destructured mutateAsync with mutate (i.e., const { mutate: castCommit } =
useCastCommit(...)) and call castCommit(...) from handleCommit, and remove the
unnecessary async keyword from handleCommit so the function is synchronous and
relies on the hook's onSuccess/onError callbacks; references: useCastCommit,
mutateAsync → mutate, castCommit, and handleCommit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: Contracts Court smart contracts Package: Subgraph Court subgraph Package: Web Court web frontend Type: Toolchain ⚒️ Build tools configuration, CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants