Repository: MystenLabs/sui
File: crates/sui-core/src/transaction_driver/effects_certifier.rs (Rust)

Code before:

        // Add initial request to the pending set alongside fallbacks for uniform handling
        let initial_request = self.get_full_effects(
            initial_client,
            tx_digest,
            tx_type,
            consensus_position,
            options,
        );
        pending_requests.push(Box::pin(
            async move { (initial_target, initial_request.await) },
        ));

        let mut fallback_delay = tokio::time::interval(GET_FULL_EFFECTS_FALLBACK_DELAY);

        loop {
            tokio::select! {
                Some((validator, result)) = pending_requests.next() => {
                    if validator == initial_target {
                        // Initial request completed - always return its result (success or error)
                        return (result, validator);
                    }
                    // Fallback request completed
                    if result.is_ok() {
                        return (result, validator);
                    }
                    tracing::debug!(?validator, "Fallback get_full_effects failed, continuing to wait");
                }

                // After delay, try to start a fallback request to an acked validator
                _ = fallback_delay.tick() => {
                    // Drain all available acked validators and pick one we haven't tried
                    while let Ok(acked_validator) = acked_validators_rx.try_recv() {
                        // We send ack requests to all validators, so skip if the acked validator was the initial target
                        if acked_validator == initial_target {
                            continue;
                        }

                        let Some(client) = authority_aggregator.authority_clients.get(&acked_validator) else {
                            continue;
                        };

                        tracing::debug!(
                            ?acked_validator,
                            "Starting fallback get_full_effects request"
                        );

                        let client = client.clone();
                        let fut = self.get_full_effects(
                            client,
                            tx_digest,

Review comment (refactor):
When the txn is rejected, fallbacks fire until the rejection gets certified. A more optimal approach seems to be to not fire fallback request the effect digest is certified. Then ignoring errors make more sense.
Maybe merging the fallback logic into get_certified_finalized_effects() is more ideal. But that function is already a bit complex, so refactoring is needed. We can merge this for now and do a followup.

Code after:
        // Add initial request to the pending set alongside fallbacks for uniform handling
        let initial_request = self.get_full_effects(
            initial_client,
            tx_digest,
            tx_type,
            consensus_position,
            options,
        );
        pending_requests.push(Box::pin(
            async move { (initial_target, initial_request.await) },
        ));

        let mut fallback_delay = tokio::time::interval(GET_FULL_EFFECTS_FALLBACK_DELAY);
        fallback_delay.reset();

        loop {
            tokio::select! {
                Some((validator, result)) = pending_requests.next() => {
                    // Return as soon as any request (including fallback)completes - the caller handles retries for errors
                    return (result, validator);
                }

                // After delay, try to start a fallback request to an acked validator
                _ = fallback_delay.tick() => {
                    // Drain all available acked validators and pick one we haven't tried
                    while let Ok(acked_validator) = acked_validators_rx.try_recv() {
                        // We send ack requests to all validators, so skip if the acked validator was the initial target
                        if acked_validator == initial_target {
                            continue;
                        }

                        let Some(client) = authority_aggregator.authority_clients.get(&acked_validator) else {
                            continue;
                        };

                        tracing::debug!(
                            ?acked_validator,
                            "Starting fallback get_full_effects request"
                        );

                        let fut = self.get_full_effects(
                            client.clone(),
                            tx_digest,
                            tx_type,
                            consensus_position,
                            options,
                        );

                        pending_requests.push(Box::pin(async move {
                            (acked_validator, fut.await)
                        }));