feat: using ProposalPart's ProposalCommitment for validation

crates/pathfinder/src/consensus/inner/consensus_task.rs

@@ -18,8 +18,22 @@ use std::vec;
 use anyhow::Context;
 use p2p::consensus::HeightAndRound;
 use p2p_proto::common::{Address, Hash, L1DataAvailabilityMode};
-use p2p_proto::consensus::{BlockInfo, ProposalFin, ProposalInit, ProposalPart};
-use pathfinder_common::{BlockNumber, ChainId, ConsensusInfo, ContractAddress, ProposalCommitment};
+use p2p_proto::consensus::{
+    BlockInfo,
+    ProposalCommitment as ProposalCommitmentProto,
+    ProposalFin,
+    ProposalInit,
+    ProposalPart,
+};
+use pathfinder_common::{
+    BlockHash,
+    BlockNumber,
+    ChainId,
+    ConsensusInfo,
+    ContractAddress,
+    ProposalCommitment,
+    StarknetVersion,
+};
 use pathfinder_consensus::{
     Config,
     Consensus,
@@ -38,6 +52,11 @@ use tokio::sync::{mpsc, watch};
 use super::fetch_validators::L2ValidatorSetProvider;
 use super::{ConsensusTaskEvent, ConsensusValue, HeightExt, P2PTaskEvent};
 use crate::config::ConsensusConfig;
+use crate::state::block_hash::{
+    calculate_event_commitment,
+    calculate_receipt_commitment,
+    calculate_transaction_commitment,
+};
 use crate::validator::{FinalizedBlock, ValidatorBlockInfoStage};
 
 #[allow(clippy::too_many_arguments)]
@@ -359,14 +378,57 @@ fn create_empty_proposal(
         l1_data_gas_price_wei: 1,
         eth_to_strk_rate: 1_000_000_000,
     };
+    let current_block = BlockNumber::new(height).context("Invalid height")?;
+    let parent_proposal_commitment_hash = if let Some(parent_number) = current_block.parent() {
+        let mut db_conn = storage
+            .connection()
+            .context("Creating database connection")?;
+        let db_txn = db_conn
+            .transaction()
+            .context("Create database transaction")?;
+        let hash = db_txn.block_hash(parent_number.into())?.unwrap_or_default();
+        db_txn.commit()?;
+        hash
+    } else {
+        BlockHash::ZERO
+    };
     let db_conn = storage
         .connection()
         .context("Creating database connection")?;
     let validator = ValidatorBlockInfoStage::new(chain_id, proposal_init.clone())?
         .validate_consensus_block_info(block_info.clone(), db_conn)?;
     let validator = validator.consensus_finalize0()?;
     let finalized_block = validator.finalize(storage.clone())?;
-    let proposal_commitment = Hash(finalized_block.header.state_diff_commitment.0);
+    let proposal_commitment_hash = Hash(finalized_block.header.state_diff_commitment.0);
+
+    // the only version handled by consensus, so far
+    let starknet_version = StarknetVersion::new(0, 14, 0, 0);
+    let transactions = vec![];
+    let transaction_commitment = calculate_transaction_commitment(&transactions, starknet_version)?;
+    let transaction_events = vec![];
+    let event_commitment = calculate_event_commitment(&transaction_events, starknet_version)?;
+    let receipts = vec![];
+    let receipt_commitment = calculate_receipt_commitment(&receipts)?;
+    let proposal_commitment = ProposalCommitmentProto {
+      block_number: height,
+        parent_commitment: Hash(parent_proposal_commitment_hash.0),
+        builder: proposer,
+        timestamp,
+        protocol_version: starknet_version.to_string(),
+        old_state_root: Default::default(), // not used by 0.14.0
+        version_constant_commitment: Default::default(), // TODO
+        state_diff_commitment: proposal_commitment_hash,
+        transaction_commitment: Hash(transaction_commitment.0),
+        event_commitment: Hash(event_commitment.0),
+        receipt_commitment: Hash(receipt_commitment.0),
+        concatenated_counts: Default::default(), // should be the sum of lengths of inputs to *_commitment
+        l1_gas_price_fri: 1000,
+        l1_data_gas_price_fri: 2000,
+        l2_gas_price_fri: 3000,
+        l2_gas_used: 4000,
+        next_l2_gas_price_fri: 3000,
+        l1_da_mode: L1DataAvailabilityMode::Calldata,
+    };
 
     Ok((
         vec![
@@ -375,8 +437,9 @@ fn create_empty_proposal(
             // TODO empty proposal in the spec actually skips this part,
             // make sure our code handles the case where this part is missing
             ProposalPart::TransactionBatch(vec![]),
+            ProposalPart::ProposalCommitment(proposal_commitment),
             ProposalPart::Fin(ProposalFin {
-                proposal_commitment,
+                proposal_commitment: proposal_commitment_hash,
             }),
         ],
         finalized_block,

crates/pathfinder/src/consensus/inner/p2p_task.rs

@@ -509,6 +509,28 @@ async fn handle_incoming_proposal_part(
             );
             Ok(None)
         }
+        ProposalPart::ProposalCommitment(proposal_commitment) => {
+            let Some(validator_stage) = validator_cache.remove(&height_and_round) else {
+                anyhow::bail!(
+                    "No ValidatorTransactionBatchStage for height and round {}",
+                    height_and_round
+                );
+            };
+
+            let ValidatorStage::TransactionBatch(mut validator) = validator_stage else {
+                anyhow::bail!(
+                    "Wrong validator stage for height and round {}",
+                    height_and_round
+                );
+            };
+
+            validator.record_proposal_commitment(proposal_commitment)?;
+            validator_cache.insert(
+                height_and_round,
+                ValidatorStage::TransactionBatch(validator),
+            );
+            Ok(None)
+        }
         ProposalPart::Fin(ProposalFin {
             proposal_commitment,
         }) => {
@@ -526,6 +548,13 @@ async fn handle_incoming_proposal_part(
                 );
             };
 
+            if !validator.has_proposal_commitment() {
+                anyhow::bail!(
+                    "Transaction batch missing proposal commitment for height and round {}",
+                    height_and_round
+                );
+            }
+
             parts.push(proposal_part);
             let ProposalPart::Init(ProposalInit { proposer, .. }) =
                 parts.first().expect("Proposal Init")
@@ -554,10 +583,6 @@ async fn handle_incoming_proposal_part(
             // TODO
             Ok(None)
         }
-        ProposalPart::ProposalCommitment(_proposal_commitment) => {
-            // TODO
-            Ok(None)
-        }
     }
 }
 

crates/pathfinder/src/validator.rs

@@ -5,7 +5,12 @@ use anyhow::Context;
 use p2p::sync::client::conv::TryFromDto;
 use p2p_proto::class::Cairo1Class;
 use p2p_proto::common::Hash;
-use p2p_proto::consensus::{BlockInfo, ProposalInit, TransactionVariant as ConsensusVariant};
+use p2p_proto::consensus::{
+    BlockInfo,
+    ProposalCommitment,
+    ProposalInit,
+    TransactionVariant as ConsensusVariant,
+};
 use p2p_proto::sync::transaction::{DeclareV3WithoutClass, TransactionVariant as SyncVariant};
 use p2p_proto::transaction::DeclareV3WithClass;
 use pathfinder_common::class_definition::{SelectorAndFunctionIndex, SierraEntryPoints};
@@ -18,12 +23,17 @@ use pathfinder_common::{
     BlockHash,
     BlockHeader,
     BlockNumber,
+    BlockTimestamp,
     ChainId,
     EntryPoint,
+    EventCommitment,
     L1DataAvailabilityMode,
+    ReceiptCommitment,
     SequencerAddress,
     StarknetVersion,
     StateCommitment,
+    StateDiffCommitment,
+    TransactionCommitment,
     TransactionHash,
 };
 use pathfinder_executor::types::{to_starknet_api_transaction, BlockInfoPriceConverter};
@@ -145,6 +155,7 @@ impl ValidatorBlockInfoStage {
         Ok(ValidatorTransactionBatchStage {
             chain_id,
             block_info,
+            expected_block_header: None,
             block_executor,
             transactions: Vec::new(),
             receipts: Vec::new(),
@@ -223,6 +234,7 @@ impl ValidatorBlockInfoStage {
         Ok(ValidatorTransactionBatchStage {
             chain_id,
             block_info,
+            expected_block_header: None,
             block_executor,
             transactions: Vec::new(),
             receipts: Vec::new(),
@@ -234,6 +246,7 @@ impl ValidatorBlockInfoStage {
 pub struct ValidatorTransactionBatchStage {
     chain_id: ChainId,
     block_info: pathfinder_executor::types::BlockInfo,
+    expected_block_header: Option<BlockHeader>,
     block_executor: BlockExecutor,
     transactions: Vec<Transaction>,
     receipts: Vec<Receipt>,
@@ -325,6 +338,51 @@ impl ValidatorTransactionBatchStage {
         Ok(())
     }
 
+    pub fn has_proposal_commitment(&self) -> bool {
+        self.expected_block_header.is_some()
+    }
+
+    pub fn record_proposal_commitment(
+        &mut self,
+        proposal_commitment: ProposalCommitment,
+    ) -> anyhow::Result<()> {
+        let expected_block_header = BlockHeader {
+            hash: BlockHash::ZERO,        // UNUSED
+            parent_hash: BlockHash::ZERO, // UNUSED
+            number: BlockNumber::new(proposal_commitment.block_number)
+                .context("ProposalCommitment block number exceeds i64::MAX")?,
+            timestamp: BlockTimestamp::new(proposal_commitment.timestamp)
+                .context("ProposalCommitment timestamp exceeds i64::MAX")?,
+            // TODO prices should be validated against proposal_commitment values
+            eth_l1_gas_price: self.block_info.eth_l1_gas_price,
+            strk_l1_gas_price: self.block_info.strk_l1_gas_price,
+            eth_l1_data_gas_price: self.block_info.eth_l1_data_gas_price,
+            strk_l1_data_gas_price: self.block_info.strk_l1_data_gas_price,
+            eth_l2_gas_price: self.block_info.eth_l2_gas_price,
+            strk_l2_gas_price: self.block_info.strk_l2_gas_price,
+            sequencer_address: SequencerAddress(proposal_commitment.builder.0),
+            starknet_version: StarknetVersion::from_str(&proposal_commitment.protocol_version)?,
+            event_commitment: EventCommitment(proposal_commitment.event_commitment.0),
+            state_commitment: StateCommitment::ZERO, // UNUSED
+            transaction_commitment: TransactionCommitment(
+                proposal_commitment.transaction_commitment.0,
+            ),
+            transaction_count: 0, // TODO validate concatenated_counts
+            event_count: 0,       // TODO validate concatenated_counts
+            l1_da_mode: match proposal_commitment.l1_da_mode {
+                p2p_proto::common::L1DataAvailabilityMode::Blob => L1DataAvailabilityMode::Blob,
+                p2p_proto::common::L1DataAvailabilityMode::Calldata => {
+                    L1DataAvailabilityMode::Calldata
+                }
+            },
+            receipt_commitment: ReceiptCommitment(proposal_commitment.receipt_commitment.0),
+            state_diff_commitment: StateDiffCommitment(proposal_commitment.state_diff_commitment.0),
+            state_diff_length: 0, // TODO validate concatenated_counts
+        };
+        self.expected_block_header = Some(expected_block_header);
+        Ok(())
+    }
+
     /// Finalizes the block, producing a header with all commitments except
     /// the state commitment and block hash, which are computed in the last
     /// stage. Also verifies that the computed proposal commitment matches the
@@ -353,6 +411,7 @@ impl ValidatorTransactionBatchStage {
     pub(crate) fn consensus_finalize0(self) -> anyhow::Result<ValidatorFinalizeStage> {
         let Self {
             block_info,
+            expected_block_header,
             block_executor,
             transactions,
             receipts,
@@ -403,12 +462,12 @@ impl ValidatorTransactionBatchStage {
             // Computed in ValidatorFinalizeStage::finalize()
             state_commitment: StateCommitment::ZERO,
             transaction_commitment,
-            transaction_count: transactions.len(),
-            event_count: events.iter().flatten().count(),
+            transaction_count: 0, // TODO validate concatenated_counts
+            event_count: 0,       // TODO validate concatenated_counts
             l1_da_mode: self.block_info.l1_da_mode,
             receipt_commitment,
             state_diff_commitment,
-            state_diff_length: state_update.state_diff_length(),
+            state_diff_length: 0, // TODO validate concatenated_counts
         };
 
         debug!(
@@ -417,6 +476,12 @@ impl ValidatorTransactionBatchStage {
             start.elapsed().as_millis()
         );
 
+        if let Some(expected_header) = expected_block_header {
+            if header != expected_header {
+                anyhow::bail!("expected {:?}, actual {:?}", expected_header, header);
+            }
+        }
+
         Ok(ValidatorFinalizeStage {
             header,
             state_update,