From c11e81699ecfd88811fa398b9c56c513200d22c6 Mon Sep 17 00:00:00 2001 From: Leonardo Saturnino Date: Sat, 6 Jun 2026 02:21:02 -0300 Subject: [PATCH] fix(electrum): gate low-fee estimate fallback to non-mainnet networks When the Electrum fee oracle returns no estimate for any confirmation target, EstimateSatPerVByteFee returned a hardcoded 2 sat/vByte for every network. On mainnet this turns a fail-safe (error, no broadcast) into a fail-broadcast: the tBTC sweep and redemption fee paths build and broadcast transactions at a feerate that can be left unconfirmable or evicted under congestion, stalling sweeps and redemptions. Gate the fallback to test networks (testnet, testnet4, regtest) via a new Electrum Config.Network field resolved from the client configuration. On mainnet, and on any unset or unknown network, the oracle failure is surfaced as an error so callers do not broadcast at a guessed feerate. The field is tagged mapstructure:"-" and set after config unmarshal, so a config key cannot re-enable the fallback on mainnet. Add a unit test covering the fallback decision per network and propagate the resolved network into the Electrum integration test configs. --- config/electrum.go | 9 ++ config/electrum_test.go | 11 ++- pkg/bitcoin/electrum/config.go | 12 ++- pkg/bitcoin/electrum/electrum.go | 66 +++++++++++-- .../electrum/electrum_integration_test.go | 13 +++ pkg/bitcoin/electrum/electrum_test.go | 92 +++++++++++++++++++ 6 files changed, 189 insertions(+), 14 deletions(-) diff --git a/config/electrum.go b/config/electrum.go index b3e9b20847..2d3b763e4c 100644 --- a/config/electrum.go +++ b/config/electrum.go @@ -34,6 +34,15 @@ func readElectrumUrls(network bitcoin.Network) ( func (c *Config) resolveElectrum(rng *rand.Rand) error { network := c.Bitcoin.Network + // Propagate the resolved Bitcoin network into the Electrum config so the + // client can gate network-sensitive behavior. The Electrum fee-estimate + // fallback (a fixed low feerate used when the fee oracle is unavailable) is + // only safe on test networks; on mainnet an underpriced transaction can be + // left unconfirmable or evicted. This is set from the resolved network and + // never from the config file (the field is `mapstructure:"-"`), so a config + // key cannot re-enable the fallback on mainnet. + c.Bitcoin.Electrum.Network = network + // Return if Electrum is already set. if len(c.Bitcoin.Electrum.URL) > 0 { return nil diff --git a/config/electrum_test.go b/config/electrum_test.go index 8f8d2d3ca3..30b9166fee 100644 --- a/config/electrum_test.go +++ b/config/electrum_test.go @@ -19,21 +19,24 @@ func TestResolveElectrum(t *testing.T) { bitcoin.Mainnet: { expectedConfig: []electrum.Config{ { - URL: "wss://electrum.boar.network:2083", + URL: "wss://electrum.boar.network:2083", + Network: bitcoin.Mainnet, }, }, }, bitcoin.Testnet: { expectedConfig: []electrum.Config{ { - URL: "wss://electrum.testnet.boar.network:443/QxbJgaSLUHqrgAa9BW7bDpnGPxrlhnCa", + URL: "wss://electrum.testnet.boar.network:443/QxbJgaSLUHqrgAa9BW7bDpnGPxrlhnCa", + Network: bitcoin.Testnet, }, }, }, bitcoin.Testnet4: { expectedConfig: []electrum.Config{ { - URL: "ssl://mempool.space:40002", + URL: "ssl://mempool.space:40002", + Network: bitcoin.Testnet4, }, }, }, @@ -42,6 +45,7 @@ func TestResolveElectrum(t *testing.T) { { URL: "", KeepAliveInterval: 0, + Network: bitcoin.Regtest, }, }, }, @@ -50,6 +54,7 @@ func TestResolveElectrum(t *testing.T) { { URL: "", KeepAliveInterval: 0, + Network: bitcoin.Unknown, }, }, }, diff --git a/pkg/bitcoin/electrum/config.go b/pkg/bitcoin/electrum/config.go index 43942c933b..590378736b 100644 --- a/pkg/bitcoin/electrum/config.go +++ b/pkg/bitcoin/electrum/config.go @@ -1,6 +1,10 @@ package electrum -import "time" +import ( + "time" + + "github.com/keep-network/keep-core/pkg/bitcoin" +) const ( // DefaultConnectTimeout is a default timeout used for a single attempt of @@ -36,4 +40,10 @@ type Config struct { // An Electrum server may disconnect clients that have not sent any requests // for roughly 10 minutes. KeepAliveInterval time.Duration + // Network is the Bitcoin network this connection operates on. It is set + // internally from the resolved client configuration, not decoded from the + // config file (`mapstructure:"-"`), so it cannot be used to alter + // network-sensitive client behavior such as the low-fee estimate fallback. + // A zero value (bitcoin.Unknown) disables that fallback (fail-safe). + Network bitcoin.Network `mapstructure:"-"` } diff --git a/pkg/bitcoin/electrum/electrum.go b/pkg/bitcoin/electrum/electrum.go index f40c4f935c..db38d5993b 100644 --- a/pkg/bitcoin/electrum/electrum.go +++ b/pkg/bitcoin/electrum/electrum.go @@ -984,8 +984,11 @@ func feeEstimateWithFallbackTargets(primary uint32) []uint32 { // defaultFallbackSatPerVByteWhenEstimateFails is used when Electrum cannot // return a fee for any confirmation target (typical on testnet4 / quiet -// mempools: -32603 for all N). Relay policy still accepts low feerates; -// deposit sweep max-fee checks on the Bridge bound the total fee. +// mempools: -32603 for all N). It is only applied on test networks (see +// lowFeeFallbackAllowed): on mainnet a fixed low feerate can leave a +// transaction unconfirmable or evicted under congestion, so the oracle failure +// is surfaced as an error (fail-safe) rather than broadcast at a guessed +// feerate. const defaultFallbackSatPerVByteWhenEstimateFails int64 = 2 // isElectrumFeeOracleFailure reports whether the error is the usual @@ -1062,16 +1065,59 @@ func (c *Connection) EstimateSatPerVByteFee(blocks uint32) (int64, error) { } if sawFeeOracleFailure { - logger.Warnf( - "Electrum returned no fee estimate for any target %v; using "+ - "fallback [%d] sat/vbyte (last error: [%v])", - targets, - defaultFallbackSatPerVByteWhenEstimateFails, - lastErr, - ) - return defaultFallbackSatPerVByteWhenEstimateFails, nil + if lowFeeFallbackAllowed(c.config.Network) { + logger.Warnf( + "Electrum returned no fee estimate for any target %v on [%v] "+ + "network; using fallback [%d] sat/vbyte (last error: [%v])", + targets, + c.config.Network, + defaultFallbackSatPerVByteWhenEstimateFails, + lastErr, + ) + } else { + logger.Warnf( + "Electrum returned no fee estimate for any target %v on [%v] "+ + "network; low-fee fallback is not permitted on this network, "+ + "failing safe (last error: [%v])", + targets, + c.config.Network, + lastErr, + ) + } + } + + return feeFallbackResult(c.config.Network, sawFeeOracleFailure, lastErr, targets) +} + +// lowFeeFallbackAllowed reports whether the hardcoded low-fee estimate fallback +// is acceptable for the given Bitcoin network. It is permitted only on test +// networks, where an underpriced transaction has no real economic consequence. +// Mainnet and any unset/unrecognized network fail closed, so an oracle failure +// is surfaced as an error rather than broadcast at a guessed feerate. +func lowFeeFallbackAllowed(network bitcoin.Network) bool { + switch network { + case bitcoin.Testnet, bitcoin.Testnet4, bitcoin.Regtest: + return true + default: + return false } +} +// feeFallbackResult resolves the result of EstimateSatPerVByteFee when no +// Electrum fee estimate could be obtained for any confirmation target. On a +// fee-oracle failure it returns the low-fee fallback only where +// lowFeeFallbackAllowed permits it; otherwise (mainnet, an unset network, or a +// transport-level failure) it returns an error so the caller does not broadcast +// a transaction at a guessed feerate. +func feeFallbackResult( + network bitcoin.Network, + sawFeeOracleFailure bool, + lastErr error, + targets []uint32, +) (int64, error) { + if sawFeeOracleFailure && lowFeeFallbackAllowed(network) { + return defaultFallbackSatPerVByteWhenEstimateFails, nil + } if lastErr != nil { return 0, fmt.Errorf("failed to get fee: [%v]", lastErr) } diff --git a/pkg/bitcoin/electrum/electrum_integration_test.go b/pkg/bitcoin/electrum/electrum_integration_test.go index 6da4d4c4c0..e3d3c8680b 100644 --- a/pkg/bitcoin/electrum/electrum_integration_test.go +++ b/pkg/bitcoin/electrum/electrum_integration_test.go @@ -42,6 +42,19 @@ type testConfig struct { network bitcoin.Network } +// init propagates each test config's Bitcoin network into its Electrum +// connection config. In production the network is injected during config +// resolution; mirroring that here ensures the integration tests exercise the +// same network-gated behavior (e.g. the low-fee estimate fallback) instead of +// leaving Config.Network at its zero value (bitcoin.Unknown), which would +// disable the fallback. +func init() { + for key, tc := range testConfigs { + tc.clientConfig.Network = tc.network + testConfigs[key] = tc + } +} + // Servers details were taken from a public Electrum servers list published // at https://1209k.com/bitcoin-eye/ele.php?chain=tbtc. var testConfigs = map[string]testConfig{ diff --git a/pkg/bitcoin/electrum/electrum_test.go b/pkg/bitcoin/electrum/electrum_test.go index 2b09742a2d..b09d03ec51 100644 --- a/pkg/bitcoin/electrum/electrum_test.go +++ b/pkg/bitcoin/electrum/electrum_test.go @@ -1,10 +1,12 @@ package electrum import ( + "fmt" "reflect" "testing" "github.com/keep-network/keep-core/internal/testutils" + "github.com/keep-network/keep-core/pkg/bitcoin" ) func TestFeeEstimateWithFallbackTargets(t *testing.T) { @@ -96,3 +98,93 @@ func TestConvertBtcKbToSatVByte(t *testing.T) { }) } } + +func TestFeeFallbackResult(t *testing.T) { + t.Parallel() + + oracleFailure := fmt.Errorf("cannot estimate fee") + transportFailure := fmt.Errorf("request failed: [connection refused]") + targets := []uint32{1, 6, 25} + + for _, tc := range []struct { + name string + network bitcoin.Network + sawFeeOracleFailure bool + lastErr error + wantFee int64 + wantErr bool + }{ + { + name: "mainnet oracle failure fails safe", + network: bitcoin.Mainnet, + sawFeeOracleFailure: true, + lastErr: oracleFailure, + wantErr: true, + }, + { + name: "unknown network oracle failure fails safe", + network: bitcoin.Unknown, + sawFeeOracleFailure: true, + lastErr: oracleFailure, + wantErr: true, + }, + { + name: "testnet4 oracle failure uses fallback", + network: bitcoin.Testnet4, + sawFeeOracleFailure: true, + lastErr: oracleFailure, + wantFee: defaultFallbackSatPerVByteWhenEstimateFails, + }, + { + name: "testnet oracle failure uses fallback", + network: bitcoin.Testnet, + sawFeeOracleFailure: true, + lastErr: oracleFailure, + wantFee: defaultFallbackSatPerVByteWhenEstimateFails, + }, + { + name: "regtest oracle failure uses fallback", + network: bitcoin.Regtest, + sawFeeOracleFailure: true, + lastErr: oracleFailure, + wantFee: defaultFallbackSatPerVByteWhenEstimateFails, + }, + { + name: "testnet4 transport failure does not use fallback", + network: bitcoin.Testnet4, + sawFeeOracleFailure: false, + lastErr: transportFailure, + wantErr: true, + }, + { + name: "mainnet transport failure errors", + network: bitcoin.Mainnet, + sawFeeOracleFailure: false, + lastErr: transportFailure, + wantErr: true, + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + fee, err := feeFallbackResult( + tc.network, + tc.sawFeeOracleFailure, + tc.lastErr, + targets, + ) + if tc.wantErr { + if err == nil { + t.Fatalf("expected an error, got fee [%d]", fee) + } + return + } + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if fee != tc.wantFee { + t.Fatalf("expected fee [%d], got [%d]", tc.wantFee, fee) + } + }) + } +}