blockifier: verify that the proof facts block number matches the block hash

Author: meship-starkware

crates/apollo_gateway/src/gateway_test.rs

@@ -38,7 +38,7 @@ use apollo_network_types::network_types::BroadcastedMessageMetadata;
 use apollo_test_utils::{get_rng, GetTestInstance};
 use blockifier::blockifier::config::ContractClassManagerConfig;
 use blockifier::context::ChainInfo;
-use blockifier::test_utils::initial_test_state::fund_account;
+use blockifier::test_utils::initial_test_state::{fund_account, setup_block_hash_from_proof_facts};
 use blockifier_test_utils::cairo_versions::{CairoVersion, RunnableCairo1};
 use blockifier_test_utils::calldata::create_trivial_calldata;
 use blockifier_test_utils::contracts::FeatureContract;
@@ -52,6 +52,7 @@ use starknet_api::executable_transaction::AccountTransaction;
 use starknet_api::rpc_transaction::{
     InternalRpcTransaction,
     RpcDeclareTransaction,
+    RpcInvokeTransaction,
     RpcTransaction,
     RpcTransactionLabelValue,
 };
@@ -276,14 +277,21 @@ async fn setup_mock_state(
 
     setup_transaction_converter_mock(&mut mock_dependencies.mock_transaction_converter, tx_args);
 
+    // Setup state: fund account and store proof block hash if needed.
+    let state_reader =
+        &mut mock_dependencies.state_reader_factory.state_reader.blockifier_state_reader;
     let address = expected_internal_tx.contract_address();
     fund_account(
         &mock_dependencies.config.chain_info,
         address,
         VALID_ACCOUNT_BALANCE,
-        &mut mock_dependencies.state_reader_factory.state_reader.blockifier_state_reader,
+        state_reader,
     );
 
+    if let RpcTransaction::Invoke(RpcInvokeTransaction::V3(invoke_tx)) = &input_tx {
+        setup_block_hash_from_proof_facts(&invoke_tx.proof_facts, state_reader);
+    }
+
     let mempool_add_tx_args = AddTransactionArgs {
         tx: expected_internal_tx.clone(),
         account_state: AccountState { address, nonce: *input_tx.nonce() },

crates/blockifier/src/test_utils.rs

@@ -416,3 +416,11 @@ pub fn maybe_dummy_block_hash_and_number(block_number: BlockNumber) -> Option<Bl
         hash: BlockHash(StarkHash::ONE),
     })
 }
+
+/// Returns the contract address for the block hash contract used in tests.
+pub fn block_hash_contract_address() -> ContractAddress {
+    VersionedConstants::create_for_testing()
+        .os_constants
+        .os_contract_addresses
+        .block_hash_contract_address()
+}

crates/blockifier/src/test_utils/initial_test_state.rs

@@ -5,7 +5,8 @@ use starknet_api::block::FeeType;
 use starknet_api::contract_class::compiled_class_hash::HashVersion;
 use starknet_api::core::ContractAddress;
 use starknet_api::felt;
-use starknet_api::transaction::fields::Fee;
+use starknet_api::state::StorageKey;
+use starknet_api::transaction::fields::{Fee, ProofFacts, ProofFactsVariant};
 use strum::IntoEnumIterator;
 
 use crate::blockifier::config::ContractClassManagerConfig;
@@ -16,6 +17,7 @@ use crate::state::state_reader_and_contract_manager::{
     FetchCompiledClasses,
     StateReaderAndContractManager,
 };
+use crate::test_utils::block_hash_contract_address;
 use crate::test_utils::contracts::FeatureContractData;
 use crate::test_utils::dict_state_reader::DictStateReader;
 
@@ -36,6 +38,20 @@ pub fn fund_account(
     }
 }
 
+/// Sets up block hash in the block hash contract from SNOS proof facts.
+///
+/// This is a test helper used to prepare state for proof-facts validation.
+pub fn setup_block_hash_from_proof_facts(
+    proof_facts: &ProofFacts,
+    state_reader: &mut DictStateReader,
+) {
+    if let Ok(ProofFactsVariant::Snos(snos_proof_facts)) = proof_facts.try_into() {
+        let contract = block_hash_contract_address();
+        let storage_key = StorageKey::from(snos_proof_facts.block_number.0);
+        state_reader.storage_view.insert((contract, storage_key), snos_proof_facts.block_hash.0);
+    }
+}
+
 /// Sets up a state reader for testing:
 /// * "Declares" a Cairo0 ERC20 contract (class hash => class mapping set).
 /// * "Deploys" two ERC20 contracts (address => class hash mapping set) at the fee token addresses
@@ -83,6 +99,7 @@ pub fn setup_test_state(
     }
 }
 
+// TODO(Meshi): create a client-side test state.
 pub fn test_state(
     chain_info: &ChainInfo,
     initial_balances: Fee,

crates/blockifier/src/transaction/account_transaction.rs

@@ -1,13 +1,14 @@
 use std::sync::Arc;
 
 use starknet_api::abi::abi_utils::selector_from_name;
-use starknet_api::block::GasPriceVector;
+use starknet_api::block::{BlockNumber, GasPriceVector};
 use starknet_api::calldata;
 use starknet_api::contract_class::EntryPointType;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce};
 use starknet_api::data_availability::DataAvailabilityMode;
 use starknet_api::executable_transaction::{AccountTransaction as Transaction, TransactionType};
 use starknet_api::execution_resources::GasAmount;
+use starknet_api::state::StorageKey;
 use starknet_api::transaction::fields::Resource::{L1DataGas, L1Gas, L2Gas};
 use starknet_api::transaction::fields::{
     AccountDeploymentData,
@@ -24,6 +25,8 @@ use starknet_api::transaction::{constants, TransactionHash, TransactionVersion};
 use starknet_types_core::felt::Felt;
 
 use super::errors::ResourceBoundsError;
+use crate::abi::constants::STORED_BLOCK_HASH_BUFFER;
+use crate::blockifier_versioned_constants::OsConstants;
 use crate::context::{BlockContext, GasCounter, TransactionContext};
 use crate::execution::call_info::CallInfo;
 use crate::execution::common_hints::ExecutionMode;
@@ -246,19 +249,57 @@ impl AccountTransaction {
         }
     }
 
-    fn validate_proof_facts(&self) -> TransactionPreValidationResult<()> {
-        if let Transaction::Invoke(tx) = &self.tx {
-            if tx.tx.version() == TransactionVersion::THREE {
-                let proof_facts_variant = ProofFactsVariant::try_from(&tx.tx.proof_facts())
-                    .map_err(|e| TransactionPreValidationError::InvalidProofFacts(e.to_string()))?;
-                match proof_facts_variant {
-                    ProofFactsVariant::Empty => {}
-                    ProofFactsVariant::Snos(_snos_proof_facts) => {
-                        // TODO(Meshi/ AvivG): add proof facts validations.
-                    }
-                }
-            }
+    fn validate_proof_facts(
+        &self,
+        os_constants: &OsConstants,
+        current_block_number: BlockNumber,
+        state: &mut dyn State,
+    ) -> TransactionPreValidationResult<()> {
+        // Only Invoke V3 transactions can carry proof facts.
+        let Transaction::Invoke(invoke_tx) = &self.tx else {
+            return Ok(());
+        };
+        if invoke_tx.version() != TransactionVersion::THREE {
+            return Ok(());
+        }
+
+        // Parse proof facts.
+        let proof_facts = invoke_tx.proof_facts();
+        let snos_proof_facts = match ProofFactsVariant::try_from(&proof_facts)
+            .map_err(|e| TransactionPreValidationError::InvalidProofFacts(e.to_string()))?
+        {
+            ProofFactsVariant::Empty => return Ok(()),
+            ProofFactsVariant::Snos(snos_proof_facts) => snos_proof_facts,
+        };
+
+        // Proof block must be old enough to have a stored block hash.
+        // Stored block hashes are guaranteed only up to: current - STORED_BLOCK_HASH_BUFFER.
+        let max_allowed = current_block_number.0.saturating_sub(STORED_BLOCK_HASH_BUFFER);
+
+        let proof_block_number = snos_proof_facts.block_number.0;
+        if proof_block_number > max_allowed {
+            return Err(TransactionPreValidationError::InvalidProofFacts(format!(
+                "Invalid proof block number {proof_block_number}. Block hashes are only available \
+                 for blocks ≤ current_block_number - STORED_BLOCK_HASH_BUFFER \
+                 ({current_block_number} - {STORED_BLOCK_HASH_BUFFER} = {max_allowed})."
+            )));
+        }
+
+        // Compare the proof's block hash with the stored block hash.
+        let contract = os_constants.os_contract_addresses.block_hash_contract_address();
+
+        let stored_block_hash =
+            state.get_storage_at(contract, StorageKey::from(proof_block_number))?;
+        assert_ne!(stored_block_hash, Felt::ZERO, "Stored block hash is zero");
+
+        let proof_block_hash = snos_proof_facts.block_hash.0;
+        if stored_block_hash != proof_block_hash {
+            return Err(TransactionPreValidationError::InvalidProofFacts(format!(
+                "Block hash mismatch for block {proof_block_number}. Proof block hash: \
+                 {proof_block_hash}, stored block hash: {stored_block_hash}."
+            )));
         }
+
         Ok(())
     }
 
@@ -278,7 +319,11 @@ impl AccountTransaction {
             verify_can_pay_committed_bounds(state, tx_context).map_err(Box::new)?;
         }
 
-        self.validate_proof_facts()?;
+        self.validate_proof_facts(
+            &tx_context.block_context.versioned_constants.os_constants,
+            tx_context.block_context.block_info.block_number,
+            state,
+        )?;
 
         Ok(())
     }

crates/starknet_os_flow_tests/src/test_manager.rs

@@ -8,7 +8,7 @@ use blockifier::state::cached_state::{CachedState, StateMaps};
 use blockifier::state::state_api::{StateReader, UpdatableState};
 use blockifier::state::stateful_compression_test_utils::decompress;
 use blockifier::test_utils::dict_state_reader::DictStateReader;
-use blockifier::test_utils::ALIAS_CONTRACT_ADDRESS;
+use blockifier::test_utils::{block_hash_contract_address, ALIAS_CONTRACT_ADDRESS};
 use blockifier::transaction::objects::TransactionExecutionInfo;
 use blockifier::transaction::transaction_execution::Transaction as BlockifierTransaction;
 use blockifier_test_utils::calldata::create_calldata;
@@ -47,7 +47,7 @@ use starknet_api::invoke_tx_args;
 use starknet_api::state::{SierraContractClass, StorageKey};
 use starknet_api::test_utils::invoke::{invoke_tx, InvokeTxArgs};
 use starknet_api::test_utils::{NonceManager, CHAIN_ID_FOR_TESTS};
-use starknet_api::transaction::fields::{Calldata, Fee, Tip};
+use starknet_api::transaction::fields::{Calldata, Fee, ProofFactsVariant, Tip};
 use starknet_api::transaction::{L1HandlerTransaction, L1ToL2Payload, MessageToL1};
 use starknet_committer::block_committer::input::{
     IsSubset,
@@ -118,6 +118,36 @@ pub(crate) static STRK_FEE_TOKEN_ADDRESS: LazyLock<ContractAddress> = LazyLock::
 pub(crate) static FUNDED_ACCOUNT_ADDRESS: LazyLock<ContractAddress> =
     LazyLock::new(|| get_initial_deploy_account_tx().contract_address);
 
+/// The block hash contract address, cached to avoid repeated VersionedConstants creation.
+static BLOCK_HASH_CONTRACT_ADDRESS: LazyLock<ContractAddress> =
+    LazyLock::new(block_hash_contract_address);
+
+/// Stores block hash for invoke transactions with SNOS proof facts.
+/// This is needed so the OS can verify block hash proofs during execution.
+// TODO(Meshi): change it so the block number to block hash mapping will be a part of the state and
+// not written during the transaction execution.
+fn store_block_hash_from_transaction<S: UpdatableState>(tx: &BlockifierTransaction, state: &mut S) {
+    if let BlockifierTransaction::Account(account_tx) = tx {
+        if let AccountTransaction::Invoke(invoke_tx) = &account_tx.tx {
+            if let Ok(ProofFactsVariant::Snos(snos_proof_facts)) =
+                ProofFactsVariant::try_from(&invoke_tx.proof_facts())
+            {
+                let storage_writes = StateMaps {
+                    storage: HashMap::from([(
+                        (
+                            *BLOCK_HASH_CONTRACT_ADDRESS,
+                            StorageKey::from(snos_proof_facts.block_number.0),
+                        ),
+                        snos_proof_facts.block_hash.0,
+                    )]),
+                    ..Default::default()
+                };
+                state.apply_writes(&storage_writes, &HashMap::new());
+            }
+        }
+    }
+}
+
 #[derive(Default)]
 pub(crate) struct TestBuilderConfig {
     pub(crate) use_kzg_da: bool,
@@ -257,6 +287,25 @@ impl<S: FlowTestState> OsTestOutput<S> {
         );
     }
 
+    /// Adds the proof facts block hash storage mapping to the decompressed state diff.
+    /// This is needed because the Blockifier writes block hash storage for proof facts
+    /// verification, but the OS doesn't include this in its output.
+    // TODO(Meshi): Remove this once we have a solution that dose not write to the state during the
+    // transaction execution.
+    pub(crate) fn add_proof_facts_block_hash_storage(
+        &mut self,
+        block_number: BlockNumber,
+        block_hash: BlockHash,
+    ) {
+        let storage_key = StarknetStorageKey(StorageKey::from(block_number.0));
+        let storage_value = StarknetStorageValue(block_hash.0);
+        self.decompressed_state_diff
+            .storage_updates
+            .entry(*BLOCK_HASH_CONTRACT_ADDRESS)
+            .or_default()
+            .insert(storage_key, storage_value);
+    }
+
     fn perform_global_validations(&self) {
         // TODO(Dori): Implement global validations for the OS test output.
 
@@ -749,7 +798,10 @@ impl<S: FlowTestState> TestBuilder<S> {
 
             let (block_txs, revert_reasons): (Vec<_>, Vec<_>) = block_txs_with_reason
                 .into_iter()
-                .map(|flow_test_tx| (flow_test_tx.tx, flow_test_tx.expected_revert_reason))
+                .map(|flow_test_tx| {
+                    store_block_hash_from_transaction(&flow_test_tx.tx, &mut state);
+                    (flow_test_tx.tx, flow_test_tx.expected_revert_reason)
+                })
                 .unzip();
             // Clone the block info for later use.
             let block_info = block_context.block_info().clone();

crates/starknet_os_flow_tests/src/tests.rs

@@ -55,6 +55,7 @@ use starknet_api::transaction::fields::{
     ContractAddressSalt,
     Fee,
     ProofFacts,
+    ProofFactsVariant,
     ResourceBounds,
     Tip,
     TransactionSignature,
@@ -1086,7 +1087,7 @@ async fn test_new_class_execution_info(#[values(true, false)] use_kzg_da: bool)
             main_contract_address, test_execution_info_selector_name, &expected_execution_info
         ),
         resource_bounds: *NON_TRIVIAL_RESOURCE_BOUNDS,
-        proof_facts,
+        proof_facts: proof_facts.clone(),
     };
     // Put the tx hash in the signature.
     let tx = InvokeTransaction::create(invoke_tx(invoke_tx_args.clone()), chain_id).unwrap();
@@ -1165,7 +1166,29 @@ async fn test_new_class_execution_info(#[values(true, false)] use_kzg_da: bool)
     );
 
     // Run the test.
-    let test_output = test_builder.build_and_run().await;
+    let mut test_output = test_builder.build_and_run().await;
+
+    // Reconcile proof facts block-hash storage between Blockifier and OS outputs.
+    //
+    // Context: To allow testing transactions with proof_facts validations, the Blockifier writes a
+    // storage entry: (block_hash_contract, proof_block_number) -> proof_block_hash.
+    //
+    // However, the OS doesn't emit this storage write in its output (the proof facts verification
+    // was not yet added to the OS, and thus having non non-empty (block_number -> block_hash)
+    // mapping is not yet required for the tests to pass).
+    //
+    // This causes a mismatch: Blockifier's state diff includes the write, but OS output doesn't.
+    // We manually add it here so the test comparison passes.
+    //
+    // TODO(Meshi): Move block hash storage setup to not affect the writes, then remove this fixup.
+
+    if let Ok(ProofFactsVariant::Snos(snos_proof_facts)) = ProofFactsVariant::try_from(&proof_facts)
+    {
+        test_output.add_proof_facts_block_hash_storage(
+            snos_proof_facts.block_number,
+            snos_proof_facts.block_hash,
+        );
+    }
 
     // Perform general validations and storage update validations.
     test_output.perform_default_validations();