feat: zone fee manager#680
Conversation
# Conflicts: # crates/evm/src/executor.rs # crates/evm/src/lib.rs
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review
PR #680 adds a zone-native fee manager, but the change has several cross-layer integration issues around txpool fee-token admission, TIP-403 policy enforcement, L1 token-enable caching, default-token initialization, and transient RPC error classification. I left the actionable findings inline.
Reviewer Callouts
- ⚡ Prover/witness coverage for fee-time L1 reads:
collect_fee_pre_tx -> ensure_enabled -> is_enabled_tokennow performs implicit L1 storage reads for every charged transaction, outside the explicitTempoState.readTempoStorageSlotuser-call surface. Confirm the proving/witness pipeline records these reads deterministically. - ⚡ T8/TIP-1042 fee-policy semantics: when making fee collection L1-policy-aware, align sender/recipient authorization exemptions with Tempo's fee semantics so the FeeManager recipient is not accidentally made a blocker for otherwise valid fee tokens.
- ⚡ Pool/execution consistency: fixing the slot-collision issue should include a broader check that public txpool validation and block execution use the same zone fee-token resolution, portal-enable checks, and no-FeeAMM assumptions.
| /// `TipFeeManager` storage slots are read or overwritten. | ||
| #[contract(addr = ZONE_FEE_MANAGER_ADDRESS)] | ||
| pub struct ZoneFeeManager { | ||
| user_tokens: Mapping<Address, Address>, |
There was a problem hiding this comment.
🚨 [SECURITY] ZoneFeeManager slot-0 layout collides with Tempo txpool AMM liquidity cache
ZoneFeeManager is deployed at the same 0xfeEC...0000 account as Tempo's TipFeeManager, but this first field makes user_tokens occupy mapping slot 0. The zone txpool still reuses Tempo's AmmLiquidityCache, which reads slot 0 at this address as TipFeeManager.validator_tokens[beneficiary]; the PR also removes the executor-side override_validator_token write that previously kept that view aligned with each tx. Non-default portal-enabled fee tokens can therefore be rejected from the mempool for missing FeeAMM liquidity, and a sequencer setting a non-default user token can make pathUSD transactions look unliquid.
Recommended Fix: Use a zone-specific txpool validator/ZoneEvmConfig that accepts portal-enabled fee tokens without FeeAMM routing, or otherwise reserve/restore the old slot-0 validator-token semantics so txpool cache reads cannot observe ZoneFeeManager.user_tokens.
| ) -> Result<Address> { | ||
| self.ensure_enabled(provider, fee_token)?; | ||
|
|
||
| let mut token = TIP20Token::from_address(fee_token)?; |
There was a problem hiding this comment.
🚨 [SECURITY] Zone fee debits and distributions bypass L1 TIP-403 policy enforcement
The fee manager instantiates vanilla TIP20Token directly for fee collection (and later does the same for distributeFees). On zones, normal TIP-20 calls are routed through ZoneTip20Token/ZoneTip403ProxyRegistry so L1-backed transfer policies are enforced; vanilla TIP20Token consults only the local L2 registry, which is not the canonical zone policy source. A policy-restricted account can move restricted tokens to the sequencer by paying gas fees, and distribution can complete the transfer without the L1-backed policy check a normal transfer would require.
Recommended Fix: Thread the same PolicyProvider/ZoneTip403ProxyRegistry used by ZoneTip20Token into the fee path. Enforce L1-backed sender authorization before fee debit and enforce or explicitly define a protocol exemption for FeeManager -> sequencer distributions; add deny-all policy regression tests for both paths.
|
|
||
| // Preserve Tempo's TIP-20 call inference and default-token behavior for | ||
| // users that have not selected a zone preference. | ||
| journal.get_fee_token(tx, fee_payer, spec, actions) |
There was a problem hiding this comment.
🚨 [SECURITY] Default pathUSD fees are rejected on zones created with a custom initial token
This preserves Tempo's fallback to DEFAULT_FEE_TOKEN/pathUSD for users without an explicit or stored preference, but ZoneFeeManager::collect_fee_pre_tx now requires the resolved token to be enabled in the L1 ZonePortal registry. The custom-initial-token creation flow enables only the operator-selected initialToken on the portal, while zone genesis still always creates pathUSD as the default fee token. On a zone initialized with a non-pathUSD token, ordinary transactions that fall back to pathUSD fail ensure_enabled(pathUSD) and are rejected until pathUSD is separately enabled on L1.
Recommended Fix: Keep genesis/default fee-token setup and the portal-enabled token set consistent: enable pathUSD on the portal in addition to a custom initial token, make genesis default to the chosen initial token, or explicitly special-case/document the genesis default token with tests for the custom-initial-token flow.
| block_number: u64, | ||
| ) -> core::result::Result<bool, PrecompileError> { | ||
| let slot = keccak256((token, PORTAL_TOKEN_CONFIGS_SLOT).abi_encode()); | ||
| let value = self.read_l1_storage(self.zone_portal_address(), slot, block_number)?; |
There was a problem hiding this comment.
🚨 [SECURITY] Pre-enable false cache entries can poison ZoneFeeManager token validation
Fee-token enablement now reads ZonePortal._tokenConfigs[token].enabled through the shared cache-first L1StateProvider. That cache returns the latest value at or before the requested finalized L1 block, and RPC misses are written back into the shared cache. Because TokenEnabled events are decoded but not written into L1StateCache for this mapping slot, a public isEnabledToken/fee read before enablement can cache false; later, after L1 enables the token, the same node can keep rejecting it while other nodes fetch true.
Recommended Fix: When processing TokenEnabled, update or invalidate the corresponding portal token-config slot in L1StateCache at the event block, including startup/backfill seeding for initially enabled tokens. Avoid treating RPC-fetched values for mutable untracked slots as future-block baselines unless all writes to those slots are tracked.
| self.__initialize() | ||
| } | ||
|
|
||
| fn map_reader_error(error: PrecompileError) -> TempoPrecompileError { |
There was a problem hiding this comment.
[zone rpc] errors before builder classification
map_reader_error stringifies PrecompileError. For PrecompileError::Fatal("[zone rpc] ..."), revm-precompile displays this as "fatal: [zone rpc] ..."; the payload builder's transient-RPC skip logic checks starts_with("[zone rpc]"). A bounded/failing L1 reader on this fee path would therefore be classified as a generic fatal EVM error and abort block building instead of skipping only the affected pool transaction. The current production provider retries indefinitely, so this is latent today, but the marker propagation contract is broken.
Recommended Fix: Preserve the inner string when mapping PrecompileError::Fatal(s) into TempoPrecompileError::Fatal(s), add tests that fee-token validation [zone rpc] errors hit the builder's skip path, and consider making the classifier robust to standard fatal-display prefixes.
# Conflicts: # crates/evm/src/lib.rs
👁️ Cyclops Security Review
🧭 Verifying · mode=
Findings
⚙️ Controls
📜 40 events🔍 |
2 participants
This PR implements a zone fee manager that allows users to pay transaction fees in any token enabled for the zone. Fees are collected and credited to the sequencer in the same token, without requiring FeeAMM liquidity or swaps.
ZoneFeeManagerreplaces Tempo's fee manager precompile at0xfeEC000000000000000000000000000000000000and is wired into Tempo'sProtocolFeeManagerhooks.