Skip to content

Shutterized Dispute Kit #1965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 64 commits into
base: dev
Choose a base branch
from
Open

Shutterized Dispute Kit #1965

wants to merge 64 commits into from

Conversation

jaybuidl
Copy link
Member

@jaybuidl jaybuidl commented Apr 30, 2025

Devnet contract deployed

DisputeKitShutter

Critical changes needed:

See #1314


PR-Codex overview

This PR introduces the DisputeKitShutter, enhancing the dispute resolution system by implementing new features and adjustments for handling votes and appeals, while integrating with the Shutter API for encryption and decryption.

Detailed summary

  • Added DisputeKitShutter contract with voting and commitment features.
  • Updated Voting and Appeal components to handle Shutter disputes.
  • Enhanced error handling in OptionsContainer for selection.
  • Integrated Shutter API for vote encryption and decryption.
  • Modified routes to support Shutter-specific voting.
  • Adjusted state management in various components for new functionality.
  • Updated environment variables to include Shutter API endpoint.
  • Improved query handling in useDisputeDetailsQuery.

The following files were skipped due to too many changes: contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol, subgraph/core/src/DisputeKitClassic.ts, subgraph/core-neo/subgraph.template.yaml, yarn.lock, subgraph/core/subgraph.template.yaml, contracts/scripts/shutter.ts, contracts/scripts/keeperBotShutter.ts, contracts/scripts/keeperBot.ts, contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Proxy.json, contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json, contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Implementation.json

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

Summary by CodeRabbit

  • New Features

    • Introduced DisputeKitShutter contract with encrypted voting and Shutter Network integration for enhanced privacy.
    • Added deployment artifacts, proxy contracts, and deployment scripts for DisputeKitShutter on Arbitrum Sepolia Devnet.
    • Enhanced subgraph manifests and configurations to support indexing of DisputeKitShutter events and dynamic handling of multiple dispute kits.
    • Added encryption/decryption CLI script and automated vote reveal script leveraging Shutter Network API.
    • Enhanced keeper bot for multi-dispute-kit support with dynamic dispute kit resolution and shutter auto-reveal.
    • Added React components and hooks for Shutter dispute kit voting, committing, revealing, and appeal funding UI.
    • Updated web client to dynamically render UI based on dispute kit type (Classic vs Shutter).
    • Added Shutter encryption/decryption utilities and integrated Shutter SDK assets into the web build.
  • Bug Fixes

    • Improved subgraph entity handling by dynamically extracting dispute kit IDs, replacing hardcoded values and adding defensive null checks.
  • Documentation

    • Updated README with DisputeKitShutter deployed addresses.
    • Added autogenerated file comments in subgraph configs.
  • Chores

    • Updated package versions and replaced unused dependencies.
    • Refined deployment and subgraph update scripts to exclude undeployed contracts and ensure configuration consistency.
    • Extended contract type mappings and fetching utilities to include DisputeKitShutter.
    • Added error handling improvements in UI components.
    • Upgraded DisputeKitClassic initializer version and added DisputeKitShutter initializer in upgrade scripts.
    • Added Vite plugin configuration to include Shutter SDK assets in web build.
  • Tests

    • Extended tests to cover DisputeKitShutter and updated expected error messages related to vote commitment verification.

@jaybuidl jaybuidl self-assigned this Apr 30, 2025
Copy link
Contributor

coderabbitai bot commented Apr 30, 2025

Walkthrough

This update adds the DisputeKitShutter contract, its deployment, proxy, and event handling infrastructure. It refactors core and subgraph logic for dynamic dispute kit handling, introduces new subgraph templates, and extends SDK and CLI scripts for Shutter encryption/decryption automation.

Changes

File(s) / Group Summary
contracts/package.json Removed node-fetch; added @shutter-network/shutter-sdk and isomorphic-fetch dependencies.
contracts/scripts/shutter.ts New script for Shutter API interactions, encryption, decryption, CLI, and SDK integration.
contracts/scripts/keeperBotShutter.ts New script automating vote reveal by decrypting and casting votes using Shutter dispute kit.
contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol Refactored commit/vote logic into internal functions; added hashVote for customizable vote hashing.
contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol New contract extending DisputeKitClassicBase with Shutter-specific logic and event emission.
contracts/README.md Added DisputeKitShutter addresses for Arbitrum Sepolia Devnet.
contracts/deploy/00-home-chain-arbitration.ts Updated deployment script to deploy/register DisputeKitShutter and enable it on the court.
contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter*.json Deployment artifacts for DisputeKitShutterProxy (UUPS proxy).
contracts/src/proxy/KlerosProxies.sol Added DisputeKitShutterProxy inheriting UUPSProxy.
contracts/test/arbitration/index.ts Extended tests to deploy and verify DisputeKitShutter events and addresses.
contracts/test/foundry/KlerosCore.t.sol Updated expected revert messages for vote commitment validation.
contracts/scripts/utils/contracts.ts Added DisputeKitShutter contract type and retrieval logic for all core types.
subgraph/core-neo/subgraph.template.yaml New subgraph template supporting multiple dispute kits and event handlers.
subgraph/core-university/subgraph.template.yaml New subgraph template supporting multiple data sources and entities.
subgraph/core/src/DisputeKitClassic.ts Updated to extract dispute kit IDs dynamically, with null checks, and handle events accordingly.
subgraph/core/src/KlerosCore.ts Updated comment to clarify applicability to both Classic and Shutter dispute kits.
subgraph/core/src/entities/ClassicContribution.ts Dynamically determines dispute kit ID for contributions; removed hardcoded ID.
subgraph/core/src/entities/ClassicDispute.ts Updated to accept dispute kit ID and round index; dynamic ID construction.
subgraph/core/src/entities/ClassicRound.ts Handles dynamic dispute kit IDs for round creation and funding updates.
subgraph/core/src/utils.ts Added utility extractDisputeKitIDFromExtraData to parse dispute kit ID from extraData bytes.
subgraph/core/subgraph.template.yaml New subgraph template supporting multiple dispute kits and event handlers.
subgraph/core/subgraph.yaml Added DisputeKitShutter as a new data source with event handlers; updated metadata/comments.
subgraph/dispute-template-registry/subgraph.template.yaml New subgraph template for Dispute Template Registry.
subgraph/dispute-template-registry/subgraph.yaml Added auto-generated comment and metadata fields.
subgraph/core-neo/subgraph.yaml Updated with new DisputeKitShutter data source; added header comment.
subgraph/core-university/subgraph.yaml Updated addresses and start blocks for PolicyRegistry and EvidenceModule.
subgraph/package.json Updated version to 0.16.4, drtVersion to 0.12.2.
subgraph/scripts/update.sh Added check for contract deployment artifacts; filtered out undeployed contracts before update.
contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol Updated version string; renamed and incremented reinitializer function.
contracts/scripts/keeperBot.ts Added multi-dispute-kit support; dynamic dispute kit resolution; shutter auto-reveal integration; improved GraphQL queries and shutdown logging.
web/package.json Added vite-plugin-static-copy and @shutter-network/shutter-sdk dependencies.
web/src/hooks/queries/useDisputeDetailsQuery.ts Extended GraphQL query to include disputeKit field in current round.
web/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsx New React component for appeal funding with wallet and contract interaction.
web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx New React component for Shutter appeal crowdfunding UI with popup and mini guide.
web/src/pages/Cases/CaseDetails/Appeal/index.tsx Conditional rendering of appeal components based on dispute kit type (classic or shutter).
web/src/pages/Cases/CaseDetails/Timeline.tsx Exported getDeadline function for external use.
web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx Fixed import paths for OptionsContainer.
web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx Fixed import paths for JustificationArea.
web/src/pages/Cases/CaseDetails/Voting/Classic/Vote.tsx Fixed import paths for OptionsContainer.
web/src/pages/Cases/CaseDetails/Voting/OptionsContainer.tsx Added error handling around async option selection callback.
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx New React component for Shutter commit phase with encryption and signing.
web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx New React component for Shutter reveal phase with stored reveal data and transaction handling.
web/src/pages/Cases/CaseDetails/Voting/Shutter/index.tsx New React component managing Shutter voting phases and rendering commit/reveal components.
web/src/pages/Cases/CaseDetails/Voting/index.tsx Conditional rendering of voting components based on dispute kit type; added dispute prop.
web/src/pages/Cases/CaseDetails/index.tsx Passed dispute prop to Voting component in routing.
web/src/utils/shutter.ts New utility module for Shutter encryption/decryption using Shutter API and SDK.
web/vite.config.js Added vite-plugin-static-copy plugin to copy Shutter SDK dist files during build.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ShutterScript (CLI)
    participant Shutter API
    participant Shutter SDK

    User->>ShutterScript: encrypt <message>
    ShutterScript->>Shutter API: fetchShutterData(decryptionTimestamp)
    Shutter API-->>ShutterScript: {pubkey, params, identity}
    ShutterScript->>Shutter SDK: encryptData(message, pubkey, params, sigma)
    Shutter SDK-->>ShutterScript: encryptedCommitment
    ShutterScript-->>User: {encryptedCommitment, identity}

    User->>ShutterScript: decrypt <encryptedMessage> <identity>
    ShutterScript->>Shutter API: fetchDecryptionKey(identity)
    Shutter API-->>ShutterScript: decryptionKey
    ShutterScript->>Shutter SDK: decrypt(encryptedMessage, decryptionKey)
    Shutter SDK-->>ShutterScript: decryptedMessage
    ShutterScript-->>User: decryptedMessage
Loading
sequenceDiagram
    participant Core
    participant DisputeKitClassic
    participant DisputeKitShutter
    participant Subgraph

    Core->>DisputeKitClassic: createDispute(...)
    Core->>DisputeKitShutter: createDispute(...)
    DisputeKitClassic-->>Core: DisputeKitCreated(id=1)
    DisputeKitShutter-->>Core: DisputeKitCreated(id=2)
    Core-->>Subgraph: DisputeKitCreated events
    Subgraph->>DisputeKitClassic: handle events (dynamic kit ID)
    Subgraph->>DisputeKitShutter: handle events (dynamic kit ID)
Loading

Possibly related PRs

Suggested reviewers

  • alcercu

Poem

🐇
In the warren of contracts, a Shutter appears,
Encrypting our votes, dispelling old fears.
Proxies and scripts now hop into view,
Subgraphs and IDs—dynamic and new!
With every commit and a cryptic event,
The rabbit rejoices—on progress, we're bent!

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jaybuidl jaybuidl added Type: Feature🗿 Package: Web Court web frontend Package: Contracts Court smart contracts Package: Bots Court bots Package: Subgraph Court subgraph labels Apr 30, 2025
Copy link

netlify bot commented Apr 30, 2025

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

Name Link
🔨 Latest commit d43e68e
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-university/deploys/6839b0caf478b20008c89839

Copy link

netlify bot commented Apr 30, 2025

Deploy Preview for kleros-v2-testnet ready!

Name Link
🔨 Latest commit d43e68e
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-testnet/deploys/6839b0ca2e562400084ff59e
😎 Deploy Preview https://deploy-preview-1965--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.

Copy link

netlify bot commented Apr 30, 2025

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

Name Link
🔨 Latest commit d43e68e
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-neo/deploys/6839b0cafdf06100088851fe

Copy link

netlify bot commented Apr 30, 2025

Deploy Preview for kleros-v2-testnet-devtools ready!

Name Link
🔨 Latest commit d43e68e
🔍 Latest deploy log https://app.netlify.com/projects/kleros-v2-testnet-devtools/deploys/6839b0cab01da20008da5bfa
😎 Deploy Preview https://deploy-preview-1965--kleros-v2-testnet-devtools.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.

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

🧹 Nitpick comments (5)
contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol (1)

439-453: Added virtual hashVote function for extensible vote hashing

This new hashVote function is a key architectural change that enables the Shutter integration:

  1. It's declared as virtual so derived contracts can override it
  2. The base implementation hashes only _choice and _salt (backward compatible)
  3. It accepts but doesn't use the _justification parameter in the base implementation
  4. The comment explicitly indicates that derived contracts may use the unused parameters

This design enables the specialized Shutter implementation to include the justification in the hash calculation.

This is a well-designed extension point that maintains backward compatibility while enabling new functionality in derived contracts. The virtual function pattern allows for specialized implementations like DisputeKitShutter to include the justification in the hash while keeping the base functionality unchanged.

contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol (2)

30-33: Unnecessary external self-call incurs gas & re-entrancy risk

this.castCommit(...) performs an external call to the same contract, paying
extra gas for ABI encoding/decoding and exposing the function to re-entrancy
checks (albeit low-risk here).

Since castCommit is declared external, you can call it internally without
this by using the fully-qualified function name:

-        this.castCommit(_coreDisputeID, _voteIDs, _commit);
+        castCommit(_coreDisputeID, _voteIDs, _commit);

This is cheaper and avoids the external-call context switch.


85-92: Natspec parameter order does not match function signature

The docstring lists the parameters as (_choice, _justification, _salt)
while the signature is (_choice, _salt, _justification).
This can confuse automated doc generators.

- * @param _justification The justification for the vote
- * @param _salt A random salt for commitment
+ * @param _salt A random salt for commitment
+ * @param _justification The justification for the vote
contracts/scripts/shutter.ts (2)

33-82: Missing retry / back-off on Shutter API call

fetchShutterData fails the whole encryption flow on the first transient HTTP
error. Consider adding a simple exponential back-off or at least one retry to
improve UX when the Shutter API momentarily flakes.

Not blocking, but worth considering.


160-186: Leaking raw Shutter keys in logs

encrypt() logs the eon key, identity and sigma to stdout. These are
cryptographically sensitive materials and should not be printed in production
mode.

Recommend gating the console.log statements behind an environment variable
(e.g. DEBUG) or removing them entirely.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01edde5 and 8e46e05.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (7)
  • contracts/package.json (1 hunks)
  • contracts/scripts/shutter.ts (1 hunks)
  • contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol (1 hunks)
  • contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol (3 hunks)
  • contracts/src/arbitration/dispute-kits/DisputeKitGated.sol (1 hunks)
  • contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol (1 hunks)
  • contracts/test/foundry/KlerosCore.t.sol (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-university
  • GitHub Check: Header rules - kleros-v2-university
  • GitHub Check: Pages changed - kleros-v2-university
  • GitHub Check: SonarCloud
  • GitHub Check: Analyze (javascript)
  • GitHub Check: contracts-testing
🔇 Additional comments (8)
contracts/src/arbitration/dispute-kits/DisputeKitGated.sol (1)

30-30: Version update from 0.8.0 to 0.9.0

The version constant has been updated as part of the broader Shutter integration changes across dispute kit contracts.

contracts/package.json (1)

159-160:

❓ Verification inconclusive

Added Shutter SDK and related dependencies

These new dependencies are required to support the Shutter Network integration for encrypted commitments:

  1. @shutter-network/shutter-sdk - Core SDK for Shutter Network functionality
  2. isomorphic-fetch - Cross-environment fetch implementation

Note that the Shutter SDK is at version 0.0.1, which suggests it's in early development.


🏁 Script executed:

#!/bin/bash
# Check for newer versions of the Shutter SDK and potential issues

echo "Checking for latest version of @shutter-network/shutter-sdk"
npm view @shutter-network/shutter-sdk versions

echo "Checking for any reported issues or vulnerabilities"
npm audit --json | grep -i shutter

Length of output: 682


🏁 Script executed:

#!/bin/bash
# Generate a lockfile to enable auditing
npm install --package-lock-only

echo "Re-checking for any reported issues or vulnerabilities after lockfile creation"
npm audit --json | grep -i shutter

Length of output: 796


Verify Shutter SDK and Fetch Dependency
The PR adds:

  • @shutter-network/shutter-sdk @ 0.0.1
  • isomorphic-fetch @ ^3.0.0

Because this project uses workspace-protocols, running npm audit in CI failed—please manually:
• Confirm there are no known vulnerabilities in these packages (e.g. via a local npm audit or yarn audit).
• Verify that 0.0.1 of the Shutter SDK is the intended stable release (bump when a newer stable version is available).

contracts/src/arbitration/dispute-kits/DisputeKitClassicBase.sol (3)

241-241: Improved parameter documentation

The _commit parameter documentation is updated to more clearly indicate it's a "commitment hash" rather than a general commit.


286-286: Added vote hash calculation for verification

The code now uses the new hashVote function to generate a vote hash, which enables customized hashing strategies in derived contracts.


292-293: Updated vote hash verification logic and error message

The verification logic now compares against the computed voteHash instead of directly hashing the choice and salt. The error message has been updated accordingly.

contracts/src/arbitration/dispute-kits/DisputeKitClassic.sol (1)

14-14: Version update from 0.8.0 to 0.9.0

The version constant has been updated as part of the broader Shutter integration changes across dispute kit contracts.

contracts/src/arbitration/dispute-kits/DisputeKitShutter.sol (1)

56-72: Event is emitted even when castCommit reverts

Because castCommitShutter emits CommitCastShutter after the external call,
a failure inside castCommit will revert the whole transaction and the event
will not be emitted.
If your intention was to guarantee the event only when the commit succeeds,
this is fine. If you need to record the attempt, move the emit in a
try/catch block.

contracts/scripts/shutter.ts (1)

88-127: Error parsing assumes specific JSON structure

fetchDecryptionKey throws if jsonResponse.message is undefined, but the
Shutter API returns the payload at jsonResponse root according to the spec
used earlier. Align the two helpers or add a type-guard to avoid false
positives.

@jaybuidl jaybuidl linked an issue Apr 30, 2025 that may be closed by this pull request
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: 0

🧹 Nitpick comments (2)
contracts/test/arbitration/index.ts (2)

65-65: Test function updated but consider expanding test coverage

The deployContracts function is properly updated to include DisputeKitShutter. However, while the deployment and registration of DisputeKitShutter are verified, there are no specific tests for its unique functionality related to Shutter integration.

Consider adding specific test cases for the DisputeKitShutter's encryption/decryption capabilities and its integration with the Shutter API to ensure complete coverage.

Also applies to: 71-71, 73-73


6-6: Update test description to reflect expanded scope

The describe block is still labeled "DisputeKitClassic" even though the tests now cover both classic and shutter dispute kits. Consider renaming this to "DisputeKits" or "Arbitration System" to more accurately reflect the expanded test scope.

-describe("DisputeKitClassic", async () => {
+describe("DisputeKits", async () => {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 8e46e05 and 98ec3ba.

📒 Files selected for processing (6)
  • contracts/README.md (1 hunks)
  • contracts/deploy/00-home-chain-arbitration.ts (1 hunks)
  • contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json (1 hunks)
  • contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Proxy.json (1 hunks)
  • contracts/src/proxy/KlerosProxies.sol (1 hunks)
  • contracts/test/arbitration/index.ts (2 hunks)
✅ Files skipped from review due to trivial changes (4)
  • contracts/README.md
  • contracts/src/proxy/KlerosProxies.sol
  • contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter_Proxy.json
  • contracts/deployments/arbitrumSepoliaDevnet/DisputeKitShutter.json
⏰ Context from checks skipped due to timeout of 90000ms (15)
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet-devtools
  • GitHub Check: Header rules - kleros-v2-testnet-devtools
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet-devtools
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: SonarCloud
  • GitHub Check: contracts-testing
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
contracts/deploy/00-home-chain-arbitration.ts (1)

99-106: Good integration of DisputeKitShutter

The implementation properly deploys the new DisputeKitShutter contract as upgradeable and registers it with the core contract. This aligns with the PR objective of introducing Shutter API integration for message encryption in the dispute kit system.

Consider adding a brief comment explaining what DisputeKitShutter is for and how it differs from DisputeKitClassic, which would improve code documentation.

contracts/test/arbitration/index.ts (4)

3-3: LGTM!

Correctly imports the new DisputeKitShutter type.


9-9: LGTM!

Properly declares the disputeKitShutter variable for use in tests.


13-13: LGTM!

Updated to destructure the disputeKitShutter from the return value of deployContracts().


18-18: LGTM!

Test correctly verifies both dispute kits are created with the expected IDs and addresses.

Also applies to: 21-22

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: 0

♻️ Duplicate comments (4)
contracts/scripts/keeperBotShutter.ts (4)

40-48: Add validation to prevent runtime crashes.

The decode function lacks validation for the message format, which can cause runtime errors if the message is malformed.

Apply this diff to add proper validation:

-const decode = (message: string) => {
-  const SEPARATOR = "-";
-  const [choice, salt, justification] = message.split(SEPARATOR);
-  return {
-    choice: BigInt(choice),
-    salt,
-    justification,
-  };
-};
+const decode = (message: string) => {
+  const SEPARATOR = "-";
+  const parts = message.split(SEPARATOR);
+  if (parts.length !== 3) {
+    throw new Error(`Malformed decrypted message: expected 3 parts, got ${parts.length} in "${message}"`);
+  }
+  const [choice, salt, justification] = parts;
+  try {
+    return {
+      choice: BigInt(choice),
+      salt,
+      justification,
+    };
+  } catch (e) {
+    throw new Error(`Invalid choice value "${choice}" in message "${message}": ${e}`);
+  }
+};

55-58: Add validation to prevent silent failures.

The parseGraphVoteId function can silently produce NaN values if the input is malformed, leading to downstream issues.

Apply this diff to add proper validation:

-const parseGraphVoteId = (graphVoteId: string) => {
-  const [disputeKitID, localDisputeID, localRoundID, voteID] = graphVoteId.split("-").map(Number);
-  return { disputeKitID, localDisputeID, localRoundID, voteID };
-};
+const parseGraphVoteId = (graphVoteId: string) => {
+  const parts = graphVoteId.split("-");
+  if (parts.length !== 4) {
+    throw new Error(`Invalid graphVoteId format: expected 4 parts, got ${parts.length} in "${graphVoteId}"`);
+  }
+  const [dk, ld, lr, v] = parts.map((n) => {
+    const parsed = Number(n);
+    if (Number.isNaN(parsed)) {
+      throw new Error(`Invalid graphVoteId segment "${n}" in "${graphVoteId}"`);
+    }
+    return parsed;
+  });
+  return { disputeKitID: dk, localDisputeID: ld, localRoundID: lr, voteID: v };
+};

169-177: Fix potential undefined access.

The code accesses filteredLocalRounds[0] without checking if the array is empty, which can cause runtime errors.

Apply this diff to add proper validation:

-  const disputeVotes = filteredRounds.map((round) => {
-    const dk = round.dispute.disputeKitDispute[0];
-    const idx = Number(dk.currentLocalRoundIndex);
-    const filteredLocalRounds = dk.localRounds.filter((_, i) => i === idx);
-    return {
-      coreDispute: dk.coreDispute,
-      votes: filteredLocalRounds[0].votes,
-    };
-  });
+  const disputeVotes = filteredRounds.map((round) => {
+    const dk = round.dispute.disputeKitDispute[0];
+    const idx = Number(dk.currentLocalRoundIndex);
+    const currentLocalRound = dk.localRounds[idx];
+    if (!currentLocalRound) {
+      throw new Error(`No local round found at index ${idx} for dispute ${dk.coreDispute.id}`);
+    }
+    return {
+      coreDispute: dk.coreDispute,
+      votes: currentLocalRound.votes,
+    };
+  });

211-217: Fix event filter for indexed bytes32 parameter.

Using getBytes(vote.commit) may break indexed event lookup if the commit is a bytes32 indexed parameter.

Apply this diff to fix the filter:

-      const filter = disputeKitShutter.filters.CommitCastShutter(coreDispute.id, vote.juror.id, getBytes(vote.commit));
+      const filter = disputeKitShutter.filters.CommitCastShutter(coreDispute.id, vote.juror.id, vote.commit);
🧹 Nitpick comments (3)
contracts/scripts/keeperBotShutter.ts (3)

295-296: Remove unnecessary continue statement.

The continue statement is unnecessary as it's at the end of the loop iteration.

Apply this diff to remove the unnecessary statement:

       } catch (e) {
         logger.error(e, "Failure");
-        continue;
       }
🧰 Tools
🪛 Biome (1.9.4)

[error] 296-296: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)


194-195: Remove debug console.log in production code.

The console.log statement should be replaced with proper logging or removed for production.

Apply this diff:

   const shutterDisputes = await getShutterDisputesToReveal(disputeKitShutter);
-  console.log(JSON.stringify(shutterDisputes, null, 2));
+  logger.debug(`Found ${shutterDisputes.length} Shutter disputes to reveal`);

305-305: Remove hardcoded dispute IDs from production code.

The hardcoded dispute IDs in the main function should be parameterized or removed for production use.

Apply this diff to make it configurable:

 async function main() {
   logger.debug("Starting...");
   const { disputeKitShutter } = await getContracts();
-  await shutterAutoReveal(disputeKitShutter, ["44", "45", "51", "54"]);
+  const disputesToSkip = env.optional("DISPUTES_TO_SKIP", "").split(",").map(s => s.trim());
+  await shutterAutoReveal(disputeKitShutter, disputesToSkip);
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bafbd14 and 99f9852.

📒 Files selected for processing (2)
  • contracts/scripts/keeperBot.ts (7 hunks)
  • contracts/scripts/keeperBotShutter.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
contracts/scripts/keeperBotShutter.ts

[error] 296-296: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)

⏰ Context from checks skipped due to timeout of 90000ms (15)
  • GitHub Check: SonarCloud
  • GitHub Check: contracts-testing
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Redirect rules - kleros-v2-university
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-university
  • GitHub Check: Redirect rules - kleros-v2-testnet-devtools
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-university
  • 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
🔇 Additional comments (14)
contracts/scripts/keeperBot.ts (14)

3-3: LGTM!

The import addition for DisputeKitShutter is correctly placed and follows the existing import pattern.


7-7: LGTM!

The import of shutterAutoReveal function is correctly added for the new Shutter integration functionality.


10-10: LGTM!

The environment variable SHUTTER_AUTO_REVEAL_ONLY provides a clean way to run only the Shutter auto-reveal functionality when needed.


81-106: LGTM! Well-designed abstraction for multi-dispute kit support.

The getDisputeKit function provides excellent abstraction for resolving dispute kits dynamically. The logic correctly:

  • Queries the core contract for round information
  • Maps dispute kit addresses to contract instances
  • Returns both the dispute kit instance and local IDs
  • Includes proper error handling for unknown dispute kits

109-122: LGTM! Improved GraphQL query structure.

The refactoring to use gql template literals and typed generics improves code clarity and type safety compared to string-based queries.


126-147: LGTM! Consistent GraphQL query improvements.

The same beneficial refactoring pattern is applied consistently across all GraphQL queries in the file.


151-164: LGTM! Consistent GraphQL query improvements.

The query structure improvements are consistently applied throughout the file.


172-188: LGTM! Consistent GraphQL query improvements.

The refactoring maintains consistency across all GraphQL operations.


364-421: LGTM! Well-integrated dynamic dispute kit resolution.

The withdrawAppealContribution function correctly uses the new getDisputeKit helper to:

  • Dynamically resolve the appropriate dispute kit
  • Use local dispute and round IDs for the specific kit
  • Maintain proper logging with core dispute/round references

The integration is seamless and maintains backward compatibility.


510-513: LGTM! Proper graceful shutdown.

The new shutdown function provides clean logging and ensures proper exit with a reasonable delay for log message processing.


516-516: LGTM! Correct contract destructuring.

The addition of disputeKitShutter to the destructuring is correct for the new functionality.


557-563: LGTM! Clean integration of Shutter auto-reveal functionality.

The integration correctly:

  • Calls shutterAutoReveal with the appropriate parameters
  • Provides early exit when SHUTTER_AUTO_REVEAL_ONLY is true
  • Maintains proper logging and shutdown flow

685-686: LGTM! Correct dynamic dispute kit resolution in repartitions.

The change correctly replaces the hardcoded disputeKitClassic usage with dynamic dispute kit resolution, enabling proper support for both classic and Shutter dispute kits during repartition execution.


750-750: LGTM! Proper cleanup integration.

The call to the new shutdown function ensures proper cleanup and logging before exit.

@jaybuidl
Copy link
Member Author

Follow-up feature: #2016

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: 10

🧹 Nitpick comments (7)
web/src/pages/Cases/CaseDetails/Voting/index.tsx (1)

58-58: Consider making the dispute prop required.

The dispute prop is marked as optional in the interface but appears to be essential for Shutter dispute kit functionality.

If the dispute data is always expected to be available when this component is rendered, consider making it required:

interface IVoting {
  arbitrable?: `0x${string}`;
  currentPeriodIndex: number;
-  dispute: DisputeDetailsQuery["dispute"];
+  dispute: NonNullable<DisputeDetailsQuery["dispute"]>;
}

Or add runtime validation if it should remain optional.

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

1-17: Consider adding JSDoc documentation for the new component.

This is a new component with specific business logic for the Shutter dispute kit. Adding documentation would help other developers understand its purpose and usage.

Add JSDoc documentation:

+/**
+ * Shutter voting component that handles the commit and reveal phases
+ * for disputes using the Shutter dispute kit.
+ * 
+ * @param arbitrable - The arbitrable contract address
+ * @param setIsOpen - Function to control popup visibility
+ * @param dispute - The dispute data from the query
+ * @param currentPeriodIndex - The current period index of the dispute
+ */
const Shutter: React.FC<IShutter> = ({ arbitrable, setIsOpen, dispute, currentPeriodIndex }) => {
web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx (2)

8-9: Consider code reuse opportunities with the Classic appeal component.

The component imports Options from the Classic appeal but implements its own Fund component. This might indicate code duplication or inconsistent patterns.

Consider whether the appeal components can share more common logic:

  1. Shared base component: Create a common base that both Classic and Shutter can extend
  2. Shared utility functions: Extract common state management and popup logic
  3. Consistent component structure: Ensure both appeal types follow the same patterns

This would improve maintainability and reduce the risk of divergent behavior between dispute kit types.


1-49: Consider adding PropTypes or more detailed TypeScript interfaces.

The component interface is minimal but could benefit from more detailed type definitions for better development experience and error catching.

Enhance the interface with more detailed types:

interface IShutter {
+  /** Controls visibility of the appeal mini guide */
  isAppealMiniGuideOpen: boolean;
+  /** Function to toggle the appeal mini guide visibility */
  toggleAppealMiniGuide: () => void;
}

Additionally, consider if the component needs any error boundaries or loading states for better user experience.

web/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsx (1)

52-62: Consider simplifying the useNeedFund logic.

The conditional logic is complex and could benefit from clearer variable naming or breaking into smaller functions for better readability.

const useNeedFund = () => {
  const { loserSideCountdown } = useCountdownContext();
  const { fundedChoices, winningChoice } = useFundingContext();
+  
+  const hasActiveLoserCountdown = (loserSideCountdown ?? 0) > 0;
+  const hasUnfundedWinningChoice = !isUndefined(fundedChoices) &&
+    !isUndefined(winningChoice) &&
+    fundedChoices.length > 0 &&
+    !fundedChoices.includes(winningChoice);
+    
-  return (
-    (loserSideCountdown ?? 0) > 0 ||
-    (!isUndefined(fundedChoices) &&
-      !isUndefined(winningChoice) &&
-      fundedChoices.length > 0 &&
-      !fundedChoices.includes(winningChoice))
-  );
+  return hasActiveLoserCountdown || hasUnfundedWinningChoice;
};
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (1)

85-86: Consider security implications of localStorage for vote data.

Storing vote choice, salt, and justification in localStorage could pose privacy risks if the device is compromised. Consider if this sensitive data needs additional protection.

While localStorage is convenient for persistence, consider:

  1. Adding encryption for the stored vote data
  2. Implementing automatic cleanup after successful reveal
  3. Warning users about device security when storing sensitive voting information

The current implementation does clean up on successful reveal (line 83 in Reveal.tsx), which is good practice.

web/src/utils/shutter.ts (1)

33-33: Consider making API URL configurable.

The Shutter API URL is hardcoded, which makes it difficult to use different environments (development, staging, production) or handle API endpoint changes.

+ const SHUTTER_API_BASE_URL = process.env.REACT_APP_SHUTTER_API_URL || "https://shutter-api.shutter.network";
+
async function fetchShutterData(decryptionTimestamp: number): Promise<ShutterApiMessageData> {
  try {
    const identityPrefix = generateRandomBytes32();

-   const response = await fetch("https://shutter-api.shutter.network/api/register_identity", {
+   const response = await fetch(`${SHUTTER_API_BASE_URL}/api/register_identity`, {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99f9852 and 469039a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (17)
  • web/package.json (2 hunks)
  • web/src/hooks/queries/useDisputeDetailsQuery.ts (1 hunks)
  • web/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Appeal/index.tsx (2 hunks)
  • web/src/pages/Cases/CaseDetails/Timeline.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Vote.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/OptionsContainer.tsx (2 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/Shutter/index.tsx (1 hunks)
  • web/src/pages/Cases/CaseDetails/Voting/index.tsx (5 hunks)
  • web/src/pages/Cases/CaseDetails/index.tsx (1 hunks)
  • web/src/utils/shutter.ts (1 hunks)
  • web/vite.config.js (2 hunks)
✅ Files skipped from review due to trivial changes (5)
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Vote.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx
  • web/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsx
  • web/src/hooks/queries/useDisputeDetailsQuery.ts
  • web/package.json
🧰 Additional context used
🧠 Learnings (1)
web/vite.config.js (1)
Learnt from: kemuru
PR: kleros/kleros-v2#1994
File: web/vite.config.js:26-33
Timestamp: 2025-05-23T17:47:39.947Z
Learning: The viteStaticCopy plugin configuration in web/vite.config.js correctly copies Shutter SDK files from the installed node_modules location. The path resolve(__dirname, "../node_modules/@shutter-network/shutter-sdk/dist/*") works when dependencies are properly installed, despite initial analysis suggesting otherwise due to sandbox environment limitations.
🧬 Code Graph Analysis (3)
web/src/pages/Cases/CaseDetails/Voting/index.tsx (2)
web/src/hooks/queries/useDisputeDetailsQuery.ts (1)
  • DisputeDetailsQuery (8-8)
web/src/consts/index.ts (1)
  • getDisputeKitName (46-56)
web/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsx (1)
web-devtools/src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (3-22)
web/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsx (4)
web/src/hooks/queries/useDisputeDetailsQuery.ts (1)
  • DisputeDetailsQuery (8-8)
web-devtools/src/hooks/useLocalStorage.ts (1)
  • useLocalStorage (3-22)
web/src/pages/Cases/CaseDetails/Timeline.tsx (1)
  • getDeadline (143-154)
web/src/utils/shutter.ts (1)
  • encrypt (149-173)
⏰ Context from checks skipped due to timeout of 90000ms (16)
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-university
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-university
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: contracts-testing
  • GitHub Check: SonarCloud
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-university
  • GitHub Check: Mend Security Check
🔇 Additional comments (14)
web/src/pages/Cases/CaseDetails/index.tsx (1)

82-82: LGTM! Clean integration for Shutter dispute kit support.

Adding the dispute prop to the Voting component enables conditional rendering between Classic and Shutter voting interfaces based on the dispute kit type. This change aligns well with the broader Shutter dispute kit integration.

web/src/pages/Cases/CaseDetails/Timeline.tsx (1)

143-154: Good refactoring for code reusability.

Exporting the getDeadline function enables its reuse across the codebase, particularly for keeper bots and scripts that need consistent deadline calculations for Shutter dispute kit functionality. The function logic remains unchanged and correctly calculates deadlines based on period changes.

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

73-83: Excellent error handling improvement.

The try-catch-finally block ensures proper state cleanup regardless of whether handleSelection succeeds or fails. The simplified dependency array is also correct since handleSelection is the only unstable dependency. This robust error handling is especially important for the voting flow in the new Shutter dispute kit.

web/vite.config.js (2)

5-9: Necessary imports for Shutter SDK integration.

The added imports support the viteStaticCopy plugin configuration and ES module compatibility for path resolution.


26-33: Correct Shutter SDK static copy configuration.

Based on previous validation in the retrieved learnings, this configuration correctly copies Shutter SDK files from the installed node_modules location to support frontend encryption/decryption functionality. The path resolution and plugin configuration are appropriate for the Shutter dispute kit integration.

web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx (1)

2-2:

✅ Verification successful

Verify context compatibility between Classic and Shutter appeal flows.

The component uses useSelectedOptionContext from the classic appeal context, but it's unclear if this context is appropriate for Shutter appeals or if they have different option structures.

Verify that the classic appeal context is compatible with Shutter appeal requirements:


🏁 Script executed:

#!/bin/bash
# Description: Check if the selected option context is used consistently across appeal components
# and verify the Options component structure for compatibility

# Find the classic appeal context implementation
ast-grep --pattern 'export const $_ = createContext'

# Check how Options component is used in both Classic and Shutter appeals
rg -A 10 "Options.*setAmount" --type tsx

Length of output: 125


I’ll dig deeper into the context and Options usage to confirm compatibility.


🏁 Script executed:

#!/bin/bash
# 1. Locate where useSelectedOptionContext is defined
rg -n "export (const|function) useSelectedOptionContext" -g "*.ts*" -A5

# 2. Find all createContext calls to see the context’s shape
rg -n "createContext" -g "*.ts" -A5

# 3. Find the Options component implementation
rg -n "export (const|function) Options" -g "*.tsx" -A5

# 4. See how Options is used in the Classic appeal flow
rg -n "<Options" -g "*.tsx" | grep -i Classic

# 5. See how Options is used in the Shutter appeal flow
rg -n "<Options" -g "*.tsx" | grep -i Shutter

Length of output: 1791


I’ll inspect the context definitions and the Options component implementation to confirm they align for Shutter.


🏁 Script executed:

#!/bin/bash
# 1. Locate all createContext calls (to see context shapes)
rg -n "createContext" -g "*.{ts,tsx}" -A5

# 2. Show SelectedOptionContext and OptionsContext definitions
rg -n "SelectedOptionContext" -g "web/src/hooks/useClassicAppealContext.tsx" -A10
rg -n "OptionsContext" -g "web/src/hooks/useClassicAppealContext.tsx" -A10

# 3. Inspect the Classic Options component implementation
rg -n "export const Options" -g "web/src/pages/Cases/CaseDetails/Appeal/Classic/Options/*.tsx" -A20

# 4. Inspect Shutter index usage of useSelectedOptionContext
rg -n "useSelectedOptionContext" -g "web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx" -A20

Length of output: 21940


Gathering where the provider is defined and how Classic wraps its children:


🏁 Script executed:

#!/bin/bash
# 1. Find the exported Provider in the classic context file
rg -n "export .*Provider" -g "web/src/hooks/useClassicAppealContext.tsx"

# 2. Inspect the top of the Classic appeal index to see if it uses that Provider
sed -n '1,60p' web/src/pages/Cases/CaseDetails/Appeal/Classic/index.tsx

Length of output: 1954


Context compatibility confirmed
Both Classic and Shutter flows share the same SelectedOptionContext shape and OptionsContext, and the Options component’s API (accepting setAmount and reading the selected option) is identical in each. Shutter’s use of selectedOption?.title mirrors Classic’s popup logic, so no context changes are needed.

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

40-52: Good defensive programming with data validation.

The parsing logic properly validates the required fields (salt, choice, justification) and includes comprehensive error handling. This prevents runtime errors when localStorage contains malformed data.


71-93: Well-structured async operation with proper error handling.

The reveal handling includes appropriate loading states, error checking, and cleanup operations. The transaction flow with toast notifications provides good user feedback.


33-33:

✅ Verification successful

Verify the useLocalStorage hook signature.

The code destructures three values [storedData, _, removeStoredData] from useLocalStorage, but the relevant code snippet shows the hook only returns two values [storedValue, setValue]. Please verify that the actual hook implementation in this project supports a third value for removal functionality.


🏁 Script executed:

#!/bin/bash
# Search for the useLocalStorage hook implementation to verify its signature
rg -A 10 "export.*useLocalStorage" --type typescript

Length of output: 89


Let’s locate where useLocalStorage is imported and defined in the repo:


🏁 Script executed:

#!/bin/bash
# Find all references to useLocalStorage to identify its import location and implementation
rg -n "useLocalStorage" .

Length of output: 2685


useLocalStorage destructuring is correct

You’re importing useLocalStorage from react-use, which returns a three-element tuple [value, setValue, removeValue]. The third element is indeed the removal function, so the [storedData, _, removeStoredData] destructuring is valid. No changes required here.

web/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsx (1)

94-110: Robust input validation and user experience.

The debouncing, balance checking, and comprehensive disable conditions provide excellent user experience and prevent common input errors.

web/src/utils/shutter.ts (4)

28-72: Excellent error handling and validation.

The function includes comprehensive error handling for different failure scenarios, proper JSON parsing validation, and meaningful error messages. The response validation ensures required data is present.


137-142: Secure random number generation.

Good use of crypto.getRandomValues() for generating cryptographically secure random bytes. The implementation correctly converts to hex format.


99-108: User-friendly error handling for timing constraints.

The specific handling of "timestamp not reached yet" errors provides clear guidance to users about when they can attempt decryption again. This improves the user experience significantly.


149-173: Verify decryption delay calculation consistency.

Ensure that the decryption delay calculation here aligns with the usage in the Commit component, particularly the 300-second buffer addition mentioned in Commit.tsx line 91.

The encrypt function accepts decryptionDelay as a parameter and adds it to the current timestamp. Verify that callers are consistently handling the timing calculations, especially the buffer time mentioned in the Commit component.

alcercu
alcercu previously approved these changes May 29, 2025
Copy link
Contributor

@alcercu alcercu left a comment

Choose a reason for hiding this comment

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

Frontend part lgtm 🚀

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: 0

♻️ Duplicate comments (5)
contracts/scripts/keeperBotShutter.ts (4)

40-48: ⚠️ Potential issue

Add input validation to prevent runtime crashes.

The decode function lacks validation for the message format, which can cause runtime errors if the message is malformed.

Apply this diff to add proper validation:

-const decode = (message: string) => {
-  const SEPARATOR = "-";
-  const [choice, salt, justification] = message.split(SEPARATOR);
-  return {
-    choice: BigInt(choice),
-    salt,
-    justification,
-  };
-};
+const decode = (message: string) => {
+  const SEPARATOR = "-";
+  const parts = message.split(SEPARATOR);
+  if (parts.length !== 3) {
+    throw new Error(`Malformed decrypted message (${message})`);
+  }
+  const [choice, salt, justification] = parts;
+  try {
+    return {
+      choice: BigInt(choice),
+      salt,
+      justification,
+    };
+  } catch (e) {
+    throw new Error(`Invalid choice value in message (${message}): ${e}`);
+  }
+};

55-58: ⚠️ Potential issue

Guard against malformed Graph vote IDs.

Number() can silently yield NaN, propagating bad data downstream.

Apply this diff to add validation:

-const parseGraphVoteId = (graphVoteId: string) => {
-  const [disputeKitID, localDisputeID, localRoundID, voteID] = graphVoteId.split("-").map(Number);
-  return { disputeKitID, localDisputeID, localRoundID, voteID };
-};
+const parseGraphVoteId = (graphVoteId: string) => {
+  const [dk, ld, lr, v] = graphVoteId.split("-").map((n) => {
+    const parsed = Number(n);
+    if (Number.isNaN(parsed)) throw new Error(`Invalid graphVoteId segment "${n}" in "${graphVoteId}"`);
+    return parsed;
+  });
+  return { disputeKitID: dk, localDisputeID: ld, localRoundID: lr, voteID: v };
+};

169-177: ⚠️ Potential issue

Prevent undefined access when no current local round exists.

filteredLocalRounds[0] will throw when localRounds is empty, causing the script to abort.

Apply this diff to add safety checks:

-  const disputeVotes = filteredRounds.map((round) => {
-    const dk = round.dispute.disputeKitDispute[0];
-    const idx = Number(dk.currentLocalRoundIndex);
-    const filteredLocalRounds = dk.localRounds.filter((_, i) => i === idx);
-    return {
-      coreDispute: dk.coreDispute,
-      votes: filteredLocalRounds[0].votes,
-    };
-  });
+  const disputeVotes = filteredRounds.map((round) => {
+    const dk = round.dispute.disputeKitDispute[0];
+    const idx = Number(dk.currentLocalRoundIndex);
+    const currentLocalRound = dk.localRounds[idx];
+    if (!currentLocalRound) return null; // skip gracefully
+    return {
+      coreDispute: dk.coreDispute,
+      votes: currentLocalRound.votes,
+    };
+  }).filter(Boolean);

213-219: ⚠️ Potential issue

Fix event filter to match indexed bytes32 parameter.

getBytes(vote.commit) may break indexed event lookup if it returns a byte array that doesn't match the topic encoding.

Apply this diff to fix the filter:

-      const filter = disputeKitShutter.filters.CommitCastShutter(coreDispute.id, vote.juror.id, getBytes(vote.commit));
+      const filter = disputeKitShutter.filters.CommitCastShutter(coreDispute.id, vote.juror.id, vote.commit);
contracts/scripts/shutter.ts (1)

156-161: ⚠️ Potential issue

Fix Node.js compatibility issue with crypto API.

crypto.getRandomValues is a Web Crypto API that's not available in Node.js environments.

Apply this diff to use Node.js crypto instead:

-function generateRandomBytes32(): `0x${string}` {
-  return ("0x" +
-    crypto
-      .getRandomValues(new Uint8Array(32))
-      .reduce((acc, byte) => acc + byte.toString(16).padStart(2, "0"), "")) as Hex;
-}
+function generateRandomBytes32(): `0x${string}` {
+  return ("0x" + crypto.randomBytes(32).toString("hex")) as Hex;
+}
🧹 Nitpick comments (1)
contracts/scripts/keeperBotShutter.ts (1)

298-298: Remove unnecessary continue statement.

The continue statement at the end of the catch block is unnecessary as it's the last statement in the loop iteration.

-        continue;
🧰 Tools
🪛 Biome (1.9.4)

[error] 298-298: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bbe6df and c3f6611.

📒 Files selected for processing (3)
  • contracts/scripts/keeperBot.ts (8 hunks)
  • contracts/scripts/keeperBotShutter.ts (1 hunks)
  • contracts/scripts/shutter.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
contracts/scripts/keeperBotShutter.ts

[error] 298-298: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)

⏰ Context from checks skipped due to timeout of 90000ms (13)
  • GitHub Check: contracts-testing
  • GitHub Check: Redirect rules - kleros-v2-university
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-university
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-university
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
🔇 Additional comments (5)
contracts/scripts/keeperBot.ts (3)

91-116: Excellent dynamic dispute kit resolution implementation!

The getDisputeKit function properly handles the transition from hardcoded dispute kit usage to dynamic resolution based on the dispute kit address. This enables seamless support for both Classic and Shutter dispute kits within the same keeper bot.


598-612: Well-integrated auto-reveal functionality for Shutter disputes.

The auto-reveal section is properly placed in the workflow and includes proper period transition handling. The SHUTTER_AUTO_REVEAL_ONLY flag provides useful operational flexibility for running dedicated auto-reveal instances.


374-431: Properly updated withdrawal logic for dynamic dispute kits.

The withdrawAppealContribution function correctly uses the dynamically resolved dispute kit and local IDs, ensuring compatibility with both Classic and Shutter dispute kits.

contracts/scripts/shutter.ts (2)

168-194: Well-structured encryption function with comprehensive error handling.

The encrypt function properly handles the Shutter API workflow, including timestamp calculation, data fetching, format validation, and encryption. The logging provides good visibility into the encryption process.


202-215: Robust decryption implementation with proper error handling.

The decrypt function correctly handles the decryption workflow with appropriate error handling and format validation. The hex string conversion is properly handled.

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: 0

♻️ Duplicate comments (4)
contracts/scripts/keeperBotShutter.ts (4)

40-48: Add basic validation to decode() to avoid runtime crashes

If an unexpected message arrives (missing separator / wrong format), BigInt(choice) will throw and bring the whole bot down.


55-58: Guard against malformed Graph vote IDs

Number() can silently yield NaN, propagating bad data down-stream. Add validation and a clear error message.


169-177: Possible undefined access when no current local round exists

filteredLocalRounds[0] will throw when localRounds is empty, causing the whole script to abort.


215-221: getBytes(vote.commit) may break indexed event lookup

_commit is likely an indexed bytes32. Passing a byte array returned by getBytes may not match the topic encoding, leading to empty event sets and missed reveals.

🧹 Nitpick comments (2)
contracts/scripts/keeperBotShutter.ts (2)

300-300: Remove unnecessary continue statement.

The continue statement at the end of the catch block is unnecessary since it's the last statement in the loop iteration.

-        continue;
🧰 Tools
🪛 Biome (1.9.4)

[error] 300-300: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)


309-309: Consider making dispute skip list configurable.

The hardcoded dispute IDs to skip reduce maintainability. Consider using an environment variable or configuration file.

+const DISPUTES_TO_SKIP = env.optional("DISPUTES_TO_SKIP", "44,45,51,54").split(",");
-  await shutterAutoReveal(disputeKitShutter, ["44", "45", "51", "54"]);
+  await shutterAutoReveal(disputeKitShutter, DISPUTES_TO_SKIP);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3f6611 and 959b765.

📒 Files selected for processing (1)
  • contracts/scripts/keeperBotShutter.ts (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
contracts/scripts/keeperBotShutter.ts

[error] 300-300: Unnecessary continue statement

Unsafe fix: Delete the unnecessary continue statement

(lint/correctness/noUnnecessaryContinue)

⏰ Context from checks skipped due to timeout of 90000ms (15)
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-university
  • GitHub Check: Header rules - kleros-v2-university
  • GitHub Check: Pages changed - kleros-v2-university
  • GitHub Check: contracts-testing
  • GitHub Check: SonarCloud
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-neo
🔇 Additional comments (1)
contracts/scripts/keeperBotShutter.ts (1)

1-34: Well-structured imports and configuration setup.

The imports, environment variable handling, and contract retrieval logic are properly implemented with appropriate error handling.

Copy link

codeclimate bot commented May 30, 2025

Code Climate has analyzed commit d43e68e and detected 124 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 7
Duplication 7
Style 110

View more on Code Climate.

Copy link

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

🧹 Nitpick comments (2)
web/src/utils/shutter.ts (2)

45-46: Remove incomplete logging comment

There's an incomplete comment about logging the response status with no actual implementation.

-    // Log the response status
-
+    

123-131: Add JSDoc documentation for helper functions

The helper functions lack documentation which would improve maintainability.

+/**
+ * Ensures a string is a valid hex string with 0x prefix
+ * @param hexString The hex string to validate
+ * @returns The validated hex string with 0x prefix
+ * @throws Error if hexString is undefined, null, or invalid
+ */
 function ensureHexString(hexString: string | undefined): `0x${string}` {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 959b765 and d43e68e.

📒 Files selected for processing (8)
  • contracts/.env.example (1 hunks)
  • web/.env.devnet-neo.public (1 hunks)
  • web/.env.devnet-university.public (1 hunks)
  • web/.env.devnet.public (1 hunks)
  • web/.env.local.public (1 hunks)
  • web/.env.mainnet-neo.public (1 hunks)
  • web/.env.testnet.public (1 hunks)
  • web/src/utils/shutter.ts (1 hunks)
✅ Files skipped from review due to trivial changes (7)
  • web/.env.testnet.public
  • web/.env.devnet-neo.public
  • web/.env.devnet.public
  • web/.env.local.public
  • web/.env.devnet-university.public
  • web/.env.mainnet-neo.public
  • contracts/.env.example
⏰ Context from checks skipped due to timeout of 90000ms (15)
  • GitHub Check: contracts-testing
  • GitHub Check: SonarCloud
  • GitHub Check: Redirect rules - kleros-v2-neo
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-neo
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Redirect rules - kleros-v2-testnet-devtools
  • GitHub Check: Header rules - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-neo
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Pages changed - kleros-v2-testnet
  • GitHub Check: Header rules - kleros-v2-testnet-devtools
  • GitHub Check: Pages changed - kleros-v2-testnet-devtools
🔇 Additional comments (2)
web/src/utils/shutter.ts (2)

1-21: LGTM: Well-structured interfaces and imports

The imports and interface definitions are clean and properly typed. The interfaces correctly model the Shutter API response structure with appropriate optional fields.


137-142:

✅ Verification successful

Verify cryptographic randomness availability

The function uses crypto.getRandomValues without checking if it's available. Consider adding a fallback or validation.


🏁 Script executed:

#!/bin/bash
# Description: Check if there are any compatibility concerns with crypto.getRandomValues in the codebase
# Expected: Find usage patterns and any existing fallbacks

rg -A 3 -B 3 "crypto\.getRandomValues|window\.crypto" --type ts --type tsx --type js --type jsx

Length of output: 125


🏁 Script executed:

#!/bin/bash
# Re-run search without unsupported --type flags, targeting relevant file extensions
rg -A 3 -B 3 'crypto\.getRandomValues|window\.crypto' \
  --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'

Length of output: 113


No fallback needed for crypto.getRandomValues in browser code

After searching the repository, there are no other uses of crypto.getRandomValues or any existing fallbacks—and since this utility lives under web/src, it targets modern browsers where crypto.getRandomValues is universally available. No changes required here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: Bots Court bots Package: Contracts Court smart contracts Package: Subgraph Court subgraph Package: Web Court web frontend Type: Feature🗿
Projects
None yet
3 participants