chore: tally decoding updated to only sum up the correct coefficients#951
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds on-chain tally decoding: a public Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant CRISPProgram
participant Enclave
Caller->>CRISPProgram: decodeTally(e3Id)
CRISPProgram->>Enclave: getE3(e3Id)
Enclave-->>CRISPProgram: E3 { plaintextOutput, ... }
rect rgb(220,245,220)
Note over CRISPProgram: Decode & tally extraction
CRISPProgram->>CRISPProgram: ABI-decode plaintextOutput -> uint256[] tally
CRISPProgram->>CRISPProgram: compute START_INDEX_Y / START_INDEX_N
CRISPProgram->>CRISPProgram: extract slices, apply decreasing bit-weights
CRISPProgram->>CRISPProgram: accumulate yes and no totals
end
CRISPProgram-->>Caller: (yes, no)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-11-05T14:12:57.791ZApplied to files:
📚 Learning: 2025-09-19T11:16:53.825ZApplied to files:
📚 Learning: 2024-10-01T02:51:17.718ZApplied to files:
📚 Learning: 2025-10-10T12:56:40.538ZApplied to files:
🧬 Code graph analysis (1)examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
🪛 Biome (2.1.2)examples/CRISP/packages/crisp-sdk/src/vote.ts[error] 100-100: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 101-101: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 102-102: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 105-105: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 106-106: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 108-108: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) [error] 109-109: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) ⏰ 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (1)
28-145: Preserve the top bit when decoding tallies
MAXIMUM_VOTE_VALUEis2^28, so a valid tally can legitimately set the 29th bit. By only summing the last 28 coefficients you zero out that MSB, leading to incorrect yes/no totals at the advertised limit. Please widen the window (and the initial weight) so every significant bit survives decoding; otherwise tallies at the limit regress.Apply this diff:
- /// @notice Half of the largest minimum degree used to fit votes - /// inside the plaintext polynomial - uint256 public constant HALF_LARGEST_MINIMUM_DEGREE = 28; + /// @notice Half of the largest minimum degree used to fit votes + /// inside the plaintext polynomial (supports MAXIMUM_VOTE_VALUE = 2^28) + uint256 public constant HALF_LARGEST_MINIMUM_DEGREE = 29; @@ - uint256 weight = 2 ** (HALF_LARGEST_MINIMUM_DEGREE - 1); + uint256 weight = 1 << (HALF_LARGEST_MINIMUM_DEGREE - 1); @@ - weight /= 2; // Right shift equivalent + weight >>= 1; // Right shift equivalent @@ - weight = 2 ** (HALF_LARGEST_MINIMUM_DEGREE - 1); + weight = 1 << (HALF_LARGEST_MINIMUM_DEGREE - 1); @@ - weight /= 2; + weight >>= 1;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(5 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(3 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(2 hunks)examples/CRISP/packages/crisp-sdk/package.json(2 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(2 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-01T02:51:17.718Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 119
File: packages/evm/contracts/test/MockE3Program.sol:15-17
Timestamp: 2024-10-01T02:51:17.718Z
Learning: In mock contracts used for testing, it's acceptable for functions to lack access control since they do not pose security risks.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
📚 Learning: 2025-09-11T12:56:39.601Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/test/Enclave.spec.ts:804-807
Timestamp: 2025-09-11T12:56:39.601Z
Learning: In Hardhat v3, the chai matchers syntax changed to support multiple network connections: `.to.not.be.revert(ethers)` is the correct new syntax, replacing the old `.to.not.be.reverted` (without parameters). The ethers object must be passed as a parameter to the revert/reverted matchers in Hardhat v3. Similarly, `.revertedWithoutReason(ethers)` replaces `.revertedWithoutReason`.
Applied to files:
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
📚 Learning: 2025-10-03T08:38:36.786Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 781
File: examples/CRISP/test/governanceCircuit.test.ts:15-50
Timestamp: 2025-10-03T08:38:36.786Z
Learning: Hardhat v3 supports Node.js's native test runner (node:test) including importing describe and it from 'node:test'. Tests using this interface are compatible with Hardhat v3's test task.
Applied to files:
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
📚 Learning: 2025-09-11T13:09:03.800Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.800Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
HALF_LARGEST_MINIMUM_DEGREE(24-24)
🪛 Biome (2.1.2)
examples/CRISP/packages/crisp-sdk/src/vote.ts
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 105-105: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 106-106: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: build_enclave_cli
- GitHub Check: test_contracts
- GitHub Check: rust_integration
- GitHub Check: rust_unit
6bf76a9 to
0dfcf2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(5 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(3 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(2 hunks)examples/CRISP/packages/crisp-sdk/package.json(2 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(2 hunks)examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(1 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/CRISP/packages/crisp-zk-inputs/package.json
- examples/CRISP/packages/crisp-sdk/package.json
- examples/CRISP/packages/crisp-contracts/package.json
- examples/CRISP/packages/crisp-sdk/src/constants.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-11T13:09:03.800Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.800Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
📚 Learning: 2024-10-01T02:51:17.718Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 119
File: packages/evm/contracts/test/MockE3Program.sol:15-17
Timestamp: 2024-10-01T02:51:17.718Z
Learning: In mock contracts used for testing, it's acceptable for functions to lack access control since they do not pose security risks.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
HALF_LARGEST_MINIMUM_DEGREE(17-17)
🪛 Biome (2.1.2)
examples/CRISP/packages/crisp-sdk/src/vote.ts
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 105-105: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 106-106: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: test_contracts
- GitHub Check: rust_integration
- GitHub Check: build_enclave_cli
- GitHub Check: rust_unit
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: build_sdk
0dfcf2f to
b5178d5
Compare
b5178d5 to
bf9f152
Compare
bf9f152 to
085eb06
Compare
Decode plaintext output on CRISP program contract
fix #948
Summary by CodeRabbit
New Features
Chores