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) + } + }) + } +}