Zero fee htlc prep part #3: Block estimates, and the Return of RBF #6120
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
changed the title
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
2 times, most recently
from
ec1f116
to
b04ed0a
Compare
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
b04ed0a
to
5cacaa1
Compare
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
5cacaa1
to
afc1996
Compare
cdecker
approved these changes
Apr 6, 2023
Adding a new field with `added` fails: ``` AssertionError: Field Feerates.perkb.estimates[] does not have an `added` annotation ``` Looks like this assertion is wrong: we should get an added from the field itself or from the .msggen.json file. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Turns out the two bcli replacements I checked (`sauron` and `trustedcoin`) don't even implement this, and the multiplier makes more sense in lightningd, especially as we move to bcli just providing raw feerate estimates. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have tal_arr_remove and tal_arr_append already. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we're messing with feerates, it's good to test this directly upfront. Also, fix documentation! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have the FEERATE_FLOOR constant if you don't care, but usually you want to use the current bitcoind lower limit, so call get_feerate_floor() (which is currently the same, but coming!). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than have specific-purpose levels, have an array of [blockcount, feerate], and rebuild the specific-purpose levels for now on top. We also keep a *separate* smoothed feerate, so you can ask for that explicitly. Since all the plugins used the same formula to derive the different named fee levels, we apply the reverse to return to the underlying estimates: updating the interface comes next. This is ugly for now, but various specific-purpose levels will be going away, as we shift to deadline-driven fees. This temporarily breaks the floor calculation, so that test is disabled. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Drop try_get_feerate() in favor of explicit feerate_for_deadline() and smoothed_feerate_for_deadline(). This shows us everywhere we deal with old-style feerates by names. `delayed_to_us` and `htlc_resolution` will be moving to dynamic fees, so deprecate those. Note that "penalty" is still used for generating penalty txs for watchtowers, and "unilateral_close" still used until we get zero-fee anchors. Changelog-Added: JSON-RPC: `feerates` `estimates` array shows fee estimates by blockcount from underlying plugin (usually *bcli*). Changelog-Changed: JSON-RPC: `close`, `fundchannel`, `fundpsbt`, `multifundchannel`, `multiwithdraw`, `txprepare`, `upgradewallet`, `withdraw` `feerate` (`feerange` for `close`) value *slow* is now 100 block-estimate, not half of 100-block estimate. Changelog-Deprecated: JSON-RPC: `close`, `fundchannel`, `fundpsbt`, `multifundchannel`, `multiwithdraw`, `txprepare`, `upgradewallet`, `withdraw` `feerate` (`feerange` for `close`) expressed as, "delayed_to_us", "htlc_resolution", "max_acceptable" or "min_acceptable". Use explicit block counts or *slow*/*normal*/*urgent*/*minimum*. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And consolidate descriptions into lightning-feerates(). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `close`, `fundchannel`, `fundpsbt`, `multifundchannel`, `multiwithdraw`, `txprepare`, `upgradewallet`, `withdraw` now allow "minimum" and NN"blocks" as `feerate` (`feerange` for `close`).
…meters. Changelog-Added: Plugins: `estimatefees` can return explicit `fee_floor` and `feerates` by block number. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#4473 Changelog-Deprecated: Plugins: `estimatefees` returning feerates by name (e.g. "opening"); use `fee_floor` and `feerates`. Changelog-Fixed: Plugins: `bcli` now tells us the minimal possible feerate, such as with mempool congestion, rather than assuming 1 sat/vbyte.
…will accept Changelog-Added: JSON-RPC: `feerates`: added `floor` field for current minimum feerate bitcoind will accept Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Splitting into onchaind_tx() into onchaind_tx_unsigned() and sign_and_get_witness() makes it easier to reuse for RBF. Adding more information in onchain_signing_info is required too. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would only set it the first time, which was OK for how we were using it before. Now we want to also set it for rebroadcast. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We also do it on every block, but since bitcoind can't always be counted to rebroadcast for us, we might as well be aggressive! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
afc1996
to
bdcd4c2
Compare
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
bdcd4c2
to
3649e15
Compare
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
3649e15
to
52799f7
Compare
Now we've set everything up, the replacement code is quite simple. Some tests now have to deal with RBF though, and our rbf tests need work since they look for the old onchaind messages. In particular, when we can't afford the fee we want, we back off to the next blockcount estimate, rather than spending all on fees (necessarily). So test_penalty_rbf_burn no longer applies. Changelog-Changed: Protocol: spending unilateral close transactions now use dynamic fees based on deadlines (and RBF), instead of fixed fees. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
zero-fee-htlc-prep-3
branch
from
52799f7
to
c31153c
Compare
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.
(Built on #6074 so see "plugins/bcli: move commit-fee (dev-max-fee-multiplier) and into core." onwards)MERGED!We have the idea of a deadline, but our fee backend doesn't give us block estimates. That was a bad mistake, do deprecate the old "for purposes" estimates and return a simpler "for blocks" estimates.
Once that's done, now we RBF again! Not just for penalty txs, but for any tx. The rules are simple:
bcli).