Conversation
|
cyclops audit |
|
cc @0xKitsune Cyclops audit event published. View workflow run Config: config: |
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
PR #1433 enables dynamic T7-style Zone base fees at Z1 and updates the redacted RPC fee surfaces. The normal builder/validator path mostly computes the same fee, but the change makes previously inert fork-schedule, verifier, redaction, and fee-recipient assumptions security- and availability-critical. I left the diff-mapped findings inline.
Body-only findings that could not be safely mapped to changed lines:
- 🚨 [POTENTIAL-VULNERABILITY] Enclave settlement verifier synthesizes a Zone spec that never activates Z1 (
bin/prover/enclave/src/main.rs:253). The Nitro enclave clones the parent Tempo genesis and overrides only the Zone chain ID, so the Zone-onlyz1Timeis absent and post-Z1 replay derivesbasefee = 0while canonical nodes use the T7-derived fee. This causes post-Z1 SPF batches to fail verifier replay and can attest rules that honest nodes reject. Recommended fix: load trusted Zone genesis in the enclave or bind the Zone fork schedule/base-fee rule into SPF public inputs, with a regression test comparing enclave and nodeZoneChainSpecbehavior. ⚠️ [ISSUE] Post-Z1 fee collection depends on the rotating sequencer being TIP-403-recipient authorized (crates/precompiles/src/zone_fee_manager/mod.rs:41). Once base fees are nonzero, normal zero-tip user transactions runcollect_fee_pre_tx, which checks the block beneficiary as a TIP-403 recipient. Startup andzone_setLeadervalidate sequencer membership but not recipient authorization for default/enabled fee tokens, so a policy-gated default token or unauthorized leader can make affected transactions invalid after Z1. Recommended fix: validate recipient authorization at startup/handover or route fees to a fixed authorized fee vault, and add a policy-gated token test across Z1.
Reviewer Callouts
- ⚡ SPF public-input binding:
crates/spf/src/types.rsdoes not commit to the Zone fork schedule or base-fee rule the prover used. Even after fixing enclave spec construction, attestations should identify the replay rules they bind. - ⚡ Z1 rollout tooling: the shipped Zone genesis templates and
xtaskcommands do not provide az1Timeflag, making hand-edited JSON the activation path. That increases the chance of silent fork-schedule mismatches unless tooling and startup diagnostics are added.
| fn next_block_base_fee(&self, _parent: &TempoHeader, _target_timestamp: u64) -> Option<u64> { | ||
| Some(0) | ||
| fn next_block_base_fee(&self, parent: &TempoHeader, target_timestamp: u64) -> Option<u64> { | ||
| let fork = self.zone_hardfork_at(target_timestamp); |
There was a problem hiding this comment.
🚨 [POTENTIAL-VULNERABILITY] Malformed z1Time can now split consensus
This new fee path trusts zone_hardfork_at(target_timestamp), but insert_zone_fork_activations silently omits Z1 when extra_fields["z1Time"] is malformed or mis-cased. A node with such a genesis keeps computing baseFeePerGas = 0, while a correctly configured node computes the T7 fee, so every post-activation block fails with BaseFeeDiff.
Recommended Fix:
Fail closed on present-but-unparseable or likely mis-cased z1Time, add tooling/startup logging for the resolved Zone fork schedule, and test malformed encodings.
| let parent_base_fee = parent | ||
| .inner | ||
| .base_fee_per_gas | ||
| .expect("Zone blocks are expected to have a base fee"); |
There was a problem hiding this comment.
expect can panic in SPF replay
Canonical parents should have baseFeePerGas, but SPF replay takes BatchWitness.parent_header from the prover/batch submitter and reaches next_evm_env before any portal-committed previous-block comparison. A witness parent with no base fee can unwind the local SPF worker, and would unwind the Nitro enclave once its Z1 spec synthesis is fixed.
Recommended Fix:
Return None or a typed verifier error instead of panicking, and validate witness.parent_header.base_fee_per_gas before EVM replay.
| .inner | ||
| .base_fee_per_gas | ||
| .expect("Zone blocks are expected to have a base fee"); | ||
| Some(tempo_t7_next_block_base_fee( |
There was a problem hiding this comment.
This recurrence is applied even when the parent is a Z0 block with baseFeePerGas = 0. That makes the activation child clamp to TEMPO_T7_BASE_FEE_FLOOR (600M) instead of the T7 activation cap (12B), allowing pre-positioned Z0 traffic to consume activation-block capacity at up to a 20x discount.
Recommended Fix:
Add an explicit Z0→Z1 transition branch that returns TEMPO_T7_BASE_FEE_CAP, then apply tempo_t7_next_block_base_fee only for already-Z1 parents.
| .eth | ||
| .api | ||
| .provider() | ||
| .latest_header() |
There was a problem hiding this comment.
eth_gasPrice quotes the sealed-head fee with no legacy headroom
Reading latest_header().base_fee_per_gas() here returns the parent block's fee, but inclusion is checked against the next child fee. After an over-target parent the child fee is higher, and the legacy fill path sets gasPrice exactly to this stale value, so transactions priced by the node's own fee oracle can be accepted but parked/filtered until fees decay.
Recommended Fix:
Quote the pending child base fee via chain_spec.next_block_base_fee(&latest_header, next_timestamp) (or an equivalent pending-header source) and give legacy gasPrice the same safety headroom used for dynamic-fee defaults.
| fn redact_fee_history(history: &mut FeeHistory) { | ||
| history.base_fee_per_gas.fill(u128::from(TEMPO_T0_BASE_FEE)); | ||
| fn redact_fee_history(history: &mut FeeHistory, fork: ZoneHardfork) { | ||
| if !fork.is_z1() { |
There was a problem hiding this comment.
🚨 [POTENTIAL-VULNERABILITY] Preserving Z1 base fees leaks redacted gas usage
After this change, fee history keeps the exact Z1 base_fee_per_gas series while still zeroing gas_used_ratio. Because the new base-fee rule is a deterministic function of the parent gas_used, consecutive public base fees let an authenticated redacted-RPC client recover the gas usage that the API is trying to hide whenever the controller is not clamped.
Recommended Fix:
Keep Z1 base fees redacted or quantized on the redacted RPC, or explicitly document that base fees disclose aggregate activity and remove the misleading gas-usage redaction guarantees.
| ) -> eyre::Result<Self::Pool> { | ||
| // Zone blocks have no protocol base fee, so allow zero-fee transactions into the pool. | ||
| // Keep Z0 transactions admissible across the Z1 transition. The pool's current block base | ||
| // fee still parks transactions that are underpriced once Z1 is active. |
There was a problem hiding this comment.
This comment assumes underpriced transactions are parked by pending-fee logic, but Tempo's EVM pool validator still validates against the latest completed header's base fee. When T7 lowers the pending child fee, a transaction capped in [next_base_fee, current_base_fee) is valid for the block being built yet is rejected at admission with gas price is less than basefee before classification.
Recommended Fix:
Do not enforce the completed block's base fee during pool prevalidation; either disable that check and let pending-fee classification/filtering handle it, or validate against the actual pending child fee and timestamp.
Gate dynamic Zone execution fees and the public RPC fee policy at Tempo T13 instead of Z1. Before T13, blocks keep a zero base fee and the RPC keeps its legacy defaults; at T13, the existing T7 fee controller and zero-tip RPC policy activate together using the inherited Tempo schedule. Update the Tempo pin to [346c22e](tempoxyz/tempo@346c22e), which introduces T13 and moves TIP-1096 to that boundary. This moves off the v1.14 release pin onto the mainline T13 commit; the lower package version labels in Cargo.lock reflect mainline's version metadata. Public-network T13 activation remains unscheduled. Update the reference ZoneFactory installer and its existing runtime validation tests to use the renamed T13 contract runtimes. Targets the branch for [#1433](#1433). Tests cover the T13 boundary, inherited activation, Z1 remaining fee-free before T13, RPC redaction, and sponsored fee settlement. The existing public-RPC test follows the DEV schedule's T13-at-genesis behavior. Validation: all four CI test partitions (929 tests), the Docker build, nightly formatting, Clippy, and dependency checks pass. The network-chaos test `test_incoming_leader_recovers_after_l1_disconnect` passed on CI's automatic retry. All 13 chainspec tests and both reference ZoneFactory installer tests also pass locally. Automated review and Cyclops audit checks remain pending. Prompted by: @0xKitsune --------- Co-authored-by: 0xKitsune <77890308+0xKitsune@users.noreply.github.com>
This PR enables dynamic execution fees on Zones behind the
Z1hardfork and makes the RPC fee surface follow the same policy.Before
Z1, Zones continue to produce blocks with a zero base fee and retain the existing public RPC defaults. OnceZ1is active, Zones use Tempo'sT7controller to derive the next base fee from the parent block's base fee and gas usage, including theT7floor and cap.At
Z1,eth_gasPricereturns only the latest base fee rather than adding Reth's sampled tip.eth_fillTransactiongives dynamic transactions base-fee headroom while defaulting an omitted priority fee to zero, consistent witheth_maxPriorityFeePerGas. Explicit priority fees are preserved.eth_feeHistorycontinues to redact activity-derived fields while preserving actual base fees afterZ1. When a historical range ends on the finalZ0block and its canonicalZ1successor is already available, the trailing fee now comes from that successor instead of being derived under theZ0rules.The transaction pool keeps the protocol floor at zero so
Z0transactions remain admissible across theZ1transition. OnceZ1is active, the pool's current block base fee prevents underpriced transactions from being selected for blocks.