diff --git a/chainreg/chainregistry.go b/chainreg/chainregistry.go index 41a2fcbb7f..4afd3c4f16 100644 --- a/chainreg/chainregistry.go +++ b/chainreg/chainregistry.go @@ -400,10 +400,16 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { // if we're using bitcoind as a backend, then we can // use live fee estimates, rather than a statically // coded value. - fallBackFeeRate := chainfee.SatPerKVByte(25 * 1000) cc.FeeEstimator, err = chainfee.NewBitcoindEstimator( *rpcConfig, bitcoindMode.EstimateMode, - fallBackFeeRate.FeePerKWeight(), + chainfee.EstimatorConfig{ + FallbackFeePerKW: chainfee.SatPerVByte( + cfg.Fee.FallbackFeeRate, + ).FeePerKWeight(), + MaxMinRelayFeePerKW: chainfee.SatPerVByte( + cfg.Fee.MaxMinRelayFeeRate, + ).FeePerKWeight(), + }, ) if err != nil { return nil, nil, err @@ -665,9 +671,15 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { // if we're using btcd as a backend, then we can use // live fee estimates, rather than a statically coded // value. - fallBackFeeRate := chainfee.SatPerKVByte(25 * 1000) cc.FeeEstimator, err = chainfee.NewBtcdEstimator( - *rpcConfig, fallBackFeeRate.FeePerKWeight(), + *rpcConfig, chainfee.EstimatorConfig{ + FallbackFeePerKW: chainfee.SatPerVByte( + cfg.Fee.FallbackFeeRate, + ).FeePerKWeight(), + MaxMinRelayFeePerKW: chainfee.SatPerVByte( + cfg.Fee.MaxMinRelayFeeRate, + ).FeePerKWeight(), + }, ) if err != nil { return nil, nil, err @@ -725,6 +737,8 @@ func NewPartialChainControl(cfg *Config) (*PartialChainControl, func(), error) { !cacheFees, cfg.Fee.MinUpdateTimeout, cfg.Fee.MaxUpdateTimeout, + chainfee.SatPerVByte(cfg.Fee.MaxMinRelayFeeRate). + FeePerKWeight(), ) if err != nil { return nil, nil, err diff --git a/config.go b/config.go index 34d84720cb..f5890690c7 100644 --- a/config.go +++ b/config.go @@ -598,8 +598,10 @@ func DefaultConfig() Config { ConnectionTimeout: tor.DefaultConnTimeout, Fee: &lncfg.Fee{ - MinUpdateTimeout: lncfg.DefaultMinUpdateTimeout, - MaxUpdateTimeout: lncfg.DefaultMaxUpdateTimeout, + MinUpdateTimeout: lncfg.DefaultMinUpdateTimeout, + MaxUpdateTimeout: lncfg.DefaultMaxUpdateTimeout, + MaxMinRelayFeeRate: lncfg.DefaultMaxMinRelayFeeRate, + FallbackFeeRate: lncfg.DefaultFallbackFeeRate, }, SubRPCServers: &subRPCServerConfigs{ diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 919d25aa49..8a3b3ef172 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -36,6 +36,9 @@ # Improvements ## Functional Updates +* [Clamp the min relay fee rate](https://github.com/lightningnetwork/lnd/pull/9106) + to prevent misconfigured bitcoin backends from causing too high fee estimates. + ## RPC Updates ## lncli Updates diff --git a/lncfg/fee.go b/lncfg/fee.go index 200ae9e819..c56caeab13 100644 --- a/lncfg/fee.go +++ b/lncfg/fee.go @@ -1,20 +1,56 @@ package lncfg -import "time" +import ( + "fmt" + "time" +) -// DefaultMinUpdateTimeout represents the minimum interval in which a -// WebAPIEstimator will request fresh fees from its API. -const DefaultMinUpdateTimeout = 5 * time.Minute +const ( + // DefaultMinUpdateTimeout represents the minimum interval in which a + // WebAPIEstimator will request fresh fees from its API. + DefaultMinUpdateTimeout = 5 * time.Minute -// DefaultMaxUpdateTimeout represents the maximum interval in which a -// WebAPIEstimator will request fresh fees from its API. -const DefaultMaxUpdateTimeout = 20 * time.Minute + // DefaultMaxUpdateTimeout represents the maximum interval in which a + // WebAPIEstimator will request fresh fees from its API. + DefaultMaxUpdateTimeout = 20 * time.Minute + + // DefaultMaxMinRelayFeeRate is the default maximum minimum relay fee + // rate in sat/vbyte. + DefaultMaxMinRelayFeeRate = 200 + + // DefaultFallbackFeeRate is the default fallback fee rate in sat/vbyte + // that will be used if the fee estimation method fails. + DefaultFallbackFeeRate = 25 +) // Fee holds the configuration options for fee estimation. // //nolint:lll type Fee struct { - URL string `long:"url" description:"Optional URL for external fee estimation. If no URL is specified, the method for fee estimation will depend on the chosen backend and network. Must be set for neutrino on mainnet."` - MinUpdateTimeout time.Duration `long:"min-update-timeout" description:"The minimum interval in which fees will be updated from the specified fee URL."` - MaxUpdateTimeout time.Duration `long:"max-update-timeout" description:"The maximum interval in which fees will be updated from the specified fee URL."` + URL string `long:"url" description:"Optional URL for external fee estimation. If no URL is specified, the method for fee estimation will depend on the chosen backend and network. Must be set for neutrino on mainnet."` + MinUpdateTimeout time.Duration `long:"min-update-timeout" description:"The minimum interval in which fees will be updated from the specified fee URL."` + MaxUpdateTimeout time.Duration `long:"max-update-timeout" description:"The maximum interval in which fees will be updated from the specified fee URL."` + MaxMinRelayFeeRate uint64 `long:"max-min-relay-feerate" description:"The maximum relay fee rate in sat/vbyte that will be used although the backend would return a higher fee rate. Must be large enough to ensure transaction propagation in a croweded mempool environment."` + FallbackFeeRate uint64 `long:"fallback-feerate" description:"The fee rate in sat/vbyte that will be used if the fee estimation method fails."` +} + +// Validate checks the values configured for the fee estimator. +func (f *Fee) Validate() error { + if f.MinUpdateTimeout < 0 { + return fmt.Errorf("min-update-timeout must be positive") + } + + if f.MaxUpdateTimeout < 0 { + return fmt.Errorf("max-update-timeout must be positive") + } + + if f.MaxMinRelayFeeRate < 10 { + return fmt.Errorf("max-min-relay-feerate must be at least 10") + } + + if f.FallbackFeeRate < 10 { + return fmt.Errorf("fallback-feerate must be at least 10") + } + + return nil } diff --git a/lnwallet/chainfee/estimator.go b/lnwallet/chainfee/estimator.go index 6f59c3f7f5..7a301fc298 100644 --- a/lnwallet/chainfee/estimator.go +++ b/lnwallet/chainfee/estimator.go @@ -58,6 +58,12 @@ var ( errEmptyCache = errors.New("fee rate cache is empty") ) +// EstimatorConfig is the configuration for the fee estimator. +type EstimatorConfig struct { + MaxMinRelayFeePerKW SatPerKWeight + FallbackFeePerKW SatPerKWeight +} + // Estimator provides the ability to estimate on-chain transaction fees for // various combinations of transaction sizes and desired confirmation time // (measured by number of blocks). @@ -147,6 +153,10 @@ type BtcdEstimator struct { // produce fee estimates. fallbackFeePerKW SatPerKWeight + // maxMinRelayFeePerKW is the maximum minimum relay fee rate in sat/kw + // that we will use. + maxMinRelayFeePerKW SatPerKWeight + // minFeeManager is used to query the current minimum fee, in sat/kw, // that we should enforce. This will be used to determine fee rate for // a transaction when the estimated fee rate is too low to allow the @@ -167,7 +177,7 @@ type BtcdEstimator struct { // the occasion that the estimator has insufficient data, or returns zero for a // fee estimate. func NewBtcdEstimator(rpcConfig rpcclient.ConnConfig, - fallBackFeeRate SatPerKWeight) (*BtcdEstimator, error) { + cfg EstimatorConfig) (*BtcdEstimator, error) { rpcConfig.DisableConnectOnNew = true rpcConfig.DisableAutoReconnect = false @@ -181,9 +191,10 @@ func NewBtcdEstimator(rpcConfig rpcclient.ConnConfig, } return &BtcdEstimator{ - fallbackFeePerKW: fallBackFeeRate, - btcdConn: chainConn, - filterManager: newFilterManager(fetchCb), + fallbackFeePerKW: cfg.FallbackFeePerKW, + maxMinRelayFeePerKW: cfg.MaxMinRelayFeePerKW, + btcdConn: chainConn, + filterManager: newFilterManager(fetchCb), }, nil } @@ -200,7 +211,8 @@ func (b *BtcdEstimator) Start() error { // can initialise the minimum relay fee manager which queries the // chain backend for the minimum relay fee on construction. minRelayFeeManager, err := newMinFeeManager( - defaultUpdateInterval, b.fetchMinRelayFee, + defaultUpdateInterval, b.maxMinRelayFeePerKW, + b.fetchMinRelayFee, ) if err != nil { return err @@ -340,6 +352,10 @@ type BitcoindEstimator struct { // produce fee estimates. fallbackFeePerKW SatPerKWeight + // maxMinRelayFeePerKW is the maximum minimum relay fee rate in sat/kw + // that we will use. + maxMinRelayFeePerKW SatPerKWeight + // minFeeManager is used to keep track of the minimum fee, in sat/kw, // that we should enforce. This will be used as the default fee rate // for a transaction when the estimated fee rate is too low to allow @@ -367,7 +383,7 @@ type BitcoindEstimator struct { // in the occasion that the estimator has insufficient data, or returns zero // for a fee estimate. func NewBitcoindEstimator(rpcConfig rpcclient.ConnConfig, feeMode string, - fallBackFeeRate SatPerKWeight) (*BitcoindEstimator, error) { + cfg EstimatorConfig) (*BitcoindEstimator, error) { rpcConfig.DisableConnectOnNew = true rpcConfig.DisableAutoReconnect = false @@ -383,10 +399,11 @@ func NewBitcoindEstimator(rpcConfig rpcclient.ConnConfig, feeMode string, } return &BitcoindEstimator{ - fallbackFeePerKW: fallBackFeeRate, - bitcoindConn: chainConn, - feeMode: feeMode, - filterManager: newFilterManager(fetchCb), + fallbackFeePerKW: cfg.FallbackFeePerKW, + maxMinRelayFeePerKW: cfg.MaxMinRelayFeePerKW, + bitcoindConn: chainConn, + feeMode: feeMode, + filterManager: newFilterManager(fetchCb), }, nil } @@ -399,7 +416,7 @@ func (b *BitcoindEstimator) Start() error { // initialise the minimum relay fee manager which will query // the backend node for its minimum mempool fee. relayFeeManager, err := newMinFeeManager( - defaultUpdateInterval, + defaultUpdateInterval, b.maxMinRelayFeePerKW, b.fetchMinMempoolFee, ) if err != nil { @@ -756,6 +773,10 @@ type WebAPIEstimator struct { // web estimator will request fresh fees from its API. maxFeeUpdateTimeout time.Duration + // maxMinRelayFeeRate is the maximum minimum relay fee rate which will + // be used. + maxMinRelayFeeRate SatPerKWeight + quit chan struct{} wg sync.WaitGroup } @@ -763,8 +784,8 @@ type WebAPIEstimator struct { // NewWebAPIEstimator creates a new WebAPIEstimator from a given URL and a // fallback default fee. The fees are updated whenever a new block is mined. func NewWebAPIEstimator(api WebAPIFeeSource, noCache bool, - minFeeUpdateTimeout time.Duration, - maxFeeUpdateTimeout time.Duration) (*WebAPIEstimator, error) { + minFeeUpdateTimeout time.Duration, maxFeeUpdateTimeout time.Duration, + maxMinRelayFeeRate SatPerKWeight) (*WebAPIEstimator, error) { if minFeeUpdateTimeout == 0 || maxFeeUpdateTimeout == 0 { return nil, fmt.Errorf("minFeeUpdateTimeout and " + @@ -784,6 +805,7 @@ func NewWebAPIEstimator(api WebAPIFeeSource, noCache bool, quit: make(chan struct{}), minFeeUpdateTimeout: minFeeUpdateTimeout, maxFeeUpdateTimeout: maxFeeUpdateTimeout, + maxMinRelayFeeRate: maxMinRelayFeeRate, }, nil } @@ -911,7 +933,14 @@ func (w *WebAPIEstimator) RelayFeePerKW() SatPerKWeight { log.Infof("Web API returning %v for min relay feerate", w.minRelayFeerate) - return w.minRelayFeerate.FeePerKWeight() + minRelayFee := w.minRelayFeerate.FeePerKWeight() + + // Clamp the minimum relay fee rate. + if minRelayFee > w.maxMinRelayFeeRate { + minRelayFee = w.maxMinRelayFeeRate + } + + return minRelayFee } // randomFeeUpdateTimeout returns a random timeout between minFeeUpdateTimeout diff --git a/lnwallet/chainfee/estimator_test.go b/lnwallet/chainfee/estimator_test.go index 3d355d5034..18bca1ccaf 100644 --- a/lnwallet/chainfee/estimator_test.go +++ b/lnwallet/chainfee/estimator_test.go @@ -188,6 +188,9 @@ func TestWebAPIFeeEstimator(t *testing.T) { minFeeUpdateTimeout = 5 * time.Minute maxFeeUpdateTimeout = 20 * time.Minute + + // Equivalent to 200 sat/vbyte. + maxMinRelayFeeRate SatPerKWeight = 50000 ) testCases := []struct { @@ -250,6 +253,7 @@ func TestWebAPIFeeEstimator(t *testing.T) { estimator, _ := NewWebAPIEstimator( feeSource, false, minFeeUpdateTimeout, maxFeeUpdateTimeout, + maxMinRelayFeeRate, ) // Test that when the estimator is not started, an error is returned. @@ -301,11 +305,15 @@ func TestGetCachedFee(t *testing.T) { minFeeUpdateTimeout = 5 * time.Minute maxFeeUpdateTimeout = 20 * time.Minute + + // Equivalent to 200 sat/vbyte. + maxMinRelayFeeRate SatPerKWeight = 50000 ) // Create a dummy estimator without WebAPIFeeSource. estimator, _ := NewWebAPIEstimator( nil, false, minFeeUpdateTimeout, maxFeeUpdateTimeout, + maxMinRelayFeeRate, ) // When the cache is empty, an error should be returned. @@ -378,10 +386,14 @@ func TestRandomFeeUpdateTimeout(t *testing.T) { var ( minFeeUpdateTimeout = 1 * time.Minute maxFeeUpdateTimeout = 2 * time.Minute + + // Equivalent to 200 sat/vbyte. + maxMinRelayFeeRate SatPerKWeight = 50000 ) estimator, _ := NewWebAPIEstimator( nil, false, minFeeUpdateTimeout, maxFeeUpdateTimeout, + maxMinRelayFeeRate, ) for i := 0; i < 1000; i++ { @@ -398,10 +410,14 @@ func TestInvalidFeeUpdateTimeout(t *testing.T) { var ( minFeeUpdateTimeout = 2 * time.Minute maxFeeUpdateTimeout = 1 * time.Minute + + // Equivalent to 200 sat/vbyte. + maxMinRelayFeeRate SatPerKWeight = 50000 ) _, err := NewWebAPIEstimator( nil, false, minFeeUpdateTimeout, maxFeeUpdateTimeout, + maxMinRelayFeeRate, ) require.Error(t, err, "NewWebAPIEstimator should return an error "+ "when minFeeUpdateTimeout > maxFeeUpdateTimeout") diff --git a/lnwallet/chainfee/minfeemanager.go b/lnwallet/chainfee/minfeemanager.go index 7eaef3e8b5..bf2baf8c03 100644 --- a/lnwallet/chainfee/minfeemanager.go +++ b/lnwallet/chainfee/minfeemanager.go @@ -5,17 +5,20 @@ import ( "time" ) -const defaultUpdateInterval = 10 * time.Minute +const ( + defaultUpdateInterval = 10 * time.Minute +) // minFeeManager is used to store and update the minimum fee that is required // by a transaction to be accepted to the mempool. The minFeeManager ensures // that the backend used to fetch the fee is not queried too regularly. type minFeeManager struct { - mu sync.Mutex - minFeePerKW SatPerKWeight - lastUpdatedTime time.Time - minUpdateInterval time.Duration - fetchFeeFunc fetchFee + mu sync.Mutex + minFeePerKW SatPerKWeight + maxMinRelayFeePerKW SatPerKWeight + lastUpdatedTime time.Time + minUpdateInterval time.Duration + fetchFeeFunc fetchFee } // fetchFee represents a function that can be used to fetch a fee. @@ -25,6 +28,7 @@ type fetchFee func() (SatPerKWeight, error) // given fetchMinFee function to set the minFeePerKW of the minFeeManager. // This function requires the fetchMinFee function to succeed. func newMinFeeManager(minUpdateInterval time.Duration, + maxMinRelayFeePerKW SatPerKWeight, fetchMinFee fetchFee) (*minFeeManager, error) { minFee, err := fetchMinFee() @@ -39,10 +43,11 @@ func newMinFeeManager(minUpdateInterval time.Duration, } return &minFeeManager{ - minFeePerKW: minFee, - lastUpdatedTime: time.Now(), - minUpdateInterval: minUpdateInterval, - fetchFeeFunc: fetchMinFee, + minFeePerKW: minFee, + maxMinRelayFeePerKW: maxMinRelayFeePerKW, + lastUpdatedTime: time.Now(), + minUpdateInterval: minUpdateInterval, + fetchFeeFunc: fetchMinFee, }, nil } @@ -66,6 +71,13 @@ func (m *minFeeManager) fetchMinFee() SatPerKWeight { return m.minFeePerKW } + // Clamp the minimum relay fee rate. This also protects against + // high config setting scenarios in the bitcoin.conf file + // (minrelaytxfee). + if newMinFee > m.maxMinRelayFeePerKW { + newMinFee = m.maxMinRelayFeePerKW + } + // By default, we'll use the backend node's minimum fee as the // minimum fee rate we'll propose for transactions. However, if this // happens to be lower than our fee floor, we'll enforce that instead. diff --git a/lnwallet/chainfee/minfeemanager_test.go b/lnwallet/chainfee/minfeemanager_test.go index b498e7962e..7594bf32ae 100644 --- a/lnwallet/chainfee/minfeemanager_test.go +++ b/lnwallet/chainfee/minfeemanager_test.go @@ -33,6 +33,8 @@ func TestMinFeeManager(t *testing.T) { // once. feeManager, err := newMinFeeManager( 100*time.Millisecond, + // 200 sat/vbyte + SatPerKWeight(50000), chainBackend.fetchFee, ) require.NoError(t, err) diff --git a/sample-lnd.conf b/sample-lnd.conf index c9fb0e4f8c..70a5cfe4cf 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -547,6 +547,14 @@ ; The maximum interval in which fees will be updated from the specified fee URL. ; fee.max-update-timeout=20m +; The maximum relay fee rate cap in sat/vbyte that will be used. Must be large +; enough to ensure transaction propagation in a croweded mempool environment. +;fee.max-min-relay-feerate=200 + +; The fee rate in sat/vbyte that will be used if the fee estimation method +; fails or is not available. +;fee.fallback-feerate=25 + [prometheus]