apollo_consensus_orchestrator: wire SNIP-35 fee_proposal into build_proposal

Author: sirandreww-starkware

crates/apollo_consensus_orchestrator/src/build_proposal.rs

@@ -76,6 +76,10 @@ pub(crate) struct ProposalBuildArguments {
     pub override_l2_gas_price_fri: Option<u128>,
     pub min_l2_gas_price_per_height: Vec<PricePerHeight>,
     pub compare_retrospective_block_hash: bool,
+    /// SNIP-35: proposer's fee_proposal for this block.
+    pub fee_proposal: GasPrice,
+    /// SNIP-35: current fee_actual from the sliding window.
+    pub fee_actual: Option<GasPrice>,
 }
 
 type BuildProposalResult<T> = Result<T, BuildProposalError>;
@@ -176,7 +180,7 @@ async fn initiate_build(args: &mut ProposalBuildArguments) -> BuildProposalResul
         starknet_version: starknet_api::block::StarknetVersion::LATEST,
         // TODO(Asmaa): Put the real value once we have it.
         version_constant_commitment: Default::default(),
-        fee_proposal_fri: None,
+        fee_proposal_fri: Some(args.fee_proposal),
     };
 
     let retrospective_block_hash = wait_for_retrospective_block_hash(
@@ -319,7 +323,7 @@ async fn get_proposal_content(
                     info.l2_gas_used,
                     args.override_l2_gas_price_fri,
                     &args.min_l2_gas_price_per_height,
-                    None,
+                    args.fee_actual,
                 );
                 let fin_payload = ProposalFinPayload {
                     commitment_parts: CommitmentParts::from(&info),

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs

@@ -75,7 +75,7 @@ use tokio::task::JoinHandle;
 use tokio::time::sleep;
 use tokio_util::sync::CancellationToken;
 use tokio_util::task::AbortOnDropHandle;
-use tracing::{error, error_span, info, instrument, trace, warn, Instrument};
+use tracing::{debug, error, error_span, info, instrument, trace, warn, Instrument};
 
 use crate::build_proposal::{build_proposal, BuildProposalError, ProposalBuildArguments};
 use crate::cende::{
@@ -94,8 +94,19 @@ use crate::metrics::{
     record_validate_proposal_failure,
     register_metrics,
     CONSENSUS_L2_GAS_PRICE,
+    SNIP35_FEE_ACTUAL,
+    SNIP35_FEE_PROPOSAL,
+    SNIP35_FEE_TARGET,
+    SNIP35_STRK_USD_RATE,
+};
+use crate::snip35::{
+    compute_fee_actual,
+    compute_fee_proposal,
+    compute_fee_target,
+    FEE_PROPOSAL_MARGIN_PPT,
+    FEE_PROPOSAL_WINDOW_SIZE,
+    TARGET_ATTO_USD_PER_L2_GAS,
 };
-use crate::snip35::FEE_PROPOSAL_WINDOW_SIZE;
 use crate::utils::{
     convert_to_sn_api_block_info,
     make_gas_price_params,
@@ -398,6 +409,7 @@ impl SequencerConsensusContext {
             sequencer,
             timestamp: BlockTimestamp(init.timestamp),
             l1_da_mode: init.l1_da_mode,
+            fee_proposal_fri: init.fee_proposal_fri,
             // TODO(guy.f): Figure out where/if to get the values below from and fill them.
             ..Default::default()
         };
@@ -416,16 +428,64 @@ impl SequencerConsensusContext {
     /// Returns the next L2 gas price without mutating context. Used when building the fin and when
     /// updating at decision time.
     fn calculate_next_l2_gas_price(&self, height: BlockNumber, l2_gas_used: GasAmount) -> GasPrice {
+        let fee_actual = compute_fee_actual(&self.fee_proposals_window, height);
         calculate_next_l2_gas_price_for_fin(
             self.l2_gas_price,
             height,
             l2_gas_used,
             self.config.dynamic_config.override_l2_gas_price_fri,
             &self.config.dynamic_config.min_l2_gas_price_per_height,
-            None,
+            fee_actual,
         )
     }
 
+    /// SNIP-35: compute the fee_proposal this node should publish, given the precomputed
+    /// `fee_actual` (median of the recent fee_proposals window). When `fee_actual` is `None`
+    /// the window is incomplete: freeze the fee_proposal at `l2_gas_price`, since without a
+    /// real clamping center we'd otherwise emit an arbitrary value. The validator computes
+    /// `fee_actual` the same way, so it also freezes — both sides agree.
+    async fn compute_snip35_fee_proposal(
+        &self,
+        fee_actual: Option<GasPrice>,
+        timestamp: u64,
+    ) -> GasPrice {
+        let Some(fee_actual) = fee_actual else {
+            warn!("fee_actual unavailable, freezing fee_proposal at l2_gas_price");
+            SNIP35_FEE_PROPOSAL.set_lossy(self.l2_gas_price.0);
+            return self.l2_gas_price;
+        };
+        SNIP35_FEE_ACTUAL.set_lossy(fee_actual.0);
+
+        // The instance held in `strk_to_usd_oracle` is configured with a STRK/USD endpoint;
+        // the trait method is generic and the rate semantics are labeled by the field.
+        let fee_target = match &self.deps.strk_to_usd_oracle {
+            Some(oracle) => match oracle.fetch_rate(timestamp).await {
+                Ok(rate) if rate > 0 => {
+                    SNIP35_STRK_USD_RATE.set_lossy(rate);
+                    let target = compute_fee_target(TARGET_ATTO_USD_PER_L2_GAS, rate);
+                    SNIP35_FEE_TARGET.set_lossy(target.0);
+                    Some(target)
+                }
+                Ok(_) => {
+                    warn!("STRK/USD oracle returned zero rate, freezing fee_proposal");
+                    None
+                }
+                Err(e) => {
+                    warn!("STRK/USD oracle error: {e:?}, freezing fee_proposal");
+                    None
+                }
+            },
+            None => {
+                debug!("No STRK/USD oracle configured, freezing fee_proposal");
+                None
+            }
+        };
+
+        let proposal = compute_fee_proposal(fee_target, fee_actual, FEE_PROPOSAL_MARGIN_PPT);
+        SNIP35_FEE_PROPOSAL.set_lossy(proposal.0);
+        proposal
+    }
+
     fn update_l2_gas_price(&mut self, height: BlockNumber, l2_gas_used: GasAmount) {
         self.l2_gas_price = self.calculate_next_l2_gas_price(height, l2_gas_used);
         let gas_price_u64 = u64::try_from(self.l2_gas_price.0).unwrap_or(u64::MAX);
@@ -619,6 +679,7 @@ impl ConsensusContext for SequencerConsensusContext {
 
         // Handles interrupting an active proposal from a previous height/round
         self.set_height_and_round(build_param.height, build_param.round).await?;
+
         assert!(
             self.active_proposal.is_none(),
             "We should not have an existing active proposal for the (height, round) when \
@@ -656,6 +717,9 @@ impl ConsensusContext for SequencerConsensusContext {
             BehaviorMode::Echonet => true,
             BehaviorMode::Starknet => false,
         };
+        let fee_actual = compute_fee_actual(&self.fee_proposals_window, build_param.height);
+        let fee_proposal =
+            self.compute_snip35_fee_proposal(fee_actual, self.deps.clock.unix_now()).await;
         let round = build_param.round;
         let args = ProposalBuildArguments {
             deps: self.deps.clone(),
@@ -689,6 +753,8 @@ impl ConsensusContext for SequencerConsensusContext {
                 .config
                 .dynamic_config
                 .compare_retrospective_block_hash,
+            fee_proposal,
+            fee_actual,
         };
 
         let handle = tokio::spawn(

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs

@@ -27,7 +27,11 @@ use apollo_l1_gas_price_types::errors::{
     L1GasPriceClientError,
     L1GasPriceProviderError,
 };
-use apollo_l1_gas_price_types::{MockL1GasPriceProviderClient, PriceInfo};
+use apollo_l1_gas_price_types::{
+    MockExchangeRateOracleClientTrait,
+    MockL1GasPriceProviderClient,
+    PriceInfo,
+};
 use apollo_protobuf::consensus::{
     BuildParam,
     CommitmentParts,
@@ -1601,3 +1605,107 @@ async fn test_initialize_fee_proposals_window(
     context.initialize_fee_proposals_window(start_height).await.unwrap();
     assert_eq!(context.fee_proposals_window, expected_window);
 }
+
+#[derive(Clone)]
+enum OracleBehavior {
+    /// No oracle is configured (`strk_to_usd_oracle = None`).
+    NotConfigured,
+    /// Oracle is configured and `fetch_rate` returns `Ok(rate)`.
+    Ok(u128),
+    /// Oracle is configured and `fetch_rate` returns `Err(_)`.
+    Err,
+}
+
+// `compute_snip35_fee_proposal` decides the published fee_proposal from the precomputed
+// `fee_actual` and the STRK/USD oracle. Each case fixes both inputs (fee_actual + oracle
+// behavior + l2_gas_price) and the expected published fee_proposal.
+//
+// Geometric bounds (with FEE_PROPOSAL_MARGIN_PPT = 2, PPT_DENOMINATOR = 1000):
+//   for fee_actual = 10_000_000_000 (10 gwei),
+//     upper = fee_actual * 1002 / 1000 = 10_020_000_000
+//     lower = fee_actual * 1000 / 1002 = 9_980_039_920
+//
+// Oracle target = TARGET_ATTO_USD_PER_L2_GAS * 1e18 / strk_usd_rate (u128 saturating):
+//   rate = 1                    -> floor = 3e27, saturates to u128::MAX (very high target)
+//   rate = 3 * 10^17            -> floor = 1e10 (exactly fee_actual; target in bounds)
+//   rate = 10^21                -> floor = 3e6 (low target, well below lower bound)
+#[rstest]
+// `fee_actual = None` => freeze at `l2_gas_price`; oracle is not consulted.
+#[case::no_fee_actual_freezes_at_l2_gas_price(
+    None,
+    OracleBehavior::NotConfigured,
+    GasPrice(7_000_000_000),
+    GasPrice(7_000_000_000)
+)]
+// No oracle => `fee_target` is `None` => `compute_fee_proposal` returns `fee_actual` unchanged.
+#[case::no_oracle_freezes_at_fee_actual(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::NotConfigured,
+    GasPrice(7_000_000_000),
+    GasPrice(10_000_000_000)
+)]
+// Oracle returns 0 => treated as invalid => freeze at `fee_actual`.
+#[case::oracle_zero_rate_freezes_at_fee_actual(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::Ok(0),
+    GasPrice(7_000_000_000),
+    GasPrice(10_000_000_000)
+)]
+// Oracle errors => freeze at `fee_actual`.
+#[case::oracle_err_freezes_at_fee_actual(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::Err,
+    GasPrice(7_000_000_000),
+    GasPrice(10_000_000_000)
+)]
+// Oracle target in bounds (== fee_actual) => published as-is.
+#[case::oracle_target_in_bounds_returns_target(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::Ok(300_000_000_000_000_000),
+    GasPrice(7_000_000_000),
+    GasPrice(10_000_000_000)
+)]
+// Oracle target very high => clamps to upper bound.
+#[case::oracle_target_above_clamps_to_upper(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::Ok(1),
+    GasPrice(7_000_000_000),
+    GasPrice(10_020_000_000)
+)]
+// Oracle target very low => clamps to lower bound.
+#[case::oracle_target_below_clamps_to_lower(
+    Some(GasPrice(10_000_000_000)),
+    OracleBehavior::Ok(1_000_000_000_000_000_000_000),
+    GasPrice(7_000_000_000),
+    GasPrice(9_980_039_920)
+)]
+#[tokio::test]
+async fn test_compute_snip35_fee_proposal(
+    #[case] fee_actual: Option<GasPrice>,
+    #[case] oracle_behavior: OracleBehavior,
+    #[case] l2_gas_price: GasPrice,
+    #[case] expected_fee_proposal: GasPrice,
+) {
+    let (mut deps, _network) = create_test_and_network_deps();
+    deps.setup_default_expectations();
+    deps.strk_to_usd_oracle = match oracle_behavior {
+        OracleBehavior::NotConfigured => None,
+        OracleBehavior::Ok(rate) => {
+            let mut mock = MockExchangeRateOracleClientTrait::new();
+            mock.expect_fetch_rate().returning(move |_| Ok(rate));
+            Some(Arc::new(mock))
+        }
+        OracleBehavior::Err => {
+            let mut mock = MockExchangeRateOracleClientTrait::new();
+            mock.expect_fetch_rate().returning(|_| {
+                Err(ExchangeRateOracleClientError::RequestError("test".to_string()))
+            });
+            Some(Arc::new(mock))
+        }
+    };
+
+    let mut context = deps.build_context();
+    context.l2_gas_price = l2_gas_price;
+    let proposal = context.compute_snip35_fee_proposal(fee_actual, 0).await;
+    assert_eq!(proposal, expected_fee_proposal);
+}

crates/apollo_consensus_orchestrator/src/snip35/mod.rs

@@ -12,8 +12,11 @@
 //! See also: `fee_market` for EIP-1559-style base-fee adjustment, which receives
 //! `fee_actual` as a floor.
 
+use std::collections::BTreeMap;
+
 use ethnum::U256;
-use starknet_api::block::GasPrice;
+use starknet_api::block::{BlockNumber, GasPrice};
+use tracing::warn;
 
 #[cfg(test)]
 mod test;
@@ -28,54 +31,74 @@ pub(crate) const PPT_DENOMINATOR: u128 = 1000;
 // TODO(AndrewL): consider moving this to versioned constants.
 pub(crate) const FEE_PROPOSAL_WINDOW_SIZE: usize = 10;
 
-/// Compute fee_actual from the last `window_size` `fee_proposal` values (SNIP-35).
+/// Maximum fee_proposal change per block in parts per thousand (SNIP-35: 0.2%).
+// TODO(AndrewL): consider moving this to versioned constants.
+pub(crate) const FEE_PROPOSAL_MARGIN_PPT: u128 = 2;
+
+/// Target USD cost per L2 gas unit in atto-USD ($3e-9 = 3_000_000_000 atto-USD).
+// TODO(AndrewL): consider moving this to a dynamic config.
+pub(crate) const TARGET_ATTO_USD_PER_L2_GAS: u128 = 3_000_000_000;
+
+/// Compute fee_actual for `height` as the median of the `fee_proposal` values
+/// recorded for heights `[height - FEE_PROPOSAL_WINDOW_SIZE, height - 1]` (SNIP-35).
 ///
-/// Median rule: for even `window_size`, the average of the two middle values rounded
-/// down; for odd `window_size`, the single middle value.
+/// Returns `None` (after logging a warning) when any of those heights is missing from
+/// `fee_proposals_window` or recorded as `None` (e.g., pre-SNIP-35 blocks). The `None`
+/// case triggers the `l2_gas_price` fallback in both proposer and validator paths.
 ///
-/// Returns `None` if `window_size == 0`, fewer than `window_size` proposals are
-/// available, or the median is zero (e.g., pre-SNIP-35 blocks with `fee_proposal == 0`).
-/// The `None` case triggers the `l2_gas_price` fallback in both proposer and validator
-/// paths.
-pub fn compute_fee_actual(proposals: &[GasPrice], window_size: usize) -> Option<GasPrice> {
-    if window_size == 0 {
+/// Median rule for even `FEE_PROPOSAL_WINDOW_SIZE`: average of the two middle values
+/// rounded down; for odd: the single middle value.
+pub fn compute_fee_actual(
+    fee_proposals_window: &BTreeMap<BlockNumber, Option<GasPrice>>,
+    height: BlockNumber,
+) -> Option<GasPrice> {
+    let window_size =
+        u64::try_from(FEE_PROPOSAL_WINDOW_SIZE).expect("FEE_PROPOSAL_WINDOW_SIZE fits in u64");
+    let Some(start) = height.0.checked_sub(window_size) else {
+        warn!(
+            "Cannot compute fee_actual for height {height}: height is below \
+             FEE_PROPOSAL_WINDOW_SIZE ({FEE_PROPOSAL_WINDOW_SIZE})"
+        );
         return None;
+    };
+    let mut window = Vec::with_capacity(FEE_PROPOSAL_WINDOW_SIZE);
+    for source_height in (start..height.0).map(BlockNumber) {
+        match fee_proposals_window.get(&source_height) {
+            Some(Some(price)) => window.push(*price),
+            Some(None) | None => {
+                warn!(
+                    "Cannot compute fee_actual for height {height}: fee_proposals_window has no \
+                     recorded fee_proposal for height {source_height}"
+                );
+                return None;
+            }
+        }
     }
-    let start = proposals.len().checked_sub(window_size)?;
-    let window = proposals.get(start..)?;
-    let mut sorted: Vec<GasPrice> = window.to_vec();
-    sorted.sort();
-    let mid = window_size / 2;
-    let median = if window_size.is_multiple_of(2) {
+    window.sort();
+    let mid = FEE_PROPOSAL_WINDOW_SIZE / 2;
+    let median = if FEE_PROPOSAL_WINDOW_SIZE.is_multiple_of(2) {
         // Even: average of the two middle values, rounded down.
         // Overflow-safe averaging: a + (b - a) / 2 (safe because sorted, so b >= a).
-        GasPrice(sorted[mid - 1].0 + (sorted[mid].0 - sorted[mid - 1].0) / 2)
+        GasPrice(window[mid - 1].0 + (window[mid].0 - window[mid - 1].0) / 2)
     } else {
-        sorted[mid]
+        window[mid]
     };
-    if median.0 == 0 { None } else { Some(median) }
+    Some(median)
 }
 
 /// Compute the fee target from STRK/USD price and a USD cost target (SNIP-35).
 ///
 /// `target_atto_usd_per_l2_gas` is in atto-USD (atto = 10⁻¹⁸; so a value of
 /// `3_000_000_000` means 3·10⁻⁹ USD = 3 nanodollars per L2 gas unit).
-/// `strk_usd_rate` is the STRK/USD price with 18 decimals (from the oracle).
-/// Result is in FRI, clamped to `[floor_min_fri, floor_max_fri]`.
-pub fn compute_fee_target(
-    target_atto_usd_per_l2_gas: u128,
-    strk_usd_rate: u128,
-    floor_min_fri: u128,
-    floor_max_fri: u128,
-) -> GasPrice {
-    if strk_usd_rate == 0 {
-        return GasPrice(floor_max_fri);
-    }
+/// `strk_usd_rate` is the STRK/USD price with 18 decimals (from the oracle); callers must
+/// filter `rate == 0` before invoking (treat as oracle failure).
+/// Returns the target in FRI; the multiplicative margin clamp in `compute_fee_proposal`
+/// keeps the published proposal bounded relative to `fee_actual`.
+pub fn compute_fee_target(target_atto_usd_per_l2_gas: u128, strk_usd_rate: u128) -> GasPrice {
     // floor_fri = target_atto_usd_per_l2_gas * 10^18 / strk_usd_rate
     let numerator = U256::from(target_atto_usd_per_l2_gas) * U256::from(FRI_DECIMALS_SCALE);
     let floor = numerator / U256::from(strk_usd_rate);
-    let floor_u128 = u128::try_from(floor).unwrap_or(u128::MAX);
-    GasPrice(floor_u128.clamp(floor_min_fri, floor_max_fri))
+    GasPrice(u128::try_from(floor).unwrap_or(u128::MAX))
 }
 
 /// Compute the fee_proposal an honest proposer should publish (SNIP-35).