blockifier: test proof facts verifications

[meship-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2026-01-07T07:22:09Z
• Started at 2026-01-07T08:42:59Z
• Started at 2026-01-08T09:18:40Z
• Started at 2026-01-08T10:15:38Z
• Started at 2026-01-08T10:15:50Z
• Started at 2026-01-08T15:30:58Z
• Started at 2026-01-08T16:40:58Z
• Started at 2026-01-08T16:47:59Z
• Started at 2026-01-11T07:01:42Z
• Started at 2026-01-11T14:35:21Z
• Started at 2026-01-12T07:15:57Z
• Started at 2026-01-12T07:16:07Z
• Started at 2026-01-12T08:53:26Z
• Started at 2026-01-12T12:11:44Z
• Started at 2026-01-12T12:11:44Z
• Started at 2026-01-12T12:53:28Z
• Started at 2026-01-12T13:32:44Z
• Started at 2026-01-13T15:05:47Z
• Started at 2026-01-14T13:11:51Z
• Started at 2026-01-14T13:11:51Z
• Started at 2026-01-14T18:15:15Z
• Started at 2026-01-15T12:43:02Z
• Started at 2026-01-15T14:21:28Z
• Started at 2026-01-15T14:21:28Z
• Started at 2026-01-15T16:38:44Z
• Started at 2026-01-15T16:38:44Z
• Started at 2026-01-18T07:59:07Z
• Started at 2026-01-18T09:11:28Z
• Started at 2026-01-19T13:55:47Z
• Started at 2026-01-19T14:47:50Z

[meship-starkware]

• blockifier: test proof facts verifications #11473 👈 (View in Graphite)
• blockifier: add a client side proving test state #11476
• blockifier: verify that the proof facts block number matches the block hash #11345 : 2 other dependent PRs (#11485 , #11704 )
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[avivg-starkware]

@avivg-starkware reviewed 1 file and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @meship-starkware and @noaov1).

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2221 at r1 (raw file):

        config_hash: felt!(0x2_u64),
    }
}

WDYT about moving it to starknet_api , next to where snos_proof_facts_for_testing() is defined? Or making them dependent somehow through a from function?

I see it's problematic with STORED_BLOCK_HASH_BUFFER, but I think it's worth considering, perhaps passing it as an argument...

Code quote:

/// Returns valid SNOS proof facts.
/// Block number must be more than STORED_BLOCK_HASH_BUFFER blocks old (difference > buffer).
fn valid_snos_proof_facts() -> SnosProofFacts {
    SnosProofFacts {
        program_hash: felt!(0x1_u64),
        block_number: BlockNumber(CURRENT_BLOCK_NUMBER - STORED_BLOCK_HASH_BUFFER - 1),
        block_hash: BlockHash(felt!(0xABCDEF_u64)),
        config_hash: felt!(0x2_u64),
    }
}

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2267 at r1 (raw file):

    #[case] proof_facts: ProofFacts,
    #[case] current_block_number: u64,
) {

Consider changing the function's args so that ProofFacts are created in the test.

Suggestion:

/// Returns valid proof facts.
fn valid_proof_facts() -> ProofFacts {
    snos_to_proof_facts(valid_snos_proof_facts())
}

/// Returns invalid proof_facts with a too recent block number.
fn too_recent_block_proof_facts() -> ProofFacts {
    snos_to_proof_facts(SnosProofFacts {
        block_number: BlockNumber(CURRENT_BLOCK_NUMBER - STORED_BLOCK_HASH_BUFFER),
        ..valid_snos_proof_facts()
    })
}

/// Returns invalid proof_facts with a mismatched block hash.
fn mismatched_hash_proof_facts() -> ProofFacts {
    snos_to_proof_facts(SnosProofFacts {
        block_hash: BlockHash(felt!(0xDEADBEEF_u64)),
        ..valid_snos_proof_facts()
    })
}

/// Returns invalid proof_facts with a block number greater than the current block number.
fn future_block_proof_facts() -> ProofFacts {
    snos_to_proof_facts(SnosProofFacts {
        block_number: BlockNumber(CURRENT_BLOCK_NUMBER + 100),
        ..valid_snos_proof_facts()
    })
}

/// Tests the `validate_proof_facts` function for Invoke V3 transactions.
/// Covers: valid proof facts, too recent block number, mismatched block hash, future block,
/// and low current block number.
#[rstest]
#[case::valid_proof_facts(valid_proof_facts(), CURRENT_BLOCK_NUMBER)]
#[should_panic(expected = "is too recent")]
#[case::too_recent_block(too_recent_block_proof_facts(), CURRENT_BLOCK_NUMBER)]
#[should_panic(expected = "Block hash mismatch")]
#[case::mismatched_block_hash(mismatched_hash_proof_facts(), CURRENT_BLOCK_NUMBER)]
#[should_panic(expected = "is less than proof block number")]
#[case::future_block(future_block_proof_facts(), CURRENT_BLOCK_NUMBER)]
fn test_validate_proof_facts(
    default_all_resource_bounds: ValidResourceBounds,
    #[case] proof_block_number: u64,
    #[case] proof_block_hash: Felt,
    #[case] current_block_number: u64,
) {

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2272 at r1 (raw file):

    let chain_info = &block_context.chain_info;
    let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0);

Why not Cairo1?

Code quote:

CairoVersion::Cairo0

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2286 at r1 (raw file):

    // Valid block number: more than STORED_BLOCK_HASH_BUFFER blocks old.
    let valid_proof_block_number = CURRENT_BLOCK_NUMBER - STORED_BLOCK_HASH_BUFFER - 1;
    let stored_block_hash = felt!(0xABCDEF_u64);

Is it a random value?
If the only constraint for "valid block hash" in this case is a value equal to this, consider adding a cont that'll be used for the func input and here (with a small comment explaining)

Code quote:

0xABCDEF_u64

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2288 at r1 (raw file):

    let stored_block_hash = felt!(0xABCDEF_u64);

    // Store the block hash in the block hash contract.

Could you provide a bit more context as to why we need to add it here? Is it because in the default test setup, the values in this contract are empty?

Code quote:

// Store the block hash in the block hash contract.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2301 at r1 (raw file):

        resource_bounds: default_all_resource_bounds,
        version: TransactionVersion::THREE,
        proof_facts: proof_facts,

Suggestion:

proof_facts,

[meship-starkware]

@meship-starkware made 10 comments and resolved 1 discussion.
Reviewable status: 1 of 2 files reviewed, 11 unresolved discussions (waiting on @avivg-starkware and @noaov1).

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2221 at r1 (raw file):

Previously, avivg-starkware wrote…

WDYT about moving it to starknet_api , next to where snos_proof_facts_for_testing() is defined? Or making them dependent somehow through a from function?

I see it's problematic with STORED_BLOCK_HASH_BUFFER, but I think it's worth considering, perhaps passing it as an argument...

WDYT about the current sloution?

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2272 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

+1

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2286 at r1 (raw file):

Previously, avivg-starkware wrote…

Is it a random value?
If the only constraint for "valid block hash" in this case is a value equal to this, consider adding a cont that'll be used for the func input and here (with a small comment explaining)

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2288 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why not use the generate_block_hash_storage_updates?

We can definitely do it before we implement this solution.
Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2215 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Informative enough

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2224 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What is this value? Can you please use the valida value(?), i.e., snos_proof_facts.block_hash and manipulate it?

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2229 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

How is it different from too_recent_block_proof_facts? 🤔

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2274 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

It is not yet stored :)

Done.

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2288 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Is it needed?

seems like the reasorce bound is nedded

---

crates/starknet_api/src/transaction/fields.rs line 710 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Is it invalid though? It is simply not a snos proof fact, no?

I can change it to optional if you think this is better. I thought that if you want to create a Snos proof facts from proof facts if the proof is not snos it is invalid

[noaov1]

@noaov1 reviewed 1 file and all commit messages, and resolved 6 discussions.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avivg-starkware and @meship-starkware).

[meship-starkware]

@meship-starkware made 1 comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avivg-starkware).

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2267 at r1 (raw file):

Previously, avivg-starkware wrote…

Consider changing the function's args so that ProofFacts are created in the test.

I do like this solution better as it reduces the test cases as we will add program hash and chain id in the future, but it will increase the proof fact creations functions

[avivg-starkware]

@avivg-starkware reviewed 2 files and all commit messages, made 4 comments, and resolved 5 discussions.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @meship-starkware).

---

crates/starknet_api/src/transaction/fields.rs line 704 at r5 (raw file):

}

impl TryFrom<ProofFacts> for SnosProofFacts {

like like

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2216 at r5 (raw file):

    let mut snos_proof_facts =
        SnosProofFacts::try_from(ProofFacts::snos_proof_facts_for_testing()).unwrap();
    // Block number at the boundary: current - STORED_BLOCK_HASH_BUFFER, which is >= max_allowed.

Suggestion:

// Set the proof block number to the first invalid value:
// `current_block_number - STORED_BLOCK_HASH_BUFFER`
// (i.e. last allowed block + 1).

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2225 at r5 (raw file):

    let mut snos_proof_facts =
        SnosProofFacts::try_from(ProofFacts::snos_proof_facts_for_testing()).unwrap();
    //

oops?

Code quote:

//

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2287 at r5 (raw file):

    let tx_context = block_context.to_tx_context(&tx);
    tx.perform_pre_validation_stage(&mut state, &tx_context).unwrap();
}

It was a bit hard for me to follow the cases, which led me to this.
Main suggestion here:

• Change the funcs' names above to start with "proof_facts_with"
• Change the case names to start by clarifying which proof facts field it tests

The idea for using "error" instead of "panic" is mainly to make the case list seem more readable.

WDYT?

Suggestion:

/// Test for proof-facts validation during the pre-validation stage
/// of Invoke V3 transactions.
///
/// Covers:
/// - valid proof facts
/// - invalid block number (below buffer size, too recent)
/// - block hash mismatch
/// - zero block hash
/// - invalid program hash
///
/// The test runs `perform_pre_validation_stage` and asserts that invalid cases
/// fail with `InvalidProofFacts`, while valid cases pass.
#[rstest]
#[case::valid(
    ProofFacts::snos_proof_facts_for_testing(),
    CURRENT_BLOCK_NUMBER,
    false
)]
#[case::invalid_block_number_too_recent(
    proof_facts_with_block_number_at_boundary(),
    CURRENT_BLOCK_NUMBER,
    true
)]
#[case::invalid_block_number_below_buffer_size(
    proof_facts_with_block_number_at_boundary(),
    CURRENT_BLOCK_NUMBER,
    true
)]
#[case::invalid_block_hash_mismatch(
    proof_facts_with_mismatched_block_hash(),
    CURRENT_BLOCK_NUMBER,
    true
)]
#[case::invalid_block_hash_zero(
    proof_facts_with_zero_block_hash(),
    CURRENT_BLOCK_NUMBER,
    true
)]
#[case::invalid_program_hash(
    proof_facts_with_invalid_program_hash(),
    CURRENT_BLOCK_NUMBER,
    true
)]
fn test_validate_proof_facts(
    default_all_resource_bounds: ValidResourceBounds,
    #[case] proof_facts: ProofFacts,
    #[case] current_block_number: u64,
    #[case] should_fail: bool,
) {
    let mut block_context = BlockContext::create_for_account_testing();
    block_context.block_info.block_number = BlockNumber(current_block_number);

    let chain_info = &block_context.chain_info;
    let account =
        FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1(RunnableCairo1::Casm));
    let mut state = test_state(chain_info, BALANCE, &[(account, 1u16)]);
    let account_address = account.get_instance_address(0_u16);

    let tx = invoke_tx_with_default_flags(invoke_tx_args! {
        sender_address: account_address,
        resource_bounds: default_all_resource_bounds,
        proof_facts,
    });

    let tx_context = block_context.to_tx_context(&tx);
    let res = tx.perform_pre_validation_stage(&mut state, &tx_context);

    if should_fail {
        assert_matches!(
            res.unwrap_err(),
            TransactionExecutionError::TransactionPreValidationError(
                TransactionPreValidationError::InvalidProofFacts(_)
            )
        );
    } else {
        assert_matches!(res, Ok(()));
    }
}

[meship-starkware]

@meship-starkware made 1 comment and resolved 2 discussions.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware and @noaov1).

---

crates/blockifier/src/transaction/account_transactions_test.rs line 2287 at r5 (raw file):

Previously, avivg-starkware wrote…

It was a bit hard for me to follow the cases, which led me to this.
Main suggestion here:

• Change the funcs' names above to start with "proof_facts_with"
• Change the case names to start by clarifying which proof facts field it tests

The idea for using "error" instead of "panic" is mainly to make the case list seem more readable.

WDYT?

I don't like adding the cases to the comments, as it is easy to forget them when the test updates.
All other comments are done.

[avivg-starkware]

@avivg-starkware reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware).