-
Notifications
You must be signed in to change notification settings - Fork 49
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
base: dev
Are you sure you want to change the base?
Shutterized Dispute Kit #1965
Conversation
WalkthroughThis update adds the Changes
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
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)
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this 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 hashingThis new
hashVote
function is a key architectural change that enables the Shutter integration:
- It's declared as
virtual
so derived contracts can override it- The base implementation hashes only
_choice
and_salt
(backward compatible)- It accepts but doesn't use the
_justification
parameter in the base implementation- 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 declaredexternal
, 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 signatureThe 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 votecontracts/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
⛔ 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.0The 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:
@shutter-network/shutter-sdk
- Core SDK for Shutter Network functionalityisomorphic-fetch
- Cross-environment fetch implementationNote 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 shutterLength 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 shutterLength of output: 796
Verify Shutter SDK and Fetch Dependency
The PR adds:
@shutter-network/shutter-sdk
@ 0.0.1isomorphic-fetch
@ ^3.0.0Because 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 localnpm audit
oryarn 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 documentationThe
_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 verificationThe 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 messageThe 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.0The 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 whencastCommit
revertsBecause
castCommitShutter
emitsCommitCastShutter
after the external call,
a failure insidecastCommit
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 theemit
in a
try/catch
block.contracts/scripts/shutter.ts (1)
88-127
: Error parsing assumes specific JSON structure
fetchDecryptionKey
throws ifjsonResponse.message
is undefined, but the
Shutter API returns the payload atjsonResponse
root according to the spec
used earlier. Align the two helpers or add a type-guard to avoid false
positives.
There was a problem hiding this 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 coverageThe 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 scopeThe 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)
📒 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 DisputeKitShutterThe 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
There was a problem hiding this 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 produceNaN
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 abytes32
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
📒 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 newgetDisputeKit
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.
Follow-up feature: #2016 |
…ring feat: shutter support in dispute commiting & appeal
There was a problem hiding this 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 ownFund
component. This might indicate code duplication or inconsistent patterns.Consider whether the appeal components can share more common logic:
- Shared base component: Create a common base that both Classic and Shutter can extend
- Shared utility functions: Extract common state management and popup logic
- 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:
- Adding encryption for the stored vote data
- Implementing automatic cleanup after successful reveal
- 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
⛔ 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 sincehandleSelection
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 tsxLength 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 ShutterLength 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" -A20Length 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.tsxLength 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 ofselectedOption?.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]
fromuseLocalStorage
, 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 typescriptLength 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 correctYou’re importing
useLocalStorage
fromreact-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 acceptsdecryptionDelay
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontend part lgtm 🚀
and support for the Shutter testnet API endpoint
There was a problem hiding this 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 issueAdd 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 issueGuard against malformed Graph vote IDs.
Number()
can silently yieldNaN
, 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 issuePrevent undefined access when no current local round exists.
filteredLocalRounds[0]
will throw whenlocalRounds
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 issueFix 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 issueFix 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
📒 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.
There was a problem hiding this 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 todecode()
to avoid runtime crashesIf 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 yieldNaN
, propagating bad data down-stream. Add validation and a clear error message.
169-177
: Possibleundefined
access when no current local round exists
filteredLocalRounds[0]
will throw whenlocalRounds
is empty, causing the whole script to abort.
215-221
:getBytes(vote.commit)
may break indexed event lookup
_commit
is likely an indexedbytes32
. Passing a byte array returned bygetBytes
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
📒 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.
Code Climate has analyzed commit d43e68e and detected 124 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
There was a problem hiding this 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 commentThere's an incomplete comment about logging the response status with no actual implementation.
- // Log the response status - +
123-131
: Add JSDoc documentation for helper functionsThe 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
📒 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 importsThe 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 jsxLength 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 underweb/src
, it targets modern browsers wherecrypto.getRandomValues
is universally available. No changes required here.
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
DisputeKitShutter
contract with voting and commitment features.Voting
andAppeal
components to handle Shutter disputes.OptionsContainer
for selection.useDisputeDetailsQuery
.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests