fix: uniswapx tx liveness

[alrxy]

No description provided.

[oxsteins]

Reviewed the txmanager rewrite end to end. Build is green, the -race suite passes, and I traced the single-lifecycle semaphore, the nonce-conflict flow, and the fee arithmetic. The serialization/result-delivery invariants, the receipt round-robin and reorg handling, the write/read routing plus chain-ID check, the fatal/readiness wiring, and the never-exceeds-maxFeeGwei ceiling all hold up. A few things worth addressing, most consequential first.

1. A replacement/cancellation can be broadcast underpriced and permanently stall the nonce lane

In nextReplacementFees (internal/txmanager/txmanager.go:703-706), next.tip is capped down to maxTip = maxFee - baseFee, then the acceptance guard only checks strict > previous, not the node's required bump (geth wants +10%, the design says +12.5%). When base fee rises between attempts, the tip gets squeezed below the bump floor, so the node rejects the send as replacement transaction underpriced.

That rejection then compounds. signAndSend / tryReplace (:900-904, :619) do not treat an underpriced rejection as definite for an existing lifecycle, so pending.fees is ratcheted to the rejected values and the attempt is tracked. On the next interval the poisoned baseline (often already at the cap) yields errReplacementLimitReached, and rebroadcastLatestAttempt re-sends the same rejected tx forever. The stuck nonce never cancels, and because it is single-lifecycle, all future fills are blocked. This is most acute for cancellation during a fee spike, which is exactly when cancellation matters.

Fix: require next.tip >= bumpFee(previous.tip) and next.maxFee >= bumpFee(previous.maxFee) after the caps (return errReplacementLimitReached otherwise), and do not update pending.fees or track the hash when the send was a definite underpriced rejection. This is the one I would gate the merge on.

2. Graceful shutdown can hang unbounded

complete runs on context.WithoutCancel(ctx), so waitForPendingTransaction only exits on a terminal receipt. On shutdown with an active nonce conflict (tryReplace no-ops via hasNonceConflict, :579), a sustained write-RPC outage, or base fee above the cap, no attempt ever mines or cancels, so lifecycleWG.Wait() (:218) blocks forever and run.go's background.Wait() hangs. The process then needs SIGKILL, which is the mid-flight kill the design is trying to avoid.

Fix: bound the drain. Select on stopping (or the real parent ctx) with a deadline so a wedged lifecycle can abandon on shutdown instead of waiting for an impossible receipt.

3. A replacement that races the original mining spuriously pauses the whole lane

sendSigned (:920-922) calls markNonceConflict on "nonce too low" regardless of existingLifecycle. When a periodic replacement or cancellation is signed just as the original mines, it gets "nonce too low" and marks a conflict even though the tx succeeded. That pauses admissions and drops /readyz, rejecting new fills for the full confirmation window (~24s at 2 confirmations). It is frequent for uniswapx (replacementIntervalMs: 5000).

Fix: skip markNonceConflict when existingLifecycle is true. A replacement seeing nonce-too-low means a sibling attempt at the same nonce landed, and the receipt poll resolves it cleanly.

4. ValidateTxManager does not check tipGwei <= maxFeeGwei

internal/config/config.go:186-200 validates each field independently. tipGwei: 60 with maxFeeGwei: 50 passes config, then every send fails at runtime in currentFees with "fee limit reached". It fails closed, but silently disables sending on a config that looked valid. Worth validating the relationship.

5. (low) All-zero p75 fee history gives a 0-wei tip and 1-wei replacement steps

With tipGwei: 0 (the shipped uniswapx example), a run of zero-reward blocks makes feeHistoryTip return 0, and bumpFee(0) steps by 1 wei, so replacements cannot outbid. Mitigated by setting a positive tipGwei floor, but worth calling out given the example ships tipGwei: 0.

Nits

• confirmations == 0 in waitForConfirmations skips the canonical recheck, so a 0-confirmation terminal result is not reorg-safe. Not reachable today (config clamps to 2 and nothing sets the per-request override), but the Request.Confirmations API plus the fast path leave the door open.
• waitForPendingTransaction's ctx-cancel path returns attempts[0].hash with a cancellation-style error. Safe only because complete runs on WithoutCancel, so this branch never fires. Worth a comment tying it to that contract, since wiring a cancellable ctx here would silently return the wrong hash.
• The LI.FI worker drops an accepted fill's local completion (completion log and capacity release) on shutdown, where uniswapx keeps draining. Not a lost fill (the txmanager owns the lifecycle and the process waits on its drain), just asymmetric between the two solvers.

[oxsteins]

Re-reviewed at cd18784. The important fixes land correctly: nextReplacementFees now enforces the full 12.5% bump on both fee and tip (I re-derived the cancellation boundary, a normal attempt at the reserved limit bumps to exactly the global cap), ValidateFeeHeadroom closes the tip-vs-cap config gap, and the stable-head ancestry walk in confirmReceiptAncestry is a solid answer to fork-serving load balancers. Race suite is green. What remains is short but it all gates merge.

1. Shutdown still hangs unbounded when the active lifecycle cannot resolve

Unchanged from my last review. complete runs on the detached context and the stop path waits with no deadline:

• internal/txmanager/txmanager.go:228 lifecycleCtx := context.WithoutCancel(ctx)
• internal/txmanager/txmanager.go:232 m.lifecycleWG.Wait()
• internal/txmanager/txmanager.go:593 if m.hasNonceConflict(pending.nonce) { return }

With an active nonce conflict, line 593 makes every cancellation attempt a no-op, no tracked hash can ever confirm, and line 232 blocks forever. Same for a sustained write-RPC outage. SIGTERM then requires SIGKILL, which is the mid-flight kill this design is trying to avoid. The README now documents the fail-closed restart, but the drain itself still needs a bound.

2. A replacement racing the original's inclusion still pauses the lane

internal/txmanager/txmanager.go:942:

if isNonceConsumedError(err) || (!existingLifecycle && isPendingNonceCollision(err)) {
    m.markNonceConflict(signed.Nonce(), signed.Hash())

A periodic replacement signed just as the original mines gets "nonce too low" and marks a conflict even though we own the nonce. Readiness and admissions drop for the full confirmation window on a healthy fill. Skipping the mark when existingLifecycle is true fixes it, the receipt poll already resolves ownership.

3. New in this update: removing CancelAt from lifi and rfq loses deadline cancellation

• internal/solvers/lifi/submission.go:40-42 sends with Label: "lifi-fill" and no CancelAt
• internal/solvers/rfq/execution.go:200 res := e.txm.Send(ctx, txmanager.Request{To: e.executor, Data: calldata, Label: "rfq-fill"})

Previously these fills switched to same-nonce cancellation at the order/discount deadline. Now a fill whose order expires mid-lifecycle keeps getting fee-bumped until the global 5m pendingTimeout, holding the single nonce lane on a dead order and paying replacement gas the whole way. uniswapx kept its deadline via the skew-safe fillCancellationDeadline translation; reusing that for lifi/rfq seems better than dropping the deadline entirely.

4. Merge-order conflict with #117

This PR and #117 rewrite the same seams with contradictory designs. cmd/vault-solver/run.go and internal/solver/solver.go conflict textually. Worse, the LI.FI completion paths auto-merge cleanly but contradict: this PR releases the reservation immediately in internal/solvers/lifi/submission.go:59 (s.releaseReservation(fill.reservationKey)), while #117 defers release until after its capacity-retry batch (its execution.go:557) precisely so capacity is never transiently freed. A clean merge keeps both and breaks #117's invariant. Whichever lands second needs a deliberate reconciliation, worth agreeing the target model now.

[oxsteins]

Re-reviewed at 39e973e. All three items from my last comment are fixed properly: the bounded shutdown drain (shutdownTimeoutMs plus the compose grace period, with the conflict, write-outage, and blocked-signer tests), the replacement-race reconciliation against a canonical receipt instead of a blanket pause, and CancelAt restored for lifi/rfq through the shared skew-safe liquidlane.CancellationDeadline. Full race suite is green. Consider those threads closed.

One finding I had not put on the record yet, and it is the main thing I would still fix in this PR. It follows directly from the single-lifecycle redesign.

UniswapX quoting reopens at fill admission, not at lifecycle terminal

quoteBlocked (internal/solvers/uniswapx/server.go:219-227) gates quoting on planningFills and txm.Available(). But planningFills drops to zero the moment startFill returns, i.e. right after SendAsync accepts (execution.go: fill, err := s.startFill(...) immediately followed by s.endFillPlanning()), and Available() is only false during a nonce conflict. Neither gate knows that an accepted fill now occupies the single nonce lane until it is terminal: receipt plus 2 confirmations (~36s best case), up to pendingTimeoutMs (5m) through replacement and cancellation cycles.

Before this PR that was fine: nonces pipelined, so a second fill was signed at nonce+1 and broadcast within seconds. Now the timeline is:

1. Fill A is admitted, the slot is taken, quoting reopens.
2. We win a quote. Order B is created on-chain exclusive to our executor, with an exclusivity window of tens of seconds.
3. The fill loop plans B and calls SendAsync. B blocks in admission behind A until A is terminal.
4. Either B's CancelAt fires pre-admission (never signed, exclusivity lapses, another filler takes it via decay), or B is admitted late and reverts or fills into decay.
5. Uniswap's fade system records the miss (external, permanent), and later sweepExclusive classifies it missed-exclusive (state.go:321) and opens the exclusive breaker, after the damage.

So under single-lifecycle, the solver overcommits: it keeps winning exclusive obligations at a rate it can no longer physically serve. The existing gates show the invariant "do not quote when you cannot fill" was intended; it is just anchored to the wrong endpoint of a fill's life.

Fix direction: an atomic in-flight counter incremented at admission (next to setPendingReservations) and decremented in completePendingFill, added to quoteBlocked. The resulting ~36s quote blackout per fill is the honest cost of the single-lifecycle model; declining a quote is free, winning and fading is not.

Two related smaller items, same root cause

1. Pre-admission failures are booked as real fill failures. completePendingFill (execution.go:408-418) treats every result.Err the same: recordOrderFillFailure -> fade breaker, plus observeFill("failed"). Under the new manager a Result{Err} can mean "never signed" (errNonceLanePaused, errManagerStopped, pre-admission CancelAt expiry, errShutdownTimeout). During a lane pause, open orders retry-fail instantly every backoff cycle, so Breaker.MaxFailures is reached from failures with no transaction behind them, and fills_total{result="failed"} reports never-signed attempts (with tx=0x000...0) as failed fills. Classifying the pre-admission sentinels out of the breaker, with a distinct metric label like not-admitted, fixes both.

2. Full planning runs while the lane is provably unusable. fillLoop never checks txm.Available() before working. While paused, each claimed order still does the chain reads, strategy decision, resolveDiscount (which consumes a freshly signed discount from the backend), calldata build, and preflight, then gets the instant buffered errNonceLanePaused; setPendingReservations fires and is immediately cleared, churning quote state. One guard at the top of the order branch (if !s.txm.Available() { retry; continue }) avoids burning signed discounts and RPC for the duration of every pause.

All three are one root cause: the PR changed what "busy" means but only the txmanager knows it. The quote gate still thinks busy means planning, the breaker still thinks every error was on-chain, and the fill loop still thinks the lane is always worth preparing for. The first one is the one that costs reputation.

[oxsteins]

Reviewed through b610dfc. All three items from my last comment are fixed and test-covered: the manager-level Idle() gate blocks quoting and readiness from slot demand through the terminal result (a better shape than the solver-side counter I suggested, since it also covers waiters and fills from other solvers sharing the lane), manager-level admission failures are now typed NotAdmitted and routed to uniswapx_fills_total{outcome="not-admitted"} without touching the fade breaker (while fee/gas/signing failures deliberately still count, which is right), and the fill loop defers claimed orders before any chain reads or discount resolution while the lane is paused. I traced the admissionDemand accounting across every path and it balances. Full race suite passes.

One thing left before this can land: with #117 merged into stage, this branch now reports a real merge conflict. The reconciliation touches exactly the seams where the two PRs made contradictory choices, so it deserves care rather than a mechanical merge:

• cmd/vault-solver/run.go: #117's ShutdownPreparer/drain-monitor shutdown vs this PR's watchReadiness/ReportFatal plus the bounded txmanager drain. The two models overlap; pick one composition rather than stacking both timers.
• internal/solver/solver.go: both PRs added interfaces; textual conflict, semantically composable.
• The LI.FI completion path is the dangerous one: this branch releases the fill reservation immediately in completeFill (internal/solvers/lifi/submission.go), while merged #117 defers the release until after its capacity-retry batch (execution.go, via SnapshotExcluding) precisely so capacity is never transiently freed. These merge cleanly textually and contradict semantically; keep #117's deferred-release sequence and drop the immediate release, or the retry planning can observe transiently freed capacity and over-quote.

Approving on the strength of the branch as it stands. Happy to re-review the stage reconciliation commit when it is up, since that is where the remaining risk lives.