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

bchain/coins/eth/ethrpc.go

@@ -154,6 +154,15 @@ func (c *Configuration) AlternativeMempoolTxTimeoutDuration() (time.Duration, er
 	return defaultAlternativeMempoolTxTimeout, nil
 }
 
+// mempoolRetentionInverted reports whether the alternative-provider cache is configured to outlive
+// the wrapped Blockbook mempool. Every cache exit clears the wrapped mempool too, but the mempool's
+// own timeout sweep is the one exit that does NOT clear the cache: inverted, that sweep drops a
+// private transaction's address index while the cache keeps serving its body as pending, and nothing
+// reconciles the two. Only an explicit timeout pair can invert it; the defaults cannot.
+func mempoolRetentionInverted(alternativeTimeout, mempoolTimeout time.Duration) bool {
+	return alternativeTimeout >= mempoolTimeout
+}
+
 // AverageBlockTimeDuration returns AverageBlockTimeMs as a time.Duration.
 func (c *Configuration) AverageBlockTimeDuration() (time.Duration, error) {
 	if c.AverageBlockTimeMs <= 0 {
@@ -711,6 +720,11 @@ func (b *EthereumRPC) CreateMempool(chain bchain.BlockChain) (bchain.Mempool, er
 		b.Mempool = bchain.NewMempoolEthereumType(chain, mempoolTxTimeout, b.ChainConfig.QueryBackendOnMempoolResync)
 		glog.Info("mempool created, MempoolTxTimeout=", mempoolTxTimeout, ", QueryBackendOnMempoolResync=", b.ChainConfig.QueryBackendOnMempoolResync, ", DisableMempoolSync=", b.ChainConfig.DisableMempoolSync)
 		if b.alternativeSendTxProvider != nil {
+			// warned here, not in Validate: the effective mempool retention depends on the
+			// env-configured provider existing
+			if mempoolRetentionInverted(b.alternativeSendTxProvider.mempoolTxsTimeout, mempoolTxTimeout) {
+				glog.Warningf("alternativeMempoolTxTimeout=%s is not shorter than mempoolTxTimeout=%s: the wrapped mempool may drop a private transaction's address index while the provider cache still serves it as pending", b.alternativeSendTxProvider.mempoolTxsTimeout, mempoolTxTimeout)
+			}
 			b.alternativeSendTxProvider.SetupMempool(b.Mempool, b.removeTransactionFromMempool)
 		}
 
@@ -1954,8 +1968,6 @@ func GetStringFromMap(p string, params map[string]interface{}) (string, bool) {
 
 // EthereumTypeEstimateGas returns estimation of gas consumption for given transaction parameters
 func (b *EthereumRPC) EthereumTypeEstimateGas(params map[string]interface{}) (uint64, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), b.Timeout)
-	defer cancel()
 	msg := ethereum.CallMsg{}
 	if s, ok := GetStringFromMap("from", params); ok && len(s) > 0 {
 		msg.From = ethcommon.HexToAddress(s)
@@ -1977,19 +1989,54 @@ func (b *EthereumRPC) EthereumTypeEstimateGas(params map[string]interface{}) (ui
 		msg.GasPrice, _ = hexutil.DecodeBig(s)
 	}
 
-	if b.alternativeSendTxProvider != nil {
+	// Route eth_estimateGas through the provider ONLY for a sender that recently sent a private tx
+	// through it (see useForNonces), which may have pending state the primary RPC does not know about.
+	// Every other estimate - the overwhelming majority, since the wallet calls estimateFee on each
+	// send-form keystroke - goes straight to the primary, so the hot endpoint no longer burns the
+	// provider's rate-limit quota (#1629). A missing `from` takes the primary path too.
+	if b.alternativeSendTxProvider != nil && msg.From != (ethcommon.Address{}) &&
+		b.alternativeSendTxProvider.useForNonces(msg.From) {
 		result, err := b.alternativeSendTxProvider.callHttpStringResult(
-			b.alternativeSendTxProvider.urls[0],
+			b.alternativeSendTxProvider.nonceURL(msg.From),
 			"eth_estimateGas",
 			params,
 		)
 		if err == nil {
-			return hexutil.DecodeUint64(result)
+			// Count success only once the result decodes: a malformed hex quantity is a provider
+			// failure, so fall through to the primary RPC rather than returning the decode error
+			// to the caller (and keep the "success" label honest for the quota dashboard).
+			if gas, decErr := hexutil.DecodeUint64(result); decErr == nil {
+				b.observeAlternativeEstimateGasRequest("success")
+				return gas, nil
+			} else {
+				err = decErr
+			}
 		}
-	}
+		// Previously the provider error was swallowed silently, hiding quota exhaustion (#1629);
+		// log it and fall back to the primary RPC below.
+		b.observeAlternativeEstimateGasRequest("error")
+		glog.Warningf("Alternative provider failed for eth_estimateGas: %v, falling back to primary RPC", err)
+	}
+	// Build the primary-RPC deadline here rather than at function entry: a slow or rate-limited
+	// alternative-provider round-trip above runs on its own independent context and can consume
+	// most of b.Timeout, so a context created at entry could already be expired by the time the
+	// fallback reaches the healthy primary backend (the exact #1629 slow-relay scenario).
+	ctx, cancel := context.WithTimeout(context.Background(), b.Timeout)
+	defer cancel()
 	return b.Client.EstimateGas(ctx, msg)
 }
 
+// observeAlternativeEstimateGasRequest records an eth_estimateGas call routed to the alternative
+// send-tx provider, labeled by result: success (provider answered) or error (provider failed and
+// the estimate fell back to the primary RPC). Only recent private senders are routed here (see
+// useForNonces), so this counts the gated subset rather than every estimateFee request.
+func (b *EthereumRPC) observeAlternativeEstimateGasRequest(result string) {
+	if b.metrics == nil || b.metrics.EthAlternativeEstimateGasRequests == nil {
+		return
+	}
+	b.metrics.EthAlternativeEstimateGasRequests.With(common.Labels{"result": result}).Inc()
+}
+
 // bigIntToFloat converts a wei amount to float64 for gauge export. float64 holds integers
 // exactly up to 2^53 (~9e15 wei), far above any realistic gas price, so no precision is lost;
 // keeping the metric in raw wei (base units) matches the repo convention and Grafana divides
@@ -2055,6 +2102,18 @@ func (b *EthereumRPC) observeAlternativeNonceRequest(result string) {
 	b.metrics.EthAlternativeNonceRequests.With(common.Labels{"result": result}).Inc()
 }
 
+// observePendingFloorRaised records that raiseToPendingFloor lifted a getTransactionCount answer above
+// the backend's own pending nonce because the cache still holds a higher-nonce private tx, labeled by
+// source: provider (the relay's own pending count had already dropped the still-cached tx) or primary
+// (the fallback primary RPC never knew the private tx). A sustained rate is the precursor to the
+// queue-behind-a-dead-nonce hang (#1638 review).
+func (b *EthereumRPC) observePendingFloorRaised(source string) {
+	if b.metrics == nil || b.metrics.EthAlternativePendingFloorRaised == nil {
+		return
+	}
+	b.metrics.EthAlternativePendingFloorRaised.With(common.Labels{"source": source}).Inc()
+}
+
 // eip1559BaseFeeMultiplier is the headroom applied to the projected base fee when deriving
 // maxFeePerGas for the on-chain EIP-1559 estimate (maxFeePerGas = multiplier*baseFee + tip).
 // 2x is the EIP-1559-standard buffer: it keeps a transaction mineable across ~6 consecutive full
@@ -2197,8 +2256,12 @@ func (b *EthereumRPC) SendRawTransaction(hex string, disableAlternativeRPC bool)
 	}
 
 	txid, retErr = b.callRpcStringResult("eth_sendRawTransaction", hex)
-	if b.ChainConfig.DisableMempoolSync {
-		// add transactions submitted by us to mempool if sync is disabled
+	if b.ChainConfig.DisableMempoolSync && retErr == nil {
+		// With no newPendingTransactions feed this add is the only thing that makes an own send visible
+		// for its addresses. Gated on success: with an empty txid it indexed the zero hash instead - one
+		// primary eth_getTransactionByHash, then the pruned-index recovery's eth_getTransactionReceipt,
+		// each on its own b.Timeout - two more rpc_timeouts spent on nothing, on the path where the
+		// wallet has already waited one for the relay broadcast and one for the primary send.
 		b.Mempool.AddTransactionToMempool(txid)
 	}
 	return txid, retErr
@@ -2279,7 +2342,11 @@ func (b *EthereumRPC) EthereumTypeGetNonces(addrDesc bchain.AddressDescriptor, w
 			// view: Blink-style relays stop counting a still-pending tx at the pending tag
 			// while Blockbook keeps exposing it until the cache timeout (see
 			// reconcileMempoolTxs).
-			return b.alternativeSendTxProvider.raiseToPendingFloor(ethAddress, pending), confirmed, confirmedOK, nil
+			raised := b.alternativeSendTxProvider.raiseToPendingFloor(ethAddress, pending)
+			if raised > pending {
+				b.observePendingFloorRaised("provider")
+			}
+			return raised, confirmed, confirmedOK, nil
 		}
 		b.observeAlternativeNonceRequest("error")
 		glog.Warningf("Alternative provider failed for eth_getTransactionCount: %v, falling back to primary RPC", err)
@@ -2297,7 +2364,11 @@ func (b *EthereumRPC) EthereumTypeGetNonces(addrDesc bchain.AddressDescriptor, w
 		// primary answer below the floor would contradict the pending tx Blockbook still
 		// displays. The floor is a local scan of a usually-empty map, so it costs nothing on
 		// the hot path.
-		pending = b.alternativeSendTxProvider.raiseToPendingFloor(ethAddress, pending)
+		raised := b.alternativeSendTxProvider.raiseToPendingFloor(ethAddress, pending)
+		if raised > pending {
+			b.observePendingFloorRaised("primary")
+		}
+		pending = raised
 	}
 	return pending, confirmed, confirmedOK, nil
 }

bchain/coins/eth/ethrpc_estimate_gas_test.go

@@ -0,0 +1,187 @@
+package eth
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	ethcommon "github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/ethclient"
+	"github.com/ethereum/go-ethereum/rpc"
+)
+
+// countingEstimateGasServer answers eth_estimateGas with a fixed hex gas value and counts how many
+// times it was hit, so a test can assert whether a given path (provider vs. primary) was consulted.
+func countingEstimateGasServer(t *testing.T, gasHex string) (*httptest.Server, *int32) {
+	t.Helper()
+	var hits int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.Header().Set("Content-Type", "application/json")
+		if _, err := w.Write([]byte(`{"jsonrpc":"2.0","id":1,"result":"` + gasHex + `"}`)); err != nil {
+			t.Errorf("Write() error = %v", err)
+		}
+	}))
+	t.Cleanup(server.Close)
+	return server, &hits
+}
+
+// newEstimateGasTestRPC wires an EthereumRPC whose primary Client points at primaryURL and whose
+// alternative send-tx provider points at providerURL, so a routing decision is observable by which
+// server receives the eth_estimateGas hit.
+func newEstimateGasTestRPC(t *testing.T, primaryURL, providerURL string) *EthereumRPC {
+	t.Helper()
+	primaryRPC, err := rpc.DialContext(context.Background(), primaryURL)
+	if err != nil {
+		t.Fatalf("dial primary: %v", err)
+	}
+	t.Cleanup(primaryRPC.Close)
+	return &EthereumRPC{
+		Client:  &EthereumClient{Client: ethclient.NewClient(primaryRPC)},
+		Timeout: 2 * time.Second,
+		alternativeSendTxProvider: &AlternativeSendTxProvider{
+			urls:              []string{providerURL},
+			mempoolTxsTimeout: time.Hour,
+			rpcTimeout:        2 * time.Second,
+			recentSenders:     map[ethcommon.Address]recentSender{},
+		},
+	}
+}
+
+// TestEthereumTypeEstimateGasSkipsProviderForNonRecentSender is the core of #1629: a sender that has
+// not recently sent a private transaction through the alternative provider must not have its gas
+// estimate routed there - it goes straight to the primary backend, so the hot estimateFee endpoint
+// does not burn the provider's rate-limit quota.
+func TestEthereumTypeEstimateGasSkipsProviderForNonRecentSender(t *testing.T) {
+	primary, primaryHits := countingEstimateGasServer(t, "0x5208")
+	provider, providerHits := countingEstimateGasServer(t, "0x9999")
+	b := newEstimateGasTestRPC(t, primary.URL, provider.URL)
+
+	gas, err := b.EthereumTypeEstimateGas(map[string]interface{}{
+		"from": "0x2222222222222222222222222222222222222222",
+		"to":   "0x3333333333333333333333333333333333333333",
+	})
+	if err != nil {
+		t.Fatalf("EthereumTypeEstimateGas() error = %v", err)
+	}
+	if gas != 0x5208 {
+		t.Fatalf("gas = %#x, want 0x5208 (primary backend value)", gas)
+	}
+	if got := atomic.LoadInt32(providerHits); got != 0 {
+		t.Fatalf("provider hits = %d, want 0 (non-recent sender must not touch the provider)", got)
+	}
+	if got := atomic.LoadInt32(primaryHits); got != 1 {
+		t.Fatalf("primary hits = %d, want 1", got)
+	}
+}
+
+// TestEthereumTypeEstimateGasRoutesRecentSenderToProvider confirms the provider path is preserved
+// for the case it exists for: a sender with a recent private transaction is routed to the provider
+// URL that accepted its send (see nonceURL), which may know a pending tx the primary does not.
+func TestEthereumTypeEstimateGasRoutesRecentSenderToProvider(t *testing.T) {
+	primary, primaryHits := countingEstimateGasServer(t, "0x5208")
+	provider, providerHits := countingEstimateGasServer(t, "0x9999")
+	b := newEstimateGasTestRPC(t, primary.URL, provider.URL)
+
+	sender := ethcommon.HexToAddress("0x2222222222222222222222222222222222222222")
+	b.alternativeSendTxProvider.recentSenders[sender] = recentSender{
+		time: time.Now(),
+		url:  provider.URL,
+		gen:  1,
+	}
+
+	gas, err := b.EthereumTypeEstimateGas(map[string]interface{}{
+		"from": sender.Hex(),
+		"to":   "0x3333333333333333333333333333333333333333",
+	})
+	if err != nil {
+		t.Fatalf("EthereumTypeEstimateGas() error = %v", err)
+	}
+	if gas != 0x9999 {
+		t.Fatalf("gas = %#x, want 0x9999 (provider value)", gas)
+	}
+	if got := atomic.LoadInt32(providerHits); got != 1 {
+		t.Fatalf("provider hits = %d, want 1 (recent sender must be routed to the provider)", got)
+	}
+	if got := atomic.LoadInt32(primaryHits); got != 0 {
+		t.Fatalf("primary hits = %d, want 0", got)
+	}
+}
+
+// TestEthereumTypeEstimateGasFallsBackWhenProviderFails checks that a provider error is not fatal:
+// a recent sender whose provider call fails still gets an estimate from the primary backend.
+func TestEthereumTypeEstimateGasFallsBackWhenProviderFails(t *testing.T) {
+	primary, primaryHits := countingEstimateGasServer(t, "0x5208")
+	// a provider server that always errors the JSON-RPC call
+	provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":1,"error":{"code":-32005,"message":"rate limited"}}`))
+	}))
+	t.Cleanup(provider.Close)
+	b := newEstimateGasTestRPC(t, primary.URL, provider.URL)
+
+	sender := ethcommon.HexToAddress("0x2222222222222222222222222222222222222222")
+	b.alternativeSendTxProvider.recentSenders[sender] = recentSender{time: time.Now(), url: provider.URL, gen: 1}
+
+	gas, err := b.EthereumTypeEstimateGas(map[string]interface{}{"from": sender.Hex()})
+	if err != nil {
+		t.Fatalf("EthereumTypeEstimateGas() error = %v", err)
+	}
+	if gas != 0x5208 {
+		t.Fatalf("gas = %#x, want 0x5208 (primary fallback value)", gas)
+	}
+	if got := atomic.LoadInt32(primaryHits); got != 1 {
+		t.Fatalf("primary hits = %d, want 1 (must fall back after provider error)", got)
+	}
+}
+
+// TestEthereumTypeEstimateGasFallsBackWhenProviderReturnsMalformedResult confirms a provider that
+// answers without a transport error but with a non-decodable gas value is treated as a failure:
+// the estimate falls back to the primary backend rather than surfacing the decode error.
+func TestEthereumTypeEstimateGasFallsBackWhenProviderReturnsMalformedResult(t *testing.T) {
+	primary, primaryHits := countingEstimateGasServer(t, "0x5208")
+	// a provider that returns a non-hex string result for eth_estimateGas
+	provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"jsonrpc":"2.0","id":1,"result":"not-a-hex-quantity"}`))
+	}))
+	t.Cleanup(provider.Close)
+	b := newEstimateGasTestRPC(t, primary.URL, provider.URL)
+
+	sender := ethcommon.HexToAddress("0x2222222222222222222222222222222222222222")
+	b.alternativeSendTxProvider.recentSenders[sender] = recentSender{time: time.Now(), url: provider.URL, gen: 1}
+
+	gas, err := b.EthereumTypeEstimateGas(map[string]interface{}{"from": sender.Hex()})
+	if err != nil {
+		t.Fatalf("EthereumTypeEstimateGas() error = %v, want fallback to primary", err)
+	}
+	if gas != 0x5208 {
+		t.Fatalf("gas = %#x, want 0x5208 (primary fallback value)", gas)
+	}
+	if got := atomic.LoadInt32(primaryHits); got != 1 {
+		t.Fatalf("primary hits = %d, want 1 (malformed provider result must fall back)", got)
+	}
+}
+
+// TestEthereumTypeEstimateGasNoFromUsesPrimary confirms an estimate without a sender takes the
+// primary path - the gate cannot apply without a from address.
+func TestEthereumTypeEstimateGasNoFromUsesPrimary(t *testing.T) {
+	primary, primaryHits := countingEstimateGasServer(t, "0x5208")
+	provider, providerHits := countingEstimateGasServer(t, "0x9999")
+	b := newEstimateGasTestRPC(t, primary.URL, provider.URL)
+
+	if _, err := b.EthereumTypeEstimateGas(map[string]interface{}{
+		"to": "0x3333333333333333333333333333333333333333",
+	}); err != nil {
+		t.Fatalf("EthereumTypeEstimateGas() error = %v", err)
+	}
+	if got := atomic.LoadInt32(providerHits); got != 0 {
+		t.Fatalf("provider hits = %d, want 0", got)
+	}
+	if got := atomic.LoadInt32(primaryHits); got != 1 {
+		t.Fatalf("primary hits = %d, want 1", got)
+	}
+}

bchain/coins/eth/ethrpc_mempool_timeout_test.go

@@ -99,6 +99,50 @@ func TestConfigurationAlternativeMempoolTxTimeoutDuration(t *testing.T) {
 	}
 }
 
+// TestMempoolRetentionInverted covers the retention-order check: the provider cache must expire
+// before the wrapped mempool, whose timeout sweep is the only exit that does not clear the cache.
+func TestMempoolRetentionInverted(t *testing.T) {
+	tests := []struct {
+		name        string
+		alternative time.Duration
+		mempool     time.Duration
+		want        bool
+	}{
+		{
+			name:        "defaults are ordered correctly",
+			alternative: defaultAlternativeMempoolTxTimeout,
+			mempool:     defaultMempoolTxTimeoutWithAlternativeProvider,
+			want:        false,
+		},
+		{
+			name:        "cache outliving the mempool is inverted",
+			alternative: 30 * time.Minute,
+			mempool:     10 * time.Minute,
+			want:        true,
+		},
+		{
+			name:        "equal retentions are inverted",
+			alternative: 10 * time.Minute,
+			mempool:     10 * time.Minute,
+			want:        true,
+		},
+		{
+			name:        "a zero mempool retention is inverted",
+			alternative: 5 * time.Minute,
+			mempool:     0,
+			want:        true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := mempoolRetentionInverted(tt.alternative, tt.mempool); got != tt.want {
+				t.Fatalf("mempoolRetentionInverted(%s, %s) = %v, want %v", tt.alternative, tt.mempool, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestNewEthereumRPCRejectsInvalidMempoolTimeouts(t *testing.T) {
 	tests := []struct {
 		name   string