[Fix]:: Redeploy Zylith and Verify Private Swap Flow #25
Conversation
📝 WalkthroughWalkthroughThis PR redeploys the Zylith protocol with a new contract address, refactors Garaga verifier interfaces to use generic naming conventions, removes hardcoded verifier constants, adds JavaScript witness calculators for circuit witness generation, includes new Groth16 verification key JSONs, and updates build scripts and documentation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/integration/frontend-integration-guide.md (1)
84-87: Critical: Verifier addresses inconsistent with frontend config.The verifier addresses in this documentation do not match the addresses in
frontend/src/lib/config.ts:Documentation (this file):
- Membership:
0x066448de8e457554d16155f215386dc9c8052a5d99212586840494142aedc165- Swap:
0x0432a5184b4e187cf68a7c476c653528b7da14f6851de8c8e1ce76b1e1bb9e36- Withdraw:
0x037f7a9fed4daa5ec5ff69e5a101ccf40c219f6cb3c0cb081c64d34ac4a26ad0- LP:
0x0745acde8db05d4b4a49dc1f2cd313a3a8960e812d41d0b71ff90457f8ebbe7efrontend/src/lib/config.ts:
- Membership:
0x011c0deb3618f2358dcba0dda14f43ef47f40b7be681d6708f554ce3d0ad5432- Swap:
0x04e7dc3190830a31c626e88182630b1eb71f8f6c6f9562adb358697f4754093b- Withdraw:
0x04ade28020ebb5676a8a55219bba7f4ef175ae8f8f8189491193b1153e991330- LP:
0x0202fa77f1158fce60dbb3d62b503dba0ce9f360003507f459d21cbed52c87d6The addresses in
config.tsmatchzylith/CONTRACT_ADDRESS.md, suggesting the documentation here is outdated.🔎 Proposed fix
-- **Membership Verifier**: `0x066448de8e457554d16155f215386dc9c8052a5d99212586840494142aedc165` -- **Swap Verifier**: `0x0432a5184b4e187cf68a7c476c653528b7da14f6851de8c8e1ce76b1e1bb9e36` -- **Withdraw Verifier**: `0x037f7a9fed4daa5ec5ff69e5a101ccf40c219f6cb3c0cb081c64d34ac4a26ad0` -- **LP Verifier**: `0x0745acde8db05d4b4a49dc1f2cd313a3a8960e812d41d0b71ff90457f8ebbe7e` +- **Membership Verifier**: `0x011c0deb3618f2358dcba0dda14f43ef47f40b7be681d6708f554ce3d0ad5432` +- **Swap Verifier**: `0x04e7dc3190830a31c626e88182630b1eb71f8f6c6f9562adb358697f4754093b` +- **Withdraw Verifier**: `0x04ade28020ebb5676a8a55219bba7f4ef175ae8f8f8189491193b1153e991330` +- **LP Verifier**: `0x0202fa77f1158fce60dbb3d62b503dba0ce9f360003507f459d21cbed52c87d6`
🧹 Nitpick comments (6)
circuits/build/membership/membership_js/generate_witness.js (1)
10-19: Missing error handling for promise rejection.If WASM compilation or witness calculation fails, the rejection is unhandled. Consider adding a
.catch()handler for better error reporting.Suggested fix
wc(buffer).then(async witnessCalculator => { const buff= await witnessCalculator.calculateWTNSBin(input,0); writeFile(process.argv[4], buff, function(err) { if (err) throw err; }); - }); + }).catch(err => { + console.error("Error generating witness:", err); + process.exit(1); + });circuits/build/withdraw/withdraw_js/witness_calculator.js (1)
1-331: Duplicate witness calculator module.This file is identical to the membership circuit's
witness_calculator.js. While this duplication is typical for circom-generated build artifacts, consider extracting a shared module to reduce maintenance burden if these files are version-controlled long-term.The implementation is correct for witness computation.
zylith/CONTRACT_ADDRESS.md (1)
6-7: Optional: Consider wrapping bare URLs for markdown compliance.The markdown linter flags bare URLs. While this doesn't affect functionality, you can improve markdown compliance by wrapping URLs in angle brackets.
Example fix for bare URLs
-🔍 **Explorer:** https://sepolia.starkscan.co -🔗 **RPC:** https://api.cartridge.gg/x/starknet/sepolia +🔍 **Explorer:** <https://sepolia.starkscan.co> +🔗 **RPC:** <https://api.cartridge.gg/x/starknet/sepolia>Apply similar changes to other bare URLs in the file.
Also applies to: 25-26, 45-45, 56-56, 67-67, 78-78
circuits/build/vkeys/membership_vk.json (1)
1-99: Document verification key provenance and validation.These Groth16 verification keys are critical cryptographic parameters. Ensure:
- Keys are generated from a trusted setup or verifiable circuit compilation process
- The generation process is documented (e.g., which circuit version, compilation command, date)
- Keys have been tested with valid/invalid proofs to confirm correct verification behavior
Consider adding a README or comments documenting the key generation process for auditability.
circuits/package.json (2)
5-12: Consider splitting long scripts for maintainability.The inline scripts are quite long and concatenate multiple commands. For better maintainability, consider extracting these into separate shell scripts (e.g.,
scripts/compile.sh,scripts/setup.sh) and referencing them from package.json. This would also make debugging easier when individual steps fail.
14-18: Consider pinning exact versions for cryptographic dependencies for better reproducibility.While the current versions are current (snarkjs has a minor update to 0.7.5 available, circomlib is at latest 2.0.5), caret ranges can introduce subtle version drift across environments. For security-sensitive cryptographic libraries, exact version pinning (
1.2.3instead of^1.2.3) provides better control and reproducibility, especially for snarkjs which is still in pre-1.0 status.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
circuits/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (57)
.gitignoreasp/README.mdasp/src/bin/initialize_pool.rsasp/src/main.rsasp/start.shcircuits/bin/circomcircuits/build/garaga/lp_verifier/.tool-versionscircuits/build/garaga/lp_verifier/Scarb.tomlcircuits/build/garaga/lp_verifier/src/groth16_verifier.cairocircuits/build/garaga/lp_verifier/src/groth16_verifier_constants.cairocircuits/build/garaga/lp_verifier/src/lib.cairocircuits/build/garaga/lp_verifier/tests/test_contract.cairocircuits/build/garaga/membership_verifier/.tool-versionscircuits/build/garaga/membership_verifier/Scarb.tomlcircuits/build/garaga/membership_verifier/src/groth16_verifier.cairocircuits/build/garaga/membership_verifier/src/groth16_verifier_constants.cairocircuits/build/garaga/membership_verifier/src/lib.cairocircuits/build/garaga/membership_verifier/tests/test_contract.cairocircuits/build/garaga/swap_verifier/.tool-versionscircuits/build/garaga/swap_verifier/Scarb.tomlcircuits/build/garaga/swap_verifier/src/groth16_verifier.cairocircuits/build/garaga/swap_verifier/src/groth16_verifier_constants.cairocircuits/build/garaga/swap_verifier/src/lib.cairocircuits/build/garaga/swap_verifier/tests/test_contract.cairocircuits/build/garaga/withdraw_verifier/.tool-versionscircuits/build/garaga/withdraw_verifier/Scarb.tomlcircuits/build/garaga/withdraw_verifier/src/groth16_verifier.cairocircuits/build/garaga/withdraw_verifier/src/groth16_verifier_constants.cairocircuits/build/garaga/withdraw_verifier/src/lib.cairocircuits/build/garaga/withdraw_verifier/tests/test_contract.cairocircuits/build/lp/lp_js/generate_witness.jscircuits/build/lp/lp_js/witness_calculator.jscircuits/build/membership/membership_js/generate_witness.jscircuits/build/membership/membership_js/witness_calculator.jscircuits/build/swap/swap_js/generate_witness.jscircuits/build/swap/swap_js/witness_calculator.jscircuits/build/vkeys/lp_vk.jsoncircuits/build/vkeys/membership_vk.jsoncircuits/build/vkeys/swap_vk.jsoncircuits/build/vkeys/withdraw_vk.jsoncircuits/build/withdraw/withdraw_js/generate_witness.jscircuits/build/withdraw/withdraw_js/witness_calculator.jscircuits/lp_verifier/.tool-versionscircuits/lp_verifier/src/groth16_verifier_constants.cairocircuits/membership_verifier/.tool-versionscircuits/membership_verifier/src/groth16_verifier_constants.cairocircuits/package.jsoncircuits/swap_verifier/.tool-versionscircuits/swap_verifier/src/groth16_verifier_constants.cairocircuits/withdraw_verifier/src/groth16_verifier_constants.cairodocs/integration/frontend-integration-guide.mdfrontend/README.mdfrontend/src/components/portfolio/__tests__/NotesList.test.tsxfrontend/src/hooks/__tests__/use-private-deposit.test.tsfrontend/src/lib/config.tszylith/CONTRACT_ADDRESS.mdzylith/deployment_sepolia_20251226_112137.json
💤 Files with no reviewable changes (8)
- circuits/withdraw_verifier/src/groth16_verifier_constants.cairo
- circuits/lp_verifier/src/groth16_verifier_constants.cairo
- circuits/swap_verifier/.tool-versions
- circuits/swap_verifier/src/groth16_verifier_constants.cairo
- .gitignore
- circuits/membership_verifier/.tool-versions
- circuits/lp_verifier/.tool-versions
- circuits/membership_verifier/src/groth16_verifier_constants.cairo
🧰 Additional context used
🧬 Code graph analysis (6)
circuits/build/withdraw/withdraw_js/witness_calculator.js (4)
circuits/build/lp/lp_js/generate_witness.js (3)
wc(1-1)input(7-7)buff(15-15)circuits/build/membership/membership_js/generate_witness.js (3)
wc(1-1)input(7-7)buff(15-15)circuits/build/swap/swap_js/generate_witness.js (3)
wc(1-1)input(7-7)buff(15-15)circuits/build/withdraw/withdraw_js/generate_witness.js (3)
wc(1-1)input(7-7)buff(15-15)
circuits/build/swap/swap_js/witness_calculator.js (1)
circuits/build/lp/lp_js/witness_calculator.js (6)
err(22-22)instance(19-64)j(94-94)i(297-297)res(278-278)n(328-328)
circuits/build/withdraw/withdraw_js/generate_witness.js (3)
circuits/build/lp/lp_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/membership/membership_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/swap/swap_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)
circuits/build/lp/lp_js/generate_witness.js (3)
circuits/build/membership/membership_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/swap/swap_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/withdraw/withdraw_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)
circuits/build/membership/membership_js/generate_witness.js (3)
circuits/build/lp/lp_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/swap/swap_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/withdraw/withdraw_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)
circuits/build/swap/swap_js/generate_witness.js (3)
circuits/build/lp/lp_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/membership/membership_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)circuits/build/withdraw/withdraw_js/generate_witness.js (5)
wc(1-1)require(2-2)input(7-7)buffer(9-9)buff(15-15)
🪛 markdownlint-cli2 (0.18.1)
zylith/CONTRACT_ADDRESS.md
6-6: Bare URL used
(MD034, no-bare-urls)
7-7: Bare URL used
(MD034, no-bare-urls)
25-25: Bare URL used
(MD034, no-bare-urls)
26-26: Bare URL used
(MD034, no-bare-urls)
32-32: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
45-45: Bare URL used
(MD034, no-bare-urls)
56-56: Bare URL used
(MD034, no-bare-urls)
67-67: Bare URL used
(MD034, no-bare-urls)
78-78: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (30)
circuits/build/garaga/swap_verifier/Scarb.toml (1)
16-16: The snforge_std 0.53.0 version is correctly specified and compatible with the current toolchain. Starknet Foundry 0.53.x requires Scarb >= 2.13 (current version 2.14.0 meets this requirement), and snforge_std 0.53 aligns with Foundry 0.53 as specified in .tool-versions. This downgrade is intentional and ensures proper toolchain alignment across the verifier crates.circuits/build/membership/membership_js/witness_calculator.js (2)
1-106: Auto-generated circom witness calculator - looks correct.This is standard snarkjs/circom-generated code for WASM-based witness computation. The WebAssembly compilation, runtime hooks for exception handling, and message buffering are correctly implemented.
108-331: WitnessCalculator class and utilities are correctly implemented.The class properly handles:
- Prime field initialization from WASM exports
- Input signal hashing via FNV-1a
- Input validation with clear error messages
- Multiple witness output formats (array, binary, WTNS)
circuits/build/withdraw/withdraw_js/generate_witness.js (1)
1-20: Functionally correct, same issues as other generate_witness scripts.This script mirrors the membership circuit's generator. The same improvements apply: add
process.exit(1)after usage and add.catch()for error handling.circuits/build/lp/lp_js/generate_witness.js (1)
1-20: Consistent with other circuit generators.This script follows the same pattern as the membership, withdraw, and swap circuits. The implementation is correct for its purpose.
circuits/build/swap/swap_js/generate_witness.js (1)
1-20: Consistent implementation for swap circuit witness generation.The script correctly orchestrates WASM-based witness computation and writes the WTNS binary output.
circuits/build/lp/lp_js/witness_calculator.js (1)
1-331: Identical witness calculator for LP circuit.This is the same auto-generated circom witness calculator used across all circuits. Implementation is correct.
circuits/build/swap/swap_js/witness_calculator.js (1)
1-331: Final witness calculator module - completes the circuit set.All four witness calculator modules (membership, withdraw, lp, swap) are identical auto-generated files from the circom toolchain. This is expected for build artifacts. The implementation correctly handles WebAssembly-based witness computation with proper input validation and multiple output formats.
frontend/README.md (1)
47-47: LGTM!The contract address has been correctly updated to match the new Sepolia deployment.
frontend/src/hooks/__tests__/use-private-deposit.test.ts (1)
1-116: LGTM!Test file updated with consistent formatting (double quotes) and new contract address. No functional changes to test logic.
frontend/src/components/portfolio/__tests__/NotesList.test.tsx (1)
1-179: LGTM!Excellent expansion of test coverage with new test cases for edge cases (empty notes, missing leaf indices, partial withdrawals) while maintaining consistency with the new contract address.
frontend/src/lib/config.ts (1)
3-20: LGTM!Configuration updated with new contract addresses for Zylith and all verifiers. Formatting consistently uses double quotes.
zylith/CONTRACT_ADDRESS.md (1)
20-76: LGTM!Contract addresses are correctly documented and consistent with
frontend/src/lib/config.ts.asp/src/bin/initialize_pool.rs (1)
22-22: LGTM!Default contract address updated to match the new Sepolia deployment.
asp/README.md (1)
20-22: Documentation updated correctly.The documentation examples have been updated to reflect the new contract address, keeping the README in sync with the code changes in
main.rsandstart.sh.Also applies to: 44-46
asp/start.sh (1)
33-35: Shell script configuration updated correctly.The default CONTRACT_ADDRESS environment variable has been updated to match the changes in
main.rsand the documentation. All three files now consistently reference the new redeployed contract address.asp/src/main.rs (1)
65-67: Configuration update is correct.The default CONTRACT_ADDRESS has been updated to the redeployed Zylith contract on Sepolia. The new address is properly formatted for Starknet and consistently applied across the codebase with no remaining references to the previous address.
circuits/build/garaga/swap_verifier/.tool-versions (1)
1-2: No action needed—all verifier modules have consistent versions.All four verifier modules (.tool-versions files) already use identical versions: scarb 2.14.0 and starknet-foundry 0.53.0. There is no version inconsistency across lp_verifier, membership_verifier, swap_verifier, and withdraw_verifier.
Likely an incorrect or invalid review comment.
circuits/build/garaga/membership_verifier/Scarb.toml (1)
16-16: Confirm test compatibility with snforge_std 0.53.0, especially for cheatcode-based tests.The downgrade from 0.54.0 to 0.53.0 aligns with the
.tool-versionsconstraint (starknet-foundry 0.53.0), making it intentional. However, 0.54.0 includes a bug fix for cheatcodes reflection when calling contracts from test code—a potential concern if this project's tests rely on cheatcodes. Verify that:
- All tests pass with snforge_std 0.53.0
- No cheatcode-based test failures are introduced by the downgrade
circuits/build/garaga/swap_verifier/tests/test_contract.cairo (1)
1-90: Auto-generated test file looks correct.The test structure follows Garaga's standard pattern for Groth16 verification testing. The fork-based approach against Sepolia testnet is appropriate for accessing the Garaga ECIP ops contract.
Note: The module docstring (lines 8-10) mentions testing "both positive (valid proof) and negative (invalid proof) cases," but only the positive case is implemented. Consider adding a negative test case in a follow-up to verify that invalid proofs are correctly rejected.
circuits/build/garaga/withdraw_verifier/tests/test_contract.cairo (1)
1-90: LGTM!Structurally identical to other Garaga-generated verifier tests with appropriate module imports for
withdraw_verifier. The test pattern is consistent across all verifier modules.circuits/build/garaga/membership_verifier/tests/test_contract.cairo (1)
1-90: LGTM!Consistent with the other Garaga-generated verifier test files. The test correctly exercises the membership proof verification flow.
circuits/build/garaga/lp_verifier/tests/test_contract.cairo (1)
1-90: LGTM!Consistent test structure for the LP verifier, matching the pattern established across all Groth16 verifier test modules.
circuits/package.json (1)
7-7: Powers of Tau ceremony setup is for development only.The
setupscript creates a trusted setup with a single contribution (--name="First contribution"). This is appropriate for development/testing but should not be used for production deployments. Production setups require multiple independent contributions from different parties to ensure security.circuits/build/garaga/lp_verifier/src/groth16_verifier.cairo (1)
1-94: LGTM!Auto-generated verifier code consistent with the membership verifier. The verification logic is identical; circuit-specific parameters are imported from
groth16_verifier_constants.circuits/build/garaga/swap_verifier/src/groth16_verifier.cairo (1)
1-94: LGTM!The swap verifier is central to the PR's objective of verifying the private swap flow. The generated code follows the same Garaga pattern as the other verifiers, with circuit-specific verification key constants imported separately.
circuits/build/garaga/membership_verifier/src/groth16_verifier.cairo (1)
1-94: Auto-generated verifier code—do not manually edit.This file is generated by Garaga v1.0.1. The
ECIP_OPS_CLASS_HASH(line 23-24) is consistent across all verifier files (0x312d1dd5f967eaf6f86965e3fa7acbc9d0fbd979066a17721dd913736af2f5e). Verify this class hash is valid for your target network (Sepolia) when redeploying.circuits/build/garaga/withdraw_verifier/src/groth16_verifier.cairo (3)
29-92: LGTM! Implementation logic is sound.The verification logic correctly implements Groth16 proof verification for BN254:
- Deserializes and validates proof points
- Computes
vk_xvia multi-scalar multiplication with public inputs- Performs the final pairing check against the verification key
The formatting improvements (trailing commas) align with Cairo conventions.
1-1: Confirm whether changes are from Garaga generator or manual edits.This file is generated by Garaga version 1.0.1 (line 1). If the trait/module renaming was done manually, those changes could be lost when regenerating the verifier. Clarify whether:
- This is output from a new Garaga version/configuration with generic naming, or
- Manual edits were applied post-generation (which should be avoided or documented).
23-24: Verify the ECIP_OPS_CLASS_HASH is deployed and correct for Starknet Sepolia.The
ECIP_OPS_CLASS_HASHconstant (0x312d1dd5f967eaf6f86965e3fa7acbc9d0fbd979066a17721dd913736af2f5e) references the Garaga ECIP ops external contract used for multi-scalar multiplication operations. This hash does not appear on Starknet Sepolia explorer, suggesting either the class hash is incorrect for Sepolia or the Garaga ECIP ops contract needs to be declared separately before deployment. Confirm this hash matches the actual deployed Garaga ECIP ops class on Sepolia.
| #[starknet::interface] | ||
| pub trait IMembershipGroth16VerifierBN254<TContractState> { | ||
| pub trait IGroth16VerifierBN254<TContractState> { | ||
| fn verify_groth16_proof_bn254( | ||
| self: @TContractState, | ||
| full_proof_with_hints: Span<felt252>, | ||
| self: @TContractState, full_proof_with_hints: Span<felt252>, | ||
| ) -> Result<Span<u256>, felt252>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the old trait name across the codebase
echo "=== Searching for old trait name: IWithdrawGroth16VerifierBN254 ==="
rg -n 'IWithdrawGroth16VerifierBN254' -C2
echo -e "\n=== Searching for new trait name to understand scope ==="
rg -n 'IGroth16VerifierBN254' -C1 | head -50
echo -e "\n=== Checking if old module name is still referenced ==="
rg -n 'WithdrawGroth16VerifierBN254' -C1 | head -30Repository: SunsetLabs-Game/Zylith
Length of output: 7278
Generated file uses new trait name but main codebase still uses old name — this creates an incompatibility.
The trait in this generated file has been renamed to the generic IGroth16VerifierBN254, but the main codebase implementation at zylith/src/privacy/verifiers/withdraw/groth16_verifier.cairo still defines IWithdrawGroth16VerifierBN254. Active code in zylith/src/zylith.cairo:32-33 imports the old trait name, and the deployment script references the old contract name. The generated file's new naming must be coordinated with updates to the main codebase implementation.
🤖 Prompt for AI Agents
In circuits/build/garaga/withdraw_verifier/src/groth16_verifier.cairo around
lines 5 to 10, the generated trait was renamed to IGroth16VerifierBN254 which
breaks compatibility with the rest of the codebase that expects
IWithdrawGroth16VerifierBN254; to fix this, restore the original trait name
(IWithdrawGroth16VerifierBN254) in this generated file or, alternatively, update
the main implementation
(zylith/src/privacy/verifiers/withdraw/groth16_verifier.cairo), the import in
zylith/src/zylith.cairo (lines ~32-33), and any deployment scripts to use the
new IGroth16VerifierBN254 identifier so all references match the generated name.
| #[starknet::contract] | ||
| mod MembershipGroth16VerifierBN254 { | ||
| use starknet::SyscallResultTrait; | ||
| use garaga::definitions::{G1Point, G1G2Pair}; | ||
| use garaga::groth16::{multi_pairing_check_bn254_3P_2F_with_extra_miller_loop_result, Groth16ProofRawTrait}; | ||
| mod Groth16VerifierBN254 { | ||
| use garaga::definitions::{G1G2Pair, G1Point}; | ||
| use garaga::ec_ops::{G1PointTrait, ec_safe_add}; | ||
| use garaga::utils::calldata::{deserialize_full_proof_with_hints_bn254}; | ||
| use super::{N_PUBLIC_INPUTS, vk, ic, precomputed_lines}; | ||
| use garaga::groth16::{ | ||
| Groth16ProofRawTrait, multi_pairing_check_bn254_3P_2F_with_extra_miller_loop_result, | ||
| }; | ||
| use garaga::utils::calldata::deserialize_full_proof_with_hints_bn254; | ||
| use starknet::SyscallResultTrait; | ||
| use super::{N_PUBLIC_INPUTS, ic, precomputed_lines, vk}; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for old module name references
echo "=== Searching for old module name 'WithdrawGroth16VerifierBN254' ==="
rg -n 'WithdrawGroth16VerifierBN254' --type cairo
echo -e "\n=== Searching for all Groth16 verifier module declarations ==="
fd -e cairo -path '*/verifier/*' | xargs grep -h "^mod " | sort -u
echo -e "\n=== Checking all verifier directories ==="
fd -type d -name '*_verifier' | sort
echo -e "\n=== Listing groth16_verifier.cairo files ==="
fd -name 'groth16_verifier.cairo'Repository: SunsetLabs-Game/Zylith
Length of output: 905
🏁 Script executed:
#!/bin/bash
# Search for old module name references in Cairo files
echo "=== Searching for old module name 'WithdrawGroth16VerifierBN254' ==="
find . -name '*.cairo' -type f | head -50
echo -e "\n=== Searching for WithdrawGroth16VerifierBN254 ==="
grep -r 'WithdrawGroth16VerifierBN254' . 2>/dev/null || echo "Not found"
echo -e "\n=== Searching for all Groth16Verifier module declarations ==="
grep -r '^mod Groth16Verifier' . 2>/dev/null
echo -e "\n=== All verifier-related files ==="
find . -path '*verifier*' -name '*.cairo' -type f | sortRepository: SunsetLabs-Game/Zylith
Length of output: 5008
🏁 Script executed:
#!/bin/bash
echo "=== Check trait/interface names in withdraw verifier ==="
grep -n "pub trait" ./zylith/src/privacy/verifiers/withdraw/groth16_verifier.cairo
echo -e "\n=== Check trait/interface names in lp verifier ==="
grep -n "pub trait" ./zylith/src/privacy/verifiers/lp/groth16_verifier.cairo
echo -e "\n=== Check trait/interface names in membership verifier ==="
grep -n "pub trait" ./zylith/src/privacy/verifiers/membership/groth16_verifier.cairo
echo -e "\n=== Check trait/interface names in swap verifier ==="
grep -n "pub trait" ./zylith/src/privacy/verifiers/swap/groth16_verifier.cairo
echo -e "\n=== Check what verifier traits are actually imported in zylith.cairo ==="
grep -n "Dispatcher" ./zylith/src/zylith.cairo | head -20Repository: SunsetLabs-Game/Zylith
Length of output: 2215
🏁 Script executed:
#!/bin/bash
echo "=== Check deploy script for all verifier declarations ==="
grep -n "declare --contract-name" ./zylith/scripts/deploy.sh
echo -e "\n=== Check if other verifiers have similar patterns ==="
grep -n "GrothVerifier" ./zylith/scripts/deploy.shRepository: SunsetLabs-Game/Zylith
Length of output: 803
🏁 Script executed:
#!/bin/bash
echo "=== Check actual module names in zylith verifier files ==="
grep -n "^mod \|^#\[starknet::contract\]" ./zylith/src/privacy/verifiers/withdraw/groth16_verifier.cairo
echo -e "\n=== Check lp verifier ==="
grep -n "^mod \|^#\[starknet::contract\]" ./zylith/src/privacy/verifiers/lp/groth16_verifier.cairo
echo -e "\n=== Check membership verifier ==="
grep -n "^mod \|^#\[starknet::contract\]" ./zylith/src/privacy/verifiers/membership/groth16_verifier.cairo
echo -e "\n=== Check swap verifier ==="
grep -n "^mod \|^#\[starknet::contract\]" ./zylith/src/privacy/verifiers/swap/groth16_verifier.cairoRepository: SunsetLabs-Game/Zylith
Length of output: 215
🏁 Script executed:
#!/bin/bash
echo "=== Check if zylith verifier files exist ==="
ls -la ./zylith/src/privacy/verifiers/*/groth16_verifier.cairo
echo -e "\n=== Content of withdraw verifier ==="
cat ./zylith/src/privacy/verifiers/withdraw/groth16_verifier.cairo
echo -e "\n=== Content of lp verifier ==="
cat ./zylith/src/privacy/verifiers/lp/groth16_verifier.cairoRepository: SunsetLabs-Game/Zylith
Length of output: 1136
Verify and fix naming inconsistency in deploy script.
Module renaming is consistent across all verifiers in circuits/build/garaga/—all use the generic Groth16VerifierBN254. However, the deploy script shows an inconsistency: line 89 still declares the contract as "WithdrawGroth16VerifierBN254" (old format), while the other verifiers use the new pattern: "MembershipGroth16VerifierBN254", "SwapGroth16VerifierBN254", and "LPGroth16VerifierBN254". Update line 89 in zylith/scripts/deploy.sh to use "WithdrawGroth16VerifierBN254" only if it matches the actual contract name; otherwise, align it with the naming pattern used by other verifiers.
🤖 Prompt for AI Agents
In zylith/scripts/deploy.sh around line 89, the contract name string is
inconsistent with the verifier module naming used elsewhere; open the Groth16
verifier Cairo module
(circuits/build/garaga/withdraw_verifier/src/groth16_verifier.cairo) to confirm
the actual contract name (module is declared as Groth16VerifierBN254) then
update line 89 in deploy.sh: if the compiled/declared contract is
Groth16VerifierBN254, replace "WithdrawGroth16VerifierBN254" with
"Groth16VerifierBN254"; if the compiled artifact actually uses the
Withdraw-prefixed name, leave it as "WithdrawGroth16VerifierBN254" — otherwise
change the deploy string to align with the pattern used by other verifiers
(e.g., MembershipGroth16VerifierBN254, SwapGroth16VerifierBN254,
LPGroth16VerifierBN254).
| if (process.argv.length != 5) { | ||
| console.log("Usage: node generate_witness.js <file.wasm> <input.json> <output.wtns>"); | ||
| } else { |
There was a problem hiding this comment.
Script continues after printing usage message.
When incorrect arguments are provided, the script prints usage but doesn't exit. Add process.exit(1) to prevent silent continuation.
Suggested fix
if (process.argv.length != 5) {
console.log("Usage: node generate_witness.js <file.wasm> <input.json> <output.wtns>");
+ process.exit(1);
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (process.argv.length != 5) { | |
| console.log("Usage: node generate_witness.js <file.wasm> <input.json> <output.wtns>"); | |
| } else { | |
| if (process.argv.length != 5) { | |
| console.log("Usage: node generate_witness.js <file.wasm> <input.json> <output.wtns>"); | |
| process.exit(1); | |
| } else { |
🤖 Prompt for AI Agents
In circuits/build/membership/membership_js/generate_witness.js around lines 4 to
6, the script prints a usage message when the argument count is incorrect but
continues execution; modify the branch that handles the wrong-argument case to
call process.exit(1) immediately after printing the usage so the script
terminates with a non-zero exit code and does not continue executing the rest of
the file.
| const sanityCheck = | ||
| options | ||
| // options && | ||
| // ( | ||
| // options.sanityCheck || | ||
| // options.logGetSignal || | ||
| // options.logSetSignal || | ||
| // options.logStartComponent || | ||
| // options.logFinishComponent | ||
| // ); |
There was a problem hiding this comment.
Potential issue: sanityCheck receives the entire options object instead of a boolean.
The commented-out code suggests sanityCheck should be a boolean derived from specific option flags, but currently the entire options object is assigned. This causes sanityCheck to always be truthy when any options are passed. Since the WitnessCalculator uses this for conditional initialization ((this.sanityCheck || sanityCheck) ? 1 : 0), this may cause unintended sanity checks to be enabled.
This appears to be inherited from the circom generator, so it may be intentional for this use case.
🤖 Prompt for AI Agents
In circuits/build/membership/membership_js/witness_calculator.js around lines 66
to 75, sanityCheck is assigned the entire options object instead of a boolean
derived from specific flags; change the assignment so sanityCheck is a boolean
expression (e.g., options && (options.sanityCheck || options.logGetSignal ||
options.logSetSignal || options.logStartComponent ||
options.logFinishComponent)) so downstream checks that expect a boolean behave
correctly.
| ZYLITH_CONTRACT: | ||
| "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update hardcoded address to match new deployment.
The ZYLITH_CONTRACT address in the example should match the new deployment address used elsewhere in the codebase.
🔎 Proposed fix
ZYLITH_CONTRACT:
- "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d",
+ process.env.REACT_APP_ZYLITH_CONTRACT ||
+ "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d",Actually, looking more carefully, the address is already correct. Consider adding the environment variable fallback pattern for consistency with the actual implementation in frontend/src/lib/config.ts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ZYLITH_CONTRACT: | |
| "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d", | |
| ZYLITH_CONTRACT: | |
| process.env.REACT_APP_ZYLITH_CONTRACT || | |
| "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d", |
🤖 Prompt for AI Agents
In docs/integration/frontend-integration-guide.md around lines 242 to 243, the
example hardcodes ZYLITH_CONTRACT; update the snippet to show the
environment-variable fallback pattern used in frontend/src/lib/config.ts (use
process.env.ZYLITH_CONTRACT or equivalent with the deployed address as default)
so the docs mirror the actual implementation and remain consistent with the
codebase.
| { | ||
| "profile": "sepolia", | ||
| "timestamp": "2025-12-26T11:22:24+05:30", | ||
| "contracts": { | ||
| "zylith": { | ||
| "class_hash": "0x0", | ||
| "address": "" | ||
| }, | ||
| "membership_verifier": { | ||
| "class_hash": "0x0", | ||
| "address": "" | ||
| }, | ||
| "swap_verifier": { | ||
| "class_hash": "0x0", | ||
| "address": "" | ||
| }, | ||
| "withdraw_verifier": { | ||
| "class_hash": "0x0", | ||
| "address": "" | ||
| }, | ||
| "lp_verifier": { | ||
| "class_hash": "0x0", | ||
| "address": "" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Deployment record is incomplete.
The PR description states that deployment tasks are "completed" and new addresses should be recorded, but this file contains only placeholder values (0x0 for class hashes and empty strings for addresses).
Per the PR objectives, this file should contain the actual deployed contract addresses for:
- zylith contract
- membership_verifier
- swap_verifier
- withdraw_verifier
- lp_verifier
Expected structure with actual values
The file should look like:
{
"profile": "sepolia",
"timestamp": "2025-12-26T11:22:24+05:30",
"contracts": {
"zylith": {
- "class_hash": "0x0",
- "address": ""
+ "class_hash": "0x390d09b07f161b3e1b8b10dedd091e3415ba493c67ff4b63ba908aec3e3d435",
+ "address": "0x002c6ced7ef107e71fb10b6b04b301d52116ab1803b19a0b88b35874d207db1d"
},
"membership_verifier": {
- "class_hash": "0x0",
- "address": ""
+ "class_hash": "0x22d131697d55ce44510d04575e3633d90c3219963ab0c554c1deb5962fd91f5",
+ "address": "0x011c0deb3618f2358dcba0dda14f43ef47f40b7be681d6708f554ce3d0ad5432"
},
// ... (similarly for other verifiers)
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In zylith/deployment_sepolia_20251226_112137.json around lines 1 to 26, the
deployment record contains placeholder class_hashes ("0x0") and empty addresses;
replace each placeholder with the actual deployed values for zylith,
membership_verifier, swap_verifier, withdraw_verifier, and lp_verifier: update
each contract object to set the correct class_hash and address strings as
produced by the deployment artifacts (or the network explorer/transaction
receipts), ensure values are properly quoted JSON strings, and commit the file
so it reflects the real deployments.
2 participants
TODO: Redeploy Zylith & Verify Private Swap
Deployment
scarb build)Configuration
.env.envTesting
Verification
Option::unwrap failedor multicall errorsIf Issues Persist
sqrtPriceLimitX128is passed asu256Close #24
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.