Conversation
Restore the concrete prover witness type definitions so the unified zone spec remains self-contained for implementers. Add a schematic of the witness tree and restore the missing queued-deposit type definitions referenced by the batch input interfaces. Made-with: Cursor
Replace the prover input flowchart with a nested-box schematic that shows the full BatchWitness container structure. Make the repeated collections clearer by showing representative entries for blocks, deposits, account witnesses, and Tempo reads. Made-with: Cursor
Replace the dense per-field diagram with a smaller set of multiline boxes so the witness structure stays readable in rendered docs. Keep the nested container shape and include the deposit payload variants without shrinking the entire figure. Made-with: Cursor
Simplify the prover witness by removing the per-read node_path field from L1StateRead. Clarify that reads are verified by walking the bound Tempo trie using nodes from the shared deduplicated pool. Made-with: Cursor
Clarify how to read and verify the zone-state and Tempo-state proof witnesses in the unified spec. Add plain-language guidance for interpreting storage proof vectors and the shared deduplicated node pool. Made-with: Cursor
Clarify that ZoneStateWitness proofs are relative to the initial batch state and are verified once during initialization. Spell out that block execution then operates on the materialized state rather than re-verifying Merkle proofs on each access. Made-with: Cursor
Explain how missing account and storage leaves are represented in the witness and interpreted during verification. Clarify that deleted accounts and zeroed storage slots are absent from the current trie rather than preserved as explicit zero-valued leaves. Made-with: Cursor
Make the contrast between zone-state and Tempo-state witnesses explicit in the unified spec. State clearly that Tempo reads are verified against the root currently bound in TempoState at each read site, not the Tempo root at batch start. Made-with: Cursor
Update ZoneStateWitness to use the same deduplicated node_pool design as BatchStateProof. Replace per-account proof vectors with decoded account and storage read descriptors, and rewrite the schematic and proof interpretation text to match. Made-with: Cursor
Clean up the proving spec by defining the shared node_pool trie-proof encoding once and referencing it from both ZoneStateWitness and BatchStateProof. Remove wording that depended on earlier spec revisions and make the zone-vs-Tempo distinction purely about when the shared format is applied. Made-with: Cursor
Replace the brief block execution summary with a full step-by-step stateless verification procedure for BatchWitness. Factor out the shared trie proof format so the zone-state and Tempo-state witness sections read top-down for a new reader. Made-with: Cursor
Tighten the stateless prover execution procedure so it reads as a cleaner ordered algorithm. Remove redundant setup wording and renumber the verification steps to match the current proof flow. Made-with: Cursor
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review — PR #385
This PR restores and concretizes the stateless prover input definitions. The rewritten spec introduces several material divergences from the live zone runtime and contracts, and its new prose legitimizes existing cross-layer gaps that can be exploited for chain-halting or policy bypass.
7 findings survived deduplication across 9 worker reports:
- 🚨 3 SECURITY (encrypted deposit chain halt, encrypted deposit policy bypass, witness bytecode unauthenticated)
⚠️ 3 ISSUE (missingenabledTokens, wrongfinalizeWithdrawalBatchmodel, TIP-403 not on witness path)- 🛡️ 1 DEFENSE-IN-DEPTH (refund-path prose overstates CP proof handling)
🚨 Encrypted Deposits Bypass TIP-403 Recipient Restrictions
depositEncrypted() is presented as the private version of deposit(), but it does not preserve the same authorization rules. Plaintext deposits require the target to satisfy both Recipient and MintRecipient TIP-403 checks. The encrypted path never checks Recipient at any layer: L1 portal skips it (recipient hidden), sequencer builder only checks MintRecipient (crates/tempo-zone/src/l1.rs:1196-1233), zone-side mint also only enforces MintRecipient (crates/precompiles/src/ztip20.rs:248-279). A blacklisted recipient can receive bridged funds via the encrypted path.
Fix: After decryption, enforce both AuthRole::Recipient and AuthRole::MintRecipient on dec.to.
Reviewer Callouts
- ⚡ Fork Safety Claims vs Implementation: The PR changes fork-activation language from future tense to present tense, removing the caveat that verifier rotation was only a target design.
ZonePortalstill stores a singleaddress public immutable verifierwith noforkVerifieror rotation mechanism.ZoneFactoryhas noprotocolVersionorsetForkVerifier. - ⚡ Forward-Integration Risk: The verifier is currently a stub. Witness-schema findings (bytecode binding, missing inputs, wrong finalization model) become critical when proof enforcement is enabled.
- ⚡ CI Blind Spot:
zone_spec.mdis not consumed by any build/lint/test pipeline. Manual review is the only guardrail against spec/runtime drift.
| 4. Executes `finalizeWithdrawalBatch` if present (required in the final block, absent in intermediate blocks). | ||
| 5. Computes the block hash from the simplified zone header fields (see [Block Header Format](#block-header-format)). | ||
| 6. **Process deposits and encrypted deposit decryptions inside `advanceTempo`.** | ||
| Using the now-bound Tempo root for this block, verify the Tempo-side reads needed by `ZoneInbox` such as the portal's current deposit queue hash. Process the `deposits` in witness order, enforcing the queue semantics specified in [Deposit Queue](#deposit-queue). For encrypted deposits, verify the supplied `DecryptionData` and Chaum-Pedersen proof, decode the recipient and memo when valid, and apply the fallback mint-to-sender path when decryption verification fails as specified in [Onchain Decryption Verification](#onchain-decryption-verification). |
There was a problem hiding this comment.
🚨 [SECURITY] Encrypted Deposit Refund Path Can Permanently Halt the L2 Chain
This line claims failed encrypted-deposit decryptions safely fall back to minting to the sender. However, ZoneInbox.advanceTempo calls IZoneToken(token).mint(sender, amount) without try-catch (ZoneInbox.sol:274-281). The zone TIP-20 precompile enforces AuthRole::MintRecipient on every mint and reverts with PolicyForbids for unauthorized recipients.
Since depositEncrypted() on L1 never checks sender authorization and advanceTempo is a must-succeed system transaction, an attacker excluded from MintRecipient can submit garbage-ciphertext encrypted deposits, causing the refund mint to revert, the payload builder to abort (builder.rs:235-259), and the L1 block to remain stuck at the queue head permanently.
🛡️ [DEFENSE-IN-DEPTH] Additionally, this prose overstates the fallback scope: the live contract hard-reverts on invalid Chaum-Pedersen proofs and only falls back after a valid proof when AES-GCM/plaintext validation fails. The builder compensates by manufacturing a valid proof for garbage ciphertext. A prover implemented literally from this text could accept batch traces the contract would reject.
Recommended Fix:
- Wrap
mint(...)calls inZoneInbox.advanceTempointry-catch. On failure, emitDepositMintFailedand continue. - On L1, validate
msg.senderis an authorizedMintRecipientbefore accepting encrypted deposits. - Narrow the prose: invalid CP proof → revert. AES-GCM/plaintext failure after valid proof → mint to sender.
There was a problem hiding this comment.
This finding is likely correct but will be fixed with bouncebacks on deposits
| /// Sequencer-only: finalize a batch (only in final block, must be last) | ||
| /// Required for the final block in a batch; must be absent in intermediate blocks. | ||
| /// Uses U256 to match Solidity `finalizeWithdrawalBatch(uint256 count)`. | ||
| pub finalize_withdrawal_batch_count: Option<U256>, |
There was a problem hiding this comment.
ZoneBlock:
1. Missing enabledTokens: The ZoneBlock schema and execution algorithm model advanceTempo as taking only tempo_header_rlp, deposits, and decryptions. The live ABI requires a fourth argument EnabledToken[] enabledTokens, and ZoneInbox.advanceTempo enables tokens before deposit processing (ZoneInbox.sol:175-194). A prover built from this spec diverges on any block that enables a token and deposits it in the same block. Fix: Add enabled_tokens: Vec<EnabledToken> to ZoneBlock.
2. Wrong finalizeWithdrawalBatch model: This field is documented as final-block-only and the execution algorithm (step 8, line 1061) only calls finalizeWithdrawalBatch(count) once. The live executor unconditionally calls finalizeWithdrawalBatch(count, blockNumber, encryptedSenders) after every zone block, incrementing withdrawalBatchIndex each time. The documented ABI is also missing two required arguments. Fix: Document per-block finalization and the full three-argument ABI.
There was a problem hiding this comment.
@0xKitsune These are spec/implementation mismatches, can we fix them in the implementation, or if they need to be as implemented, add a spec PR with justification?
| Using the now-bound Tempo root for this block, verify the Tempo-side reads needed by `ZoneInbox` such as the portal's current deposit queue hash. Process the `deposits` in witness order, enforcing the queue semantics specified in [Deposit Queue](#deposit-queue). For encrypted deposits, verify the supplied `DecryptionData` and Chaum-Pedersen proof, decode the recipient and memo when valid, and apply the fallback mint-to-sender path when decryption verification fails as specified in [Onchain Decryption Verification](#onchain-decryption-verification). | ||
|
|
||
| 7. **Execute user transactions in order.** | ||
| Run each user transaction against the materialized zone state using the current block environment. Whenever execution calls `TempoState.readTempoStorageSlot`, satisfy that call by locating the corresponding `L1StateRead`, proving it against the Tempo root currently bound for this block, and requiring the decoded value to match the witness entry. Any zone-state or Tempo-state access not covered by the witness is an error. |
There was a problem hiding this comment.
This step only constrains reads that go through TempoState.readTempoStorageSlot(), and line 592 claims TIP-403 policy state uses that path. In live execution, TIP-20 transfer authorization goes through ZoneTip20Token::enforce_transfer → ZoneTip403ProxyRegistry::is_transfer_authorized → PolicyProvider (cache-first / L1 RPC fallback). That path never touches TempoState. A prover built from this spec would not witness-bind policy-gated transfer outcomes at all.
Recommended Fix: Either route live TIP-403 reads through TempoState.readTempoStorageSlot so they are witness-bound, or update the spec to describe the actual proxy/provider path and add a corresponding witness mechanism.
There was a problem hiding this comment.
This finding seems incorrect. According to the spec, the zone TIP-403 Registry calls TempoState.readTempoStorageAt.
There does not seem to be a mock contract showing this behaviour at the moment, maybe we can add that.
Document the zone-side TIP-403 registry as a read-only mirror of Tempo policy state. Allow the mirror contract to read finalized Tempo storage so the mock contracts and zone spec stay aligned. Made-with: Cursor
Require zone witness ingestion to check that any supplied account bytecode preimage matches the witnessed code hash. This makes the batch-start materialization rules explicit in both the shared proof format and the step-by-step execution flow. Made-with: Cursor
Require TempoStateReader to reject non-TempoState callers so Tempo storage reads always flow through the finalized Tempo binding in TempoState. Document the restriction in the zone spec and interface appendix to keep the implementation and spec aligned. Made-with: Cursor
3 participants
Summary
docs/specs/zone_spec.md, including the nested Rust-style input structs that were dropped during the spec mergeBatchWitness,ZoneBlock,ZoneStateWitness, andBatchStateProoffit togetherDepositTypeandQueuedDepositcommon types and add the missingWithdrawal Requestheading so the spec stays internally consistentTest plan
docs/specs/zone_spec.mddocs/specs/zone_spec.mdMade with Cursor