Zero fee htlc: Finally add support (experimental!) #6137

rustyrussell · 2023-04-01T04:08:42Z

~~Builds on #6136 , so start from hsmd: command to sign anchor spends.~~ MERGED

This is still experimental since it needs more testing (in particular, interop!), and we don't yet reserve utxos, so if you have no utxos you will find yourself unable to push a unilateral close through!

But it includes CPFP using anchors, and RBF using HTLC tx's SIGHASH_SINGLE|SIGHASH_ANYONECANPAY, so zero-fee htlcs are trivial to support.

cdecker

ACK 3e883a0

lightningd/chaintopology.c

This is kind of a withdrawl to ourselves, except we also spend a channel to-local anchor. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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 <rusty@rustcorp.com.au>

We don't actually use it anywhere, but we actually want to now for CPFP. So give it more parameters and make it return bool so it can be set without necessarily suppressing rexmit. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

…nt tx. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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 <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `feerates` has new output `unilateral_anchor_close` to show the feerate used for anchor channels (currently experimental)

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 <rusty@rustcorp.com.au>

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 <rusty@rustcorp.com.au>

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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 <rusty@rustcorp.com.au> Changelog-Experimental: protocol: added support for zero-fee-htlc anchors (`option_anchors_zero_fee_htlc_tx`), using `--experimental-anchors`.

rustyrussell · 2023-04-10T08:03:34Z

Rebased on master, fixed up hsm_version.h to include newer checksums.

Ack 0241ea1

vincenzopalazzo

LGTM, there are a couple of nits from the compiler

cc wallet/db_sqlite3_sqlgen.c
lightningd/onchain_control.c:941:42: error: variable 'fee' is uninitialized when used here [-Werror,-Wuninitialized]
            || !amount_sat_sub(&change, change, fee)) {
                                                ^~~
lightningd/onchain_control.c:870:2: note: variable 'fee' is declared here
        struct amount_sat fee, excess, change;
        ^
lightningd/onchain_control.c:938:30: error: variable 'psbt' is uninitialized when used here [-Werror,-Wuninitialized]
                                BITCOIN_TX_RBF_SEQUENCE, psbt);
                                                         ^~~~
lightningd/onchain_control.c:876:25: note: initialize the variable 'psbt' to silence this warning
        struct wally_psbt *psbt;
                               ^
                                = NULL
2 errors generated.
make: *** [Makefile:292: lightningd/onchain_control.o] Error 1
make: *** Waiting for unfinished jobs....
rm external/x86_64-pc-linux-gnu/libwally-core-build/src/secp256k1/libsecp256k1.la
ld lightningd/lightning_hsmd
ld lightningd/lightning_gossipd
ld lightningd/lightning_openingd
ld lightningd/lightning_dualopend
ld lightningd/lightning_channeld
ld lightningd/lightning_closingd
ld lightningd/lightning_onchaind
ld lightningd/lightning_connectd
ld lightningd/lightning_websocketd
cc lightningd/onchain_control.c
lightningd/onchain_control.c:941:42: error: variable 'fee' is uninitialized when used here [-Werror,-Wuninitialized]
            || !amount_sat_sub(&change, change, fee)) {
                                                ^~~
lightningd/onchain_control.c:870:2: note: variable 'fee' is declared here
        struct amount_sat fee, excess, change;
        ^
lightningd/onchain_control.c:938:30: error: variable 'psbt' is uninitialized when used here [-Werror,-Wuninitialized]
                                BITCOIN_TX_RBF_SEQUENCE, psbt);
                                                         ^~~~
lightningd/onchain_control.c:876:25: note: initialize the variable 'psbt' to silence this warning
        struct wally_psbt *psbt;
                               ^
                                = NULL
2 errors generated.
make: *** [Makefile:292: lightningd/onchain_control.o] Error 1
Error: Process completed with exit code 2.

rustyrussell · 2023-04-13T02:38:24Z

I worked on this quite a bit, it was definitely not ready. I am still failing on final tests, and am deferring it to next release.

instagibbs · 2023-04-13T19:45:17Z

lightningd/onchain_control.c

+	const u8 *msg;
+	struct lightningd *ld = channel->peer->ld;
+
+	/* We can't do much without anchor outputs (we could CPFP?) */


cpfp only works in the case of revocation? making sure I understand the comment

instagibbs · 2023-04-13T19:51:05Z

lightningd/onchain_control.c

+	utxos[0] = wallet_find_utxo(utxos, ld->wallet,
+				    get_block_height(ld->topology),
+				    NULL,
+				    0, /* FIXME: unused! */


please annotate which one is unused (feerate_per_kw)

instagibbs · 2023-04-13T19:57:18Z

lightningd/onchain_control.c

+	/* OK, we need to attach another input! */
+	/* FIXME: Maybe more than one! */
+	utxos = tal_arr(tmpctx, struct utxo *, 1);
+	utxos[0] = wallet_find_utxo(utxos, ld->wallet,


IIUC, this will return a random utxo from the wallet, but the value will be arbitrary. This means we will keep trying with random utxos until we get one that is big enough, is that right?

If so, this may allow an attacker to dust you wallet to slow down progress at picking a "good enough" utxo. Might want to just keep grabbing utxos rather than aborting at a min?

We should likely ensure that we aren't actually reducing our feerate by adding tiny inputs, that don't even pay for themselves. That'd also fix the dusting issue mentioned by @instagibbs.

instagibbs · 2023-04-13T20:01:45Z

lightningd/onchain_control.c

+			/* Eliminate this output altogether. */
+			bitcoin_tx_remove_output(newtx, change_output);
+		} else {
+			bitcoin_tx_output_set_amount(newtx, change_output, amt);


if this hits at dust values, and new tx meets_feerate, I think this makes dusty tx?

instagibbs · 2023-04-13T20:02:52Z

lightningd/onchain_control.c

+	}
+
+	/* Now, get HSM to sign off. */
+	msg = towire_hsmd_sign_htlc_tx_mingle(NULL,


SIGHASH_SINGLE, ready to MINGLE

cdecker · 2023-06-05T12:53:36Z

Looking quite good I think, love the anchor details handling in the first couple of commits.

cdecker · 2023-06-05T11:48:19Z

bitcoin/script.c

+	if (script_len != 34 + 1 + 1 + 1 + 1 + 1 + 1)
+		return false;
+	if (script[0] != OP_PUSHBYTES(33))
+		return false;
+	if (script[34] != OP_CHECKSIG)
+		return false;
+	if (script[35] != OP_IFDUP)
+		return false;
+	if (script[36] != OP_NOTIF)
+		return false;
+	if (script[37] != 0x50 + 16)
+		return false;
+	if (script[38] != OP_CHECKSEQUENCEVERIFY)
+		return false;
+	if (script[39] != OP_ENDIF)
+		return false;
+	return true;


A template comparison system would be nice here, but probably overkill 🤔

cdecker · 2023-06-05T12:47:36Z

lightningd/onchain_control.c

+	/* OK, we need to attach another input! */
+	/* FIXME: Maybe more than one! */
+	utxos = tal_arr(tmpctx, struct utxo *, 1);
+	utxos[0] = wallet_find_utxo(utxos, ld->wallet,


We should likely ensure that we aren't actually reducing our feerate by adding tiny inputs, that don't even pay for themselves. That'd also fix the dusting issue mentioned by @instagibbs.

rustyrussell · 2023-06-15T01:42:21Z

closed-by #6334

rustyrussell added this to the v23.05 milestone Apr 1, 2023

rustyrussell requested a review from cdecker as a code owner April 1, 2023 04:08

rustyrussell force-pushed the zero-fee-htlc-prep-5 branch 2 times, most recently from 894228c to 3e883a0 Compare April 3, 2023 05:20

cdecker approved these changes Apr 6, 2023

View reviewed changes

lightningd/chaintopology.c Show resolved Hide resolved

rustyrussell force-pushed the zero-fee-htlc-prep-5 branch 2 times, most recently from 04dc118 to 17caa31 Compare April 10, 2023 07:58

rustyrussell added 11 commits April 10, 2023 17:31

hsmd: command to sign anchor spends.

c7153d9

This is kind of a withdrawl to ourselves, except we also spend a channel to-local anchor. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd: code to spend anchor outputs if we want to boost commitme…

e20cb83

…nt tx. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

pytest: test CPFP using anchors.

bada362

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

hsmd: routine to sign HTLC tx merged with our own tx.

9b33dca

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 <rusty@rustcorp.com.au>

onchaind: use our own inputs to add to HTLC txs (iff using anchors).

d23c30f

common/channel_type: add zero-fee-anchor type.

ab40dc0

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

rustyrussell force-pushed the zero-fee-htlc-prep-5 branch from 17caa31 to 0241ea1 Compare April 10, 2023 08:02

vincenzopalazzo reviewed Apr 10, 2023

View reviewed changes

rustyrussell marked this pull request as draft April 13, 2023 02:37

rustyrussell modified the milestones: v23.05, v23.08 Apr 13, 2023

instagibbs reviewed Apr 13, 2023

View reviewed changes

morehouse mentioned this pull request Apr 27, 2023

[bug]: Unhandled commitment type in channel acceptor request with zero conf channels from CLN lightningnetwork/lnd#7642

Closed

rustyrussell added feature protocol These issues are protocol level issues that should be discussed on the protocol spec repo labels May 9, 2023

cdecker added the needs-rebase label Jun 5, 2023

cdecker reviewed Jun 6, 2023

View reviewed changes

cdecker mentioned this pull request Jun 14, 2023

Anchor with zero fee htlc support (aka. experimental-anchors) #6334

Merged

rustyrussell closed this Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zero fee htlc: Finally add support (experimental!) #6137

Zero fee htlc: Finally add support (experimental!) #6137

rustyrussell commented Apr 1, 2023 •

edited

Loading

cdecker left a comment

rustyrussell commented Apr 10, 2023

vincenzopalazzo left a comment

rustyrussell commented Apr 13, 2023

instagibbs Apr 13, 2023

instagibbs Apr 13, 2023

instagibbs Apr 13, 2023

instagibbs Apr 13, 2023

cdecker Jun 5, 2023

instagibbs Apr 13, 2023

instagibbs Apr 13, 2023

cdecker commented Jun 5, 2023

cdecker Jun 5, 2023

cdecker Jun 5, 2023

rustyrussell commented Jun 15, 2023

Zero fee htlc: Finally add support (experimental!) #6137

Zero fee htlc: Finally add support (experimental!) #6137

Conversation

rustyrussell commented Apr 1, 2023 • edited Loading

cdecker left a comment

Choose a reason for hiding this comment

rustyrussell commented Apr 10, 2023

vincenzopalazzo left a comment

Choose a reason for hiding this comment

rustyrussell commented Apr 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdecker commented Jun 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rustyrussell commented Jun 15, 2023

rustyrussell commented Apr 1, 2023 •

edited

Loading