-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Clamp minimum relay feerate #9106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ziggie1984
wants to merge
2
commits into
lightningnetwork:master
Choose a base branch
from
ziggie1984:fix-min-relay-fee
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmmm think this may be too large |
||
return fmt.Errorf("fallback-feerate must be at least 10") | ||
} | ||
|
||
return nil | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,15 +773,19 @@ 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 | ||
} | ||
|
||
// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. think we need a warning log here |
||
} | ||
|
||
return minRelayFee | ||
} | ||
|
||
// randomFeeUpdateTimeout returns a random timeout between minFeeUpdateTimeout | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think we can call this
MaxRelayFeeRate