chore: remove semaphore and cleanup crisp#833
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR removes Semaphore-based registration and group logic across client, server, and contracts, introduces ownership-gated round data in CRISPInputValidator, updates factories and program constructor, adjusts vote payloads to exclude semaphore proofs, updates env addresses, and deletes related contracts, hooks, components, and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Client (Web)
participant Server as Server (API)
participant Program as CRISPProgram
participant Validator as CRISPInputValidator
participant Verifier as NoirVerifier
rect rgba(200,230,255,0.25)
note over Program,Validator: One-time setup (owner)
actor Owner
Owner->>Program: setImageId(bytes32)
Owner->>Program: setVerifier(address)
Owner->>Validator: setRoundData(root, token, threshold, snapshotBlock)
end
User->>Client: Select option and cast vote
Client->>Server: POST /vote { encryptedVote, noirProof, publicInputs }
Server->>Program: broadcastVote(encodeCrispInputs(noirProof, publicInputs, encryptedVote))
Program->>Validator: validate(evidence)
Validator->>Verifier: verify(noirProof, publicInputs)
Verifier-->>Validator: ok/err
Validator-->>Program: valid/invalid
Program-->>Server: tx success/failure
Server-->>Client: result
sequenceDiagram
autonumber
actor Owner
participant Validator as CRISPInputValidator (Ownable)
Owner->>Validator: setRoundData(root, token, threshold, snapshotBlock)
alt already set
Validator-->>Owner: revert RoundDataAlreadySet
else success
Validator-->>Owner: Round data committed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ctrlc03
force-pushed
the
chore/update-crisp
branch
from
7f970df to
02ef1c7
Compare
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
70-70: Consider making the block explorer URL configurable.The Sepolia Etherscan URL is hardcoded. If the application needs to support multiple chains (mainnet, other testnets, or custom networks), this will break or show incorrect links.
Consider adding a helper function that constructs the explorer URL based on the chain ID:
const getExplorerUrl = (chainId: number, txHash: string): string => { const explorers: Record<number, string> = { // Add other chains as needed }; const baseUrl = explorers[chainId] || 'https://etherscan.io'; return `${baseUrl}/tx/${txHash}`; };Then use it in the success case:
case 'success': { - const url = `https://sepolia.etherscan.io/tx/${broadcastVoteResponse.tx_hash}`; + const url = getExplorerUrl(CHAIN_ID, broadcastVoteResponse.tx_hash); setTxUrl(url);Alternatively, if the chain configuration is already available via context or environment, use that directly.
examples/CRISP/client/.env.example (1)
4-4: LGTM! Consider adding quotes for consistency.The updated E3_PROGRAM_ADDRESS correctly matches the server configuration and reflects the new CRISPProgram deployment. The removal of
VITE_SEMAPHORE_ADDRESSaligns with the PR objectives.For consistency with environment file best practices, consider wrapping the address value in quotes:
-VITE_E3_PROGRAM_ADDRESS=0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1 # Default E3 program address from anvil +VITE_E3_PROGRAM_ADDRESS="0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1" # Default E3 program address from anvilThis prevents potential parsing issues with certain environment variable processors.
examples/CRISP/contracts/CRISPProgram.sol (1)
109-114: Remove trailing whitespace on line 114.Line 114 contains trailing whitespace that should be removed.
Apply this diff:
inputValidator = IInputValidator( INPUT_VALIDATOR_FACTORY.deploy( address(HONK_VERIFIER), owner() ) ); - + return (ENCRYPTION_SCHEME_ID, inputValidator);examples/CRISP/contracts/CRISPInputValidator.sol (1)
71-72: Name the unused address parameter for clarity.The first parameter in the validate function is unnamed, which reduces code readability. Consider naming it even if it's unused (e.g.,
address /* sender */oraddress _sender).Apply this diff:
- function validate( - address, + function validate( + address /* sender */, bytes memory data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
examples/CRISP/.gitignore(1 hunks)examples/CRISP/client/.env.example(1 hunks)examples/CRISP/client/package.json(0 hunks)examples/CRISP/client/src/components/RegistrationModal.tsx(0 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx(0 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts(0 hunks)examples/CRISP/client/src/hooks/semaphore/useSemaphoreGroupManagement.ts(0 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(1 hunks)examples/CRISP/client/src/model/vote.model.ts(0 hunks)examples/CRISP/client/src/pages/Landing/components/DailyPoll.tsx(2 hunks)examples/CRISP/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/contracts/CRISPChecker.sol(0 hunks)examples/CRISP/contracts/CRISPCheckerFactory.sol(0 hunks)examples/CRISP/contracts/CRISPInputValidator.sol(1 hunks)examples/CRISP/contracts/CRISPInputValidatorFactory.sol(1 hunks)examples/CRISP/contracts/CRISPPolicy.sol(0 hunks)examples/CRISP/contracts/CRISPPolicyFactory.sol(0 hunks)examples/CRISP/contracts/CRISPProgram.sol(1 hunks)examples/CRISP/contracts/README.md(1 hunks)examples/CRISP/deploy/Deploy.s.sol(0 hunks)examples/CRISP/package.json(0 hunks)examples/CRISP/remappings.txt(0 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/server/src/server/models.rs(0 hunks)examples/CRISP/server/src/server/routes/voting.rs(0 hunks)examples/CRISP/test/crisp.spec.ts(0 hunks)
💤 Files with no reviewable changes (16)
- examples/CRISP/client/src/model/vote.model.ts
- examples/CRISP/package.json
- examples/CRISP/server/src/server/routes/voting.rs
- examples/CRISP/contracts/CRISPCheckerFactory.sol
- examples/CRISP/server/src/server/models.rs
- examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts
- examples/CRISP/remappings.txt
- examples/CRISP/test/crisp.spec.ts
- examples/CRISP/client/package.json
- examples/CRISP/client/src/hooks/semaphore/useSemaphoreGroupManagement.ts
- examples/CRISP/contracts/CRISPPolicyFactory.sol
- examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx
- examples/CRISP/client/src/components/RegistrationModal.tsx
- examples/CRISP/contracts/CRISPChecker.sol
- examples/CRISP/contracts/CRISPPolicy.sol
- examples/CRISP/deploy/Deploy.s.sol
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
examples/CRISP/client/.env.example
[warning] 4-4: [UnorderedKey] The VITE_E3_PROGRAM_ADDRESS key should go before the VITE_ENCLAVE_API key
(UnorderedKey)
[warning] 4-4: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
examples/CRISP/server/.env.example
[warning] 19-19: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: ciphernode_integration_test (persist)
- GitHub Check: ciphernode_integration_test (base)
- GitHub Check: template_integration
- GitHub Check: crisp_e2e
- GitHub Check: Build & Push Image
🔇 Additional comments (13)
examples/CRISP/server/.env.example (1)
19-19: LGTM!The updated E3_PROGRAM_ADDRESS aligns with the new CRISPProgram deployment that no longer depends on Semaphore. The address format and context (Default Anvil Deployments) are appropriate for local testing.
examples/CRISP/.gitignore (1)
50-52: LGTM!Explicitly tracking
ERC20Votes.jsonis appropriate given the shift to token-based voter eligibility. This ensures the SDK artifact remains version-controlled while other build outputs are ignored.examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
42-46: LGTM!The simplified validation correctly reflects the removal of Semaphore-based registration. Checking only
userandroundStateis appropriate for the new Merkle tree-based voting flow.examples/CRISP/client/src/pages/Landing/components/DailyPoll.tsx (2)
7-7: LGTM!The removal of Identity and RegistrationModal imports correctly reflects the elimination of Semaphore-based registration from the voting flow.
120-126: LGTM!The simplified button state logic and updated label correctly reflect the streamlined voting flow. The disabled condition now appropriately checks only for poll selection, loading states, round availability, and vote casting in progress—without unnecessary registration checks.
examples/CRISP/client/src/utils/proof-encoding.ts (3)
7-7: LGTM!The updated imports correctly reflect the removal of Semaphore-specific functionality. The retained imports (
encodeAbiParameters,parseAbiParameters,bytesToHex) are sufficient for the simplified encoding logic.
9-11: LGTM!The simplified ABI structure
(bytes, bytes32[], bytes)correctly reflects the removal of Semaphore proof encoding and aligns with the updated CRISP input validation flow.
13-25: LGTM!The function rename from
encodeSemaphoreProoftoencodeCrispInputsclearly communicates the updated purpose. The removal of thesemaphoreProofparameter and the simplified encoding logic align perfectly with the PR objectives to eliminate Semaphore-based proofs.examples/CRISP/contracts/CRISPProgram.sol (1)
49-70: LGTM! Constructor simplification aligns with Semaphore removal.The constructor signature has been correctly updated to remove Semaphore and factory dependencies. The validation of input parameters is appropriate and the initialization logic is sound.
examples/CRISP/contracts/CRISPInputValidatorFactory.sol (1)
19-27: LGTM! Factory signature updated correctly.The deploy function signature and encoding have been correctly updated to match the new InputValidator initialization requirements. The parameter order (verifier, owner) aligns with the changes in CRISPInputValidator._initialize().
examples/CRISP/contracts/CRISPInputValidator.sol (2)
10-16: LGTM! Ownable integration is correct.The addition of Ownable inheritance and the initialization in the constructor is properly implemented.
81-92: LGTM! Input decoding and proof verification simplified correctly.The removal of Semaphore proof handling and the updated decoding to
(bytes, bytes32[], bytes)aligns with the PR objectives to remove Semaphore-based validation. The Noir proof verification logic is correctly implemented.examples/CRISP/contracts/README.md (1)
19-23: Document Merkle-based gating and preconditions in READMEThe README no longer mentions Semaphore (good) but still omits the new Merkle workflow and guard logic. Update it to:
- Admin must call
setRoundData(uint256 root, address token, uint256 balanceThreshold, uint256 snapshotBlock)once before any votes.validate(...)should revert if the Merkle root isn’t set—ensure theisDataSetguard is active (currently commented out).- Each vote input must include a Merkle proof against
censusMerkleRoot.- Token-balance gating uses the supplied threshold and snapshot block when building the tree.
- Refer explicitly to
Enclave.publishInputfor vote submission andGrecofor proof verification.Confirm that
setRoundDatais the correct root-publication function and whether the “no voting before root” guard is enforced (or still pending) so the steps above can reference the exact behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
examples/CRISP/client/.env.example (1)
4-4: Quote value to satisfy dotenv-linter.dotenv-linter flags this line for
ValueWithoutQuotes; wrapping the address in quotes keeps the inline comment while avoiding warnings.-VITE_E3_PROGRAM_ADDRESS=0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1 # Default E3 program address from anvil +VITE_E3_PROGRAM_ADDRESS="0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1" # Default E3 program address from anvilexamples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
42-44: Good preflight guard; also check votingRound/pk_bytes upfront for clearer UX.Currently, missing votingRound triggers an exception later. Add an early check to show a friendlier message.
if (!user || !roundState) { console.error("Cannot cast vote: Missing user or round state."); showToast({ type: 'danger', message: 'Cannot cast vote. Ensure you are connected, and the round is active.' }); return; } + if (!votingRound || !votingRound.pk_bytes?.length) { + console.error("Cannot cast vote: Missing voting round public key."); + showToast({ type: 'danger', message: 'Round is not ready yet. Please try again shortly.' }); + return; + }examples/CRISP/client/src/pages/Landing/components/DailyPoll.tsx (1)
120-120: Remove redundant condition in disabled prop.Button renders only when roundState exists, so
!roundStateis unnecessary.- disabled={noPollSelected || loading || !roundState || isEnded || isCastingVote} + disabled={noPollSelected || loading || isEnded || isCastingVote}examples/CRISP/client/src/utils/proof-encoding.ts (1)
13-25: Strengthen types and validate inputs to prevent runtime ABI encoding errors.
- Return type should be
0x${string}(viem returns hex).- Validate each public input is a 32‑byte hex; current cast can mask errors.
-export const encodeCrispInputs = ( - noirProof: Uint8Array, - noirPublicInputs: string[], - encryptedVote: Uint8Array -): string => { - return encodeAbiParameters(crispAbi, [ - [ - bytesToHex(noirProof), - noirPublicInputs.map(input => input as `0x${string}`), - bytesToHex(encryptedVote) - ] - ]) -} +export const encodeCrispInputs = ( + noirProof: Uint8Array, + noirPublicInputs: readonly `0x${string}`[], + encryptedVote: Uint8Array +): `0x${string}` => { + const isBytes32 = (x: string) => /^0x[0-9a-fA-F]{64}$/.test(x); + if (!noirPublicInputs.every(isBytes32)) { + throw new Error('noirPublicInputs must be 0x-prefixed 32-byte hex strings (bytes32).'); + } + return encodeAbiParameters(crispAbi, [ + [ + bytesToHex(noirProof), + noirPublicInputs, + bytesToHex(encryptedVote), + ], + ]) as `0x${string}`; +}Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
examples/CRISP/.gitignore(1 hunks)examples/CRISP/client/.env.example(1 hunks)examples/CRISP/client/package.json(0 hunks)examples/CRISP/client/src/components/RegistrationModal.tsx(0 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx(0 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts(0 hunks)examples/CRISP/client/src/hooks/semaphore/useSemaphoreGroupManagement.ts(0 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(1 hunks)examples/CRISP/client/src/model/vote.model.ts(0 hunks)examples/CRISP/client/src/pages/Landing/components/DailyPoll.tsx(2 hunks)examples/CRISP/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/contracts/CRISPChecker.sol(0 hunks)examples/CRISP/contracts/CRISPCheckerFactory.sol(0 hunks)examples/CRISP/contracts/CRISPInputValidator.sol(1 hunks)examples/CRISP/contracts/CRISPInputValidatorFactory.sol(1 hunks)examples/CRISP/contracts/CRISPPolicy.sol(0 hunks)examples/CRISP/contracts/CRISPPolicyFactory.sol(0 hunks)examples/CRISP/contracts/CRISPProgram.sol(1 hunks)examples/CRISP/contracts/README.md(1 hunks)examples/CRISP/deploy/Deploy.s.sol(0 hunks)examples/CRISP/package.json(0 hunks)examples/CRISP/remappings.txt(0 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/server/src/server/models.rs(0 hunks)examples/CRISP/server/src/server/routes/voting.rs(0 hunks)examples/CRISP/test/crisp.spec.ts(0 hunks)
💤 Files with no reviewable changes (16)
- examples/CRISP/contracts/CRISPPolicyFactory.sol
- examples/CRISP/client/src/model/vote.model.ts
- examples/CRISP/server/src/server/models.rs
- examples/CRISP/remappings.txt
- examples/CRISP/client/package.json
- examples/CRISP/client/src/components/RegistrationModal.tsx
- examples/CRISP/server/src/server/routes/voting.rs
- examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts
- examples/CRISP/client/src/hooks/semaphore/useSemaphoreGroupManagement.ts
- examples/CRISP/contracts/CRISPPolicy.sol
- examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx
- examples/CRISP/package.json
- examples/CRISP/contracts/CRISPCheckerFactory.sol
- examples/CRISP/contracts/CRISPChecker.sol
- examples/CRISP/test/crisp.spec.ts
- examples/CRISP/deploy/Deploy.s.sol
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
examples/CRISP/server/.env.example
[warning] 19-19: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
examples/CRISP/client/.env.example
[warning] 4-4: [UnorderedKey] The VITE_E3_PROGRAM_ADDRESS key should go before the VITE_ENCLAVE_API key
(UnorderedKey)
[warning] 4-4: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: ciphernode_integration_test (persist)
- GitHub Check: ciphernode_integration_test (base)
- GitHub Check: template_integration
- GitHub Check: crisp_e2e
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (2)
examples/CRISP/client/src/pages/Landing/components/DailyPoll.tsx (1)
123-125: Button label update looks good.Clear, consistent with isCastingVote state.
examples/CRISP/client/src/utils/proof-encoding.ts (1)
9-11: Confirm tuple shape matches nested args to encodeAbiParameters.You pass values as
[[...]], which implies a single tuple parameter. Ensure the parsed ABI represents one tuple param. If you intended three top‑level params, pass a flat array instead.Would you confirm that parseAbiParameters('(bytes, bytes32[], bytes)') yields a single tuple param in your viem version? If not, either:
- Use a flat args array:
[bytesHex, inputs, encHex], or- Make the tuple explicit (e.g., 'tuple(bytes, bytes32[], bytes)').
Based on learnings
cedoor
force-pushed
the
chore/update-crisp
branch
from
0a52edf to
5e1484e
Compare
0xjei
left a comment
0xjei
left a comment
There was a problem hiding this comment.
serveris not working due to.envconfigs (expect them at root level)clientnot showing up the current round
|
@coderabbitai configuration |
CodeRabbit configurationlanguage: en-US
tone_instructions: ''
early_access: false
enable_free_tier: true
reviews:
profile: chill
request_changes_workflow: false
high_level_summary: true
high_level_summary_placeholder: '@coderabbitai summary'
high_level_summary_in_walkthrough: false
auto_title_placeholder: '@coderabbitai'
auto_title_instructions: ''
review_status: true
commit_status: true
fail_commit_status: false
collapse_walkthrough: false
changed_files_summary: true
sequence_diagrams: true
estimate_code_review_effort: true
assess_linked_issues: true
related_issues: true
related_prs: true
suggested_labels: true
auto_apply_labels: false
suggested_reviewers: true
auto_assign_reviewers: false
in_progress_fortune: true
poem: true
labeling_instructions: []
path_filters: []
path_instructions: []
abort_on_close: true
disable_cache: false
auto_review:
enabled: true
auto_incremental_review: true
ignore_title_keywords: []
labels: []
drafts: false
base_branches: []
ignore_usernames: []
finishing_touches:
docstrings:
enabled: true
unit_tests:
enabled: true
pre_merge_checks:
docstrings:
mode: warning
threshold: 80
title:
mode: warning
requirements: ''
description:
mode: warning
issue_assessment:
mode: warning
custom_checks: []
tools:
ast-grep:
rule_dirs: []
util_dirs: []
essential_rules: true
packages: []
shellcheck:
enabled: true
ruff:
enabled: true
markdownlint:
enabled: true
github-checks:
enabled: true
timeout_ms: 90000
languagetool:
enabled: true
enabled_rules: []
disabled_rules: []
enabled_categories: []
disabled_categories: []
enabled_only: false
level: default
biome:
enabled: true
hadolint:
enabled: true
swiftlint:
enabled: true
phpstan:
enabled: true
level: default
phpmd:
enabled: true
phpcs:
enabled: true
golangci-lint:
enabled: true
yamllint:
enabled: true
gitleaks:
enabled: true
checkov:
enabled: true
detekt:
enabled: true
eslint:
enabled: true
flake8:
enabled: true
rubocop:
enabled: true
buf:
enabled: true
regal:
enabled: true
actionlint:
enabled: true
pmd:
enabled: true
cppcheck:
enabled: true
semgrep:
enabled: true
circleci:
enabled: true
clippy:
enabled: true
sqlfluff:
enabled: true
prismaLint:
enabled: true
pylint:
enabled: true
oxc:
enabled: true
shopifyThemeCheck:
enabled: true
luacheck:
enabled: true
brakeman:
enabled: true
dotenvLint:
enabled: true
htmlhint:
enabled: true
checkmake:
enabled: true
osvScanner:
enabled: true
chat:
art: true
auto_reply: true
integrations:
jira:
usage: auto
linear:
usage: auto
knowledge_base:
opt_out: false
web_search:
enabled: true
code_guidelines:
enabled: true
filePatterns: []
learnings:
scope: auto
issues:
scope: auto
jira:
usage: auto
project_keys: []
linear:
usage: auto
team_keys: []
pull_requests:
scope: auto
mcp:
usage: auto
disabled_servers: []
code_generation:
docstrings:
language: en-US
path_instructions: []
unit_tests:
path_instructions: []
|
cedoor
left a comment
cedoor
left a comment
There was a problem hiding this comment.
Should we get rid of the commented code in crisp.contracts.test.ts file? It's using Semaphore.
ctrlc03
force-pushed
the
chore/update-crisp
branch
from
5e1484e to
09e1262
Compare
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
ctrlc03
force-pushed
the
chore/update-crisp
branch
from
09e1262 to
23fff2f
Compare
3 participants
fix #758 and fix #757
Summary by CodeRabbit
New Features
Refactor
Chores
Documentation