starknet_os_flow_tests: migrate test_os_logic and some small refactors

[dorimedini-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[dorimedini-starkware]

• starknet_os_flow_tests: migrate test_v1_bound_accounts_cairo1 #9496 : 2 dependent PRs (#9501 , #9514 )
• starknet_os_flow_tests: add V1-bound cairo1 account for flow tests #9495
• starknet_api: fix compiler->SNAPI contract class ABI string conversion for tests #9494
• starknet_os_flow_tests: migrate test_v1_bound_accounts_cairo0 #9493
• starknet_os_flow_tests: split get_class_info_of_cairo0_contract into function #9492
• starknet_os_flow_tests: more informative execution failure #9491
• starknet_os_flow_tests: add V1-bound cairo0 account for flow tests #9490
• starknet_os_flow_tests: localize expected storage updates in test_os_logic #9485
• starknet_os_flow_tests: migrate test_os_logic and some small refactors #9484 👈 (View in Graphite)
• blockifier_test_utils: add functionality to test contract for test_os_logic #9483
• blockifier_test_utils: add test_contract2 cairo0 contract #9482
• blockifier_test_utils: add delegate_proxy cairo0 contract #9481
• starknet_os_flow_tests: add control for expected reverting txs #9480
• starknet_os_flow_tests: migrate test_reverted_l1_handler_tx #9474
• starknet_os_flow_tests: reduce boilerplate with add_funded_account_invoke #9563
• starknet_os_flow_tests: move nonce manager into test manager #9562
• starknet_os_flow_tests: migrate test_reverted_invoke_tx #9473
• starknet_os_flow_tests: reduce boilerplate with perform_default_validations #9472
• main

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

[AvivYossef-starkware]

Q:

The Py test parametrized with private_keys and os_test_variant. What are the equivalents here?

possible TODOs:

1. Use a macro or a function to create calldata with an inner call
2. I don't think that the test adds an empty block in the multi-block ( let me know if I`m wrong )

@AvivYossef-starkware reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @meship-starkware and @Yoni-Starkware)

---

crates/starknet_os_flow_tests/src/initial_state.rs line 306 at r1 (raw file):

        nonce,
        resource_bounds,
        Felt::ONE,

Suggestion:

// Default salt.
Felt::ONE

---

crates/starknet_os_flow_tests/src/initial_state.rs line 321 at r1 (raw file):

    nonce: Nonce,
    resource_bounds: ValidResourceBounds,
    contract_address_salt: Felt,

Suggestion:

contract_address_salt: ContractAddressSalt

---

crates/starknet_os_flow_tests/src/tests.rs line 419 at r1 (raw file):

        .add_cairo0_declare_tx(tx, get_class_hash_of_feature_contract(cairo0_test_contract));

    // Deploy some instances of the test contract.

Suggestion:

// Deploy some instances of the deprecated test contract.

---

crates/starknet_os_flow_tests/src/tests.rs line 440 at r1 (raw file):

    let calldata = create_calldata(
        contract_addresses[0],
        "test_storage_read_write",

Note that in the py test we call set_value but Im ok with that.

Code quote:

test_storage_read_write

---

crates/starknet_os_flow_tests/src/tests.rs line 495 at r1 (raw file):

    if block_number_offset * txs_per_block < test_manager.total_txs() + 1 {
        block_number_offset += 1;
    }

I don't think that we add an empty block here, like in split_txs_into_blocks_in_multi_blocks

Code quote:

    // Calculate the block number of the next transaction.
    let txs_per_block = n_expected_txs / n_blocks_in_multi_block;
    // Calculate the block number of tx 'len(txs) + 1' without the added empty block.
    let mut block_number_offset = (test_manager.total_txs() + 1) / txs_per_block;
    if block_number_offset * txs_per_block < test_manager.total_txs() + 1 {
        block_number_offset += 1;
    }

---

crates/starknet_os_flow_tests/src/tests.rs line 516 at r1 (raw file):

        contract_addresses[0],
        "test_call_contract",
        &[**contract_addresses[0], selector_from_name("send_message").0, Felt::ONE, Felt::from(85)],

Suggestion:

    // Call contract -> send message to L1.
    let inner_selector_name = "test_call_contract";
    let inner_calldata = Felt::from(85);
    let inner_calldata_len = Felt::ONE;
    let calldata = create_calldata(
        contract_addresses[0],
        "test_call_contract",
        &[**contract_addresses[0], selector_from_name("send_message").0, inner_calldata_len, inner_calldata],

---

crates/starknet_os_flow_tests/src/tests.rs line 525 at r1 (raw file):

    };

    // Test get_contract_address.

Suggestion:

// Test get_caller_address syscall.

---

crates/starknet_os_flow_tests/src/tests.rs line 532 at r1 (raw file):

            *contract_addresses[1].0.key(),
            selector_from_name("test_get_caller_address").0,
            Felt::ONE,

Suggestion:

Felt::ONE, // Inner calldata len.

---

crates/starknet_os_flow_tests/src/tests.rs line 682 at r1 (raw file):

        ],
    );
    let signature = TransactionSignature(Arc::new(vec![Felt::from(100)]));

Why is it needed?

Code quote:

let signature = TransactionSignature(Arc::new(vec![Felt::from(100)]));

---

crates/starknet_os_flow_tests/src/tests.rs line 699 at r1 (raw file):

        ],
    );
    let signature = TransactionSignature(Arc::new(vec![Felt::from(100)]));

Why is it needed?

Code quote:

let signature = TransactionSignature(Arc::new(vec![Felt::from(100)]));

[dorimedini-starkware]

good catch! this parametrization was added after I migrated the test...
I'll add the private keys parametrization now.
the os_test_variant is unecessary; it's just for converage. I'll update the coverage PR (later in the stack)

as for your questions:

1. this, and other improvements (e.g. reduce boilerplate when adding declare/deploy txs), are a good idea, but the stack is already huge; we can do this later :)
2. correct, no empty block is added. do you think it's important?

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware, @meship-starkware, and @Yoni-Starkware)

---

crates/starknet_os_flow_tests/src/tests.rs line 495 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

I don't think that we add an empty block here, like in split_txs_into_blocks_in_multi_blocks

yes, this is why the expected_block_number has the -1

---

crates/starknet_os_flow_tests/src/tests.rs line 682 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

Why is it needed?

seems it really doesn't test anything interesting (was like this in the original test), removed

---

crates/starknet_os_flow_tests/src/tests.rs line 699 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

Why is it needed?

ditto

---

crates/starknet_os_flow_tests/src/tests.rs line 516 at r1 (raw file):

        contract_addresses[0],
        "test_call_contract",
        &[**contract_addresses[0], selector_from_name("send_message").0, Felt::ONE, Felt::from(85)],

how about like this? better?

[AvivYossef-starkware]

1. Sababa
2. @meship-starkware WDYT?

@AvivYossef-starkware reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @meship-starkware, and @Yoni-Starkware)

---

crates/starknet_os_flow_tests/src/tests.rs line 516 at r1 (raw file):

Previously, dorimedini-starkware wrote…

how about like this? better?

Yes

[AvivYossef-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)

[dorimedini-starkware]

good catch! this parametrization was added after I migrated the test...
I'll add the private keys parametrization now.
the os_test_variant is unecessary; it's just for converage. I'll update the coverage PR (later in the stack)

as for your questions:

1. this, and other improvements (e.g. reduce boilerplate when adding declare/deploy txs), are a good idea, but the stack is already huge; we can do this later :)
2. correct, no empty block is added. do you think it's important?

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)

[github-actions]

Artifacts upload workflows:

• Started at 2025-10-22T15:30:33Z

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 4 files at r1, 2 of 2 files at r2, 10 of 10 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware and @Yoni-Starkware)