From 5b83eb4ee3505208fbb5aa445a2ca8caa5e4073f Mon Sep 17 00:00:00 2001 From: erhant Date: Wed, 19 Aug 2026 21:36:39 +0300 Subject: [PATCH] =?UTF-8?q?fix(fees):=20review=20fixes=20=E2=80=94=20clamp?= =?UTF-8?q?=20actual=20fee,=20bind=20cross-zone=20producer,=20doc=20contra?= =?UTF-8?q?cts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lez/common/src/block.rs | 5 ++ lez/indexer/core/src/cross_zone_verifier.rs | 55 +++++++++++++------- lez/programs/fee/core/src/assess.rs | 21 ++++++-- lez/sequencer/core/src/cross_zone_watcher.rs | 7 +++ 4 files changed, 65 insertions(+), 23 deletions(-) diff --git a/lez/common/src/block.rs b/lez/common/src/block.rs index e1fffb2d0..17bee2bfb 100644 --- a/lez/common/src/block.rs +++ b/lez/common/src/block.rs @@ -84,6 +84,11 @@ impl Block { /// Whether the header signature verifies against the embedded producer /// key. Every valid block must satisfy this. + /// + /// This attests only that the producer signed the *declared* `header.hash`, + /// not that the hash matches the block contents — so it is not an + /// authenticity check on its own. Pair it with a `recompute_hash` check (as + /// `validate_against_tip` does) before trusting it. #[must_use] pub fn has_valid_producer_signature(&self) -> bool { self.header diff --git a/lez/indexer/core/src/cross_zone_verifier.rs b/lez/indexer/core/src/cross_zone_verifier.rs index be17e6f3c..8b44091db 100644 --- a/lez/indexer/core/src/cross_zone_verifier.rs +++ b/lez/indexer/core/src/cross_zone_verifier.rs @@ -414,15 +414,24 @@ impl CrossZoneVerifier { .await?; // Equivocation defense: the source block must be signed by the peer's - // pinned block-signing key, not merely inscribed on the channel. - if let Some(expected) = self.peer_pubkeys.get(&msg.src_zone) - && !peer_block.is_signed_by(expected) - { - return Err(CrossZoneVerifyError::Forged(anyhow!( - "forged cross-zone dispatch: peer zone {} block {} is not signed by the pinned block-signing key", - hex::encode(msg.src_zone), - msg.src_block_id - ))); + // pinned block-signing key and declare it as the producer, not merely be + // inscribed on the channel — so it is a block the peer's home zone would + // also accept. + if let Some(expected) = self.peer_pubkeys.get(&msg.src_zone) { + if !peer_block.is_signed_by(expected) { + return Err(CrossZoneVerifyError::Forged(anyhow!( + "forged cross-zone dispatch: peer zone {} block {} is not signed by the pinned block-signing key", + hex::encode(msg.src_zone), + msg.src_block_id + ))); + } + if peer_block.header.producer != *expected { + return Err(CrossZoneVerifyError::Forged(anyhow!( + "forged cross-zone dispatch: peer zone {} block {} declares a producer other than the pinned block-signing key", + hex::encode(msg.src_zone), + msg.src_block_id + ))); + } } // Everything below is a property of the peer block just read, so a @@ -565,15 +574,25 @@ fn accept_peer_block( return false; } - if let Some(expected) = expected_pubkey - && !block.is_signed_by(expected) - { - warn!( - "Peer reader dropping block {} from {}: not signed by the pinned block-signing key", - block.header.block_id, - hex::encode(peer_zone) - ); - return false; + if let Some(expected) = expected_pubkey { + if !block.is_signed_by(expected) { + warn!( + "Peer reader dropping block {} from {}: not signed by the pinned block-signing key", + block.header.block_id, + hex::encode(peer_zone) + ); + return false; + } + // The producer must be the pinned key itself, not merely signed by it, + // so the block is one the peer's home zone would also accept. + if block.header.producer != *expected { + warn!( + "Peer reader dropping block {} from {}: producer is not the pinned block-signing key", + block.header.block_id, + hex::encode(peer_zone) + ); + return false; + } } true diff --git a/lez/programs/fee/core/src/assess.rs b/lez/programs/fee/core/src/assess.rs index dee6b92c9..730258728 100644 --- a/lez/programs/fee/core/src/assess.rs +++ b/lez/programs/fee/core/src/assess.rs @@ -65,6 +65,12 @@ impl FeeTxView { /// The amount held from the payer before execution, at the block's opening /// base fees: `gas_limit·base_fee_exec + gas_stor·base_fee_stor + tip`. +/// +/// The caller must pass a cap-validated view (gas within `MAX_GAS_*`) and a +/// well-formed `FeeState` whose base fees stay in `[MIN, BASE_FEE_*_MAX]` — the +/// bounds `next_base_fee` maintains. Under those preconditions every product +/// fits u64 and the u128 sum cannot overflow; on an unchecked view against a +/// corrupt state it could wrap silently in release. #[must_use] pub fn fee_reserve(view: &FeeTxView, fee_state: &FeeState) -> u128 { u128::from(view.gas_limit()) * u128::from(fee_state.base_fee_exec) @@ -75,12 +81,15 @@ pub fn fee_reserve(view: &FeeTxView, fee_state: &FeeState) -> u128 { /// The base fee actually owed after execution. /// /// `gas_exec·base_fee_exec + gas_stor·base_fee_stor`, where `gas_exec` is the -/// charged (clamped) cycle count for public transactions and the fixed -/// verification cost for private. +/// executed cycle count clamped to the transaction's `gas_limit` for public +/// transactions (a session may overshoot its budget by up to one instruction, +/// but a transaction is never billed past the gas it declared) and the fixed +/// verification cost for private. Clamping here keeps `actual + tip ≤ reserve` +/// regardless of what the caller passes. #[must_use] pub fn fee_actual_base(charged_cycles: u64, view: &FeeTxView, fee_state: &FeeState) -> u128 { let gas_exec = match view { - FeeTxView::Public { .. } => charged_cycles, + FeeTxView::Public { gas_limit, .. } => charged_cycles.min(*gas_limit), FeeTxView::Private { .. } => market::PRIVATE_VERIFY_GAS, }; u128::from(gas_exec) * u128::from(fee_state.base_fee_exec) @@ -131,7 +140,7 @@ mod tests { } #[test] - fn reserve_dominates_actual_within_the_limit() { + fn reserve_dominates_actual_even_past_the_limit() { let view = FeeTxView::Public { payer: payer(), gas_limit: 10_000, @@ -141,7 +150,9 @@ mod tests { }; let state = genesis(); let reserve = fee_reserve(&view, &state); - for cycles in [0, 1, 5_000, 10_000] { + // Including cycle counts above the limit: the clamp keeps the actual fee + // bounded by the reserve, so the payer is never billed past gas_limit. + for cycles in [0, 1, 5_000, 10_000, 10_001, 1_000_000, u64::MAX] { assert!(fee_actual_base(cycles, &view, &state) + u128::from(view.tip()) <= reserve); } } diff --git a/lez/sequencer/core/src/cross_zone_watcher.rs b/lez/sequencer/core/src/cross_zone_watcher.rs index 9a910df10..1de9296d3 100644 --- a/lez/sequencer/core/src/cross_zone_watcher.rs +++ b/lez/sequencer/core/src/cross_zone_watcher.rs @@ -172,6 +172,13 @@ fn link_against( return Link::OffChain("block-signing key does not match the pinned key".to_owned()); } + // The producer must also *be* the pinned key, not merely signed by it: + // otherwise the block declares a producer its home zone would reject + // (`has_valid_producer_signature` fails there) yet passes here. + if expected_pubkey.is_some_and(|key| block.header.producer != *key) { + return Link::OffChain("block producer does not match the pinned key".to_owned()); + } + let recomputed = block.recompute_hash(); if recomputed != block.header.hash { // The signature does not cover this field, so a correctly signed block