apollo_starknet_os_program: add security checks for virtual OS

[Yoni-Starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

@AvivYossef-starkware reviewed 7 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @liorgold2 and @noaov1).

---

crates/starknet_os_flow_tests/src/virtual_os_test.rs line 92 at r1 (raw file):

    test_builder.build().await.run_virtual_expect_error("Expected INVOKE_FUNCTION transaction");
}

Consider parameterizing this test with all types of transactions.

Code quote:

#[rstest]
#[tokio::test]
/// Test that the virtual OS fails when a non-invoke transaction is added.
async fn test_non_invoke_tx_os_error() {
    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1(RunnableCairo1::Casm));

    let (mut test_builder, [contract_address]) =
        TestBuilder::create_standard_virtual([(test_contract, calldata![Felt::ONE, Felt::TWO])])
            .await;

    // Add an L1 handler transaction instead of an invoke.
    let tx = ExecutableL1HandlerTransaction::create(
        L1HandlerTransaction {
            version: L1HandlerTransaction::VERSION,
            nonce: Nonce::default(),
            contract_address,
            entry_point_selector: selector_from_name("l1_handle"),
            // from_address, arg.
            calldata: calldata![Felt::ONE, Felt::TWO],
        },
        &test_builder.chain_id(),
        Fee(1_000_000),
    )
    .unwrap();
    test_builder.add_l1_handler_tx(tx, None);

    test_builder.build().await.run_virtual_expect_error("Expected INVOKE_FUNCTION transaction");
}

[Yoni-Starkware]

@Yoni-Starkware made 1 comment.
Reviewable status: 5 of 7 files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @liorgold2, and @noaov1).

---

crates/starknet_os_flow_tests/src/virtual_os_test.rs line 92 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

Consider parameterizing this test with all types of transactions.

Added a TODO

[Yoni-Starkware]

@Yoni-Starkware reviewed 1 file and all commit messages.
Reviewable status: 2 of 7 files reviewed, all discussions resolved (waiting on @AvivYossef-starkware, @liorgold2, and @noaov1).

[AvivYossef-starkware]

@AvivYossef-starkware partially reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @liorgold2 and @noaov1).

---

crates/apollo_starknet_os_program/src/virtual_os_test.rs line 26 at r4 (raw file):

    "#]]
    .assert_debug_eq(&VIRTUAL_OS_PROGRAM.data_len());
}

Have you tried it? Sometimes it doesn't work well, so you need to define constants. Additionally, consider allowing some margin; otherwise, it will need to be fixed every time someone changes the operating system.

Code quote:

#[test]
fn test_program_bytecode_lengths() {
    expect![[r#"
        15535
    "#]]
    .assert_debug_eq(&OS_PROGRAM.data_len());
    expect![[r#"
        13184
    "#]]
    .assert_debug_eq(&VIRTUAL_OS_PROGRAM.data_len());
}