Clamp minimum relay feerate #9106
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ziggie1984
force-pushed
the
fix-min-relay-fee
branch
from
f4baafc to
1255e94
Compare
For the btcd, bitcoind and webapiestimate we clamp down the max min relay feerate to protect the user. When configuring bitcoind for example the user can set the minrelayfee manually and if he sets a vary high value this will have servere consequences for example for the commitment fee rates. The default max minrelay fee rate is set to 200 sat/vbyte but it is also configurable to react to potential changes in the mempool or in the local node setup (using a very small mempool size).
ziggie1984
force-pushed
the
fix-min-relay-fee
branch
from
1255e94 to
fbd6e42
Compare
ziggie1984
added
the
fees
There was a problem hiding this comment.
While this fixed #9048, it seems to create a potential new issue - when setting the new config maxminrelayfeerate, I'd tend to set it to a larger value, just in case the fee suddenly spikes. Then it's a question of how large this value should be set, which will end up using the same rationalities when setting sweeper.maxfeerate.
Looking back at issue #9048, if this global max feerate config were taking effect it should have not happened. So my question is how about we keep it simple and use this maxfeerate config only?
| 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."` |
There was a problem hiding this comment.
think we can call this MaxRelayFeeRate
| return fmt.Errorf("max-min-relay-feerate must be at least 10") | ||
| } | ||
|
|
||
| if f.FallbackFeeRate < 10 { |
There was a problem hiding this comment.
hmmm think this may be too large
|
|
||
| // Clamp the minimum relay fee rate. | ||
| if minRelayFee > w.maxMinRelayFeeRate { | ||
| minRelayFee = w.maxMinRelayFeeRate |
There was a problem hiding this comment.
think we need a warning log here
| // high config setting scenarios in the bitcoin.conf file | ||
| // (minrelaytxfee). | ||
| if newMinFee > m.maxMinRelayFeePerKW { | ||
| newMinFee = m.maxMinRelayFeePerKW |
There was a problem hiding this comment.
same here - a warning log can help users identify this case
Avoids misconfigured backends seen in #9048.
Make the maximum configurable so users can change it and adopt it to different mempool conditions.
While add it, also makes the FallBackFeeRate configurable.
Introduces this limit for all estimators except the
StaticFeeEstimatorbecause there the relay fee rate is set manually and I don't see a benefit clamping it in this case.