Shutter API integration PoC #1964
Conversation
WalkthroughThis update introduces Shutter-based cryptographic voting functionality into the codebase. A new Solidity contract, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ShutterScript
participant ShutterAPI
participant ShutterSDK
participant Blockchain
participant DisputeKitShutterPoC
User->>ShutterScript: encrypt(message)
ShutterScript->>ShutterAPI: Register identity, get encryption params
ShutterAPI-->>ShutterScript: Return encryption data
ShutterScript->>ShutterSDK: Encrypt message
ShutterSDK-->>ShutterScript: Encrypted commitment, identity
ShutterScript-->>User: Return encrypted data
User->>ShutterAutoVote: castCommit(disputeID, voteIDs, choice, justification)
ShutterAutoVote->>ShutterScript: encrypt(message)
ShutterScript->>ShutterAPI: Register identity, get encryption params
ShutterAPI-->>ShutterScript: Return encryption data
ShutterScript->>ShutterSDK: Encrypt message
ShutterSDK-->>ShutterScript: Encrypted commitment, identity
ShutterScript-->>ShutterAutoVote: Return encrypted data
ShutterAutoVote->>DisputeKitShutterPoC: castCommit(commitHash, identity)
DisputeKitShutterPoC-->>Blockchain: Emit CommitCast
Note over ShutterAutoVote: After decryption delay
ShutterAutoVote->>ShutterScript: decrypt(encryptedCommitment, identity)
ShutterScript->>ShutterAPI: Fetch decryption key
ShutterAPI-->>ShutterScript: Return decryption key
ShutterScript->>ShutterSDK: Decrypt message
ShutterSDK-->>ShutterScript: Decrypted message
ShutterScript-->>ShutterAutoVote: Return plaintext vote data
ShutterAutoVote->>DisputeKitShutterPoC: castVote(choice, justification, salt)
DisputeKitShutterPoC-->>Blockchain: Emit VoteCast
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
contracts/deploy/09-shutter.ts (1)
10-13: Guard against undefined chain mapping to avoid “cannot read properties of undefined”
HomeChains[chainId]returnsundefinedfor unknown testnets →console.logprints “deploying to undefined”.
Consider a fallback label to improve DX:-console.log("deploying to %s with deployer %s", HomeChains[chainId], deployer); +console.log( + "deploying to %s with deployer %s", + HomeChains[chainId] ?? `chainId(${chainId})`, + deployer, +);contracts/scripts/shutter.ts (1)
199-212: Decrypt flow lacks retry/back-off for “timestamp not reached yet”Consumers will have to wrap
decryptin their own polling loop. Consider surfacing a typed error or an optionalwaitflag that sleeps/retries until the key is available to improve DX.contracts/scripts/shutterAutoVote.ts (2)
22-27: Remove unused import to avoid compilation warnings
decodeEventLogis imported but never referenced. Type-script will emit “unused identifier” warnings and some linters treat this as an error.-import { createPublicClient, createWalletClient, http, Hex, decodeEventLog, getContract } from "viem"; +import { createPublicClient, createWalletClient, http, Hex, getContract } from "viem";
127-170: Tight infinite loop without back-off or cancelation hook
while (true)with a fixed 30 s sleep will run forever even when there is nothing to do and cannot be cleanly stopped from the outside (e.g. SIGINT). Consider:
- Using
setInterval/setTimeoutin the top-level script rather than recursion.- Adding an abort signal or exit condition.
- Exponential back-off when no votes are ready.
This will make the script friendlier to ops & testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
contracts/deploy/09-shutter.ts(1 hunks)contracts/package.json(1 hunks)contracts/scripts/shutter.ts(1 hunks)contracts/scripts/shutterAutoVote.ts(1 hunks)contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
contracts/scripts/shutterAutoVote.ts (1)
contracts/scripts/shutter.ts (3)
encrypt(165-191)DECRYPTION_DELAY(7-7)decrypt(199-212)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet
- 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
🔇 Additional comments (2)
contracts/scripts/shutter.ts (1)
46-56: API spec:identityPrefixlikely expects raw bytes, not0x-prefixedThe Shutter API examples show plain hex without the
0x. Confirm and, if needed, strip the prefix beforeJSON.stringifyto avoid 400-responses.-identityPrefix, +identityPrefix: identityPrefix.slice(2),contracts/scripts/shutterAutoVote.ts (1)
88-93: Only the first voteId is included in the committed message
voteIDsis an array, yet the encoded message and subsequent hash only containvoteIDs[0]. If multiple vote IDs should be committed together (per the TODO spec), they need to be concatenated or handled individually; otherwise downstream hash verification will fail for the omitted IDs.
| // Verify the commitment hash | ||
| bytes32 computedHash = hashVote(_coreDisputeID, _voteIDs[i], _choice, _justification, _salt); | ||
| require(votes[_voteIDs[i]].commitHash == computedHash, "The commitment hash does not match."); | ||
| require(!votes[_voteIDs[i]].voted, "Vote already cast."); | ||
| votes[_voteIDs[i]].choice = _choice; | ||
| votes[_voteIDs[i]].voted = true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Commit replay / overwrite not prevented
A malicious juror can repeatedly call castCommit and overwrite an existing commitHash, effectively replacing their committed vote after seeing others’ reveals.
Add a guard:
require(votes[_voteIDs[i]].commitHash == bytes32(0), "Commit already stored");and/or introduce a separate commit phase toggle.
Committable suggestion skipped: line range outside the PR's diff.
contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol
Outdated
Show resolved
Hide resolved
contracts/scripts/shutterAutoVote.ts
Outdated
| // Generate salt and compute hash | ||
| const salt = generateSalt(); | ||
| const commitHash = await disputeKit.read.hashVote([coreDisputeID, voteIDs[0], choice, justification, salt]); | ||
|
|
||
| // Cast the commit on-chain | ||
| const txHash = await disputeKit.write.castCommit([coreDisputeID, voteIDs, commitHash, identity as Hex]); | ||
|
|
||
| // Wait for transaction to be mined |
There was a problem hiding this comment.
Mismatch with contract’s expected castCommit signature
The TODO block states that castCommit should be called with (encryptedCommitment, voteId, choice, justification), but the implementation sends (commitHash, identity) instead. Unless the ABI was changed accordingly, the transaction will revert. Please cross-check DisputeKitShutterPoC.castCommit and pass the correct arguments—most likely including encryptedCommitment.
-const txHash = await disputeKit.write.castCommit([coreDisputeID, voteIDs, commitHash, identity as Hex]);
+const txHash = await disputeKit.write.castCommit([
+ coreDisputeID,
+ voteIDs,
+ encryptedCommitment,
+ choice,
+ justification,
+ identity as Hex,
+]);Committable suggestion skipped: line range outside the PR's diff.
contracts/scripts/shutterAutoVote.ts
Outdated
| const PRIVATE_KEY = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" as const; | ||
|
|
||
| const CONTRACT_ADDRESS = "0x5FbDB2315678afecb367f032d93F642f64180aa3" as const; | ||
|
|
There was a problem hiding this comment.
Hard-coding private key & contract address is a critical security risk
Leaking a funded key in an OSS repo compromises any network the key is used on, and tightly couples the script to a single environment.
-const PRIVATE_KEY = "0x…ff80" as const;
-const CONTRACT_ADDRESS = "0x5FbDB2315678afecb367f032d93F642f64180aa3" as const;
+const PRIVATE_KEY = process.env.PRIVATE_KEY as Hex;
+const CONTRACT_ADDRESS = process.env.DISPUTE_KIT_ADDRESS as Hex;
+
+if (!PRIVATE_KEY || !CONTRACT_ADDRESS)
+ throw new Error("Missing PRIVATE_KEY or DISPUTE_KIT_ADDRESS env vars");Committable suggestion skipped: line range outside the PR's diff.
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
contracts/scripts/shutterAutoVote.ts (4)
73-79: Mismatch with contract's expectedcastCommitsignatureThere appears to be a potential mismatch between the arguments sent to the
castCommitfunction and what the contract expects. Based on the context, the contract may expect parameters like encryptedCommitment, choice, and justification.Verify the function signature by examining the contract code:
#!/bin/bash # Find the castCommit function definition in the contract fd "DisputeKitShutterPoC.sol" --type f | xargs cat | grep -A 10 "function castCommit"
22-24:⚠️ Potential issueSecurity risk: Hard-coded private key and contract address
Hard-coding private keys and contract addresses in source code is a critical security risk. This could lead to accidental exposure of private keys if committed to a repository, potentially resulting in fund loss or contract compromise.
Replace the hard-coded values with environment variables:
-const PRIVATE_KEY = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" as const; - -const CONTRACT_ADDRESS = "0x5FbDB2315678afecb367f032d93F642f64180aa3" as const; +const PRIVATE_KEY = process.env.PRIVATE_KEY as Hex; + +const CONTRACT_ADDRESS = process.env.DISPUTE_KIT_ADDRESS as Hex; + +if (!PRIVATE_KEY || !CONTRACT_ADDRESS) { + throw new Error("Missing required environment variables: PRIVATE_KEY, DISPUTE_KIT_ADDRESS"); +}🧰 Tools
🪛 Gitleaks (8.21.2)
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
83-85: 🛠️ Refactor suggestionEvent retrieval may return stale or duplicate logs
Using
disputeKit.getEvents.CommitCast()without filters fetches all historical events every time it's called. This is inefficient and can lead to processing duplicate events.Filter events by the transaction hash to only get relevant logs:
- // Watch for CommitCast event - const events = await disputeKit.getEvents.CommitCast(); - console.log("CommitCast event:", (events[0] as any).args); + // Get logs from the specific transaction + const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash }); + const commitCastTopic = disputeKit.getEventTopic("CommitCast"); + const commitLogs = receipt.logs.filter(l => l.topics[0] === commitCastTopic); + if (commitLogs.length > 0) { + const event = disputeKit.parseEventLog({ log: commitLogs[0] }); + console.log("CommitCast event:", event.args); + }
134-136: 🛠️ Refactor suggestionEvent retrieval may return stale or duplicate logs
Similar to the issue in the commit function, this code fetches all historical VoteCast events without filtering.
Filter events by the transaction hash:
- // Watch for VoteCast event - const events = await disputeKit.getEvents.VoteCast(); - console.log("VoteCast event:", (events[0] as any).args); + // Get logs from the specific transaction + const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash }); + const voteCastTopic = disputeKit.getEventTopic("VoteCast"); + const voteLogs = receipt.logs.filter(l => l.topics[0] === voteCastTopic); + if (voteLogs.length > 0) { + const event = disputeKit.parseEventLog({ log: voteLogs[0] }); + console.log("VoteCast event:", event.args); + }
🧹 Nitpick comments (3)
contracts/scripts/shutterAutoVote.ts (3)
106-154: Consider adding graceful termination option for autoVote loopThe
autoVotefunction runs in an infinite loop with no way to gracefully terminate it except for a process kill. For a more robust implementation, especially in production environments, consider adding a termination mechanism.Add a termination mechanism using a signal handler:
/** * Continuously monitor for votes ready to be decrypted and cast */ -export async function autoVote() { +export async function autoVote(options = { shouldContinue: true }) { while (true) { try { + // Check if we should terminate + if (!options.shouldContinue) { + console.log("Terminating autoVote loop"); + break; + } const currentTime = Math.floor(Date.now() / 1000);Usage example:
// Add to main or where appropriate const options = { shouldContinue: true }; // Set up signal handlers process.on('SIGINT', () => { console.log('Received SIGINT. Gracefully shutting down...'); options.shouldContinue = false; }); // Pass options to autoVote await autoVote(options);
112-112: Consider configurable buffer time for decryptionThe code adds a hardcoded 10-second buffer to the
DECRYPTION_DELAY. This value might need adjustment based on network conditions or Shutter API response times.Make the buffer time configurable:
+// Constants +const DECRYPTION_BUFFER = 10; // Additional seconds to wait after DECRYPTION_DELAY + // Find votes ready for decryption -const readyVotes = encryptedVotes.filter((vote) => currentTime - vote.timestamp >= DECRYPTION_DELAY + 10); +const readyVotes = encryptedVotes.filter((vote) => currentTime - vote.timestamp >= DECRYPTION_DELAY + DECRYPTION_BUFFER);
146-148: Consider configurable polling intervalThe code uses a hardcoded 30-second polling interval. This value might need adjustment based on network conditions or expected voting frequency.
Make the polling interval configurable:
+// Constants +const DEFAULT_POLLING_INTERVAL = 30000; // 30 seconds in milliseconds + +/** + * Continuously monitor for votes ready to be decrypted and cast + */ -export async function autoVote() { +export async function autoVote(options = { + shouldContinue: true, + pollingInterval: DEFAULT_POLLING_INTERVAL +}) { while (true) { // ... existing code ... // Sleep for polling interval - console.log("Sleeping for 30 seconds"); - await new Promise((resolve) => setTimeout(resolve, 30000)); + console.log(`Sleeping for ${options.pollingInterval / 1000} seconds`); + await new Promise((resolve) => setTimeout(resolve, options.pollingInterval));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/scripts/shutterAutoVote.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
contracts/scripts/shutterAutoVote.ts (1)
contracts/scripts/shutter.ts (3)
encrypt(165-191)DECRYPTION_DELAY(7-7)decrypt(199-212)
🪛 Gitleaks (8.21.2)
contracts/scripts/shutterAutoVote.ts
22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: 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: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
contracts/scripts/shutterAutoVote.ts
Outdated
| // Parse the decrypted message | ||
| const [coreDisputeID, , choice, justification] = decryptedMessage.split(SEPARATOR); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for decrypted message parsing
The code assumes the decrypted message will always have a valid format with all expected components. If the message is malformed, the array destructuring could assign undefined values to variables.
Add validation for the parsed message:
// Parse the decrypted message
const [coreDisputeID, , choice, justification] = decryptedMessage.split(SEPARATOR);
+
+ // Validate parsed values
+ if (!coreDisputeID || !choice) {
+ throw new Error(`Invalid decrypted message format: ${decryptedMessage}`);
+ }📝 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.
| // Parse the decrypted message | |
| const [coreDisputeID, , choice, justification] = decryptedMessage.split(SEPARATOR); | |
| // Parse the decrypted message | |
| const [coreDisputeID, , choice, justification] = decryptedMessage.split(SEPARATOR); | |
| // Validate parsed values | |
| if (!coreDisputeID || !choice) { | |
| throw new Error(`Invalid decrypted message format: ${decryptedMessage}`); | |
| } |
contracts/scripts/shutterAutoVote.ts
Outdated
| }) { | ||
| try { | ||
| // Create message with U+241F separator | ||
| const message = `${coreDisputeID}${SEPARATOR}${voteIDs[0]}${SEPARATOR}${choice}${SEPARATOR}${justification}`; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure message format is compatible with contract expectations
The message is formatted with a specific separator and order of elements. Ensure this format aligns with what the contract expects when validating votes.
Verify the message format expected by the contract:
🏁 Script executed:
#!/bin/bash
# Find the vote validation or hash computation logic in the contract
fd "DisputeKitShutterPoC.sol" --type f | xargs cat | grep -A 10 "hashVote"Length of output: 965
Update message format to match hashVote in the contract
The contract’s hashVote computes
keccak256(abi.encode(
_coreDisputeID,
_voteID,
_choice,
keccak256(bytes(_justification)),
_salt
))but the script currently builds:
const message = `${coreDisputeID}${SEPARATOR}${voteIDs[0]}${SEPARATOR}${choice}${SEPARATOR}${justification}`;It omits both the justification hash and the salt, and doesn’t use the same ABI encoding.
Please update contracts/scripts/shutterAutoVote.ts (around line 68) to:
- Compute
justificationHash = keccak256(toUtf8Bytes(justification)) - Include the same
saltvalue used on‐chain - Use a packed keccak256 (or ethers.js
solidityKeccak256) with types['uint256','uint256','uint256','bytes32','bytes32']
Example replacement snippet:
- const message = `${coreDisputeID}${SEPARATOR}${voteIDs[0]}${SEPARATOR}${choice}${SEPARATOR}${justification}`;
+ const justificationHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(justification));
+ // `salt` must match the one passed to the contract’s commit phase
+ const message = ethers.utils.solidityKeccak256(
+ ['uint256','uint256','uint256','bytes32','bytes32'],
+ [coreDisputeID, voteIDs[0], choice, justificationHash, salt]
+ );This ensures the off‐chain message hash exactly matches what hashVote expects.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol (2)
4-4: Remove console.sol import from production codeUsing
hardhat/console.solin production code inflates the bytecode and can break verification on block explorers like Etherscan or Sourcify.-import "hardhat/console.sol"; +// import "hardhat/console.sol"; // Uncomment for local debugging only
65-79:⚠️ Potential issueAdd replay protection and per-vote commitment hashes
The
castCommitfunction has two critical issues:
- It allows overwriting existing commitments (no replay protection)
- It uses the same commitment hash for all votes, which conflicts with the hash calculation in
hashVote()that includes the vote IDfunction castCommit( uint256 _coreDisputeID, uint256[] calldata _voteIDs, - bytes32 _commitHash, + bytes32[] calldata _commitHashes, bytes32 _identity ) external { + require(_voteIDs.length == _commitHashes.length, "Length mismatch"); // Store the commitment hash for each voteID for (uint256 i = 0; i < _voteIDs.length; i++) { require(votes[_voteIDs[i]].account == msg.sender, "The caller has to own the vote."); + require(votes[_voteIDs[i]].commitHash == bytes32(0), "Commit already stored"); - votes[_voteIDs[i]].commitHash = _commitHash; + votes[_voteIDs[i]].commitHash = _commitHashes[i]; } totalCommitted += _voteIDs.length; - emit CommitCast(_coreDisputeID, msg.sender, _voteIDs, _commitHash, _identity); + emit CommitCast(_coreDisputeID, msg.sender, _voteIDs, _commitHashes[0], _identity); }Note: You may need to adjust the event to handle multiple commitment hashes.
🧹 Nitpick comments (2)
contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol (2)
104-117: Simplify the winning choice logicThe current implementation for determining the winning choice and ties is somewhat complex and could be simplified for better readability and maintainability.
counts[_choice] += _voteIDs.length; -if (_choice == winningChoice) { - if (tied) tied = false; -} else { - // Voted for another choice. - if (counts[_choice] == counts[winningChoice]) { - // Tie. - if (!tied) tied = true; - } else if (counts[_choice] > counts[winningChoice]) { - // New winner. - winningChoice = _choice; - tied = false; - } -} + +// Update winning choice and tie status +if (counts[_choice] > counts[winningChoice]) { + winningChoice = _choice; + tied = false; +} else if (counts[_choice] == counts[winningChoice] && _choice != winningChoice) { + tied = true; +}This simplification maintains the same logic but with clearer conditions.
6-124: Add comprehensive NatSpec documentationThe contract lacks complete NatSpec documentation, which is essential for understanding its functionality and integration with other components. Add detailed documentation for all functions, events, and state variables.
For example:
/** * @title DisputeKitShutterPoC * @dev A proof of concept implementation of a dispute kit using Shutter Network for encrypted voting * This contract implements a commit-reveal voting scheme where votes are first committed as encrypted * hashes and later revealed. */ contract DisputeKitShutterPoC { /** * @dev Vote struct containing all relevant information about a vote * @param account The address of the juror * @param commitHash The hash of the encrypted vote * @param choice The revealed choice of the juror * @param voted Flag indicating if the vote has been revealed */ struct Vote { // ... } // ...and so on for all functions, events, and variables }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
contracts/scripts/shutterAutoVote.ts(1 hunks)contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/scripts/shutterAutoVote.ts
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
| contract DisputeKitShutterPoC { | ||
| struct Vote { | ||
| address account; // The address of the juror. | ||
| bytes32 commitHash; // The hash of the encrypted message + salt | ||
| uint256 choice; // The choice of the juror. | ||
| bool voted; // True if the vote has been cast. | ||
| } | ||
|
|
||
| Vote[] public votes; | ||
| uint256 public winningChoice; // The choice with the most votes. Note that in the case of a tie, it is the choice that reached the tied number of votes first. | ||
| mapping(uint256 => uint256) public counts; // The sum of votes for each choice in the form `counts[choice]`. | ||
| bool public tied; // True if there is a tie, false otherwise. | ||
| uint256 public totalCommitted; | ||
| uint256 public totalVoted; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add access control and contract inheritance
This contract lacks proper access control mechanisms. As a DisputeKit implementation in the Kleros system, it should inherit from either IDisputeKit or a base implementation.
-contract DisputeKitShutterPoC {
+import "../interfaces/IDisputeKit.sol";
+import "@openzeppelin/contracts/access/Ownable.sol";
+
+contract DisputeKitShutterPoC is IDisputeKit, Ownable {This would ensure proper integration with the rest of the Kleros system and provide basic access control functionality.
📝 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.
| contract DisputeKitShutterPoC { | |
| struct Vote { | |
| address account; // The address of the juror. | |
| bytes32 commitHash; // The hash of the encrypted message + salt | |
| uint256 choice; // The choice of the juror. | |
| bool voted; // True if the vote has been cast. | |
| } | |
| Vote[] public votes; | |
| uint256 public winningChoice; // The choice with the most votes. Note that in the case of a tie, it is the choice that reached the tied number of votes first. | |
| mapping(uint256 => uint256) public counts; // The sum of votes for each choice in the form `counts[choice]`. | |
| bool public tied; // True if there is a tie, false otherwise. | |
| uint256 public totalCommitted; | |
| uint256 public totalVoted; | |
| import "../interfaces/IDisputeKit.sol"; | |
| import "@openzeppelin/contracts/access/Ownable.sol"; | |
| contract DisputeKitShutterPoC is IDisputeKit, Ownable { | |
| struct Vote { | |
| address account; // The address of the juror. | |
| bytes32 commitHash; // The hash of the encrypted message + salt | |
| uint256 choice; // The choice of the juror. | |
| bool voted; // True if the vote has been cast. | |
| } | |
| Vote[] public votes; | |
| uint256 public winningChoice; // The choice with the most votes. Note that in the case of a tie, it is the choice that reached the tied number of votes first. | |
| mapping(uint256 => uint256) public counts; // The sum of votes for each choice in the form `counts[choice]`. | |
| bool public tied; // True if there is a tie, false otherwise. | |
| uint256 public totalCommitted; | |
| uint256 public totalVoted; | |
| // … rest of your code … | |
| } |
| constructor() { | ||
| address juror = msg.sender; | ||
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | ||
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | ||
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constructor needs more flexibility for real-world use
The current constructor hardcodes three votes for the deployer, which is likely only for testing purposes. For production use, this should be more flexible.
-constructor() {
- address juror = msg.sender;
- votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false}));
- votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false}));
- votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false}));
+constructor() {
+ // Initial state setup if needed for production
}Additionally, consider adding initialization parameters if needed for integration with other contracts.
📝 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.
| constructor() { | |
| address juror = msg.sender; | |
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | |
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | |
| votes.push(Vote({account: juror, commitHash: bytes32(0), choice: 0, voted: false})); | |
| } | |
| constructor() { | |
| // Initial state setup if needed for production | |
| } |
contracts/src/arbitration/dispute-kits/DisputeKitShutterPoC.sol
Outdated
Show resolved
Hide resolved
| function castVote( | ||
| uint256 _coreDisputeID, | ||
| uint256[] calldata _voteIDs, | ||
| uint256 _choice, | ||
| string memory _justification, | ||
| bytes32 _salt | ||
| ) external { | ||
| require(_voteIDs.length > 0, "No voteID provided"); | ||
|
|
||
| // TODO: what happens if hiddenVotes are not enabled? | ||
|
|
||
| // Verify the commitment hash for all votes at once | ||
| bytes32 computedHash = hashVote(_coreDisputeID, _voteIDs, _choice, _justification, _salt); | ||
|
|
||
| for (uint256 i = 0; i < _voteIDs.length; i++) { | ||
| require(votes[_voteIDs[i]].commitHash == computedHash, "The commitment hash does not match."); |
There was a problem hiding this comment.
Fix hash verification logic and add explicit phase checks
The current castVote function has issues with its hash verification:
- It computes a single hash for all vote IDs but each vote should have its own hash
- The TODO comment indicates missing logic for non-hidden votes
- Consider adding phase checks to ensure voting is only allowed during the appropriate phase
function castVote(
uint256 _coreDisputeID,
uint256[] calldata _voteIDs,
uint256 _choice,
string memory _justification,
bytes32 _salt
) external {
require(_voteIDs.length > 0, "No voteID provided");
- // TODO: what happens if hiddenVotes are not enabled?
+ // If hidden votes are not enabled, skip hash verification
+ bool hiddenVotesEnabled = true; // Replace with actual check
- // Verify the commitment hash for all votes at once
- bytes32 computedHash = hashVote(_coreDisputeID, _voteIDs, _choice, _justification, _salt);
for (uint256 i = 0; i < _voteIDs.length; i++) {
- require(votes[_voteIDs[i]].commitHash == computedHash, "The commitment hash does not match.");
+ if (hiddenVotesEnabled) {
+ // For hidden votes, compute individual hash for each vote
+ uint256[] memory singleVoteID = new uint256[](1);
+ singleVoteID[0] = _voteIDs[i];
+ bytes32 computedHash = hashVote(_coreDisputeID, singleVoteID, _choice, _justification, _salt);
+ require(votes[_voteIDs[i]].commitHash == computedHash, "The commitment hash does not match.");
+ }📝 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.
| function castVote( | |
| uint256 _coreDisputeID, | |
| uint256[] calldata _voteIDs, | |
| uint256 _choice, | |
| string memory _justification, | |
| bytes32 _salt | |
| ) external { | |
| require(_voteIDs.length > 0, "No voteID provided"); | |
| // TODO: what happens if hiddenVotes are not enabled? | |
| // Verify the commitment hash for all votes at once | |
| bytes32 computedHash = hashVote(_coreDisputeID, _voteIDs, _choice, _justification, _salt); | |
| for (uint256 i = 0; i < _voteIDs.length; i++) { | |
| require(votes[_voteIDs[i]].commitHash == computedHash, "The commitment hash does not match."); | |
| function castVote( | |
| uint256 _coreDisputeID, | |
| uint256[] calldata _voteIDs, | |
| uint256 _choice, | |
| string memory _justification, | |
| bytes32 _salt | |
| ) external { | |
| require(_voteIDs.length > 0, "No voteID provided"); | |
| // If hidden votes are not enabled, skip hash verification | |
| bool hiddenVotesEnabled = true; // TODO: replace with actual check (e.g. phase or config) | |
| for (uint256 i = 0; i < _voteIDs.length; i++) { | |
| if (hiddenVotesEnabled) { | |
| // For hidden votes, compute individual hash for each vote | |
| uint256[] memory singleVoteID = new uint256[](1); | |
| singleVoteID[0] = _voteIDs[i]; | |
| bytes32 computedHash = hashVote( | |
| _coreDisputeID, | |
| singleVoteID, | |
| _choice, | |
| _justification, | |
| _salt | |
| ); | |
| require( | |
| votes[_voteIDs[i]].commitHash == computedHash, | |
| "The commitment hash does not match." | |
| ); | |
| } | |
| // else: handle non-hidden votes (e.g. direct accept or other logic) | |
| } | |
| // …rest of castVote implementation… | |
| } |
|
|
Code Climate has analyzed commit 7fd5997 and detected 15 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
The production version is #1965
PR-Codex overview
This PR introduces a new dispute resolution contract,
DisputeKitShutterPoC, along with related deployment and voting functionalities. It enhances the project by integrating the@shutter-network/shutter-sdkfor secure vote encryption and decryption processes.Detailed summary
DisputeKitShutterPoCcontract for dispute resolution.Voteand related functions.deployArbitrationfunction for deploying the contract.shutter-sdkfor encrypting and decrypting votes.shutterAutoVotescript for automatic voting based on encrypted data.package.jsonandyarn.lockwith new dependencies.Summary by CodeRabbit
New Features
Chores
Bug Fixes