chore(l2): docker and production polish#9
chore(l2): docker and production polish#9Zena-park wants to merge 16 commits intopr/zk/03-zk-dex-e2efrom
Conversation
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 delivers a final polish to the Docker infrastructure and production readiness aspects of the project. It streamlines Docker builds, standardizes branding across services and user interfaces, and significantly enhances the user experience for withdrawals by providing real-time on-chain status and an interactive claim process. Additionally, it optimizes batch commitment logic to be more responsive to user actions while maintaining system efficiency, and introduces critical documentation for infrastructure planning and development awareness. 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
|
🤖 Kimi Code ReviewAutomated review by Kimi (Moonshot AI) |
There was a problem hiding this comment.
Code Review
This pull request introduces significant rebranding from 'ethrex' to 'TokamakAppL2 ZK-DEX' across Docker Compose configurations, UI elements (dashboard, bridge, withdrawal tracker), and documentation. Key functional changes include optimizing Docker builds by ignoring the .git directory and providing fallback git information via environment variables for vergen. The L1 committer logic is enhanced to trigger early batch commits for withdrawals, but only if the prover is idle, preventing unnecessary load. The withdrawal tracker UI is substantially improved with a new claim section, detailed status tracking, and the ability to claim withdrawals directly. Additionally, Blockscout services are configured with restart policies and disabled heavy indexing for better dev environment performance. A new documentation file outlines limitations of the ethrex L1 devnet and suggests alternatives like Anvil. A review comment highlights a potential issue in l1_committer.rs where unwrap_or could mask errors in RPC calls, suggesting explicit error handling and logging. Another comment points out a Cross-Site Scripting (XSS) vulnerability in withdraw-status.html due to direct innerHTML usage with potentially untrusted data, recommending safer DOM manipulation methods.
23759c9 to
2d33b67
Compare
338d5b1 to
8a8b1be
Compare
2d33b67 to
c0623ad
Compare
8a8b1be to
5ac4927
Compare
c0623ad to
6e74d66
Compare
5ac4927 to
a40d336
Compare
6e74d66 to
7ca60e6
Compare
a40d336 to
680b4ed
Compare
7ca60e6 to
c777815
Compare
680b4ed to
6cce894
Compare
c777815 to
56604ad
Compare
6cce894 to
7559477
Compare
56604ad to
ccf3106
Compare
7559477 to
97dd77d
Compare
ccf3106 to
791d666
Compare
97dd77d to
8eca51c
Compare
791d666 to
7611496
Compare
8eca51c to
362182d
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
Three issues fixed: 1. Timelock ownership: VK registration called OnChainProposer directly but owner is Timelock. Added Timelock.upgradeVerificationKey() with SECURITY_COUNCIL role and updated deployer to use it. 2. KZG blob verification crash: kzg-rs BLS12-381 operations crash in SP1 VM despite sp1_bls12_381 dependency. App circuits now skip KZG verification (L1 already verifies) and compute versioned hash directly. 3. Empty batch safety: Added early returns in verify_state_proofs() and compute_new_state_root() when account proofs are empty. SP1 guest execution verified: 414K cycles (42K read + 370K exec + 2K commit).
Extract inline transaction handlers from app_execution.rs into dedicated modules under common/handlers/ to fix five root causes of SP1 proof verification failure: 1. Withdrawal now credits BURN_ADDRESS with withdrawn value 2. L1Messenger.lastMessageId storage slot incremented on withdrawal 3. Withdrawal logs (WithdrawalInitiated + L1Message) properly generated instead of empty vec, fixing l1_out_messages_merkle_root mismatch 4. Gas fee distribution now debits effective_gas_price (not just base_fee) and credits coinbase/base_fee_vault/operator_fee_vault 5. Witness analyzer includes BURN_ADDRESS, messenger storage, coinbase, and fee vault accounts for proof generation
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
…, and rebrand to TokamakAppL2 - Add restart policy and disable heavy Blockscout indexers for stability - Rename network branding from ethrex to TokamakAppL2 ZK-DEX - Add L2 genesis contract sections to dashboard - Add claim withdrawal button and status tracking to withdraw-status page - Include local deployed contract addresses (.zk-dex-deployed.env)
- Add Phase 4 timeline: early batch commit, Docker tools stability, withdrawal claim UI, TokamakAppL2 rebranding, infra cost analysis - Update checklist with newly completed items - Add withdrawal E2E test as next priority - Update E2E design doc: Phase 3 completed, Phase 4 added
Only trigger early batch commit when a pending withdrawal is detected AND no batch is waiting for proof verification (committed <= verified). If the prover is busy, skip the early commit and fall back to the normal timer-based commit cycle. Also update progress and E2E design docs with detailed early batch commit design (flow diagram, code, verification table).
- Check claimedWithdrawalIDs on L1 bridge to detect already-claimed withdrawals, even across browser sessions - Query WithdrawalClaimed events for claim TX hash and timestamp - Extract showClaimedState() helper to reduce duplication - Add 'finalized' status badge style for claimed withdrawals
- Remove COPY .git from Dockerfile and Dockerfile.sp1 to prevent cache invalidation on every commit; add VERGEN_GIT_SHA/BRANCH env var fallbacks for Docker builds without .git directory - Add .git/ to .dockerignore - Update cmd/ethrex/build.rs to support env var fallback for vergen git info (branch + SHA) in Docker builds - Fix engine_type scope error in cmd/ethrex/initializers.rs by wrapping in proper #[cfg(feature = "rocksdb")] blocks - Enhance withdrawal tracker with batch commit/verify timestamps, L1/L2 tx links, and on-chain claim status display - Add ethrex L1 devnet limitations analysis document - Update local-setup-guide with gas costs, batch behavior, env vars, and Docker caching troubleshooting
Rename all Docker Compose service and container names: - ethrex_l1 → tokamak-app-l1 - ethrex_l2 → tokamak-app-l2 - contract_deployer → tokamak-app-deployer - ethrex_prover → tokamak-app-prover Also updates internal Docker network hostnames and script references.
7611496 to
7f2c47a
Compare
362182d to
db37d1d
Compare
# 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 duplicate import of compute_message_digests, get_batch_messages - Remove dead code outside #[cfg(feature = "l2")] block in serialize_input - Fix missing `fc` variable: iterate fee_configs for operator_fee_vault - Add missing has_withdrawal/has_non_privileged assignments in witness analyzer
…it logic Address Gemini Code Assist review: RPC failures in has_pending_withdrawals() and get_last_verified_batch() were silently masked by unwrap_or fallbacks. Now uses match with warn! logging so failures are visible in logs while still safely skipping the early commit check.
…ntainability - CORS: restrict origin to Tauri dev/prod allowlist (Copilot #1) - open-url: use execFile with arg arrays instead of shell exec (Copilot #2) - fs browse: restrict path traversal to home directory (Copilot #3) - test-e2e-fork: move RPC URL to SEPOLIA_RPC_URL env var (Copilot #4) - docker-remote: clear timeout on stream close, close stream on timeout (Copilot #5) - docker-remote: add shell quoting (q()) and assertSafeName for all interpolated shell args to prevent injection (Copilot #6-8) - genesis.rs: add ChainConfig::validate() for pre-startup checks (Copilot #9) - listings.js: use named params (@id, @name, ...) instead of 30 positional ? args for upsertListing (Gemini #1)
- 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
Final polish for Docker infrastructure and production readiness:
ethrextotokamak-app(TokamakAppL2)Dependencies
Part 4/4 of stacked PR series. Depends on #8 (ZK-DEX E2E).
Merge order: #6 → #7 → #8 → #9
Test plan
tokamak-app