-
Notifications
You must be signed in to change notification settings - Fork 912
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
Regtest fee control #4629
Merged
rustyrussell
merged 3 commits into
ElementsProject:master
from
rustyrussell:guilt/regtest-feecontrol
Jul 8, 2021
Merged
Regtest fee control #4629
rustyrussell
merged 3 commits into
ElementsProject:master
from
rustyrussell:guilt/regtest-feecontrol
Jul 8, 2021
Conversation
This file contains 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
rustyrussell
force-pushed
the
guilt/regtest-feecontrol
branch
from
July 3, 2021 02:47
937dcc5
to
6239684
Compare
vincenzopalazzo
approved these changes
Jul 3, 2021
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.
ack 6239684
Thanks, that sounds perfect for the tests I'm doing. |
ACK 6239684 |
rustyrussell
force-pushed
the
guilt/regtest-feecontrol
branch
from
July 8, 2021 01:20
6239684
to
431bb42
Compare
This has been reported several times on regtest, most recently by Gijs van Dam. It turns out approx_max_feerate() was returning 0 in some corner cases: we should *not* be using that value (as shown, it's overly conservative) except as a ceiling on fee *increases*. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Protocol: don't ever send 0 fee_updates (regtest bug).
Useful for regtest and testnet. Sure, you shouldn't use this on mainnet, but I haven't restricted it because our users are usually pretty clever. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Fixes: ElementsProject#1806 Changelog-Added: config: `force_feerates` option to allow overriding feerate estimates (mainly for regtest).
rustyrussell
force-pushed
the
guilt/regtest-feecontrol
branch
from
July 8, 2021 02:47
431bb42
to
52b1518
Compare
'force-feerates' already bypasses this logic, but we should still suppres Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/regtest-feecontrol
branch
from
July 8, 2021 02:59
52b1518
to
e55363b
Compare
Ack e55363b Rebased and fixed (and commented!) code which makes sure we don't reduce, if we're trying to increase and max says were too high already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Got a report from @gijswijs that we were sending 0 update_fee on regtest; I've seen this complaint before, but this time we fixed it.
Also, @t-bast wanted to override regtest fees: we used to have a dev option which we removed in favor of direct control, I think that was probably overzealous (it's far easier that writing a bcli replacement!). Of course we recommend this be in the regtest-specific config!