eth: fix alt-provider quota burn (#1629), lingering cancelled/replaced private txs (#1573) and trezor-suite#30846

[pragmaxim]

Fixes #1629 and #1573 in the EVM alternative send-tx provider, plus hardening from an xhigh review of the surrounding subsystem. Also the backend counterpart to trezor/trezor-suite#30846 — a wallet told "send failed" while Blockbook has already broadcast risks a double payment when the user retries.

Follow-up https://gist.github.com/pragmaxim/f71b03a4ed4c0030b6cba7fb0a302e96

#1629 — gas estimates burned the relay's rate quota

EthereumTypeEstimateGas routed every eth_estimateGas to the alternative provider (always urls[0], errors swallowed). The WS estimateFee handler fires on every send-form keystroke, so concurrent users drained a private relay's quota even without using private txs. Now routed only for recent private senders (useForNonces + nonceURL), matching the nonce path; others hit the primary backend. Provider errors log and fall back. New eth_alternative_estimate_gas_requests_total metric + Grafana panel.

#1573 — cancelled / replaced private txs lingered until timeout

A Blink drop-mode cancel or an accepted-but-unsurfaced RBF replacement left the superseded tx "Unconfirmed" for up to the 5-min cache timeout, because eviction waited on a fetch-back that never came. Now a same-(from, nonce) predecessor is retired the moment the relay ACKs its replacement (decoded from the raw hex), independent of fetch-back — deliberately distinct from a non-authoritative empty getTransactionByHash probe.

Counterpart to trezor-suite#30846 — a relay-accepted send must never look failed

The suite PR gives a transaction push its own 60 s deadline and stops showing "send failed" when the backend is merely slow. Both only hold if Blockbook answers within the wallet's budget and surfaces every send the relay accepted, whatever the relay does afterwards. The issues fixed here:

• an accepted send is cached from its own signed bytes — an accepted send used to be cached only if the post-send eth_getTransactionByHash round-trip surfaced it; a relay that stopped surfacing (or never surfaced) what it accepted left the send exposed nowhere — not served as pending, not in the address index, not raising the pending-nonce floor — so a wallet that retried re-signed at the next nonce and both transactions mined. The signed bytes carry everything a pending transaction needs, so the send path now decodes once and caches + indexes the transaction before the relay is ever asked; the fetch-back only refreshes that entry with the relay's own view.
• the send path stays inside the wallet's deadline — the broadcast to every configured relay used to run sequentially, so one unresponsive relay held the wallet's answer for its full rpc_timeout before the next URL was even tried, and the post-send fetch-back sat in the wait too. Both are gone: the broadcast runs concurrently (worst case a single rpc_timeout, not N), and the fetch-back runs in the background, so the wallet's own deadline is no longer consumed by relay handling.
• the background fetch-back updates, never re-creates — now that the fetch-back runs off the send path, its answer can straddle a block; re-inserting the pending body over a transaction that mined and was cleared by block sync would flip it back to Unconfirmed and re-index it as pending. It only updates a still-cached entry (or evicts on a mined answer), capped at 16 in flight so a burst of accepted sends cannot grow goroutines, sockets or relay-quota usage without bound.

Review-driven hardening (same subsystem)

• estimate-gas fallback: build the primary-RPC deadline after the alt round-trip so a slow relay can't hand the fallback an already-expired context; also fall back (not error) on a malformed provider result.
• rejected send: skip the mempool-cache path when no relay accepted, so a rejected send no longer fans out eth_getTransactionByHash(0x0…0) and logs a spurious error.
• metrics gating: lifecycle counters/residence recorded only by the goroutine that actually removed the entry — no more double-counting across concurrent reconcile / read-path / RBF evictions.
• generation ordering: an older submission's slow fetch-back can no longer evict or shadow a newer same-nonce replacement.
• read-path eviction: routes through removeMempoolTx, so an expired private tx is cleared from the wrapped mempool even when the caller's primary lookup errors.

Docs

docs/evm-send.md — Mermaid flowchart of the broadcast + pending-tx cache lifecycle; fee estimation cross-linked to fees.md; what a relay-accepted send guarantees (see above) stated precisely.

Testing

go build ./..., go vet, and the eth + common suites pass; each fix ships a regression test.

🤖 Generated with Claude Code

[cranycrane]

Reviewed at 53ee56fd. The design is sound and the three fixes are real — caching from the signed bytes is the right call, and the nonce-pin trade-off it accepts is correctly documented and instrumented. go build/go vet/gofmt clean, eth + common suites pass, -race -count=30 on the concurrency tests clean, merges cleanly on current master.

Worth calling out as a bonus: this removes a real pre-existing panic. In geth v1.17.0 newModernSigner panics on chainID == nil || chainID.Sign() <= 0, and LatestSignerForChainID routes every non-nil chain id into it — including the zero an unprotected pre-EIP-155 legacy tx decodes to. master's alternativeTxSender therefore panicked on the send path for such a raw tx. The new HomesteadSigner guard is correct, and it cannot silently recover a wrong sender (signature validation rejects v ∈ {35,36} rather than returning a garbage address).

Below are the major findings only; medium/minor ones (stale comments, doc inaccuracies, test gaps, bucket change, comment density) are separate.

---

1. The cached tx body is shared by pointer and mutated with no lock

alternativesendtx.go:778 returns the live storedTx.tx. ethrpc.go:1859 then calls EthTxToTx(..., fixEIP55=true), which writes tx.From/tx.To in place (ethparser.go:137,143) holding no lock — while pendingNonceFloor:541, txSenderAndNonce (via insertMempoolTx:677, evictReplacedByNonce:720) and refreshCachedTransaction:347 read those same fields under mempoolTxsMux. Reproducible under -race.

Pre-existing on master, but this PR is what makes it worth fixing here: it creates an entry for every accepted send instead of only surfaced ones, adds three new readers (two on a background goroutine), and the send path itself drives the writer via cacheMempoolTransaction:637 → AddTransactionToMempool → createTxEntry → GetTransaction.

• On EVM the damage is bounded — both values are 42 chars, so a torn read still yields a valid string, and HexToAddress is case-insensitive. But the cached body is permanently rewritten to EIP-55, which silently falsifies decodeAlternativeSendTx's own documented "lower-case hex addresses" contract after the first read.
• On Tron it is not bounded: tronparser.go:38 sets FormatAddressFunc = ToTronAddressFromAddress, rewriting From to a 34-char base58 T…, so txSenderAndNonce then decodes the wrong address — no RBF retirement, and pendingNonceFloor stops covering the tx, i.e. exactly the nonce-reuse exposure this PR exists to close. Latent today (no in-repo config sets TRON_ALTERNATIVE_SENDTX_URLS) but TronRPC embeds *eth.EthereumRPC, so it is env-reachable.

Fix: one line in GetTransaction — cp := *storedTx.tx; return &cp, true. A shallow copy suffices for a pending tx.

2. evictReplacedByNonce retires at most one predecessor

The break at :733 (with a single rbfTxid/rbfTime) means one pass evicts one victim. There are two eviction passes per send (:220 pre-cache, :632 post-insert), and insertMempoolTx only skips on a strictly higher gen already visible. With three concurrent sends for one (from, nonce) whose register → slotSupersededBy → insert windows interleave, all three insert, the newest pass evicts one, and the mid pass finds the newest protected — leaving two entries cached and address-indexed for one nonce slot, the state insertMempoolTx's own comment says must never exist. It does not self-heal: a later gen-4 send again evicts only one. Reproduced 174/200 trials by driving the real call sequence.

Impact: two Unconfirmed txs for one nonce served to the wallet until the winner mines or the 5-min timeout; rbf_replaced under-counts. Needs a fee-bump storm to hit, but the send-generation machinery exists precisely to make this impossible.

Fix: collect all victims into a slice, drop the break, meter each successful removeMempoolTx. That is sufficient — the max-gen send then clears every lower-gen entry, and no lower-gen send can insert afterwards. Also :700-702 says "removes any cached transaction that shares…", which is currently false.

3. A dropped fetch-back in the decode-failure branch loses the accepted send entirely

:240 sends the decErr != nil fetch-back through inBackground, and startBackground:282 drops the work when 16 are already in flight, with only a glog.Warningf. In that branch nothing was cached and registerSuccessfulSend never ran — so the accepted send is served nowhere, not address-indexed, and raises no pending-nonce floor. That is precisely the failure this PR exists to prevent, and it is unmetered (observeSendNotSurfaced does not fire on a drop, so the dashboard cannot see it).

inBackground's justification at :250-254 — "reconcileMempoolTxs revisits every cached entry within a minute anyway" — holds for the :236 call site and is false for this one.

Fix: run the decode-failure fetch-back synchronously (it is the only thing that can expose the tx), or at minimum observeSendNotSurfaced("dropped") when startBackground refuses, and split the comment per call site.

4. The fallback tail still exceeds Suite's new 60 s push deadline

ethrpc.go:2248-2267: when no relay accepts and _ALTERNATIVE_SENDTX_ONLY is not TRUE, the handler falls through to the primary eth_sendRawTransaction under b.Timeout — 25 s (relays, now concurrent) + 25 s ≈ 50 s — and on the disableMempoolSync coins (arbitrum, base, bsc, optimism, polygon) AddTransactionToMempool then issues a primary eth_getTransactionByHash under another 25 s → ≈75 s, past the 60 s that trezor/trezor-suite#30846 installs.

Pre-existing, and the wallet gets an error on that path anyway — but it is the one remaining route where the user is told "send failed" while the primary may have broadcast. At minimum docs/evm-send.md:67-72 should stop asserting "the wallet's answer waits for the broadcast and nothing else"; better, skip the fallback AddTransactionToMempool round-trip.

5. Deployment order: trezor-suite#30846 has to land first

The suite side (PUSH_TRANSACTION_TIMEOUT = 60_000, no retry — a Blockbook error and a socket timeout are indistinguishable to sendFormThunks) is satisfied by the concurrent broadcast with margin, and returning the signed-bytes hash rather than the relay's echo is an improvement for it, since synchronizeSentTransactionThunk keys its pending row on that txid.

But the coupling runs both ways: caching every accepted send means OnNewTx now pushes pending txs Blockbook previously never surfaced — including contract creations, which serialise addresses: null. Pre-#30846 getEthereumRbfParams dereferences vout[0].addresses![0] and throws inside addTransaction, failing the whole account batch. Reachable today for surfaced private txs, so not new, but this PR widens the exposure and old Suite builds stay in the field. Worth stating the order in the PR body/changelog.

---

🤖 Generated with Claude Code