Follow-up fixes for multi-threaded runtime (#810): Stellar sequence stall, Lua precision, plugin socket death, runtime-drop panic, EVM nonce race

[tirumerla]

Summary

A post-merge review of #810 (multi-threaded runtime for the transaction pipeline, shipped in v1.6.0) surfaced 1 critical and 4 important issues. Each was independently verified against the merged code (head 1455fbbb, now on main). Since #810 is in production, these should land as a follow-up PR — items 1 and 2 compound (the stall is unrecoverable and the recovery primitive has a precision bug) and are the urgent pair.

1. 🔴 Critical — sync_floor removed the only sequence-rewind path; a prepare failure after allocation permanently stalls a Stellar relayer

src/domain/transaction/stellar/stellar_transaction.rs:264

The counter stores the next sequence to allocate; prepare allocates via get_and_increment (src/domain/transaction/stellar/prepare/common.rs:137). If preparation fails after allocation (transient signer/KMS error, Soroban simulation failure — exactly the scenarios exercised by the tests in prepare/mod.rs:703,809), the allocated sequence is consumed but never reaches the chain. handle_prepare_failure (prepare/mod.rs:232) calls sync_sequence_from_chain, which now calls sync_floor(chain_next) — but since counter (S+1) > chain_next (S), sync_floor is a no-op. Under the old code, set(S) rewound the counter and healed the gap.

Failure: counter=7 after allocating seq 6, signing fails, chain still expects 6 → every subsequent transaction allocates a too-high sequence → TxBadSeq → relayer permanently stalled until manual intervention.

Suggested fix: sync_floor was introduced (correctly) because a blind set() can rewind below concurrently-allocated sequences. The rewind path needs to come back in a concurrency-safe form — e.g. a conditional sync_exact/compare-and-set Lua op used only by the prepare-failure recovery path (which knows the allocated-but-lost sequence), or explicit gap release (decrement-if-equal on failure before any other allocation).

2. 🟠 Important — sync_floor Lua script compares sequences as Lua doubles; breaks above 2^53 (all real Stellar sequences)

src/repositories/transaction_counter/transaction_counter_redis.rs:245

Redis Lua 5.1 tonumber() converts to an IEEE-754 double, exact only up to 2^53 (~9.0e15). Stellar sequences are (creation_ledger << 32) + n ≈ 2.5e17 (~2^58) today, where the double ulp is 32–64 — any account created after ledger 2,097,152 (essentially everything since ~2019) is affected.

Failure: cur=225000000000000001, floor=225000000000000010 round to the same double → tonumber(cur) < tonumber(ARGV[1]) is false → counter is NOT raised even though it is genuinely behind → allocations reuse too-low sequences and fail with TxBadSeq until the counter catches up by increments. Production callers pass real chain sequences: stellar_relayer.rs:232-245 and stellar_transaction.rs:244-266 (the TxBadSeq recovery path).

Suggested fix: compare inside Lua without float conversion — e.g. compare string lengths first, then lexicographically (both values are non-negative decimal integers), and store/return the raw string.

3. 🟠 Important — shared plugin socket listener dies permanently after ~200ms of accept errors; plugins bricked until process restart

src/services/plugins/shared_socket.rs:546

New in #810: the accept loop breaks after MAX_CONSECUTIVE_ACCEPT_ERRORS=10 failures with 20ms sleeps (~200ms). accept() fails transiently with EMFILE/ENFILE under fd exhaustion — plausible during a plugin burst, which is precisely this service's load profile. After the break, the socket file is removed, started (set via swap(true) at line 456) is never reset, and the service lives in the process-wide SHARED_SOCKET OnceLock (line 859) — so ensure_shared_socket_started() silently returns Ok(()) forever and every subsequent plugin execution fails until the relayer is restarted. Pre-#810 behavior (warn and retry forever) hot-spun but self-healed.

Suggested fix: replace break-and-die with exponential backoff (keep retrying with a capped delay), or reset started on listener exit so start() can rebuild the listener on the next plugin invocation.

4. 🟠 Important — any startup error after the pipeline runtime is built panics on Runtime drop, masking the real error

src/main.rs:110 / :247

pipeline_runtime is an owned multi-thread tokio::runtime::Runtime built inside #[actix_web::main] async fn main; shutdown_background() is called only on the success path (line 283). Every ? early-return between construction and shutdown — initialize_queue_workers, HTTP bind (port already in use is the routine ops case), metrics bind, try_join! — drops the Runtime from within the async context, which panics with "Cannot drop a runtime in a context where blocking is not allowed" instead of printing the clean eyre error. The PR's own comment (main.rs:276-281) documents the hazard but only handles the happy path.

Suggested fix: wrap the runtime in a guard that calls shutdown_background() on drop, or restructure main to capture the Result and shut the runtime down before propagating the error.

5. 🟠 Important — EVM sync_nonce still has the non-atomic get → max → set rewind race that sync_floor fixed for Stellar

src/domain/relayer/evm/nonce.rs:81 (and a second read-then-set in resolve_nonce_gaps, nonce.rs:215-229)

#810 introduced sync_floor for Stellar specifically because "under real parallelism a blind set() could rewind the counter below already-allocated sequences" — but the identical pattern remains on EVM. Both the get() and set() are awaited Redis round trips, so concurrent get_and_increment() calls from prepare jobs can interleave and be rewound. No guard prevents it: the DistributedLock in handle_health_action only serializes nonce-health jobs against each other, and sync_nonce is also reachable via check_health (evm_relayer.rs:645) from the periodic health-check handler with no lock at all, running concurrently with the pipeline.

Failure: counter rewound below an already-allocated nonce → duplicate nonce allocation → replacement-underpriced errors / stuck transactions.

Suggested fix: apply the same sync_floor primitive to the EVM counter (with the item-2 precision fix), and audit resolve_nonce_gaps for the same treatment. Note items 1/2 caveats apply: EVM also needs a story for the rewind-on-failure case.

[dylankilkenny]

Status update — 3 of the 5 items are now resolved on main or in flight:

• Item 1 (critical — Stellar sequence rewind path): fixed by fix: Rewind drifted Stellar sequence counter #832 (approved, pending merge). sync_sequence_from_chain now reconciles downward via a set_if_equals CAS, bounded by active (Pending/Sent/Submitted) transactions and the newest Confirmed transaction. The prepare-failure path heals the burned sequence at the origin (before any TxBadSeq), and the TxBadSeq recovery path resets the failing tx before syncing so its own record can't pin the rewind.
• Item 2 (Lua f64 precision above 2^53): fixed by fix: Rewind drifted EVM nonce counter and skip resubmits blocked by nonce gaps #831 (5dc4bdf0). Both sync_floor and set_if_equals now compare inside Lua with string length-then-lexicographic ordering as suggested, exact at Stellar sequence magnitudes.
• Item 5 (EVM non-atomic get→max→set rewind race): fixed by fix: Rewind drifted EVM nonce counter and skip resubmits blocked by nonce gaps #831 (5dc4bdf0). sync_nonce and resolve_nonce_gaps now raise the counter through the atomic sync_floor, and the rewind-on-failure story is covered by the CAS-based try_rewind_counter.

Items 3 (shared plugin socket listener dies permanently after ~10 accept errors) and 4 (startup error after pipeline-runtime construction panics on Runtime drop) are still present on main; a follow-up PR for both is in progress.