apollo_gateway: remove processing tx block #10592

itamar-starkware · 2025-12-07T10:10:21Z

What changed?

Removed the ProcessTxBlockingTask struct and simplified the transaction validation flow in Gateway::add_tx
Changed StatefulTransactionValidator to get the nonce directly from the state reader instead of the blockifier
Added a get_nonce method to the GatewayFixedBlockStateReader trait and its implementations
Renamed extract_state_nonce_and_run_validations to run_transaction_validations and simplified its implementation
Updated tests to match the new validation flow

Why make this change?

This refactoring simplifies the transaction validation flow by:

Eliminating unnecessary code complexity around the blocking task
Improving error handling and propagation
Making the nonce retrieval more direct by getting it from the state reader
Reducing the number of state transitions and method calls in the validation process

The changes maintain the same validation logic while making the code more maintainable and easier to understand.

reviewable-StarkWare · 2025-12-07T10:10:29Z

This change is

itamar-starkware · 2025-12-07T10:10:43Z

apollo_gateway: make StatefulTransactionValidatorTrait async #10621
apollo_gateway: remove processing tx block #10592 👈 (View in Graphite)
main-v0.14.1

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

ArniStarkware · 2025-12-07T12:19:25Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

impl<B: BlockifierStatefulValidatorTrait> StatefulTransactionValidator<B> {
    fn validate_state_preconditions(

I know this is not the main topic, but I am distracted by this.

Suggestion:

        let account_nonce = runtime
            .block_on(self.gateway_fixed_block_state_reader.get_nonce(address))
            .map_err(|e| {
                // TODO(noamsp): Fix this. Need to map the errors better.
                StarknetError::internal_with_signature_logging(
                    format!("Failed to get nonce for sender address {address}"),
                    &executable_tx.signature(),
                    e,
                )
            })?;
        self.run_transaction_validations(executable_tx, account_nonce, mempool_client, runtime)?;
        Ok(account_nonce)
    }
}

impl<B: BlockifierStatefulValidatorTrait> StatefulTransactionValidator<B> {
    fn run_transaction_validations(
        &mut self,
        executable_tx: &ExecutableTransaction,
        account_nonce: Nonce,
        mempool_client: SharedMempoolClient,
        runtime: tokio::runtime::Handle,
    ) -> StatefulTransactionValidatorResult<()> {
        self.validate_state_preconditions(executable_tx, account_nonce)?;
        runtime.block_on(validate_by_mempool(
            executable_tx,
            account_nonce,
            mempool_client.clone(),
        ))?;
        self.run_validate_entry_point(executable_tx, account_nonce, mempool_client, runtime)?;
        Ok(())
    }

    fn validate_state_preconditions(

ArniStarkware

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

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I know this is not the main topic, but I am distracted by this.

This comment is still relevant.

itamar-starkware · 2025-12-09T07:54:25Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This comment is still relevant.

I'm missing the issue here. Please explain.

ArniStarkware

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

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

Previously, itamar-starkware wrote…

I'm missing the issue here. Please explain.

Don't change unnecessary things in this PR. Which leads to the second point.
I like that the function called extract_state_nonce_and_run_validations does:

extract the state nonce (one method).
runs validations (second method). * This is the issue I refer to.

ArniStarkware · 2025-12-09T08:21:41Z

crates/apollo_gateway/src/gateway.rs line 176 at r2 (raw file):

            ),
            message: format!("Blocking task join error: {e}"),
        })??;

Join error? Blocking task?
Neither are relevant anymore (or is the spawn_blocking considered a join?)

Code quote:

            message: format!("Blocking task join error: {e}"),
        })??;

ArniStarkware · 2025-12-09T08:23:52Z

crates/apollo_gateway/src/gateway.rs line 170 at r2 (raw file):

                    &tx_signature, starknet_err
                );
                metric_counters.record_add_tx_failure(&starknet_err);

What happened to the metric? IIUC, it should be handled as one of the two ? operations of the spawn_blocking.

Code quote:

metric_counters.record_add_tx_failure(&starknet_err);

ArniStarkware

Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @itamar-starkware and @TzahiTaub)

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 492 at r2 (raw file):

    .map_err(|err| err.code);
    assert_eq!(result, expected_result_code);
}

This was a unit test for the deleted method run_transaction_validations.

In this thread. I requested you bring it back.
This would make the changes to this test unnecessary.

Code quote:

async fn run_transaction_validation_test(
    executable_tx: AccountTransaction,
    account_nonce: Nonce,
    max_allowed_nonce_gap: u32,
    expected_result_code: Result<(), StarknetErrorCode>,
) {
    let mut mock_blockifier_validator = MockBlockifierStatefulValidatorTrait::new();
    mock_blockifier_validator.expect_validate().return_once(|_| Ok(()));
    mock_blockifier_validator.expect_block_info().return_const(BlockInfo::default());

    let mut stateful_validator = StatefulTransactionValidator {
        config: StatefulTransactionValidatorConfig { max_allowed_nonce_gap, ..Default::default() },
        blockifier_stateful_tx_validator: mock_blockifier_validator,
        gateway_fixed_block_state_reader: {
            let mut mock = MockGatewayFixedBlockStateReader::new();
            mock.expect_get_nonce().return_once(move |_| Ok(account_nonce));
            Box::new(mock)
        },
    };

    let result = tokio::task::spawn_blocking(move || {
        let mut mempool_client = MockMempoolClient::new();
        mempool_client.expect_validate_tx().returning(|_| Ok(()));
        stateful_validator.extract_state_nonce_and_run_validations(
            &executable_tx,
            Arc::new(mempool_client),
            tokio::runtime::Handle::current(),
        )
    })
    .await
    .unwrap()
    .map(|_| ())
    .map_err(|err| err.code);
    assert_eq!(result, expected_result_code);
}

ArniStarkware

Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @itamar-starkware and @TzahiTaub)

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 492 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This was a unit test for the deleted method run_transaction_validations.

In this thread. I requested you bring it back.
This would make the changes to this test unnecessary.

Ditto other tests.

ArniStarkware · 2025-12-09T08:52:14Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 141 at r2 (raw file):

        let address = executable_tx.contract_address();
        let account_nonce =
            self.blockifier_stateful_tx_validator.get_nonce(address).map_err(|e| {

Go to crates/blockifier/src/blockifier/stateful_validator.rs.
Delete the method get_nonce. If this isn't simple, add a todo to delete it.

#[cfg_attr(any(test, feature = "mocks"), mockall::automock)]
pub trait StatefulValidatorTrait {
    #[allow(clippy::result_large_err)]
    fn validate(&mut self, account_tx: AccountTransaction) -> StatefulValidatorResult<()>;
    fn block_info(&self) -> &BlockInfo;
    #[allow(clippy::result_large_err)]
    fn get_nonce(&mut self, account_address: ContractAddress) -> StatefulValidatorResult<Nonce>;
}

Code quote:

self.blockifier_stateful_tx_validator.get_nonce(address)

ArniStarkware

@ArniStarkware reviewed 1 of 7 files at r1.
Reviewable status: 1 of 7 files reviewed, 5 unresolved discussions (waiting on @itamar-starkware and @TzahiTaub)

ArniStarkware

@ArniStarkware reviewed 1 of 7 files at r1.
Reviewable status: 2 of 7 files reviewed, 5 unresolved discussions (waiting on @itamar-starkware and @TzahiTaub)

ArniStarkware

@ArniStarkware reviewed 1 of 4 files at r2.
Reviewable status: 3 of 7 files reviewed, 5 unresolved discussions (waiting on @itamar-starkware and @TzahiTaub)

ArniStarkware · 2025-12-09T09:34:36Z

crates/apollo_gateway/src/gateway_fixed_block_state_reader.rs line 83 at r2 (raw file):

            Err(StateSyncClientError::StateSyncError(StateSyncError::ContractNotFound(_))) => {
                Ok(Nonce::default())
            }

What is the precedent?

Code quote:

            Err(StateSyncClientError::StateSyncError(StateSyncError::ContractNotFound(_))) => {
                Ok(Nonce::default())
            }

itamar-starkware · 2025-12-09T10:17:15Z

crates/apollo_gateway/src/gateway.rs line 170 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

What happened to the metric? IIUC, it should be handled as one of the two ? operations of the spawn_blocking.

Fixed.

itamar-starkware · 2025-12-09T10:17:19Z

crates/apollo_gateway/src/gateway.rs line 176 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Join error? Blocking task?
Neither are relevant anymore (or is the spawn_blocking considered a join?)

spawn_blocking return a JoinHandle, but I don't think we should refer to it in our message.
I changed to something that describes the issue better IMO.

itamar-starkware · 2025-12-09T10:17:27Z

crates/apollo_gateway/src/gateway_fixed_block_state_reader.rs line 83 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

What is the precedent?

On the get_nonce_at for the blockifier:

sequencer/crates/apollo_gateway/src/sync_state_reader.rs

Lines 153 to 165 in b38ede6

    
           fn get_nonce_at(&self, contract_address: ContractAddress) -> StateResult<Nonce> { 
        
               let res = self 
        
                   .runtime 
        
                   .block_on(self.state_sync_client.get_nonce_at(self.block_number, contract_address)); 
        
               match res { 
        
                   Ok(value) => Ok(value), 
        
                   Err(StateSyncClientError::StateSyncError(StateSyncError::ContractNotFound(_))) => { 
        
                       Ok(Nonce::default()) 
        
                   } 
        
                   Err(e) => Err(StateError::StateReadError(e.to_string())), 
        
               } 
        
           }

github-actions · 2025-12-09T10:17:30Z

Artifacts upload workflows:

itamar-starkware · 2025-12-09T10:17:36Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 141 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Go to crates/blockifier/src/blockifier/stateful_validator.rs.
Delete the method get_nonce. If this isn't simple, add a todo to delete it.

#[cfg_attr(any(test, feature = "mocks"), mockall::automock)]
pub trait StatefulValidatorTrait {
    #[allow(clippy::result_large_err)]
    fn validate(&mut self, account_tx: AccountTransaction) -> StatefulValidatorResult<()>;
    fn block_info(&self) -> &BlockInfo;
    #[allow(clippy::result_large_err)]
    fn get_nonce(&mut self, account_address: ContractAddress) -> StatefulValidatorResult<Nonce>;
}

Added a todo, this PR is already too long.

itamar-starkware · 2025-12-09T10:17:41Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Don't change unnecessary things in this PR. Which leads to the second point.

I like that the function called extract_state_nonce_and_run_validations does:

extract the state nonce (one method).

runs validations (second method). * This is the issue I refer to.

In the next PR we revert back to something close to that.

pre-validation call, which extracts the nonce.
validation

ArniStarkware · 2025-12-09T10:33:49Z

crates/apollo_gateway/src/stateful_transaction_validator.rs line 165 at r1 (raw file):

Previously, itamar-starkware wrote…

In the next PR we revert back to something close to that.

pre-validation call, which extracts the nonce.

validation

I would argue we can have three steps:

extract_nonce
run prevalidation checks
run validate entry point

ArniStarkware · 2025-12-09T10:55:49Z

crates/apollo_gateway/src/gateway.rs line 170 at r2 (raw file):

Previously, itamar-starkware wrote…

Fixed.

Sorry for saying "one of the two ?", because any failure should be registered as a record_add_tx_failure.

itamar-starkware · 2025-12-09T13:50:07Z

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

    .await
    .unwrap();
    assert_eq!(result.map(|_| ()), expected_result);

Done

itamar-starkware · 2025-12-09T13:50:26Z

crates/apollo_gateway/src/gateway.rs line 170 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Yes, you should.

Done.

itamar-starkware · 2025-12-09T14:04:19Z

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

            Box::new(mock)
        },
    };

Done. I should have seen it by myself.

ArniStarkware

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

ArniStarkware

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

crates/apollo_gateway/src/stateful_transaction_validator_test.rs line 117 at r6 (raw file):

    };
    let expected_run_result =
        expected_result.as_ref().map(|_| account_nonce).map_err(|blockifier_error| StarknetError {

why do we need this?

Code quote:

.map(|_| account_nonce)

ArniStarkware

@ArniStarkware reviewed 1 of 2 files at r7, all commit messages.
Reviewable status: 7 of 8 files reviewed, all discussions resolved (waiting on @TzahiTaub)

ArniStarkware

@ArniStarkware reviewed 1 of 1 files at r9.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)

ArniStarkware · 2025-12-10T08:07:51Z

Please wait for @TzahiTaub's review.

TzahiTaub

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

crates/apollo_gateway/src/gateway.rs line 168 at r8 (raw file):

                    mempool_client,
                    tokio::runtime::Handle::current(),
                )

Suggestion:

        let nonce = tokio::task::spawn_blocking(move || {
            curr_span.in_scope(|| {
                stateful_transaction_validator.extract_state_nonce_and_run_validations(
                    &executable_tx,
                    self.mempool_client.clone(),
                    tokio::runtime::Handle::current(),
                )

crates/apollo_gateway/src/gateway.rs line 173 at r8 (raw file):

        .await
        .map_err(|e| {
            let err = StarknetError {

IIUC, this helps to understand why there are two consecutive error handlers.

Suggestion:

        .map_err(|e| {
            // Handle panics in the spawned thread (see tokio::task::JoinHandle).
            let err = StarknetError {

TzahiTaub

@TzahiTaub reviewed 1 of 2 files at r7, 1 of 1 files at r9.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @itamar-starkware)

itamar-starkware · 2025-12-10T15:28:15Z

crates/apollo_gateway/src/gateway.rs line 173 at r8 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

IIUC, this helps to understand why there are two consecutive error handlers.

Done

itamar-starkware · 2025-12-10T15:28:19Z

crates/apollo_gateway/src/gateway.rs line 168 at r8 (raw file):

                    mempool_client,
                    tokio::runtime::Handle::current(),
                )

Can't be done.
We have lifetime issues here.
spawn_blocking create a closure which take ownership on all object get inside him, so working with self.mempool_client will require taking ownership on self.
The issue here, IIUC, is lifetime requiremnets. The spawn_blocking can live longer than the self object.

The compiler error here is:

borrowed data escapes outside of method
`self` escapes the method body here

This is the cursor summary of the case:

The blocking validation runs in a thread via spawn_blocking, 
whose closure must be 'static (no non-'static borrows).

Inlining self.mempool_client.clone() would capture &self, 
a non-'static borrow; instead, clone the Arc outside and
 move the owned clone into the closure.

The local mempool_client variable isn’t 'static itself, 
but moving it into the closure makes the closure 'static 
because it owns its captures.

TzahiTaub

Reviewable status: 7 of 8 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @itamar-starkware)

crates/apollo_gateway/src/gateway.rs line 168 at r8 (raw file):

Previously, itamar-starkware wrote…

Can't be done.
We have lifetime issues here.
spawn_blocking create a closure which take ownership on all object get inside him, so working with self.mempool_client will require taking ownership on self.
The issue here, IIUC, is lifetime requiremnets. The spawn_blocking can live longer than the self object.

The compiler error here is:
borrowed data escapes outside of method
`self` escapes the method body here
This is the cursor summary of the case:
The blocking validation runs in a thread via spawn_blocking, 
whose closure must be 'static (no non-'static borrows).

Inlining self.mempool_client.clone() would capture &self, 
a non-'static borrow; instead, clone the Arc outside and
 move the owned clone into the closure.

The local mempool_client variable isn’t 'static itself, 
but moving it into the closure makes the closure 'static 
because it owns its captures.

Right, then just remove the clone from &executable_tx

TzahiTaub

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

itamar-starkware · 2025-12-10T15:58:33Z

crates/apollo_gateway/src/gateway.rs line 168 at r8 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Right, then just remove the clone from &executable_tx

Done

TzahiTaub

@TzahiTaub reviewed 1 of 1 files at r11, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware)

ArniStarkware

@ArniStarkware reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware)

ArniStarkware

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

itamar-starkware self-assigned this Dec 7, 2025

itamar-starkware marked this pull request as ready for review December 7, 2025 10:11

itamar-starkware requested review from ArniStarkware and TzahiTaub December 7, 2025 11:59

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from 8790404 to bfe5849 Compare December 7, 2025 12:08

ArniStarkware requested changes Dec 7, 2025

View reviewed changes

itamar-starkware mentioned this pull request Dec 8, 2025

apollo_gateway: make StatefulTransactionValidatorTrait async #10621

Merged

ArniStarkware requested changes Dec 9, 2025

View reviewed changes

ArniStarkware reviewed Dec 9, 2025

View reviewed changes

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from bfe5849 to 1445e7c Compare December 9, 2025 10:17

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from cf08661 to fb3507f Compare December 9, 2025 14:03

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from fb3507f to 6987806 Compare December 9, 2025 14:11

ArniStarkware reviewed Dec 9, 2025

View reviewed changes

ArniStarkware requested changes Dec 9, 2025

View reviewed changes

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch 3 times, most recently from 087eff1 to e77c3e5 Compare December 9, 2025 19:25

ArniStarkware reviewed Dec 10, 2025

View reviewed changes

ArniStarkware approved these changes Dec 10, 2025

View reviewed changes

TzahiTaub requested changes Dec 10, 2025

View reviewed changes

TzahiTaub reviewed Dec 10, 2025

View reviewed changes

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from e77c3e5 to 52d9004 Compare December 10, 2025 15:28

TzahiTaub requested changes Dec 10, 2025

View reviewed changes

TzahiTaub reviewed Dec 10, 2025

View reviewed changes

apollo_gateway: remove processing tx block

500b3b7

itamar-starkware force-pushed the 12-07-apollo_gateway_remove_processing_tx_block branch from 52d9004 to 500b3b7 Compare December 10, 2025 15:58

TzahiTaub approved these changes Dec 10, 2025

View reviewed changes

ArniStarkware reviewed Dec 11, 2025

View reviewed changes

ArniStarkware approved these changes Dec 11, 2025

View reviewed changes

itamar-starkware added this pull request to the merge queue Dec 11, 2025

Merged via the queue into main-v0.14.1 with commit 15427c2 Dec 11, 2025
36 of 39 checks passed

github-actions bot locked and limited conversation to collaborators Dec 12, 2025

apollo_gateway: remove processing tx block #10592

apollo_gateway: remove processing tx block #10592

Uh oh!

Conversation

itamar-starkware commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed?

Why make this change?

Uh oh!

reviewable-StarkWare commented Dec 7, 2025

Uh oh!

itamar-starkware commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArniStarkware commented Dec 7, 2025

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

github-actions bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

ArniStarkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

itamar-starkware commented Dec 9, 2025

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

Choose a reason for hiding this comment

Uh oh!

ArniStarkware left a comment

itamar-starkware commented Dec 7, 2025 •

edited

Loading

itamar-starkware commented Dec 7, 2025 •

edited

Loading

github-actions bot commented Dec 9, 2025 •

edited

Loading