[9/9] multi: add full on chain resolution support for simple taproot channels #7472

Roasbeef · 2023-03-02T06:23:07Z

In this commit, we finalize the initial implementation of simple taproot channels by adding full on chain resolution for all contracts. This includes: HTLC sweep, breaches, anchor sweeps, remote commitment sweeps, and local commitment sweeps. Along the way, we update all the necessary witness and transaction estimation, which itself should bubble back up to the spec.

As updated itests are still missing (though commit sweeps, anchor sweeps, and breaches have been tested manually), so this PR remains as a draft PR (not to mention the other unmerged PRs in this series).

This PR body will be updated as the PR leaves the draft stage.

I think the biggest open design question is: how to handle the extra state of the tap tweaks and control blocks. As is, I've opted to spin out new storage in the contract court (taproot briefcase to house these items). All the other resolver storage is all inline, so I wanted to avoid having to do a migration. As these channels don't yet exist, we can assume that if the key is present, the the daemon has taproot channels. There're two gaps in the storage as is: breaches and the nursery. These all have their own state distinct of the resolvers, which isn't yet updated.

input/script_utils.go

Crypt-iQ · 2023-05-09T16:14:53Z

lnwallet/channel.go

 		chanState.ChanType, &chanState.LocalChanCfg,
 		&chanState.RemoteChanCfg, keyRing,
 	)
 	if err != nil {
 		return nil, err
 	}
+	if chanState.ChanType.IsTaproot() && !isLocalCommit {
+		localAnchor, remoteAnchor = remoteAnchor, localAnchor


Previously CommitScriptAnchors would return localAnchor (our anchor) and remoteAnchor (their anchor). Now it returns either (our anchor, their anchor) or (their anchor, our anchor). If we change it to be consistent inside of CommitScriptAnchors then this line can be removed.

So the change here is that the keys now differ depending on if this is the remote commit or not, since the local/remove keys are relative, while before it used the multi-sig key which is always absolute.

banging my head to understand the relationship here now...isn't the info needed already embedded in the keyRing and the correct key will be chosen there? the isLocal logic appears in both CommitScriptAnchors and here, maybe one of them is redundant?

isn't the info needed already embedded in the keyRing and the correct key will be chosen there

The keyring itself could be the local keyring or the remote keyring. So I think that is why the switch around is required here.
But yes, maybe we can instead add an "IsForLocal bool" to the keyring so that it handles this for us so that we dont have to worry about passing in the correct keyring & isLocal combination to NewAnchorResolution

contractcourt/commit_sweep_resolver.go

contractcourt/htlc_timeout_resolver.go

contractcourt/htlc_success_resolver.go

ellemouton

leaving some comments so long after doing a high level pass. Will continue once rebased & updated :)

lnwallet/commitment.go

input/size.go

lnwallet/commitment.go

input/script_utils.go

input/witnessgen.go

input/size.go

ellemouton · 2023-07-26T12:59:14Z

input/size.go

+	//      - sig_len: 1 byte
+	//      - sweep_sig: 65 bytes (worst case w/o sighash default)
+	//      - script_len: 1 byte
+	//      - taproot_to_local_script_size: 36 bytes


with updated script, I think it is now 37

input/size.go

lnwallet/commitment.go

input/witnessgen.go

input/script_utils.go

input/witnessgen.go

input/script_utils.go

ellemouton · 2023-07-27T08:49:06Z

input/script_utils.go

@@ -1513,7 +1535,7 @@ func TaprootHtlcSpendRevoke(signer Signer, signDesc *SignDescriptor,
 // NOTE: The caller MUST set the txn version, sequence number, and sign
 // descriptor's sig hash cache before invocation.
 func TaprootHtlcSpendSuccess(signer Signer, signDesc *SignDescriptor,
-	revokeKey *btcec.PublicKey, sweepTx *wire.MsgTx,


nit: the comment above: This should be used to sweep funds after the pre-image is known. isnt complete imo since this is also used for sweeping the htlc-timeout tx.

lnwallet/channel.go

lnwallet/commitment.go

input/witnessgen.go

input/script_utils.go

lnwallet/channel.go

itest/lnd_multi-hop_test.go

lnwallet/commitment.go

lnwallet/channel.go

funding/commitment_type_negotiation.go

itest/lnd_channel_backup_test.go

ellemouton

tACK, LGTM 🥕 ⚡ pending YY's comments re the neutrino backend and also the rebase issue. And then just need to update the sample conf with the re-added "simple-" prefix

input/script_desc.go

lnutils/memory.go

Crypt-iQ

bitcoind/multi_hop_receiver_chain_claim/zeroconf=true/committype=SIMPLE_TAPROOT is flaking due to the mempool
should uncomment the old chanbackup itests

In this commit, we update the breach arb to support taproot channels. We utilize the new taproot briefcase space to store both control blocks, and also the first+second level scripts for the set of HTLCs.

In this commit, we add a new interface, the `ScriptDesciptor` to abstract over details of a given output script. The purpose of this interface, and the taproot superset, is to be able to paper over the differences of a p2wsh vs a p2tr output. With this new interface, we can treat them as the same, but then use a type assertion to get at any control block related methods if needed.

…terface In this commit, we update the channel state machine to use the new ScriptDescriptor interface. This fixes some subtle issues with the existing commits, as for p2wsh we always sign the same witness script, but for p2tr, the witness script differs depending on which branch is taken. With the new abstractions, we can treat p2wsh and p2tr as the same mostly, right up until we need to obtain a control block or a tap tweak. All tests have been updated accordingly.

This is a preparatory change for the upcoming "simple channel close" feature which'll utilize RBF to allow either side to resign the co-op close transaction for broadcast at any point.

The local nonce needs to be the one that's finalized to simulate us receiving the remote nonce, then generating the local nonce in a JIT style.

Otherwise, in the itests (which are mainly based on implicit negotiation), we won't try to open taproot chans when we actually need to. We may want to revisit this however, since it may lock in parties trying to use the defaults that aren't currently setting the explicit commit type during funding.

We keep in line with the existing test that uses implicit negotiation. In the future, we should modify to also have explicit negotiation as well.

In this commit, we remove the internal call to `InitRemoteMusigNonces`. We don't need this since when we go to process the remote party's chan reest message, we'll already call this method. Otherwise, we'll get an error here since the pending verification nonce has been wiped out after each call.

The first and second level taptweaks need to be stored in order to ensure the breach arb can play revocations at both the first and second level.

Co-op close for musig2 chans needs to use the proper witness size.

We also remove the old implicit negotiation as well, as we'll be updating tests to use explciit when required.

We use a helper function to ensure that anytime we're about to make a normal sighash, we consult the channel type to check if we should use the default value or sighash all explicitly.

This adds some extra assertions to ensure things like the taproot commitment weight estimation is correct.

In this commit, we carry out a new notion introduced during a recent spec meeting to use a feature bit plus 100 before the feature has been finalized in the spec. We split into the Final and Staging bits.

The prior commit removed implicit negotiation, so we'll need to make sure to use the explicit chan type feature vector when we go to negotiate.

yyforyongyu

🎉🎉🎉LGTM🥳🥳🥳🤩🤩🤩

Roasbeef added contracts taproot taproot chans labels Mar 2, 2023

Roasbeef changed the title ~~multi: add full on chain resolution support for simple taproot channels~~ [9/9] multi: add full on chain resolution support for simple taproot channels Mar 2, 2023

saubyk added this to the v0.16.1 milestone Mar 9, 2023

saubyk assigned Roasbeef Mar 9, 2023

saubyk requested review from yyforyongyu, ellemouton and Crypt-iQ March 9, 2023 15:22

Roasbeef mentioned this pull request Mar 13, 2023

Lightning Specification Meeting 2023/03/13 lightning/bolts#1058

Closed

25 tasks

saubyk modified the milestones: v0.16.1, v0.17.0 Mar 13, 2023

Roasbeef changed the base branch from master to simple-taproot-chans-cli-rpc April 27, 2023 00:15

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch from 4e7f5f2 to aec161c Compare April 27, 2023 00:19

Roasbeef force-pushed the simple-taproot-chans-cli-rpc branch from 90ff4b1 to 077b57d Compare May 12, 2023 21:00

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch from aec161c to e3d8986 Compare May 12, 2023 21:07

Crypt-iQ reviewed May 19, 2023

View reviewed changes

Roasbeef force-pushed the simple-taproot-chans-cli-rpc branch 2 times, most recently from 4e0e94c to b7ebb59 Compare July 26, 2023 00:56

ellemouton reviewed Jul 26, 2023

View reviewed changes

Roasbeef force-pushed the simple-taproot-chans-cli-rpc branch 2 times, most recently from ba034cb to 7598dae Compare July 27, 2023 00:21

Roasbeef changed the base branch from simple-taproot-chans-cli-rpc to simple-taproot-chans-staging July 27, 2023 00:44

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch from e3d8986 to 93cdb3c Compare July 27, 2023 00:47

Roasbeef marked this pull request as ready for review July 27, 2023 00:47

ellemouton reviewed Jul 27, 2023

View reviewed changes

input/witnessgen.go Show resolved Hide resolved

input/witnessgen.go Outdated Show resolved Hide resolved

input/witnessgen.go Show resolved Hide resolved

input/witnessgen.go Show resolved Hide resolved

input/script_utils.go Show resolved Hide resolved

ellemouton reviewed Jul 27, 2023

View reviewed changes

lnwallet/channel.go Show resolved Hide resolved

lnwallet/channel.go Outdated Show resolved Hide resolved

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch 2 times, most recently from a8da336 to c8cd91c Compare August 2, 2023 04:46

yyforyongyu reviewed Aug 21, 2023

View reviewed changes

ellemouton approved these changes Aug 21, 2023

View reviewed changes

input/script_desc.go Outdated Show resolved Hide resolved

lnutils/memory.go Show resolved Hide resolved

Crypt-iQ reviewed Aug 21, 2023

View reviewed changes

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch from 67d31fe to 553e1e5 Compare August 21, 2023 19:16

Roasbeef added 23 commits August 21, 2023 12:19

contractcourt: update breach arbiter to support taproot chans

7f5386b

In this commit, we update the breach arb to support taproot channels. We utilize the new taproot briefcase space to store both control blocks, and also the first+second level scripts for the set of HTLCs.

contractcourt: update UTXO nursery to support taproot chans

d64f193

lnwallet: for taproot channels allow the co-op close tx to be RBF'd

6e67528

This is a preparatory change for the upcoming "simple channel close" feature which'll utilize RBF to allow either side to resign the co-op close transaction for broadcast at any point.

lnwallet: fix taproot co-op close nonce bug

4fdfec5

The local nonce needs to be the one that's finalized to simulate us receiving the remote nonce, then generating the local nonce in a JIT style.

feature+lncfg: restore simple- prefix for taproot chans arg

8426fb6

itest: add taproot chan support to basic channel funding

3a2199a

We keep in line with the existing test that uses implicit negotiation. In the future, we should modify to also have explicit negotiation as well.

lnwallet: add error logging to getSignedCommitTx

39fa786

itest: add taproot chans to testChannelForceClosure

c0195fd

lnwallet: add first+second level taptweaks to htlc retribution

3dba448

The first and second level taptweaks need to be stored in order to ensure the breach arb can play revocations at both the first and second level.

itest: add taproot support for all variants of runMultiHopHtlcClaimTest

2a315ea

itest: add taproot support to revocation itests

3058d82

multi: fix linter errors

37f40f7

lnwallet: for musig2 coop close fee estimation

84cbd9c

Co-op close for musig2 chans needs to use the proper witness size.

funding: add explicit chan type support for zeroconf+scid+taproot

8d57a71

We also remove the old implicit negotiation as well, as we'll be updating tests to use explciit when required.

lnwallet: uniformly use sighash default everywhere for taproot chans

de50780

We use a helper function to ensure that anytime we're about to make a normal sighash, we consult the channel type to check if we should use the default value or sighash all explicitly.

lnwallet: add taproot case to TestForceClose

72c23e7

This adds some extra assertions to ensure things like the taproot commitment weight estimation is correct.

feature: use +100 staging bit for taproot channels

4a67616

In this commit, we carry out a new notion introduced during a recent spec meeting to use a feature bit plus 100 before the feature has been finalized in the spec. We split into the Final and Staging bits.

itest: use explicit chan type negotiation for taproot chan cases

3369d21

The prior commit removed implicit negotiation, so we'll need to make sure to use the explicit chan type feature vector when we go to negotiate.

multi: add new SCB version for the taproot chan type

771491f

Roasbeef force-pushed the simple-taproot-chans-on-chain-sweeps branch from 553e1e5 to 771491f Compare August 21, 2023 19:19

yyforyongyu approved these changes Aug 21, 2023

View reviewed changes

Roasbeef merged commit b3dc4c0 into lightningnetwork:simple-taproot-chans-staging Aug 21, 2023
18 of 23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[9/9] multi: add full on chain resolution support for simple taproot channels #7472

[9/9] multi: add full on chain resolution support for simple taproot channels #7472

Roasbeef commented Mar 2, 2023 •

edited

Crypt-iQ May 9, 2023

Roasbeef Aug 2, 2023

yyforyongyu Aug 7, 2023

ellemouton Aug 10, 2023

ellemouton left a comment

ellemouton Jul 26, 2023

ellemouton Jul 27, 2023

ellemouton left a comment •

edited

Crypt-iQ left a comment

yyforyongyu left a comment

[9/9] multi: add full on chain resolution support for simple taproot channels #7472

[9/9] multi: add full on chain resolution support for simple taproot channels #7472

Conversation

Roasbeef commented Mar 2, 2023 • edited

Crypt-iQ May 9, 2023

Choose a reason for hiding this comment

Roasbeef Aug 2, 2023

Choose a reason for hiding this comment

yyforyongyu Aug 7, 2023

Choose a reason for hiding this comment

ellemouton Aug 10, 2023

Choose a reason for hiding this comment

ellemouton left a comment

Choose a reason for hiding this comment

ellemouton Jul 26, 2023

Choose a reason for hiding this comment

ellemouton Jul 27, 2023

Choose a reason for hiding this comment

ellemouton left a comment • edited

Choose a reason for hiding this comment

Crypt-iQ left a comment

Choose a reason for hiding this comment

yyforyongyu left a comment

Choose a reason for hiding this comment

Roasbeef commented Mar 2, 2023 •

edited

ellemouton left a comment •

edited