apollo_gateway: move instantiate_validator to be async

[itamar-starkware]

TL;DR

Make instantiate_validator async to avoid blocking the runtime.

What changed?

• Changed instantiate_validator in StatefulTransactionValidatorFactoryTrait to be an async function
• Updated the implementation to use await instead of block_on for asynchronous operations
• Modified process_tx in ProcessTxBlockingTask to use runtime.block_on to call the now-async instantiate_validator
• Updated tests to accommodate the async interface

How to test?

Run the existing test suite to ensure that the functionality works as expected with the new async interface.

Why make this change?

This change improves the code by avoiding blocking operations within async contexts. By making instantiate_validator async, we eliminate the need to call block_on within the method, which can lead to runtime issues when called from an async context. This creates a more consistent async/await pattern throughout the codebase and prevents potential deadlocks that can occur when blocking the runtime.

[itamar-starkware]

• apollo_gateway: test account tx in mempool influence on validation #9950
• apollo_gateway: extract async mempool query from blocking task #9941
• apollo_gateway: use get_nonce outside extract_state_nonce_and_run_validations #10009
• apollo_gateway: add get_nonce function to StatefulValidator trait #10008
• apollo_gateway: insert stateful_tx_validator creating to ProcessTxBlockingTask constructor #10007
• apollo_gateway: split instantiate_validator #10123
• apollo_gateway: move instantiate_validator to be async #10122 👈 (View in Graphite)
• apollo_gateway: move get_latest_block_info to be async #10121
• apollo_gateway: move get_block_info to be async #10120
• apollo_gateway: move get_state_reader_from_latest_block to be async #10119
• main-v0.14.1

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

[reviewable-StarkWare]

This change is 

[ArniStarkware]

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 159 at r1 (raw file):

#[rstest]
#[tokio::test(flavor = "multi_thread")]

Do we still need this?

Code quote:

#[tokio::test(flavor = "multi_thread")]

[itamar-starkware]

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 159 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Do we still need this?

No, fixed

[ArniStarkware]

@ArniStarkware reviewed 1 of 4 files at r1.
Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @TzahiTaub)

[TzahiTaub]

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

---

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 178 at r4 (raw file):

    let validator =
        stateful_validator_factory.instantiate_validator(Arc::new(mock_state_reader_factory)).await;

This can be removed, became irrelevant a long time ago apperantly

Suggestion:

    let validator =
        stateful_validator_factory.instantiate_validator(Arc::new(state_reader_factory)).await;

[itamar-starkware]

Reviewable status: 2 of 4 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware, @itamar-starkware, and @TzahiTaub)

---

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 178 at r4 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

This can be removed, became irrelevant a long time ago apperantly

Done.

[TzahiTaub]

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

---

crates/apollo_gateway/src/gateway_test.rs line 573 at r4 (raw file):

            ))
        })
    });

Revert (see comment in stateful_transaction_validator)

Code quote:

    mock_stateful_transaction_validator_factory.expect_instantiate_validator().return_once(|_| {
        Box::pin(async {
            Ok::<Box<dyn StatefulTransactionValidatorTrait>, _>(Box::new(
                mock_stateful_transaction_validator,
            ))
        })
    });

---

crates/apollo_gateway/src/gateway_test.rs line 616 at r4 (raw file):

    mock_stateful_transaction_validator_factory
        .expect_instantiate_validator()
        .return_once(|_| Box::pin(async { Err::<_, _>(expected_error) }));

Revert (see comment in stateful_transaction_validator)

Code quote:

      .return_once(|_| Box::pin(async { Err::<_, _>(expected_error) }));

---

crates/apollo_gateway/src/rpc_state_reader.rs line 116 at r5 (raw file):

        let block_header: BlockHeader = serde_json::from_value(get_block_with_tx_hashes_result)
            .map_err(serde_err_to_state_err)?;

Why was this changed? The former seems better.

Code quote:

        let block_header: BlockHeader = serde_json::from_value(get_block_with_tx_hashes_result)
            .map_err(serde_err_to_state_err)?;

---

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r4 (raw file):

#[async_trait]
#[cfg_attr(test, mockall::automock)]

See the first bullet here:

• The #[automock] attribute must appear before the crate’s attribute.

This caused the Box::Pin requirement.

Please search for similar wrongly ordered async+automock annotations in the repo and replace them as well (in a separate PR).

Suggestion:

#[cfg_attr(test, mockall::automock)]
#[async_trait]

[itamar-starkware]

crates/apollo_gateway/src/rpc_state_reader.rs line 116 at r5 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Why was this changed? The former seems better.

Fixed.

[itamar-starkware]

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r4 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

See the first bullet here:

• The #[automock] attribute must appear before the crate’s attribute.

This caused the Box::Pin requirement.

Please search for similar wrongly ordered async+automock annotations in the repo and replace them as well (in a separate PR).

Done.
This is the only place in sequencer repo with this combination.

[itamar-starkware]

crates/apollo_gateway/src/gateway_test.rs line 616 at r4 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Revert (see comment in stateful_transaction_validator)

Done.

[itamar-starkware]

crates/apollo_gateway/src/gateway_test.rs line 573 at r4 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Revert (see comment in stateful_transaction_validator)

Done.

[TzahiTaub]

@TzahiTaub reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

---

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r4 (raw file):

Previously, itamar-starkware wrote…

Done.
This is the only place in sequencer repo with this combination.

We have two more (pub trait PreconfirmedBlockWriterTrait and pub trait SignatureManagerClient)

[itamar-starkware]

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r4 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

We have two more (pub trait PreconfirmedBlockWriterTrait and pub trait SignatureManagerClient)

Sorry for that. I should have verified it better.
PR is open here:
#10438

[ArniStarkware]

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r6 (raw file):

#[cfg_attr(test, mockall::automock)]
#[async_trait]

The original change is unnecessary.

Suggestion:

#[async_trait]
#[cfg_attr(test, mockall::automock)]

[ArniStarkware]

@ArniStarkware reviewed 1 of 3 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

[ArniStarkware]

@ArniStarkware reviewed 1 of 1 files at r4, 1 of 2 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware)

---

crates/apollo_gateway/src/stateful_transaction_validator.rs line 71 at r6 (raw file):

    async fn instantiate_validator(
        &self,
        state_reader_factory: Arc<dyn StateReaderFactory>,

Why the arc? Is it explained in the upstream?

Code quote:

state_reader_factory: Arc<dyn StateReaderFactory>,

[itamar-starkware]

crates/apollo_gateway/src/stateful_transaction_validator.rs line 71 at r6 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Why the arc? Is it explained in the upstream?

We shared pointer across tasks.
The gateway and processing_tx_block both hold the state_reader.

The exact error I get:

E0195: mockall::automock can’t generate mocks for an async trait method that takes a borrowed parameter. The non-'static reference in an async fn’s signature introduces lifetimes that mockall’s automock attribute doesn’t support.

The reason is: since automock is done before async_trait (first annotation done first)
then automock tries to mock async function directly with borrowed argument (state_reader_factory).

[itamar-starkware]

crates/apollo_gateway/src/stateful_transaction_validator.rs line 53 at r6 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

The original change is unnecessary.

This documentation says otherwise:
https://docs.rs/mockall/latest/mockall/#async-traits
Am I missing something?