Refactor error handling and nonce checks in EoaExecutorWorker

[d4mr]

• Simplified error handling for balance threshold updates by consolidating conditions.
• Added logic to check nonce movement during gas bump attempts, allowing for early exit if the transaction is confirmed.
• Enhanced logging to provide clearer context on transaction states and errors encountered during processing.

Summary by CodeRabbit

• Bug Fixes

  • Added validation to detect nonce advancement and exit gas-bump flow when a transaction has already progressed, preventing redundant retries.
  • Consolidated balance-threshold update behavior and clearer failure paths when a gas-bump attempt fails without nonce movement.

• Monitoring

  • Improved logging, metrics, and health timestamps around nonce movement and confirmation steps for better observability.

[coderabbitai]

Walkthrough

Adds nonce-freshness checks and consolidated balance-threshold error handling in the gas-bump/confirmation path; exits the gas-bump flow when on-chain nonce advances, updates health timestamps on nonce movement, and preserves original error when bump fails without nonce advancement.

Changes

Cohort / File(s)

Summary

Gas bump / confirmation logic executors/src/eoa/worker/confirm.rs

Consolidates duplicated balance-threshold update/error handling into a single conditional. After a failed gas-bump build, fetches on-chain transaction counts to detect nonce movement; if the preconfirmed nonce advanced, updates health timestamps, logs/metrics, and returns Ok(true) to exit gas-bump flow. If nonce did not move, logs current nonces and returns the original error. Adds additional informational logs and metric emissions around nonce movement and health updates.

Sequence Diagram(s)

sequenceDiagram
    participant Flow as Confirmation Flow
    participant GasBump as Gas Bump Builder
    participant Balance as Balance Threshold Updater
    participant Nonce as Nonce Freshness Checker
    participant Chain as On-Chain State
    participant Health as Health Timestamps

    Flow->>GasBump: Attempt to build gas bump
    alt Build succeeds
        GasBump-->>Flow: Proceed with bump
    else Build fails (TransactionSimulationFailed / RpcError)
        GasBump->>Balance: Attempt update_balance_threshold() and log on failure
        Balance-->>GasBump: Result (ok/err)
        GasBump->>Nonce: Fetch on-chain tx counts (preconfirmed/latest)
        Nonce->>Chain: Query transaction counts
        Chain-->>Nonce: Return counts
        alt Preconfirmed nonce advanced
            note right of Nonce: Nonce moved ahead → treat as confirmed
            Nonce->>Health: Update last_nonce_movement_at & last_confirmation_at
            Health-->>Nonce: Ack (or log failure)
            Nonce-->>Flow: Return Ok(true) (exit gas-bump flow)
        else Preconfirmed nonce unchanged
            Nonce-->>Flow: Log current preconfirmed/latest nonces
            Flow-->>GasBump: Return original error (no nonce advancement)
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Review the consolidated error branch to ensure both original error variants are correctly handled and no case is suppressed.
• Verify nonce freshness queries use the correct on-chain counters and comparisons (preconfirmed vs expected_nonce).
• Check health timestamp updates (last_nonce_movement_at, last_confirmation_at) and error logging paths for correctness and race conditions.
• Confirm metric/logging emissions are accurate and not noisy in normal flow.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: refactoring error handling (consolidating balance threshold updates) and adding nonce checks (detecting nonce movement during gas bump attempts).

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pb/nonce-stall-heal-race-condition-fix

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a1b60c4 and 73c421e.

📒 Files selected for processing (1)

• executors/src/eoa/worker/confirm.rs (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-09-20T05:30:35.171Z

Learnt from: joaquim-verges
Repo: thirdweb-dev/engine-core PR: 48
File: executors/src/eoa/worker/send.rs:20-21
Timestamp: 2025-09-20T05:30:35.171Z
Learning: In executors/src/eoa/worker/send.rs, there is a critical bug where HEALTH_CHECK_INTERVAL is defined as 300 seconds but compared against millisecond timestamps, causing balance checks to occur every 300ms instead of every 5 minutes (1000x more frequent than intended).

Applied to files:

• executors/src/eoa/worker/confirm.rs

📚 Learning: 2025-09-20T06:58:40.230Z

Learnt from: d4mr
Repo: thirdweb-dev/engine-core PR: 48
File: executors/src/eoa/store/submitted.rs:229-230
Timestamp: 2025-09-20T06:58:40.230Z
Learning: The diff in executors/src/eoa/store/submitted.rs shows correct brace structure - the first closing brace closes the remove_transaction_from_redis_submitted_zset method and the second closing brace closes the impl CleanSubmittedTransactions block. The change only adds whitespace formatting.

Applied to files:

• executors/src/eoa/worker/confirm.rs

🧬 Code graph analysis (1)

executors/src/eoa/worker/confirm.rs (1)

executors/src/eoa/worker/error.rs (1)

• should_update_balance_threshold (215-235)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

executors/src/eoa/worker/confirm.rs (1)

392-402: Good consolidation of balance threshold update logic.

The refactor unifies error handling for both TransactionSimulationFailed and RpcError paths, reducing duplication while preserving the original behavior. Error handling is appropriate—failures to update the balance threshold are logged but don't prevent the subsequent nonce freshness check.

If you want to reduce the nesting further, consider extracting the update logic into a small helper:

async fn try_update_balance_threshold_for_error(&self, error: &EoaExecutorWorkerError) {
    let inner_error = match error {
        EoaExecutorWorkerError::TransactionSimulationFailed { inner_error, .. } => Some(inner_error),
        EoaExecutorWorkerError::RpcError { inner_error, .. } => Some(inner_error),
        _ => None,
    };
    
    if let Some(inner) = inner_error {
        if should_update_balance_threshold(inner) {
            if let Err(e) = self.update_balance_threshold().await {
                tracing::error!("Failed to update balance threshold: {}", e);
            }
        }
    }
}

Then call self.try_update_balance_threshold_for_error(&e).await; at line 392.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 73c421e and b59b1bf.

📒 Files selected for processing (1)

• executors/src/eoa/worker/confirm.rs (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-20T05:30:35.171Z

Learnt from: joaquim-verges
Repo: thirdweb-dev/engine-core PR: 48
File: executors/src/eoa/worker/send.rs:20-21
Timestamp: 2025-09-20T05:30:35.171Z
Learning: In executors/src/eoa/worker/send.rs, there is a critical bug where HEALTH_CHECK_INTERVAL is defined as 300 seconds but compared against millisecond timestamps, causing balance checks to occur every 300ms instead of every 5 minutes (1000x more frequent than intended).

Applied to files:

• executors/src/eoa/worker/confirm.rs

🧬 Code graph analysis (1)

executors/src/eoa/worker/confirm.rs (2)

executors/src/eoa/worker/error.rs (1)

• should_update_balance_threshold (215-235)

executors/src/eoa/store/mod.rs (1)

• now (832-834)

🔇 Additional comments (1)

executors/src/eoa/worker/confirm.rs (1)

403-464: Excellent addition of nonce freshness check—previous review concern addressed.

The nonce freshness check correctly handles the race condition where the transaction confirms between the initial nonce check and the gas bump attempt. The health timestamp update (lines 439-450) directly implements the suggestion from the previous review comment, preventing false stuck alerts in subsequent iterations.

Implementation details verified:

• Fresh transaction counts are fetched after the gas bump build fails (lines 403-426)
• When preconfirmed > expected_nonce, health timestamps are updated and the flow exits with success (lines 428-454)
• If fetching fresh counts fails, the original error is returned (line 424)
• Enhanced logging includes fresh nonce values for debugging (lines 459-460)

Note on health update resilience:
If get_eoa_health() fails at line 439, the code logs a warning but continues and returns Ok(true). This is acceptable—the regular confirmation flow will update health timestamps when it processes the confirmed transaction in the next iteration (as seen in lines 267-280 of confirm_flow).