blockifier: clean get_execution_info syscall test #11355

meship-starkware · 2026-01-01T12:39:32Z

No description provided.

reviewable-StarkWare · 2026-01-01T12:39:39Z

This change is

github-actions · 2026-01-01T12:39:40Z

Artifacts upload workflows:

meship-starkware · 2026-01-01T12:39:44Z

blockifier: clean get_execution_info syscall test #11355 👈 (View in Graphite)
blockifier: change the arguments order in the test_get_execution_info entry points #11315
blockifier: add tests to get_execution_info_v3 #11214 : 1 other dependent PR (#11282 )
blockifier: add get execution info v3 to native syscall handler #11177
main

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

meship-starkware

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

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 408 at r1 (raw file):

        let simulate_version_base = *QUERY_VERSION_BASE;
        version = TransactionVersion(simulate_version_base + version.0);
        expected_version += simulate_version_base;

I am not sure what we are doing here and why we are changing the version. What does TransactionVersion > 3 mean?

Code quote:

        let simulate_version_base = *QUERY_VERSION_BASE;
        version = TransactionVersion(simulate_version_base + version.0);
        expected_version += simulate_version_base;

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 437 at r1 (raw file):

    let (tx_info, max_fee_felt) = if version == TransactionVersion::ONE {
        common_fields.version = TransactionVersion::ONE;

I am not sure about this. Initially, I used the original version, but then I noticed that we had changed it.

Code quote:

 common_fields.version = TransactionVersion::ONE;

meship-starkware

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

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 408 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I am not sure what we are doing here and why we are changing the version. What does TransactionVersion > 3 mean?

Okay, I asked the cursor, and it means the only query flag is on, which is why most checks in this test add checks to the transaction version one specifically. This also means we don't check the proof facts with the only query flag. @noaov1 is this intentional, or should I fix it?

avivg-starkware · 2026-01-01T13:46:16Z

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 465 at r1 (raw file):

    // CallInfo: caller_address, contract_address, entry_point_selector.
    let expected_call_info = vec![
        felt!(0_u16), // Caller address.

Suggestion:

felt!(0_u16),

noaov1

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

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 408 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Okay, I asked the cursor, and it means the only query flag is on, which is why most checks in this test add checks to the transaction version one specifically. This also means we don't check the proof facts with the only query flag. @noaov1 is this intentional, or should I fix it?

Note that the version in the transaction does not contain the query flag (we add it when we use signed_tx_version), so we do check it in the code (not in the test though, as you mentioned)

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 421 at r1 (raw file):

    // Only V3 transactions support non-trivial proof facts.
    let proof_facts = if version == TransactionVersion::THREE {

Nice catch!

Code quote:

    let proof_facts = if version == TransactionVersion::THREE {

noaov1

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

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 437 at r1 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I am not sure about this. Initially, I used the original version, but then I noticed that we had changed it.

Why is this needed? Don't you set it already to this exact value? 🤔

a discussion (no related file):
Nice!! Thanks!

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 312 at r1 (raw file):

/// - `TestContract` (Cairo 1): uses `get_execution_info_v3_syscall()`, expects V3 `TxInfo`.
/// - `SierraExecutionInfoV1Contract` (Cairo 1): uses `get_execution_info_syscall()`, expects V1
///   `TxInfo`.

Why not V2?
Do we want to cover V2 as well?

Code quote:

/// - `SierraExecutionInfoV1Contract` (Cairo 1): uses `get_execution_info_syscall()`, expects V1
///   `TxInfo`.

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 376 at r1 (raw file):

    let test_contract_address = test_contract.get_instance_address(0);
    let tx_hash = tx_hash!(1991);
    let max_fee = Fee(42);

Please condition it on the transaction version

Code quote:

    let max_fee = Fee(42);

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 407 at r1 (raw file):

    if only_query {
        let simulate_version_base = *QUERY_VERSION_BASE;
        version = TransactionVersion(simulate_version_base + version.0);

Suggestion:

        query_version = TransactionVersion(simulate_version_base + version.0);

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 418 at r1 (raw file):

        l2_gas: resource_bounds,
        l1_data_gas: resource_bounds,
    });

Can move upward to where you build the transaction fields.

Code quote:

    let resource_bounds =
        ResourceBounds { max_amount: GasAmount(13), max_price_per_unit: GasPrice(61) };
    let all_resource_bounds = ValidResourceBounds::AllResources(AllResourceBounds {
        l1_gas: resource_bounds,
        l2_gas: resource_bounds,
        l1_data_gas: resource_bounds,
    });

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 429 at r1 (raw file):

    let mut common_fields = CommonAccountFields {
        transaction_hash: tx_hash,
        version: TransactionVersion::ONE,

No?

Suggestion:

        version: version,

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 436 at r1 (raw file):

    };

    let (tx_info, max_fee_felt) = if version == TransactionVersion::ONE {

Not needed. See my comment above RE the max_fee

Code quote:

    let (tx_info, max_fee_felt) = if version == TransactionVersion::ONE {

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 443 at r1 (raw file):

        )
    } else {
        common_fields.version = TransactionVersion::THREE;

See my comment above.

Code quote:

        common_fields.version = TransactionVersion::THREE;

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 468 at r1 (raw file):

        *test_contract_address.0.key(),
        entry_point_selector.0,
    ];

Personal preference (the documentation does not add more information)

Suggestion:

    let expected_call_info = vec![
        felt!(0_u16), // Caller address.
        *test_contract_address.0.key(),
        entry_point_selector.0,
    ];

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 485 at r1 (raw file):

    // TestContract uses get_execution_info_v3 which includes additional V3 fields.
    // Legacy contracts use get_execution_info_v1 and don't expect these fields.

What about SierraExecutionInfoV1Contract?

Code quote:

    // Legacy contracts use get_execution_info_v1 and don't expect these fields.

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 508 at r1 (raw file):

            Felt::ZERO, // Fee DA mode.
            Felt::ZERO, // Account deployment data.
        ];

Why is it non-empty for the legacy test contract? (as opposed to before)

Suggestion:

        let expected_unsupported_fields = vec![
            expected_tip.into(),
            Felt::ZERO, // Paymaster data.
            Felt::ZERO, // Nonce DA mode.
            Felt::ZERO, // Fee DA mode.
            Felt::ZERO, // Account deployment data.
        ];

meship-starkware

@meship-starkware made 8 comments and resolved 6 discussions.
Reviewable status: 0 of 1 files reviewed, 9 unresolved discussions (waiting on @avivg-starkware and @noaov1).

a discussion (no related file):