feat(l2): zk-dex e2e pipeline#8
feat(l2): zk-dex e2e pipeline#8Zena-park wants to merge 35 commits intopr/zk/02-circuit-frameworkfrom
Conversation
🤖 Kimi Code ReviewAutomated review by Kimi (Moonshot AI) |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a robust end-to-end infrastructure for the ZK-DEX, enabling its full functionality within the L1/L2 ecosystem. It streamlines deployment, optimizes batch processing by introducing smart contract-level empty batch verification, and enhances user interaction through dedicated UI tools. The changes ensure that the ZK-DEX guest program can operate efficiently with real SP1 proofs, laying the groundwork for advanced decentralized exchange capabilities. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive pull request that delivers a full end-to-end pipeline for the ZK-DEX. The changes are well-structured, introducing a modular framework for app-specific circuits, a complete Docker-based localnet, and significant optimizations like empty batch verification. The code quality is high, with thorough documentation and testing. I have one minor suggestion to improve the readability of a script in the Dockerfile.
d8cc431 to
954d730
Compare
23759c9 to
2d33b67
Compare
954d730 to
797f1e0
Compare
2d33b67 to
c0623ad
Compare
797f1e0 to
eacee43
Compare
c0623ad to
6e74d66
Compare
eacee43 to
c863a5c
Compare
6e74d66 to
7ca60e6
Compare
c863a5c to
f3296d8
Compare
7ca60e6 to
c777815
Compare
f3296d8 to
872ce31
Compare
c777815 to
56604ad
Compare
872ce31 to
63b8b5e
Compare
56604ad to
ccf3106
Compare
63b8b5e to
57dc13b
Compare
ccf3106 to
791d666
Compare
57dc13b to
059ab6e
Compare
791d666 to
7611496
Compare
Migrate the zk-dex guest program from the old keccak-chaining demo (DexProgramInput/execution_program) to the new AppCircuit-based architecture (AppProgramInput/execute_app_circuit with DexCircuit). - Rewrite bin/sp1-zk-dex to use execute_app_circuit(DexCircuit, input) - Add rkyv derives to AppProgramInput, StorageProof, AccountProof - Add ethrex-common dep and secp256k1 SP1 patch to sp1-zk-dex - Delete old execution.rs and types.rs from zk_dex module - Update sp1_benchmark.rs zk-dex input gen to TODO stub (Phase 3) - Replace registry test with DexCircuit-based unit test
Implement prover-side input conversion so the zk-dex guest program receives AppProgramInput (Merkle proofs) from the standard ProgramInput (full ExecutionWitness trie), eliminating the need for coordinator or protocol changes. - Add input_converter module with convert_to_app_input() that rebuilds state/storage tries from ExecutionWitness and extracts Merkle proofs - Replace ZkDexGuestProgram::serialize_input() pass-through with ProgramInput deserialization, transaction analysis, and conversion - Add analyze_zk_dex_transactions() to extract needed accounts and storage slots from batch transactions - Implement generate_zk_dex_input() in sp1_benchmark exercising the full conversion pipeline with mock state tries and transfer txs - Update tests in traits.rs and registry.rs for the new behavior
Timeline, SP1 profiling results (182x cycle reduction), Halo2 vs SP1 decision, L1/L2 infra status, and next steps.
…tration Add deployer support for registering custom guest programs (e.g., zk-dex) on L1 during contract initialization, including GuestProgramRegistry registration and SP1 verification key setup via upgradeVerificationKey(). - Add --register-guest-programs and --zk-dex-sp1-vk CLI options - Add guest program registration loop in initialize_contracts() - Add resolve_deployer_program_type_id() and get_vk_for_program() helpers - Add ZK-DEX E2E Makefile targets (deploy, l2, prover) - Add programs-zk-dex.toml prover config - Add E2E design document
Add mint, spend, liquidate, convertNote, makeOrder, takeOrder, and settleOrder operations to the DexCircuit, matching the ZkDex Solidity contract's state transitions exactly. New files: - storage.rs: slot computation for notes, orders, encryptedNotes - events.rs: NoteStateChange, OrderTaken, OrderSettled log generation - notes.rs: mint/spend/liquidate/convertNote execution - orders.rs: makeOrder/takeOrder/settleOrder execution + RLP decoder Modified: - circuit.rs: ABI selectors, calldata parsing, routing for 8 ops - mod.rs: witness analyzer for all operation storage slots
Extract makerNoteHash (input[1]) and takerStakeNoteHash (input[3]) from settleOrder calldata to include their note_state_slots in the witness. parentNote remains dependent on ExecutionWitness since it's only available in order storage. Also mark storage slot verification as done (slots 3, 4, 14 confirmed via manual inheritance chain trace).
…tness analyzer Read orders.length and parentNote from ExecutionWitness trie to compute exact storage slots for makeOrder (new order fields) and settleOrder (parentNote note state), preventing incremental MPT failures.
- Add full makeOrder→takeOrder→settleOrder lifecycle test (6 note states + order state + result.data) - Add ABI bytes offset/length overflow checks in extract_abi_bytes - Add RLP trailing bytes validation in decode_enc_datas - Add event topic uniqueness and signature verification tests - Add Solidity bytes storage short/long encoding verification tests - Remove DAI TODO (not used), clean up witness analyzer comments
… offsets Build complete pipeline to pre-deploy ZkDex + 6 Groth16 verifiers into L2 genesis block so DEX is available from Block 0. Key changes: - Add generate-zk-dex-genesis.sh: forge build → bytecode extraction → storage layout verification → genesis JSON generation - Generate l2-zk-dex.json with 7 contracts at deterministic addresses - Fix storage slot constants in witness analyzer (no packing gap at slot 2): ENCRYPTED_NOTES_SLOT 3→2, NOTES_SLOT 4→3, ORDERS_SLOT 14→13 - Update localnet script and docker-compose to use l2-zk-dex.json
L2 server mode defaults to TUI monitor which sends all tracing to an in-memory buffer instead of stdout, producing zero log output when redirected to a file. Added --no-monitor flag. Also resolved P2P port 30303 conflict between L1 and L2 by assigning L2 to port 30304. Localnet E2E verified: 7 contracts deployed with correct storage slots.
…ypeId The committer was defaulting to programTypeId=1 (evm-l2) because ETHREX_GUEST_PROGRAM_ID was not set in the localnet script. This caused L1 proof verification to fail (00e) since the VK lookup used the wrong programTypeId. Adding ETHREX_GUEST_PROGRAM_ID=zk-dex ensures batch commits use programTypeId=2, matching the prover's zk-dex guest program. E2E pipeline now fully operational: L1→Deploy→L2→SP1 Prover→L1 Verify.
- Add Section 3: E2E Localnet Benchmark with real measured data - Batch 1: 49 deposits, 5.4M cycles, 3m48s proving - Batch 2: 200 ETH transfers, 15.8M cycles, 6m46s proving - Both verified on L1 successfully - Add resolved issues table (5 major bugs documented) - Update checklist: E2E proof + benchmark complete - Add ZkDex genesis contract addresses to setup guide - Restructure sections for clarity
…ismatch The first batch after genesis may have a different state root than batch 0 when the L2 genesis includes pre-deployed contracts (e.g., ZK-DEX). Since the genesis state root is deterministic and already trusted by the L1 contract, no ZK proof is needed for this transition. - Add genesis transition check in OnChainProposer.verify(): when lastVerifiedBatch == 0 and batch has no state-changing transactions, skip proof verification - Extract _hasNoStateChangingTransactions() helper from _isEmptyBatch() - Fix proof coordinator: when a batch has no prover input but versions match, respond with empty_batch_response instead of version_mismatch
When a user submits a withdrawal on L2, the committer now detects it and triggers an immediate batch commit instead of waiting for the full commit_time_ms interval (e.g. 5 minutes). This significantly reduces withdrawal latency. The existing timer-based commit behavior is preserved for normal (non-withdrawal) traffic.
059ab6e to
f04dfd0
Compare
7611496 to
7f2c47a
Compare
Consolidate the multi-line if/else/fi block into a compact one-liner for better readability.
…serialize_input Dead code referencing RkyvError and app_input outside the #[cfg(feature = "l2")] block caused 3 compile errors (E0412, E0425, E0308) failing all CI lint jobs.
… pr/zk/03-zk-dex-e2e # Conflicts: # crates/guest-program/src/common/app_execution.rs # crates/guest-program/src/programs/zk_dex/circuit.rs # crates/guest-program/src/programs/zk_dex/mod.rs # crates/l2/contracts/src/l1/OnChainProposer.sol # crates/l2/prover/src/bin/sp1_benchmark.rs
- Remove needless borrows (&data, &slot_word, &preimage, &xxx_sel) - Use .div_ceil(32) instead of manual (x + 31) / 32 - Fix doc list item overindentation in rustdoc comments - Remove unneeded return statement - Allow mixed_case_hex_literals for EIP-55 ETH address constant - Replace map_or(true, ...) with is_none_or(...) in l1_proof_sender
The empty block fast-forward in l1_committer skipped adding empty blocks to current_blocks/current_fee_configs, excluding them from the blob. However, the prover input includes all blocks in the batch range, so verify_blob reconstructed different blob data and KZG verification failed with "Invalid KZG blob proof". Fix: push empty blocks into current_blocks and current_fee_configs before the fast-forward continue, so the blob matches the prover input.
0ce6aef to
856de48
Compare
Empty blocks were added to the batch's block list but the blob bundle was not regenerated, causing KZG proof mismatches between the committer and prover. Now generate_blobs_bundle is called in the empty-block path with graceful error handling (pop + break) when blob capacity is exceeded. Also replace undefined Make targets (bridge-address, on-chain-proposer-address) in the integration-test recipe with docker cp from the deployer container.
In based mode, block production is L1-sequenced. The has_pending_withdrawals() scan triggered rapid tiny batch commits, causing the prover to fall behind and stall the pipeline. Skip this optimisation in based mode where it is counterproductive.
…to-logout on 401 - Bug #1 (critical): getProgramById → getProgramByProgramId for slug lookup Pass program.id (UUID) to createDeployment and incrementUseCount - Bug #2: incrementUseCount now receives UUID instead of slug - Bug #3: fetch() headers properly merged (options can't overwrite auth) - Bug #8: auto-logout on 401 response (expired token)
…te detection * fix(manager): prompt — download bridge UI source, reorder firewall before tools - Step 6: download bridge UI source files (Dockerfile, HTML, entrypoint.sh) from GitHub before tools compose up (fixes build path not found) - Move firewall (Step 7→6) before tools (Step 6→7) so ports are open for external monitoring before tools deployment - Public URLs already set via VM_IP metadata for dashboard Explorer links * feat(manager): auto-detect deployment success + save & complete button - Monitor: when all services are healthy, show success message with URLs - "배포 완료 — 정보 저장" button saves IP/port to DB, sets phase=running - Navigates to dashboard detail view after save - Guide text: "아래 버튼을 누르면 대시보드 화면으로 이동합니다" * feat(messenger): Open Appchain public interface — API integration, metadata push, publish UX - Replace mock data in OpenL2View with Platform API (getPublicAppchains) - Add registration modal for external appchain publishing (name, RPC, chain ID) - OpenL2DetailView: real screenshots (IPFS), RPC status check, reviews/comments/announcements - L2DetailPublishTab: allow description/screenshots/social links input before publish toggle - Platform server: metadata-push.js — push/delete metadata to GitHub repo via Contents API - New routes: POST /deployments/:id/push-metadata, /delete-metadata - Debounced metadata push (5s) after description/screenshot/social link saves - Security: name length validation, status escalation guard, commit message sanitization, 409 retry - i18n: loading/error/retry/nativeToken/rating/reviews/comments/noScreenshots/announcements - Tests: 24 unit tests (metadata-push), e2e-store.js (11-step lifecycle test) * fix(messenger): add save button for publish tab — persist draft to localStorage before publish * fix(messenger): block publishing with localhost RPC URL — require publicly accessible endpoint * fix(messenger): fix publish toggle disabled for non-local appchains — remove false localhost check * feat(platform): server-side screenshot upload — no Pinata required - POST /api/deployments/:id/screenshots — multer-based image upload (5MB, images only) - platformAPI.uploadScreenshots() — FormData upload to Platform server - L2DetailPublishTab: use server upload when platformId exists, IPFS fallback, data URL fallback - Remove Pinata requirement for screenshot upload button * fix(messenger): detect AWS deployments correctly — show AWS badge, fix L1 chain ID, enable publishing - networkMode detection: config.mode > host_id (AWS) > l1_chain_id > local - L1 chain ID: read from DB l1_chain_id field first, then config fallback - Add AWS (orange), Mainnet (green) badges alongside existing Testnet/Local - Add hostId, publicRpcUrl, deployMethod to L2Config interface * fix(messenger): allow publishing when public URL or remote host exists, not just by networkMode * fix(messenger): add missing DB fields (l1_chain_id, host_id, public_l2_rpc_url) to DeploymentRow - Rust: add l1_chain_id, host_id, platform_deployment_id, public_l2_rpc_url, public_domain to SELECT - TS: add matching fields to DeploymentFromDB interface - Now AWS deployments show correctly (not as Local), L1 Chain ID displays properly * fix(messenger): correct networkMode detection — use l1_port to distinguish local vs testnet - l1_port exists → local (has bundled L1 node in Docker) - host_id exists → AWS (remote deployment) - l1_chain_id exists but no l1_port → testnet (external L1) - Fixes: local Docker showing as Testnet, AWS showing as Local * fix(messenger): detect AWS via config.cloud, read l1ChainId from config fallback * docs: add deployment network mode detection documentation * fix(manager): fix status showing stopped for AI-deploy local Docker — distinguish remote vs local - AI Deploy with ec2IP → SSH remote status check - AI Deploy without ec2IP (local Docker) → local docker compose ps - Remote host deployments → SSH status check - Fixes: local Docker containers showing as stopped, (pending) hostname * fix(messenger): use l2.programSlug for publish instead of non-existent 'ethrex-appchain' - Fixes 'Load failed' error when publishing — Platform has no 'ethrex-appchain' program - L2DetailPublishTab: programId = l2.programSlug || 'evm-l2' - OpenL2View registration modal: programId = 'evm-l2' - Rename labels: '오픈 앱체인 공개' → '앱체인 공개 설정' * fix: critical publish bugs — program lookup by slug, header merge, auto-logout on 401 - Bug #1 (critical): getProgramById → getProgramByProgramId for slug lookup Pass program.id (UUID) to createDeployment and incrementUseCount - Bug #2: incrementUseCount now receives UUID instead of slug - Bug #3: fetch() headers properly merged (options can't overwrite auth) - Bug #8: auto-logout on 401 response (expired token) * feat(messenger): make Platform URL configurable via VITE_PLATFORM_URL env var * feat(messenger): metadata registry submission with auto register/update detection - Add appchain-registry API client with check/submit/status endpoints - Auto-detect register vs update by checking existing file on GitHub main branch - Preserve immutable fields (l2ChainId, nativeToken, createdAt) on updates - Fetch actual chain ID from L2 RPC (eth_chainId) for accuracy - Map social links to correct schema fields (website → top-level, twitter → xUrl, etc.) - Fix bridges.url empty string validation failure (omit if no dashboardUrl) - Add GitHub PR title/body update when resubmitting to existing open PR - Add keychain-based metadata signing (SECURITY_COUNCIL role) - Open PR links in external browser via Tauri shell plugin * fix(messenger): include dashboard URL in metadata — fallback to port 3000 for remote deployments * fix(platform): sync metadata schema fields — resolve URLs from supportResources, explorers, bridges listings.js was reading legacy top-level fields (dashboardUrl, explorerUrl, bridgeUrl) that don't exist in the new appchain metadata schema. Now reads from: - supportResources.dashboardUrl → dashboard_url - explorers[0].url → explorer_url - bridges[0].url → bridge_url - top-level website → operator_website - supportResources xUrl/communityUrl/telegramUrl/documentationUrl → social_links * fix: address PR #66 code review feedback - Add cross-platform keychain support: macOS uses `security` CLI, Windows/Linux uses `keyring` crate via cfg(target_os) (#1) - Restore keychain key validation with allowed prefixes (pinata_, deployer_pk_, ai-) for security boundary (#3) - Add warning log to empty catch block in tools container status (#2) - Extract magic number 9 to LOCAL_L1_CHAIN_ID constant (#4) * fix: address Copilot PR #66 review feedback - Fix signing message indentation bug — spaces before each line caused signature mismatch with server (array.join instead of multiline string) - Fix saveDraft stale state — pass fresh result to avoid missing prNumber - Add NaN timestamp validation in submit endpoint - Add rate limit map cleanup to prevent unbounded memory growth - Strict address validation in getRepoFilePath (0x + 40 hex chars) - findOpenPR verifies actual PR files instead of title-only matching - Remove .deployed-*-tools.env from tracking, add to .gitignore
- Return 503 instead of {exists:false} on check endpoint errors (#1)
- Sanitize all error messages — log internally, return generic to client (#2,#3,#4,#5)
- Add serverless rate limit limitation comment (#6)
- Add console.warn to all empty catch blocks in github-pr.ts (#9,#10,#11)
- Note: params as Promise is correct for Next.js 15 (#7,#8)
* feat(platform): add appchain registry API routes to Next.js client Port Express server appchain-registry endpoints to Next.js API routes for Vercel deployment: - GET /api/appchain-registry/check/[l1ChainId]/[stackType]/[identityAddress] - POST /api/appchain-registry/submit - GET /api/appchain-registry/status/[prNumber] Shared logic in lib/appchain-registry.ts and lib/github-pr.ts. * fix: address PR #67 code review feedback - Return 503 instead of {exists:false} on check endpoint errors (#1) - Sanitize all error messages — log internally, return generic to client (#2,#3,#4,#5) - Add serverless rate limit limitation comment (#6) - Add console.warn to all empty catch blocks in github-pr.ts (#9,#10,#11) - Note: params as Promise is correct for Next.js 15 (#7,#8) * fix: address additional Copilot PR #67 review feedback - Add AbortSignal.timeout(15s) to all GitHub API fetch calls - Fix authHeaders error message to list both accepted env vars - Distinguish RPC errors (502) from permission denied (403) in ownership check - Add typeof validation for metadata.signedBy - Add unit test suggestion acknowledged (future work)
Summary
End-to-end ZK-DEX pipeline from L1 deployment to Docker-based proving:
Dependencies
Part 3/4 of stacked PR series. Depends on #7 (Circuit Framework).
Merge order: #6 → #7 → #8 → #9
Test plan