tx v3 two tests

[Gerson2102]

Pull Request type

Draft

Please add the labels corresponding to the type of changes your PR introduces:
Testing

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build-related changes
• Documentation content changes
• Testing
• Other (please describe):

Resolves: #1574

What is the new behavior?

• Two new tests to tx v3

[Gerson2102]

In this commit i added a line in lib.rs, related to the transactions version, because it seems for me that we had to do that, and then i tried to run the given_hardcoded_contract_run_invoke_tx_v3_fails_sender_not_deployed and still not working. The left side is failing, it is returning ok(()). And it has to return error. And the other tests that i added is failing too.

[0xSpyC]

Since cairo v0.10, transaction are splitted to two function validate and execute, this splitting allows for a protocol-level distinction between invalid and reverted transactions, so i think you should not run_non_revertible_transaction for v3 in starknet/src/lib.rs, and in your test you call get_invoke_dummy which produce a v1 invoke transaction maybe you should take a look and write an get_invoke_v3_dummy i hope this will help you out

[Gerson2102]

Right now Im working in that, in creating a new get_invoke_v3_dummy but there is a lot of parameters that i dont know how to get them...

[0xSpyC]

did you check https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-8.md ? if you still need help on get_invoke_v3_dummy go check an implementation of it at this PR : #1597 in mod.rs

[Gerson2102]

I dont know why in this code:

#[test]
fn given_hardcoded_contract_run_invoke_tx_v3_on_argent_account_then_it_works() {
    new_test_ext::<MockRuntime>().execute_with(|| {
        basic_test_setup(2);
        let none_origin = RuntimeOrigin::none();

        let chain_id = Starknet::chain_id();
        let mut transaction = get_invoke_v3_argent_dummy(chain_id);
        if let starknet_api::transaction::InvokeTransaction::V3(tx) = &mut transaction.tx {
            tx.signature = sign_message_hash(transaction.tx_hash);
        };

        assert_ok!(Starknet::invoke(none_origin, transaction));
    });
}

I'm getting this error:

failures:

---- tests::invoke_tx::given_hardcoded_contract_run_invoke_tx_v3_on_argent_account_then_it_works stdout ----
thread 'tests::invoke_tx::given_hardcoded_contract_run_invoke_tx_v3_on_argent_account_then_it_works' panicked at crates/pallets/starknet/src/tests/invoke_tx.rs:654:9:
Expected Ok(_). Got Err(
    Module(
        ModuleError {
            index: 1,
            error: [
                1,
                0,
                0,
                0,
            ],
            message: Some(
                "TransactionExecutionFailed",
            ),
        },
    ),
)

The invoke function inside of the assert_ok! is throwing that error.

The test is almost the same as this one:

#[test]
fn given_hardcoded_contract_run_invoke_on_argent_account_then_it_works() {
    new_test_ext::<MockRuntime>().execute_with(|| {
        basic_test_setup(2);
        let none_origin = RuntimeOrigin::none();

        let chain_id = Starknet::chain_id();
        let mut transaction = get_invoke_argent_dummy(chain_id);
        if let starknet_api::transaction::InvokeTransaction::V1(tx) = &mut transaction.tx {
            tx.signature = sign_message_hash(transaction.tx_hash);
        };

        assert_ok!(Starknet::invoke(none_origin, transaction));
    });
}

[Gerson2102]

Some of those tests that i just made. Are not working, there is a comment in the tests to know that. And if you see, the tests that are supposed to fail, are the ones that are working. And those tests that are supposed to passed, are failing because they return an error. So maybe im doing something wrong here :/

[tdelabro]

It is not clear what I should review.
Can you cleanup now that other PRs you integrated have been merged?

Or start a new PR

[tdelabro]

This comes from another PR.

Can you rebase please. Right now I can't tell what are your changes and what comes form the branch you merged into yours

[tdelabro]

Same this comes from another PR.