test: crisp circuit tests#978
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughRefactored rejection assertions in Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant encodeVote
participant encryptInputs as encryptVoteAndGenerateCRISPInputs
participant prover as generateProof
participant verifier as verifyProof
Test->>encodeVote: encode vote at MAXIMUM_VOTE_VALUE
encodeVote-->>Test: encodedVote
Test->>encryptInputs: build CRISP inputs (signature, merkle proof, encrypted vote)
encryptInputs-->>Test: CRISP_inputs
Test->>prover: generateProof(CRISP_inputs)
prover-->>Test: proof
Test->>verifier: verifyProof(proof)
verifier-->>Test: verificationResult (valid)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (1)
410-446: Remove variable shadowing to improve clarity.The test logic is sound and validates an important edge case. However, the local declarations of
MAXIMUM_VOTE_VALUE(line 411),votingPowerLeaf(line 412), andVOTE(lines 415-418) shadow identifiers already imported from '../src' and './constants' at lines 22 and 24, reducing code clarity.Consider either:
- Reusing the imported constants if their values match the test intent, or
- Renaming the local variables to avoid shadowing (e.g.,
maxVoteValue,maxVotingPower,maxVote)Apply this diff to remove shadowing:
- it('should succeed when the vote is the maximum value supported', { timeout: 100000 }, async () => { - const MAXIMUM_VOTE_VALUE = BigInt(Math.pow(2, 28) - 1) // 268,435,455 - const votingPowerLeaf = MAXIMUM_VOTE_VALUE // Balance at the limit - - // Vote exactly at the maximum - const VOTE = { - yes: MAXIMUM_VOTE_VALUE, - no: 0n, - } + it('should succeed when the vote is the maximum value supported', { timeout: 100000 }, async () => { + const maxVotingPower = MAXIMUM_VOTE_VALUE // Balance at the limit + + // Vote exactly at the maximum + const maxVote = { + yes: MAXIMUM_VOTE_VALUE, + no: 0n, + } - const encodedVote = encodeVote(VOTE, VotingMode.GOVERNANCE, votingPowerLeaf) + const encodedVote = encodeVote(maxVote, VotingMode.GOVERNANCE, maxVotingPower) // hardhat default private key const privateKey = '0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80' const account = privateKeyToAccount(privateKey) const signature = await account.signMessage({ message: MESSAGE }) - const leaf = hashLeaf(account.address.toLowerCase(), votingPowerLeaf.toString()) + const leaf = hashLeaf(account.address.toLowerCase(), maxVotingPower.toString()) const leaves = [...LEAVES, leaf] - const merkleProof = generateMerkleProof(0n, votingPowerLeaf, account.address.toLowerCase(), leaves, 20) + const merkleProof = generateMerkleProof(0n, maxVotingPower, account.address.toLowerCase(), leaves, 20) const inputs = await encryptVoteAndGenerateCRISPInputs({ encodedVote, publicKey, previousCiphertext, signature, message: MESSAGE, merkleData: merkleProof, - balance: votingPowerLeaf, + balance: maxVotingPower, slotAddress: account.address.toLowerCase(), isFirstVote: false, })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (5)
getCircuitOutputValue(286-292)encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)generateProof(264-272)verifyProof(294-298)examples/CRISP/packages/crisp-sdk/src/constants.ts (2)
MAXIMUM_VOTE_VALUE(23-23)MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/tests/constants.ts (4)
votingPowerLeaf(31-31)VOTE(14-14)MESSAGE(11-11)LEAVES(16-27)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 423-423: 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: rust_unit
- GitHub Check: build_sdk
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: rust_integration
🔇 Additional comments (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (1)
378-378: LGTM! Cleaner assertion pattern.The refactored rejection assertions using
await expect(...).rejects.toThrow()are more idiomatic and concise than the previous nested async function approach.Also applies to: 407-407, 474-474
ctrlc03
force-pushed
the
test/circuit-crisp-tests
branch
from
819c12f to
e4e679d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (1)
410-446: Consider using the importedMAXIMUM_VOTE_VALUEconstant.The test locally redefines
MAXIMUM_VOTE_VALUE,VOTE, andvotingPowerLeaf, which shadow the imported constant (line 22) and outer scope variables (line 24). While this makes the test self-contained, it could cause confusion.Consider these options:
- Use the imported
MAXIMUM_VOTE_VALUEdirectly- If local redefinition is intentional for clarity, add a comment explaining why
Apply this diff to use the imported constant:
- it('should succeed when the vote is the maximum value supported', { timeout: 100000 }, async () => { - const MAXIMUM_VOTE_VALUE = BigInt(Math.pow(2, 28) - 1) // 268,435,455 - const votingPowerLeaf = MAXIMUM_VOTE_VALUE // Balance at the limit - - // Vote exactly at the maximum - const VOTE = { + it('should succeed when the vote is the maximum value supported', { timeout: 100000 }, async () => { + // Use maximum value as both voting power and vote + const maxVotingPower = MAXIMUM_VOTE_VALUE + const maxVote = { yes: MAXIMUM_VOTE_VALUE, no: 0n, } - const encodedVote = encodeVote(VOTE, VotingMode.GOVERNANCE, votingPowerLeaf) + const encodedVote = encodeVote(maxVote, VotingMode.GOVERNANCE, maxVotingPower)Note: The hardcoded private key at line 423 is Hardhat's default test key and not a security concern, despite the static analysis warning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (4)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)generateProof(264-272)verifyProof(294-298)examples/CRISP/packages/crisp-sdk/src/constants.ts (2)
MAXIMUM_VOTE_VALUE(23-23)MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/tests/constants.ts (4)
votingPowerLeaf(31-31)VOTE(14-14)MESSAGE(11-11)LEAVES(16-27)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 423-423: 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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: rust_unit
- GitHub Check: rust_integration
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: build_e3_support_dev
🔇 Additional comments (3)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (3)
378-378: LGTM! Improved rejection assertion syntax.The refactor to use direct
await expect(...).rejects.toThrow()is more concise and follows modern Vitest/Jest patterns.
404-408: LGTM! Consistent refactor with clarifying comment.The refactored rejection assertion is consistent with the pattern used elsewhere, and the added comment improves clarity about what's being tested.
474-474: LGTM! Consistent refactor.The refactored rejection assertion maintains consistency with the other test cases in this file.
2 participants
fix #964
Summary by CodeRabbit