feat: generate merkle tree#826
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 WalkthroughAdds Merkle utilities and types to the CRISP SDK, updates exports, introduces blockchain balance lookup via viem, refines tree data handling to BigInt, adds tests, updates package and TypeScript configs, and makes a minor server import/format change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as App/SDK User
participant SDK as CRISP SDK
participant Utils as Utils (Merkle)
participant Server as CRISP Server
participant Chain as Blockchain (via viem)
Dev->>SDK: getTreeData(serverUrl, e3Id)
SDK->>Server: HTTP GET /treeData
Server-->>SDK: [hex leaves]
SDK-->>Dev: [bigint leaves]
Dev->>Utils: generateMerkleProof(threshold, balance, address, leaves)
Utils->>Utils: hashLeaf(address, balance)
Utils->>Utils: build LeanIMT + create proof
Utils-->>Dev: { leaf, index, proof }
Dev->>SDK: getBalanceAt(voterAddress, tokenAddress, block, chainId)
SDK->>Chain: read getPastVotes(token, voter, block)
Chain-->>SDK: balance (bigint)
SDK-->>Dev: balance (bigint)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
|
@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: 2
♻️ Duplicate comments (1)
examples/CRISP/sdk/src/token.ts (1)
9-9: Verify that the artifact file is committed and accessible.A previous review comment noted that
ERC20Votes.jsonwas not staged/committed. Ensure this artifact is now properly included in the repository.Run the following script to verify the artifact exists:
#!/bin/bash # Description: Verify ERC20Votes.json artifact exists if [ -f "examples/CRISP/sdk/src/artifacts/ERC20Votes.json" ]; then echo "✓ Artifact found" echo "Size: $(wc -c < examples/CRISP/sdk/src/artifacts/ERC20Votes.json) bytes" echo "Contains ABI: $(grep -q '"abi"' examples/CRISP/sdk/src/artifacts/ERC20Votes.json && echo "yes" || echo "no")" else echo "✗ Artifact NOT found at expected path" fi
🧹 Nitpick comments (3)
examples/CRISP/sdk/src/token.ts (1)
18-37: Consider adding error handling for malformed hex strings.While the hex-to-BigInt conversion handles both prefixed (
0x) and non-prefixed formats, it doesn't validate hex string format or handle potential conversion errors. Malformed input could cause runtime exceptions.Consider adding validation:
export const getTreeData = async (serverUrl: string, e3Id: number): Promise<bigint[]> => { const response = await fetch(`${serverUrl}/${CRISP_SERVER_TOKEN_TREE_ENDPOINT}`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ round_id: e3Id }), }) const hashes = (await response.json()) as string[] // Convert hex strings to BigInts return hashes.map((hash) => { + // Validate hex string format + const hexString = hash.startsWith('0x') ? hash : '0x' + hash + if (!/^0x[0-9a-fA-F]+$/.test(hexString)) { + throw new Error(`Invalid hex string format: ${hash}`) + } + try { + return BigInt(hexString) + } catch (error) { + throw new Error(`Failed to convert hex string to BigInt: ${hash}`) + } - // Ensure the hash is treated as a hex string - if (!hash.startsWith('0x')) { - return BigInt('0x' + hash) - } - return BigInt(hash) }) }examples/CRISP/sdk/tests/utils.test.ts (1)
15-17: Test depends on external server availability.The
beforeAllhook fetches data from an external server (CRISP_SERVER_URL). If the server is unavailable or returns unexpected data, all tests will fail. Consider:
- Mocking the server response for more reliable tests
- Adding error handling in
beforeAllwith a clear failure message- Documenting that tests require a running CRISP server
Example with error handling:
beforeAll(async () => { - leaves = await getTreeData(CRISP_SERVER_URL, 0) + try { + leaves = await getTreeData(CRISP_SERVER_URL, 0) + if (!leaves || leaves.length === 0) { + throw new Error('No leaves returned from server') + } + } catch (error) { + throw new Error(`Failed to fetch test data from CRISP server: ${error}`) + } })examples/CRISP/sdk/src/utils.ts (1)
51-60: Consider accepting a pre-built tree to avoid reconstruction.The function recreates the entire Merkle tree on each call, which is inefficient if multiple proofs need to be generated from the same tree. However, for the current use case of generating a single proof, this is acceptable.
If performance becomes a concern, consider adding an overload that accepts a pre-built tree:
export const generateMerkleProof = ( threshold: number, balance: number, address: string, leavesOrTree: bigint[] | LeanIMT ): IMerkleProof => { if (balance < threshold) { throw new Error('Balance is below the threshold') } const leaf = hashLeaf(address, balance.toString()) const tree = Array.isArray(leavesOrTree) ? generateMerkleTree(leavesOrTree) : leavesOrTree const leaves = Array.isArray(leavesOrTree) ? leavesOrTree : tree.leaves const index = leaves.findIndex((l) => l === leaf) if (index === -1) { throw new Error('Leaf not found in the tree') } const proof = tree.generateProof(index) return { leaf, index, proof, } }
📜 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 (9)
examples/CRISP/sdk/README.md(2 hunks)examples/CRISP/sdk/package.json(1 hunks)examples/CRISP/sdk/src/index.ts(1 hunks)examples/CRISP/sdk/src/token.ts(2 hunks)examples/CRISP/sdk/src/types.ts(2 hunks)examples/CRISP/sdk/src/utils.ts(1 hunks)examples/CRISP/sdk/tests/utils.test.ts(1 hunks)examples/CRISP/sdk/tsconfig.json(1 hunks)examples/CRISP/server/src/server/repo.rs(2 hunks)
🔇 Additional comments (10)
examples/CRISP/sdk/tsconfig.json (1)
12-12: LGTM!Adding JSON files to the TypeScript project inclusion is necessary for importing
ERC20Votes.jsonintoken.ts.examples/CRISP/sdk/README.md (1)
3-18: LGTM!Minor documentation formatting changes with no functional impact.
examples/CRISP/server/src/server/repo.rs (1)
248-248: LGTM!Minor formatting improvements with no functional impact.
Also applies to: 262-265
examples/CRISP/sdk/src/types.ts (1)
7-8: LGTM!The
IMerkleProofinterface is well-defined with proper typing and documentation. The integration withLeanIMTMerkleProoffrom the external library is appropriate.Also applies to: 56-63
examples/CRISP/sdk/src/token.ts (1)
47-73: Limited chain support may restrict usage.The
getBalanceAtfunction only supports Sepolia (11155111) and localhost (31337). Consider whether this limitation aligns with the SDK's intended scope or if additional chains should be supported.If broader chain support is needed, consider:
export const getBalanceAt = async (voterAddress: string, tokenAddress: string, snapshotBlock: number, chainId: number): Promise<bigint> => { let chain switch (chainId) { case 11155111: chain = sepolia break case 31337: chain = localhost break + case 1: + chain = mainnet + break + // Add other chains as needed default: throw new Error('Unsupported chainId') }Alternatively, you could dynamically create chain config or accept a chain parameter.
examples/CRISP/sdk/src/index.ts (1)
10-10: LGTM!The expanded exports properly expose the new utility functions and types (
IMerkleProof,IRoundDetailsResponse) added in this PR.Also applies to: 12-12
examples/CRISP/sdk/tests/utils.test.ts (1)
19-44: LGTM!The test suite provides good coverage of the utility functions, including both success and error cases. The test structure is clear and assertions are appropriate.
examples/CRISP/sdk/package.json (1)
40-44: Dependencies validated, no vulnerabilities detected.
All specified versions exist on npm and have no reported security advisories. Pinning viem at 2.30.6 is acceptable given the next release is a canary build.examples/CRISP/sdk/src/utils.ts (2)
27-29: LGTM!The Merkle tree construction correctly follows the LeanIMT API, providing a hash function and initial leaves.
38-49: Verify the hashLeaf call after fixing the type conversion issue.The logic correctly validates the threshold and searches for the leaf in the tree. However, line 43 calls
hashLeafwith string inputs, which will fail due to the type conversion issue flagged in thehashLeaffunction. Once that function is fixed to properly convert inputs to bigint, this code will work correctly.
* fix: server resetting data * chore: add license * feat: get round data from crisp server * feat: generate merkle tree * feat: generate voter's merkle tree locally and proof * chore: handle hex and add tests * chore: add erc20votes abi * chore: remove redundant import
* fix: server resetting data * chore: add license * feat: get round data from crisp server * feat: generate merkle tree * feat: generate voter's merkle tree locally and proof * chore: handle hex and add tests * chore: add erc20votes abi * chore: remove redundant import
* fix: server resetting data * chore: add license * feat: get round data from crisp server * feat: generate merkle tree * feat: generate voter's merkle tree locally and proof * chore: handle hex and add tests * chore: add erc20votes abi * chore: remove redundant import
* fix: server resetting data * chore: add license * feat: get round data from crisp server * feat: generate merkle tree * feat: generate voter's merkle tree locally and proof * chore: handle hex and add tests * chore: add erc20votes abi * chore: remove redundant import
2 participants
fix #807
Summary by CodeRabbit
New Features
Changes
Tests
Chores
Documentation