Prepare tap channels for v1 proofs #1779

GeorgeTsagk · 2025-09-02T14:08:50Z

Description

Adds support for v1 proofs for all channel related transactions (funding/commitments/closing/sweeping).

We also guard this feature behind a feature bit, which must be set and supported by both channel peers in order to use v1 proofs.

For some cases during sweeping, we don't really care about the feature being supported by the remote party as we're sweeping locally to our own address.

Fixes #1582

GeorgeTsagk · 2025-09-08T12:40:33Z

LiT tests are expected to fail, the related LiT PR is here

Roasbeef · 2025-09-09T00:34:19Z

tapsend/send.go

@@ -995,7 +1013,7 @@ func CreateOutputCommitments(packets []*tappsbt.VPacket,

 // commitPacket creates the output commitments for a virtual packet and merges
 // it with the existing commitments for the anchor outputs.
-func commitPacket(vPkt *tappsbt.VPacket, noSTXOProofs bool,
+func commitPacket(vPkt *tappsbt.VPacket, noSTXOProofs, existingSTXOLeaves bool,


Perhaps this should take a mini options/config struct? A common bug that can arise with two side-by-side params w/ the same type is that a caller accidentally passes them in w/ the wrong order.

Given that commitPacket is only a helper used within this package, with just one call site, I'll just wrap the two flags in a config struct to make sure the field name shows up next to the value, in order to prevent such mistakes

Roasbeef · 2025-09-09T00:35:16Z

tapsend/send.go

+
+				// Verify that the new set of alt leaves has
+				// unique keys.
+				err := asset.AddLeafKeysVerifyUnique(


What about adding this protection, or assertion at a higher level?

When can this happen (non-unique STXO leaves passed in) within the context of the protocol?

When funding a channel we need to "prematurely" create and include the alt leaves on the commitment because LND will cache the tapscript for future reference. That's the only case where we pre-generate the alt leaves, and that's why we added the option here for this "soft" check.

Generally we want to error out on alt leaf key conflict, but for this specific case we want to let this tool know that they may already exist -- in which case we only append the new leaves if no conflict occurs.

The rationale behind adding the option is to be able to re-use the commitPacket helper even from the chan funding call site, where the above special conditions are met.

Roasbeef · 2025-09-09T00:37:37Z

tapsend/send.go

+					vOut.AltLeaves = append(
+						vOut.AltLeaves, stxoAssets...,
+					)
+				}


We don't actually handle the error case here, so what good is this check?

If there was a leaf key conflict we avoid touching the existing alt leaves. We only append if the set contains unique keys -- the err == nil case.

Could change to instead keep just the unique values, but didn't want to overengineer it.

tapchannel/aux_funding_controller.go

Roasbeef · 2025-09-09T00:48:14Z

server.go

@@ -889,7 +892,9 @@ func (s *Server) ApplyHtlcView(

 	// The aux leaf creator is fully stateless, and we don't need to wait
 	// for the server to be started before being able to use it.
-	return tapchannel.ApplyHtlcView(s.chainParams, in)
+	return tapchannel.ApplyHtlcView(
+		s.chainParams, in, s.cfg.AuxChanNegotiator,


First thought at seeing this change: do we actually need to propagate this all the way here just to do something like fetch the set of aux leaves, or to apply an HTLC view?

Even before a channel is created, we know if we're going to use the STXO feature and also noop feature (for new channels). Therefore, when preparing the funding and commitment blobs, we can just store this information right there and there. So then we don't need to query for feature bits each time we want to make a new commitment or validate one from the remote peer.

The other dependnacy that pops up here is: these methods needs to always work, even if we're not currently connected to a peer. IIUC, this new aux negotiator interface can only query live peer state (what did they send us in init, etc).

Therefore, when preparing the funding and commitment blobs, we can just store this information right there and there

Where exactly are you referring to? If you're referring to the funding/commit blobs then we can't really rely on those values for future decisions. Given that (at least for a short time window) downgrading from v1 will be allowed, then we need to know what the latest state of our peer is. Last commitment could have been using v1, but then upon reconnection our peer could stop supporting it.

The other dependnacy that pops up here is: these methods needs to always work, even if we're not currently connected to a peer. IIUC, this new aux negotiator interface can only query live peer state (what did they send us in init, etc).

I think for ApplyHTLCView we know that the channel must be active for the method to be called, which means we have communicated/agreed upon feature bits with corresponding peer. You're right for some of these cases that we could skip using the AuxChanNegotiator and rely on the commit blob, will change those hooks accordingly.

Generally, if the method affects the creation of the next commitment we'll want to consult the negotiator. If it just cares about some existing commitment, we could look up the flag in the blob.

Roasbeef · 2025-09-09T00:49:07Z

tapchannel/aux_leaf_creator.go

@@ -50,10 +65,16 @@ func FetchLeavesFromView(chainParams *address.ChainParams,
 			"commit state: %w", err))
 	}

+	supportsSTXO := bitFetcher.GetChannelFeatures(


In this case, we have asccess to the blobs that we have lnd store (funding + commitment), so we can just read that state.

Same goes for all the other instances in this commit.

IMO, we only need to query this the very first time when we create a channel.

Thinking about things anew, I think the ideal implementation here would actually set a feature bit in the channel type message. Otherwise, we're falling back to the old implicit channel type negotiation, which we wanted to get away from by making the type of the channel being negotiated explicit.

With this route, an extension point would actually go near the funding manager in lnd, which is what decides if we understand a feature bit combo to open a new channel type.

See #1779 (comment), focusing on closing remark

Roasbeef · 2025-09-09T00:53:30Z

tapchannel/aux_closer.go

+	features := a.cfg.AuxChanNegotiator.GetChannelFeatures(
+		lnwire.NewChanIDFromOutPoint(desc.ChanPoint),
+	)
+	supportSTXO := features.HasFeature(tapfeatures.STXOOptional)


Same here, we already know if the channel was created with STXO support or not.

Or is the idea that if they update after the fact, we still want to use stxos in the new outputs?

At the time of running the above code lines we're crafting the coop close transaction. Both parties need to agree on whether they use v1 or not in order to craft the same transaction.

Could be the case that we used v0 for the whole lifecycle of the channel, then peer updated right before coop closing.

Roasbeef · 2025-09-09T00:54:21Z

tapchannelmsg/records.go

+
+	// STXO is a flag indicating whether this commitment supports stxo
+	// proofs.
+	STXO tlv.RecordT[tlv.TlvType5, bool]


Cool, so if we're using this here, then I think we can remove many of the perior instances in the earlier commits where we're doing queries on feature bits after channel funding.

Not really. This entry is juts a historical entry.

The fact that the last commit used STXO doesn't mean that the next commit will use STXO too. Our peer could downgrade in the meantime.

The above entry was only really introduced for our contract resolver

taproot-assets/tapchannel/aux_sweeper.go

Line 2498 in 87174c6

req.resp <- a.resolveContract(req.req)

specifically the part where we import the commit tx:

taproot-assets/tapchannel/aux_sweeper.go

Line 1588 in 87174c6

supportSTXO := commitState.STXO.Val

tapchannel/commitment.go

We change the commitment related generations to now include stxo proofs if the setting allows so. Previously we would always exclude stxo proofs, but now we add them if both parties understand & expect them.

gijswijs

Did a first pass and mad comments throughout. Starts to look good! This PR depends on the litd itest working, right? (lightninglabs/lightning-terminal#1138)

gijswijs · 2025-09-12T13:58:41Z

tapsend/send.go

+	// existingSTXOLeaves indicates whether we should be aware that alt
+	// leaves that correspond to stxo assets may already exist in asset
+	// outputs of the vpsbt.
+	existingSTXOLeaves bool


I understand the reason of this boolean and premature tapscript generation that is referenced by lnd, but I don't like it.
This complexity makes the code harder to reason about and could be fragile. Isn't there a way to establishes a single point of responsibility for adding STXO?

Thanks for this comment.

I was getting ready to write a long explanation on how this isn't a quirk of the commitment related toolkit but a characteristic of the chan funding flow -- i.e we need to craft the correct commitment even though we haven't yet finalized the vpsbt that creates it.

Then I realized that in my latest push, where I simplified a lot of code, I also omitted the (now unnecessary) part of actually appending the alt leaves to the vOut. This means that even though we do collect STXOs prematurely, we don't really attach it to the vOut, leaving it "clean" for the toolkit to handle it. This means that we actually never hit a case where there's duplicate alt leaves, as we'd never really persist them between preparation & finalization phase.

Will just drop the commit that adds WithExistingSTXOLeaves altogether.

Interesting. But if we don't append them to the vOut why do we still collect the STXOs prematurely? Where do we use them in the chan funding flow?

We directly merge them into the commitment that will be used to derive the tapscript for LND. The vOut alt leaves are just the placeholder for keeping the leaves somewhere, until we'd eventually want to merge them into the tap commitment.

So now we simply skip the step of adding the alt leaves to the vOut and later when the helper functions get the vPSBT they'll be free to re-collect/append the alt leaves without conflicts or extra ticks needed

Had some back and forth on this with @GeorgeTsagk and an LLM who will remain anonymous. I'm posting it here for future reference.

Premature Stage: Funding Transaction Construction (addToFundingCommitment)
This stage computes the correct TapscriptRoot for the on-chain P2TR funding output script. LND needs this root early, before the final proofs are generated.
Call Flow:
FundingController.processFundingReq -> pendingAssetFunding.addToFundingCommitment
Action: addToFundingCommitment calls asset.CollectSTXO and merges the resulting leaves directly into an in-memory commitment.TapCommitment. This allows for the correct TapscriptRoot to be provided to LND. This process does not modify any VPacket's AltLeaves.

Finalization Stage: Final Proof Generation (commitPacket)
This stage generates the final Taproot Asset proofs for the assets being created in the channel after the funding transaction is fully known.
Call Flow:
FundingController.processFundingReq -> FundingController.completeChannelFunding -> FundingController.anchorVPackets -> tapsend.CreateOutputCommitments -> commitPacket
Action: The commitPacket function is called on the VPackets representing the channel assets. It calls asset.CollectSTXO again and appends the resulting leaves to the vOut.AltLeaves slice before creating the final commitment for the proof.

tapchannel/aux_leaf_creator.go

tapchannel/commitment.go

This commit updates the aux funding controller to conditionally use stxo proofs, depending on whether the feature bit was negotiated or not with the remote peer. Our funding commitment will now include the alt leaves for stxos if the flag is set. The funder needs to construct the correct tapscript early on, as LND will query the tapscript root before we reach the finalization phase. That's why we manually calculate and merge the stxo alt leaves in the funding process. The fundee now also needs to calculate and merge the alt leaves that result from the asset outputs, in order to arrive to the same tapscript root.

We extend the commitment blob to also store a flag, indicating whether STXO was active when that commitment was created. This can be useful for future sweeps that need to know whether that commitment used stxo alt leaves, which affects the reconstruction of the tap commitment.

We also add stxo support for the aux leaf creation. This is crucial and needs to be the same across both channel parties. We rely on the consistency of the feature bit for whether we'll include the stxo alt leaves or not. A disagreement here could lead to a force close.

For the cooperative channel closing we also need to check if the parties have agreed upon the usage of stxo proofs. If that is the case, we'll include those commitments in the coop channel closing transaction & proof.

This commit adds stxo support to the aux sweeper. When importing a commit tx to the wallet we now consult the commit blob flag in order to find out whether that commitment was using stxo proofs or not.

GeorgeTsagk · 2025-09-15T11:29:44Z

Did a first pass and mad comments throughout. Starts to look good! This PR depends on the litd itest working, right? (lightninglabs/lightning-terminal#1138)

Yeap. Will update the LiT PR soon to reflect all of recent changes made here

gijswijs

Nearly there! I made some small remarks and some questions.

gijswijs · 2025-09-16T14:59:17Z

tapchannel/aux_funding_controller.go


 	log.Infof("Anchoring funding vPackets to funding PSBT")

+	// We have collected the stxo leaves for the outputs in a previous step.


This comment doesn't make any sense anymore now that we avoided alt leaves collisions anyway.

gijswijs · 2025-09-16T15:27:27Z

tapchannel/aux_funding_controller.go

@@ -1713,6 +1766,15 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex,
 		}
 	}



This block should be pushed a few lines down, such that the registration of the defer is still directly below the creation of the unlockLeases closure.

gijswijs · 2025-09-16T20:17:24Z

tapchannel/aux_leaf_creator.go

@@ -24,10 +26,19 @@ const (
 	DefaultTimeout = 30 * time.Second
 )

+// FeatureBitFetcher is responsible for fetching feature bits either by
+// referencing a remote peer, or an established channel.


This comment isn't up to date anymore, now that it's impossible to reference a peer. It only works byu referencing a channel.

gijswijs · 2025-09-16T20:29:38Z

tapchannel/aux_sweeper.go

-	outCommitments, err := tapsend.CreateOutputCommitments(
-		directPkts, tapsend.WithNoSTXOProofs(),
-	)
+	outCommitments, err := tapsend.CreateOutputCommitments(directPkts)


For my understanding: From now on we always create a sweeping Tx with stxo proofs.
Why didn't we do so before? Oli needed the WithNoSTXOProofs to make the channel commitments work, but it seems to me that the sweeping Tx could always have had stxo proofs. (This is more a question about Oli's PR than about this PR, but I'm still wondering.)

GeorgeTsagk self-assigned this Sep 2, 2025

levmi added this to Taproot-Assets Project Board Sep 4, 2025

github-project-automation bot moved this to 🆕 New in Taproot-Assets Project Board Sep 4, 2025

levmi moved this from 🆕 New to 🏗 In progress in Taproot-Assets Project Board Sep 4, 2025

GeorgeTsagk force-pushed the channels-v1 branch from 6b5fffb to 6fdc32e Compare September 8, 2025 10:39

GeorgeTsagk mentioned this pull request Sep 8, 2025

LiT: Custom channels feature bits lightninglabs/lightning-terminal#1138

Open

GeorgeTsagk added no-changelog tap-channels labels Sep 8, 2025

GeorgeTsagk force-pushed the feature-bits branch from 74d98bf to 611a9fc Compare September 8, 2025 12:28

GeorgeTsagk force-pushed the channels-v1 branch from 6fdc32e to 87174c6 Compare September 8, 2025 12:32

GeorgeTsagk marked this pull request as ready for review September 8, 2025 12:39

GeorgeTsagk requested review from Roasbeef and gijswijs September 8, 2025 12:40

Roasbeef requested changes Sep 9, 2025

View reviewed changes

github-project-automation bot moved this from 🏗 In progress to 👀 In review in Taproot-Assets Project Board Sep 9, 2025

GeorgeTsagk force-pushed the feature-bits branch 2 times, most recently from 3d4c18f to f9e10d7 Compare September 11, 2025 09:16

tapchannel: parameterize stxo for commitments

86a01fb

We change the commitment related generations to now include stxo proofs if the setting allows so. Previously we would always exclude stxo proofs, but now we add them if both parties understand & expect them.

GeorgeTsagk force-pushed the channels-v1 branch from 87174c6 to 24347b7 Compare September 11, 2025 11:35

GeorgeTsagk requested a review from Roasbeef September 12, 2025 11:40

gijswijs requested changes Sep 12, 2025

View reviewed changes

GeorgeTsagk added 5 commits September 15, 2025 13:13

tapchannel: use stxo on aux chan closer

fb57778

For the cooperative channel closing we also need to check if the parties have agreed upon the usage of stxo proofs. If that is the case, we'll include those commitments in the coop channel closing transaction & proof.

tapchannel: support stxo on aux sweeper

be7349d

This commit adds stxo support to the aux sweeper. When importing a commit tx to the wallet we now consult the commit blob flag in order to find out whether that commitment was using stxo proofs or not.

GeorgeTsagk force-pushed the channels-v1 branch from 24347b7 to be7349d Compare September 15, 2025 11:29

GeorgeTsagk requested a review from gijswijs September 15, 2025 11:34

gijswijs requested changes Sep 16, 2025

View reviewed changes


		log.Infof("Anchoring funding vPackets to funding PSBT")

		// We have collected the stxo leaves for the outputs in a previous step.

		@@ -1713,6 +1766,15 @@ func (f *FundingController) processFundingReq(fundingFlows fundingFlowIndex,
		}
		}

Prepare tap channels for v1 proofs #1779

Are you sure you want to change the base?

Prepare tap channels for v1 proofs #1779

Conversation

GeorgeTsagk commented Sep 2, 2025 • edited by Roasbeef Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

GeorgeTsagk commented Sep 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gijswijs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GeorgeTsagk Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GeorgeTsagk commented Sep 15, 2025

Uh oh!

gijswijs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

GeorgeTsagk commented Sep 2, 2025 •

edited by Roasbeef

Loading

GeorgeTsagk Sep 15, 2025 •

edited

Loading