apollo_consensus_orchestrator: include fee_proposal_fri in proposal commitment hash (#13949)

Author: sirandreww-starkware

crates/apollo_consensus_orchestrator/src/build_proposal.rs

@@ -41,6 +41,7 @@ use tracing::{debug, info, trace, warn};
 
 use crate::fee_market::calculate_next_l2_gas_price_for_fin;
 use crate::sequencer_consensus_context::{BuiltProposals, SequencerConsensusContextDeps};
+use crate::snip35::proposal_commitment_from;
 use crate::utils::{
     convert_to_sn_api_block_info,
     get_l1_prices_in_fri_and_wei,
@@ -269,8 +270,10 @@ async fn get_proposal_content(
                     })?;
             }
             GetProposalContent::Finished(info) => {
-                let proposal_commitment =
-                    ProposalCommitment(info.proposal_commitment.partial_block_hash.0);
+                let proposal_commitment = proposal_commitment_from(
+                    info.proposal_commitment.partial_block_hash,
+                    Some(args.fee_proposal),
+                );
                 content = truncate_to_executed_txs(&mut content, info.final_n_executed_txs);
 
                 info!(

crates/apollo_consensus_orchestrator/src/build_proposal_test.rs

@@ -8,16 +8,17 @@ use apollo_batcher_types::batcher_types::{
     ProposalCommitment,
 };
 use apollo_batcher_types::communication::BatcherClientError;
-use apollo_consensus::types::ProposalCommitment as ConsensusProposalCommitment;
 use apollo_infra::component_client::ClientError;
 use apollo_transaction_converter::{MockTransactionConverterTrait, TransactionConverterError};
 use assert_matches::assert_matches;
+use starknet_api::block::GasPrice;
 use starknet_api::block_hash::block_hash_calculator::BlockHeaderCommitments;
 use starknet_api::core::ClassHash;
 use starknet_api::execution_resources::GasAmount;
 use tokio_util::task::AbortOnDropHandle;
 
 use crate::build_proposal::{build_proposal, BuildProposalError};
+use crate::snip35::proposal_commitment_from;
 use crate::test_utils::{create_proposal_build_arguments, INTERNAL_TX_BATCH, PARTIAL_BLOCK_HASH};
 
 #[tokio::test]
@@ -44,7 +45,7 @@ async fn build_proposal_succeed() {
     tokio::time::sleep(Duration::from_millis(100)).await;
 
     let res = build_proposal(proposal_args.into()).await.unwrap();
-    assert_eq!(res, ConsensusProposalCommitment(PARTIAL_BLOCK_HASH.0));
+    assert_eq!(res, proposal_commitment_from(PARTIAL_BLOCK_HASH, Some(GasPrice::default())));
 }
 
 #[tokio::test]

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context.rs

@@ -103,6 +103,7 @@ use crate::snip35::{
     compute_fee_actual,
     compute_fee_proposal,
     compute_fee_target,
+    proposal_commitment_from,
     FeeProposalInfo,
     FEE_PROPOSAL_MARGIN_PPT,
     FEE_PROPOSAL_WINDOW_SIZE,
@@ -184,8 +185,10 @@ impl BuiltProposals {
         proposal_id: &ProposalId,
         finished_info: FinishedProposalInfo,
     ) {
-        let proposal_commitment =
-            ProposalCommitment(finished_info.proposal_commitment.partial_block_hash.0);
+        let proposal_commitment = proposal_commitment_from(
+            finished_info.proposal_commitment.partial_block_hash,
+            init.fee_proposal_fri,
+        );
 
         let height = init.height;
         let round = init.round;
@@ -591,9 +594,13 @@ impl SequencerConsensusContext {
                 compiled_class_hashes_for_migration: central_objects
                     .compiled_class_hashes_for_migration,
                 proposal_commitment: commitment,
+                // TODO(SNIP-35): plumb the parent block's `fee_proposal_fri` here once
+                // `central_objects.parent_proposal_commitment` carries it (or read from
+                // BlockHeaderWithoutHash storage). Today we pass `None`, which means
+                // pre-V0_14_3 commitments — correct for non-SNIP-35 deployments only.
                 parent_proposal_commitment: central_objects
                     .parent_proposal_commitment
-                    .map(|commitment| ProposalCommitment(commitment.partial_block_hash.0)),
+                    .map(|c| proposal_commitment_from(c.partial_block_hash, None)),
                 recent_block_hashes: self.collect_recent_block_hashes(height).await,
             })
             .await

crates/apollo_consensus_orchestrator/src/sequencer_consensus_context_test.rs

@@ -1,6 +1,6 @@
 use std::collections::BTreeMap;
 use std::future::ready;
-use std::sync::Arc;
+use std::sync::{Arc, LazyLock};
 use std::time::Duration;
 
 use apollo_batcher_types::batcher_types::{
@@ -72,6 +72,7 @@ use crate::sequencer_consensus_context::{
     SequencerConsensusContext,
     SequencerConsensusContextDeps,
 };
+use crate::snip35::proposal_commitment_from;
 use crate::test_utils::{
     create_test_and_network_deps,
     proposal_init,
@@ -96,7 +97,14 @@ fn expected_l2_gas_info_for_build_proposal_defaults() -> L2GasInfo {
 use crate::snip35::{compute_fee_actual, FEE_PROPOSAL_WINDOW_SIZE};
 use crate::utils::{apply_fee_transformations, make_gas_price_params};
 
-const TEST_PROPOSAL_COMMITMENT: ProposalCommitment = ProposalCommitment(PARTIAL_BLOCK_HASH.0);
+static TEST_PROPOSAL_COMMITMENT: LazyLock<ProposalCommitment> = LazyLock::new(|| {
+    // Default test setup: empty fee_proposals_window → `compute_fee_actual` returns `None` →
+    // `compute_snip35_fee_proposal` falls back to `self.l2_gas_price` (default
+    // `min_gas_price` = 8 gwei = 8_000_000_000 FRI). The orchestrator chains this value into
+    // the proposal commitment via `proposal_commitment_from`. The `proposal_init` test helper
+    // produces an init with `fee_proposal_fri = Some(8 gwei)` to match what the proposer emits.
+    proposal_commitment_from(PARTIAL_BLOCK_HASH, Some(GasPrice(8_000_000_000)))
+});
 const HEIGHT_0: BlockNumber = BlockNumber(0);
 const HEIGHT_1: BlockNumber = BlockNumber(1);
 
@@ -136,7 +144,7 @@ async fn validate_proposal_success() {
     let fin_receiver = context
         .validate_proposal(proposal_init(HEIGHT_0, ROUND_0), TIMEOUT, content_receiver)
         .await;
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 }
 
 #[rstest]
@@ -182,7 +190,7 @@ async fn validate_then_repropose(#[case] execute_all_txs: bool) {
         ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() });
     content_sender.send(transactions.clone()).await.unwrap();
     let fin = ProposalPart::Fin(ProposalFin {
-        proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+        proposal_commitment: *TEST_PROPOSAL_COMMITMENT,
         executed_transaction_count: n_executed_txs_count.try_into().unwrap(),
         fin_payload: Some(ProposalFinPayload {
             commitment_parts: CommitmentParts::default(),
@@ -192,11 +200,11 @@ async fn validate_then_repropose(#[case] execute_all_txs: bool) {
     content_sender.send(fin.clone()).await.unwrap();
     let fin_receiver = context.validate_proposal(init.clone(), TIMEOUT, content_receiver).await;
     content_sender.close_channel();
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 
     let build_param =
         BuildParam { round: ROUND_1, valid_round: Some(ROUND_0), ..Default::default() };
-    context.repropose(TEST_PROPOSAL_COMMITMENT, build_param).await;
+    context.repropose(*TEST_PROPOSAL_COMMITMENT, build_param).await;
     let (_, mut receiver) = network.outbound_proposal_receiver.next().await.unwrap();
     // Reproposal sends init with updated round, proposer, valid_round.
     let mut expected_init = init;
@@ -212,7 +220,7 @@ async fn validate_then_repropose(#[case] execute_all_txs: bool) {
     assert!(receiver.next().await.is_none());
 
     // Verify decision_reached uses the updated init (from reproposal round) for finalize.
-    context.decision_reached(HEIGHT_0, ROUND_1, TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
+    context.decision_reached(HEIGHT_0, ROUND_1, *TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
 }
 
 #[tokio::test]
@@ -260,14 +268,14 @@ async fn validate_then_build_then_decision_reached_round_0_uses_round_0_init() {
         ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() });
     content_sender.send(transactions.clone()).await.unwrap();
     let fin = ProposalPart::Fin(ProposalFin {
-        proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+        proposal_commitment: *TEST_PROPOSAL_COMMITMENT,
         executed_transaction_count: TX_BATCH.len().try_into().unwrap(),
         fin_payload: None,
     });
     content_sender.send(fin.clone()).await.unwrap();
     let fin_receiver = context.validate_proposal(init, TIMEOUT, content_receiver).await;
     content_sender.close_channel();
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 
     // Round 1: build - clock returns TIMESTAMP_ROUND_1 (different from TIMESTAMP_ROUND_0)
     let build_param = BuildParam { round: ROUND_1, ..Default::default() };
@@ -282,10 +290,10 @@ async fn validate_then_build_then_decision_reached_round_0_uses_round_0_init() {
     assert_eq!(build_init.timestamp, TIMESTAMP_ROUND_1);
     let _txs = receiver.next().await.unwrap();
     let _fin = receiver.next().await.unwrap();
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 
     // Decision reached for round 0 - state_sync should receive TIMESTAMP_ROUND_0
-    context.decision_reached(HEIGHT_0, ROUND_0, TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
+    context.decision_reached(HEIGHT_0, ROUND_0, *TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
 }
 
 #[tokio::test]
@@ -301,7 +309,7 @@ async fn proposals_from_different_rounds() {
     let prop_part_txs =
         ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() });
     let prop_part_fin = ProposalPart::Fin(ProposalFin {
-        proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+        proposal_commitment: *TEST_PROPOSAL_COMMITMENT,
         executed_transaction_count: INTERNAL_TX_BATCH.len().try_into().unwrap(),
         fin_payload: Some(ProposalFinPayload::default()),
     });
@@ -325,7 +333,7 @@ async fn proposals_from_different_rounds() {
     let fin_receiver_curr_round = context
         .validate_proposal(proposal_init(HEIGHT_0, ROUND_1), TIMEOUT, content_receiver)
         .await;
-    assert_eq!(fin_receiver_curr_round.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver_curr_round.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 
     // The proposal from the future round should not be processed.
     let (mut content_sender, content_receiver) =
@@ -394,7 +402,7 @@ async fn interrupt_active_proposal() {
 
     // Interrupt active proposal.
     assert!(fin_receiver_0.await.is_err());
-    assert_eq!(fin_receiver_1.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver_1.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 }
 
 #[tokio::test]
@@ -414,14 +422,16 @@ async fn build_proposal() {
         panic!("Expected ProposalPart::Init");
     };
     assert!(info.timestamp >= before && info.timestamp <= after);
+    let expected_proposal_commitment =
+        proposal_commitment_from(PARTIAL_BLOCK_HASH, info.fee_proposal_fri);
     assert_eq!(
         receiver.next().await.unwrap(),
         ProposalPart::Transactions(TransactionBatch { transactions: TX_BATCH.to_vec() })
     );
     assert_eq!(
         receiver.next().await.unwrap(),
         ProposalPart::Fin(ProposalFin {
-            proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+            proposal_commitment: expected_proposal_commitment,
             executed_transaction_count: INTERNAL_TX_BATCH.len().try_into().unwrap(),
             fin_payload: Some(ProposalFinPayload {
                 commitment_parts: CommitmentParts::default(),
@@ -430,7 +440,7 @@ async fn build_proposal() {
         })
     );
     assert!(receiver.next().await.is_none());
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), expected_proposal_commitment);
 }
 
 #[tokio::test]
@@ -564,12 +574,12 @@ async fn propose_then_repropose(#[case] execute_all_txs: bool) {
     };
     let _txs = receiver.next().await.unwrap();
     let fin = receiver.next().await.unwrap();
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 
     // Re-propose.
     let build_param =
         BuildParam { round: ROUND_1, valid_round: Some(ROUND_0), ..Default::default() };
-    context.repropose(TEST_PROPOSAL_COMMITMENT, build_param).await;
+    context.repropose(*TEST_PROPOSAL_COMMITMENT, build_param).await;
     // Re-propose sends the same proposal content but with updated init (round, proposer,
     // valid_round).
     let (_, mut receiver) = network.outbound_proposal_receiver.next().await.unwrap();
@@ -586,7 +596,7 @@ async fn propose_then_repropose(#[case] execute_all_txs: bool) {
     assert!(receiver.next().await.is_none());
 
     // Verify decision_reached uses the updated init (from reproposal round) for finalize.
-    context.decision_reached(HEIGHT_0, ROUND_1, TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
+    context.decision_reached(HEIGHT_0, ROUND_1, *TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
 }
 
 #[tokio::test]
@@ -685,7 +695,7 @@ async fn gas_price_limits(#[case] maximum: bool) {
     // Even though we used the minimum/maximum gas price, not the values we gave the provider,
     // the proposal should be still be valid due to the clamping of limit prices.
     let fin_receiver = context.validate_proposal(init, TIMEOUT, content_receiver).await;
-    assert_eq!(fin_receiver.await, Ok(TEST_PROPOSAL_COMMITMENT));
+    assert_eq!(fin_receiver.await, Ok(*TEST_PROPOSAL_COMMITMENT));
 }
 
 #[tokio::test]
@@ -731,7 +741,7 @@ async fn decision_reached_sends_correct_values() {
     let _fin = context.build_proposal(BuildParam::default(), TIMEOUT).await.unwrap().await;
     // At this point we should have a valid proposal in the context which contains the timestamp.
 
-    context.decision_reached(HEIGHT_0, ROUND_0, TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
+    context.decision_reached(HEIGHT_0, ROUND_0, *TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
 
     let metrics = recorder.handle().render();
     CONSENSUS_L2_GAS_PRICE
@@ -891,7 +901,7 @@ async fn oracle_fails_on_startup(#[case] l1_oracle_failure: bool) {
     assert_eq!(
         receiver.next().await.unwrap(),
         ProposalPart::Fin(ProposalFin {
-            proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+            proposal_commitment: *TEST_PROPOSAL_COMMITMENT,
             executed_transaction_count: INTERNAL_TX_BATCH.len().try_into().unwrap(),
             fin_payload: Some(ProposalFinPayload {
                 commitment_parts: CommitmentParts::default(),
@@ -900,7 +910,7 @@ async fn oracle_fails_on_startup(#[case] l1_oracle_failure: bool) {
         })
     );
     assert!(receiver.next().await.is_none());
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 }
 
 #[rstest]
@@ -973,7 +983,7 @@ async fn oracle_fails_on_second_block(#[case] l1_oracle_failure: bool) {
         .validate_proposal(proposal_init(HEIGHT_0, ROUND_0), TIMEOUT, content_receiver)
         .await;
     let proposal_commitment = fin_receiver.await.unwrap();
-    assert_eq!(proposal_commitment, TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(proposal_commitment, *TEST_PROPOSAL_COMMITMENT);
 
     // Decision reached
 
@@ -1006,7 +1016,7 @@ async fn oracle_fails_on_second_block(#[case] l1_oracle_failure: bool) {
     assert_eq!(
         receiver.next().await.unwrap(),
         ProposalPart::Fin(ProposalFin {
-            proposal_commitment: TEST_PROPOSAL_COMMITMENT,
+            proposal_commitment: *TEST_PROPOSAL_COMMITMENT,
             executed_transaction_count: INTERNAL_TX_BATCH.len().try_into().unwrap(),
             fin_payload: Some(ProposalFinPayload {
                 commitment_parts: CommitmentParts::default(),
@@ -1015,7 +1025,7 @@ async fn oracle_fails_on_second_block(#[case] l1_oracle_failure: bool) {
         })
     );
     assert!(receiver.next().await.is_none());
-    assert_eq!(fin_receiver.await.unwrap(), TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver.await.unwrap(), *TEST_PROPOSAL_COMMITMENT);
 }
 
 // L2 gas is a bit above the minimum gas price.
@@ -1142,7 +1152,7 @@ async fn override_prices_behavior(
         return;
     }
 
-    context.decision_reached(HEIGHT_0, ROUND_0, TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
+    context.decision_reached(HEIGHT_0, ROUND_0, *TEST_PROPOSAL_COMMITMENT, false).await.unwrap();
 
     let actual_l2_gas_price = context.l2_gas_price.0;
 
@@ -1268,7 +1278,7 @@ async fn change_gas_price_overrides() {
         .await;
 
     let proposal_commitment = fin_receiver.await.unwrap();
-    assert_eq!(proposal_commitment, TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(proposal_commitment, *TEST_PROPOSAL_COMMITMENT);
 
     context.decision_reached(HEIGHT_0, ROUND_0, proposal_commitment, false).await.unwrap();
 
@@ -1297,7 +1307,7 @@ async fn change_gas_price_overrides() {
     let content_receiver = send_proposal_to_validator_context(&mut context).await;
     let fin_receiver = context.validate_proposal(modified_init, TIMEOUT, content_receiver).await;
     let proposal_commitment = fin_receiver.await.unwrap();
-    assert_eq!(proposal_commitment, TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(proposal_commitment, *TEST_PROPOSAL_COMMITMENT);
 
     // Validate block number 1, round 1.
     let new_dynamic_config = ContextDynamicConfig {
@@ -1328,7 +1338,7 @@ async fn change_gas_price_overrides() {
     let content_receiver = send_proposal_to_validator_context(&mut context).await;
     let fin_receiver = context.validate_proposal(modified_init, TIMEOUT, content_receiver).await;
     let proposal_commitment = fin_receiver.await.unwrap();
-    assert_eq!(proposal_commitment, TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(proposal_commitment, *TEST_PROPOSAL_COMMITMENT);
 
     context.decision_reached(HEIGHT_1, ROUND_1, proposal_commitment, false).await.unwrap();
 
@@ -1347,7 +1357,7 @@ async fn change_gas_price_overrides() {
         .await
         .unwrap();
 
-    assert_eq!(fin_receiver, TEST_PROPOSAL_COMMITMENT);
+    assert_eq!(fin_receiver, *TEST_PROPOSAL_COMMITMENT);
     let (_, mut receiver) = network.outbound_proposal_receiver.next().await.unwrap();
 
     let part = receiver.next().await.unwrap();