Anchor with zero fee htlc support (aka. experimental-anchors) #6334
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
added
protocol
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
8df044a
to
6a2ce0b
Compare
|
master rebase, parameter renaming inspired by @cdecker, and moved hsm capability check to when they set --experimental-anchor for much cleaner failure! |
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
6a2ce0b
to
5b7afb1
Compare
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
5b7afb1
to
024bd11
Compare
|
Trivial rebase on master (which has CI fixes) |
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
024bd11
to
698e535
Compare
|
Now back on top of master, as that has been merged. |
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
4 times, most recently
from
046a549
to
1f45398
Compare
|
Rebased on master again, with proper rebuild of the generated python files. This time for sure! |
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
1f45398
to
c6f4e80
Compare
|
... elements test fixes (mainly, disabling some anchor + elements tests for now: I don't want to recalc all the tx sizes!). |
Otherwise a badly-timed listpeerchannels will crash. Signed-off-by: Rusty Russell <[email protected]>
It's weird to add new options into the deprecated section. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
We were setting the wrong input number: don't assume it's the same as the UTXO number, but simply the last-appended input. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Reported-by: @19710405 on GitHub Fixes: ElementsProject#6283 Signed-off-by: Rusty Russell <[email protected]>
…ro if we support anchors. It depends on whether we negotiated anchors: just document that this field doesn't apply for anchors (it becomes zero by the end of this patch series, which is weird). Signed-off-by: Rusty Russell <[email protected]>
Interesting we didn't actually test that the feerate we use is actually delivered. Signed-off-by: Rusty Russell <[email protected]>
This is kind of a withdrawl to ourselves, except we also spend a channel to-local anchor. Signed-off-by: Rusty Russell <[email protected]>
Turns out it's a single sig, identical to the already-handled case where we spend a to_remote output. We also close a temporary memleak: stack was unused, but tallocated off the psbt, so it lives as long as the PSBT. Signed-off-by: Rusty Russell <[email protected]>
…feerate. mfc->feerate_str is *never* NULL, since we set it in getfeerate; this is confusing, as many places check for NULL. Indeed, the logic in perform_fundpsbt() was *wrong* in this case: it used `normal` (if it was NULL, which it never was) instead of `opening` to fundpsbt. And the correct thing is for multifundchannel to not use a string here at all, but to use the exact feerate it is counting on (even the same string may have different values now if a block has come in). Signed-off-by: Rusty Russell <[email protected]>
Since we can CPFP, we don't have to track the feerate as closely. But it still needs to get in the mempool, so we use 10 sat/byte, or the 100 block estimate if that is higher. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: JSON-RPC: `feerates` has new fields `unilateral_anchor_close` to show the feerate used for anchor channels (currently experimental), and `unilateral_close_nonanchor_satoshis`. Changelog-Changed: JSON-RPC: `feerates` `unilateral_close_satoshis` now assumes anchor channels if enabled (currently experimental).
We need to know both, because in theory we could negotiate a non-anchor channel even if they support it. Signed-off-by: Rusty Russell <[email protected]>
We need to know both, because in theory we could negotiate a non-anchor channel even if they support it. Signed-off-by: Rusty Russell <[email protected]>
…low. Signed-off-by: Rusty Russell <[email protected]>
Since HTLC txs when using anchors are SIGHASH_SINGLE|SIGHASH_ANYONECANPAY, we can attach other inputs to give it a higher feerate. But we need the HSMd to actually sign the combo. Signed-off-by: Rusty Russell <[email protected]>
This lets us RBF htlc txs. Signed-off-by: Rusty Russell <[email protected]>
…x set. The answer, it's right in the name of the option! Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
In most cases, it's the same as option_anchor_outputs, but for fees it's different. This transformation is the simplest: pass it as a pair, and test it explicitly. In future we could rationalize some paths, but this was nice and mechanical. Signed-off-by: Rusty Russell <[email protected]>
We disabled experimental support for opening non-zero-fee anchor channels (though old nodes may still have such channels if they turned that on!). So we simply call this `experimental-anchors`, since this is the variant which we expect to be used widely. Signed-off-by: Rusty Russell <[email protected]> Changelog-EXPERIMENTAL: protocol: added support for zero-fee-htlc anchors (`option_anchors_zero_fee_htlc_tx`), using `--experimental-anchors`.
Signed-off-by: Rusty Russell <[email protected]>
They used to force option_anchor_outputs, so switch them. Signed-off-by: Rusty Russell <[email protected]>
…-anchors. We use parameterization here. The old `anchor_expected()` was for non-zero-fee anchors, and have bitrotted so there are some other changes as well. Unfortunately, all the anchor accounting seems to be broken, but I cannot understand these tests at all. I had to simply disable them for now. Signed-off-by: Rusty Russell <[email protected]>
…on failure. We were marking our inputs very late, which means any early failure would not know to unreserve them. This becomes particularly bad when we start enforcing emergency reserves. Signed-off-by: Rusty Russell <[email protected]>
If you did call fundpsbt with amount 'all' and `excess_as_change` true, you would get everything going to the change output. That's obviously not the intention, and we'd like to use this to add change outputs even for "all" when have keep emergency reserves. And change the finish_psbt() API to take an explicit change amount: at the moment it's either all or nothing, but that will change with emergency-sat reserves. Signed-off-by: Rusty Russell <[email protected]>
… logic. This was added to fundpsbt/utxopsbt in v0.10, but the txprepare plugin didn't take advantage of it, instead calculating its own change amount and output. Signed-off-by: Rusty Russell <[email protected]>
This was added to fundpsbt/utxopsbt in v0.10, but the spender plugin didn't take advantage of it, instead calculating its own change amount and output. Signed-off-by: Rusty Russell <[email protected]>
For anchors, we need some sats sitting around in case we need to CPFP a close. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: Config: `min-emergency-msat` setting for (currently experimental!) anchor channels, to keep funds in reserve for forced closes.
This is the simple version which always tries to keep some sats if we have an anchor channel. Turns out that we need something more sophisticated for multifundchannel, so that's next. Signed-off-by: Rusty Russell <[email protected]> Changelog-Changed: JSON-RPC: `withdraw` will refuse to spend funds below `min-emergency-msat` if we have any anchor channels (and `all` will be reduced appropriately). Changelog-Changed: JSON-RPC: `fundpsbt` and `utxopsbt` will refuse to spend funds below `min-emergency-msat` if we have any anchor channels.
This is needed when we know we're *opening* an anchor channel, to override the "do we already have an anchor channel open?" logic. Also, document the nonwrapped arg added in v23.02. Signed-off-by: Rusty Russell <[email protected]> Changelog-Added: JSON-RPC: `fundpsbt` and `utxopsbt` new parameter `opening_anchor_channel` so lightningd knowns it needs emergency reserve for anchors.
… reserves. If we're opening a channel with a peer which support anchors (and we do), we tell fundpsbt/utxopsbt to enforce the emergency reserve; this matters, as it doesn't know about the channel yet, and thus won't (if it's our first anchor channel). Signed-off-by: Rusty Russell <[email protected]> Changelog-Changed: JSON-RPC: `fundchannel` and `multifundchannel` will refuse to spend funds below `min-emergency-msat` if we have any anchor channels (or are opening one).
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/zero-fee-htlc
branch
from
c6f4e80
to
9088deb
Compare
|
Test flake fix for anchors test. |
|
Very nice :-) ACK 9088deb |
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.
Cleanups, new JSON fields for anchor support, anchor spending support so we can lowball our commitment tx fees, and min-emergency-msat for reserving some sats for closing anchor channels (respecting that meant quite a neatning/rework of our fundchannel and withdraw plugin ops).
I expect this to be experimental for one release, and the default in future releases.