chore(starknet_batcher): fix error message on transaction provider error on l1 validation

[ArniStarkware]

The new error message prints as:

L1Handler transaction validation failed for tx with hash 0x11faf080dfd62a9505a328e1f86c7e9e60e9752d9e18ddea5af46f8debf5205 status ConsumedOnL1OrUnknown

[reviewable-StarkWare]

This change is 

[ArniStarkware]

• chore(starknet_batcher): fix error message on transaction provider error on l1 validation #4187 : 2 dependent PRs (#4210 , #4256 ) 👈 (View in Graphite)
• chore(starknet_l1_provider_types): fix typo in validation status #4186
• main

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

[ArniStarkware]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)

---

crates/starknet_batcher/src/transaction_provider.rs line 25 at r2 (raw file):

        tx_hash.0.to_hex_string(),
        validation_status
    )]

Note the to_hex_string() is necessary; otherwise, it is printed as:

L1Handler transaction validation failed for tx with hash 508293130354954486128899687620408279632791738070753331087816184264451379717 status ConsumedOnL1OrUnknown.

i.e., the hash is presented as decimal instead of as hexadecimal.

Code quote:

    #[error(
        "L1Handler transaction validation failed for tx with hash {} status {:?}.",
        tx_hash.0.to_hex_string(),
        validation_status
    )]

[yair-starkware]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5)

[alonh5]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yair-starkware)

---

crates/starknet_batcher/src/transaction_provider_test.rs line 221 at r2 (raw file):

    assert_matches!(
        result,
        Err(TransactionProviderError::L1HandlerTransactionValidationFailed { .. })

Why? Let's check the values as well. When you're using .. it only matches the pattern

[ArniStarkware]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @yair-starkware)

---

crates/starknet_batcher/src/transaction_provider_test.rs line 221 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Why? Let's check the values as well. When you're using .. it only matches the pattern

Done.
This does revel an issue with the structure of the enum L1ValidationStatus. It has too many variants that act too diferently. What would happen if a new variant of status will be added? What if it is considered valid? or invalid?

Added a todo (For post 0.14.0).

[ArniStarkware]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @yair-starkware)

---

crates/starknet_l1_provider_types/src/lib.rs line 26 at r3 (raw file):

// TODO(Arni): Consider splitting this enum into valid and invalid status where the invalid status
// holds the flavor of the invalidity. Propagate this change to
// [TransactionProviderError::L1HandlerTransactionValidationFailed].

This TODO is handled in #4256.

Code quote:

// TODO(Arni): Consider splitting this enum into valid and invalid status where the invalid status
// holds the flavor of the invalidity. Propagate this change to
// [TransactionProviderError::L1HandlerTransactionValidationFailed].

[alonh5]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @yair-starkware)

---

crates/starknet_batcher/src/transaction_provider_test.rs line 221 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.
This does revel an issue with the structure of the enum L1ValidationStatus. It has too many variants that act too diferently. What would happen if a new variant of status will be added? What if it is considered valid? or invalid?

Added a todo (For post 0.14.0).

You don't need to test all the validation statuses, I just meant keep the tx hash and add the validation status to the assert matches.

[ArniStarkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)

---

crates/starknet_batcher/src/transaction_provider_test.rs line 221 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

You don't need to test all the validation statuses, I just meant keep the tx hash and add the validation status to the assert matches.

Discussed F2F.

[alonh5]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @alonh5 and @yair-starkware)