From fbd5d00ef0651266ff1164c69ea74f1c0d86f8f8 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 2 May 2025 14:08:46 +0200 Subject: [PATCH 1/3] tapsend: refactor out GroupProofsByAssetID for re-use The code that groups a list of proofs by their asset ID is useful in other places, so we extract it into a re-usable function. --- tapsend/allocation.go | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/tapsend/allocation.go b/tapsend/allocation.go index 31a48be73..1c2b258ab 100644 --- a/tapsend/allocation.go +++ b/tapsend/allocation.go @@ -486,19 +486,13 @@ func DistributeCoins(inputs []*proof.Proof, allocations []*Allocation, // We group the assets by asset ID, since we'll want to create a single // virtual packet per asset ID (with each virtual packet potentially // having multiple inputs and outputs). - assetIDs := fn.Map(inputs, func(input *proof.Proof) asset.ID { - return input.Asset.ID() - }) - uniqueAssetIDs := fn.NewSet(assetIDs...).ToSlice() + groupedProofs := GroupProofsByAssetID(inputs) // Each "piece" keeps track of how many assets of a specific asset ID // we have already distributed. The pieces are also the main way to // reference an asset ID's virtual packet. - pieces := make([]*piece, len(uniqueAssetIDs)) - for i, assetID := range uniqueAssetIDs { - proofsByID := fn.Filter(inputs, func(i *proof.Proof) bool { - return i.Asset.ID() == assetID - }) + pieces := make([]*piece, 0, len(groupedProofs)) + for assetID, proofsByID := range groupedProofs { sumByID := fn.Reduce( proofsByID, func(sum uint64, i *proof.Proof) uint64 { return sum + i.Asset.Amount @@ -512,12 +506,12 @@ func DistributeCoins(inputs []*proof.Proof, allocations []*Allocation, return nil, err } - pieces[i] = &piece{ + pieces = append(pieces, &piece{ assetID: assetID, totalAvailable: sumByID, proofs: proofsByID, packet: pkt, - } + }) } // Make sure the pieces are in a stable and reproducible order before we @@ -840,3 +834,22 @@ func setAllocationFieldsFromOutput(alloc *Allocation, vOut *tappsbt.VOutput) { alloc.AltLeaves = vOut.AltLeaves alloc.SiblingPreimage = vOut.AnchorOutputTapscriptSibling } + +// GroupProofsByAssetID groups the given proofs by their asset ID. +func GroupProofsByAssetID(proofs []*proof.Proof) map[asset.ID][]*proof.Proof { + assetIDs := fn.Map(proofs, func(p *proof.Proof) asset.ID { + return p.Asset.ID() + }) + uniqueAssetIDs := fn.NewSet(assetIDs...).ToSlice() + + groupedProofs := make(map[asset.ID][]*proof.Proof, len(uniqueAssetIDs)) + for _, assetID := range uniqueAssetIDs { + groupedProofs[assetID] = fn.Filter( + proofs, func(p *proof.Proof) bool { + return p.Asset.ID() == assetID + }, + ) + } + + return groupedProofs +} From cf6742617f41e434d9a5141eea942db2c582808d Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 2 May 2025 14:09:54 +0200 Subject: [PATCH 2/3] tapchannel: add CommitmentToPackets function This commit adds a new helper function that creates a set of virtual packets from an aux commitment state and a set of input proofs (as well as some negotiated values during the coop channel close flow). The purpose of this is to use the asset allocation that has already happened to distribute the assets of different asset ID pieces within the given channel commitment again for the virtual packets of a coop close transaction. The reason we want to use the already allocated outputs vs. re-allocating them again is to make sure that the distribution matches exactly that of the commitment. See explanation in next commit for more details. --- tapchannel/commitment.go | 128 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/tapchannel/commitment.go b/tapchannel/commitment.go index 21854aaf6..a562b26e6 100644 --- a/tapchannel/commitment.go +++ b/tapchannel/commitment.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "net/url" "sort" "github.com/btcsuite/btcd/btcec/v2/schnorr" @@ -28,6 +29,7 @@ import ( "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" + "golang.org/x/exp/maps" ) // DecodedDescriptor is a wrapper around a PaymentDescriptor that also includes @@ -1505,6 +1507,132 @@ func deriveFundingScriptKey(ctx context.Context, addrBook address.Storage, return fundingScriptKey, nil } +// CommitmentToPackets converts a commitment to a list of vPackets. The +// commitment must not contain any HTLCs, as this only works for coop-closed +// channels. +func CommitmentToPackets(c *cmsg.Commitment, inputs []*proof.Proof, + chainParams *address.ChainParams, localShutdownMsg, + remoteShutdownMsg cmsg.AuxShutdownMsg, localOutputIndex, + remoteOutputIndex uint32, + vPktVersion tappsbt.VPacketVersion) ([]*tappsbt.VPacket, error) { + + if len(c.IncomingHtlcAssets.Val.HtlcOutputs) > 0 || + len(c.OutgoingHtlcAssets.Val.HtlcOutputs) > 0 { + + return nil, fmt.Errorf("commitment contains HTLCs, cannot " + + "create vPackets") + } + + // We group the inputs by asset ID, so we can create a vPacket for each + // asset ID. The number of vPackets is dictated by the number of + // different asset IDs in the commitment transaction. + groupedInputs := tapsend.GroupProofsByAssetID(inputs) + vPackets := make(map[asset.ID]*tappsbt.VPacket, len(groupedInputs)) + for assetID, proofsByID := range groupedInputs { + pkt, err := tappsbt.FromProofs( + proofsByID, chainParams, vPktVersion, + ) + if err != nil { + return nil, fmt.Errorf("error creating vPacket: %w", + err) + } + + vPackets[assetID] = pkt + } + + localOutputs := c.LocalOutputs() + remoteOutputs := c.RemoteOutputs() + + // We now distribute the outputs to the vPackets. + for _, output := range localOutputs { + pkt, ok := vPackets[output.AssetID.Val] + if !ok { + return nil, fmt.Errorf("no vPacket found "+ + "for asset ID %s", output.AssetID.Val) + } + + outAsset := output.Proof.Val.Asset + vOut, err := assetToInteractiveVOutput( + outAsset, asset.V0, localShutdownMsg, + localOutputIndex, + ) + if err != nil { + return nil, fmt.Errorf("error creating "+ + "vOutput: %w", err) + } + + pkt.Outputs = append(pkt.Outputs, vOut) + } + + for _, output := range remoteOutputs { + pkt, ok := vPackets[output.AssetID.Val] + if !ok { + return nil, fmt.Errorf("no vPacket found "+ + "for asset ID %s", output.AssetID.Val) + } + + outAsset := output.Proof.Val.Asset + vOut, err := assetToInteractiveVOutput( + outAsset, asset.V0, remoteShutdownMsg, + remoteOutputIndex, + ) + if err != nil { + return nil, fmt.Errorf("error creating "+ + "vOutput: %w", err) + } + + pkt.Outputs = append(pkt.Outputs, vOut) + } + + return maps.Values(vPackets), nil +} + +// assetToInteractiveVOutput creates a VOutput for an asset that is part of an +// interactive transaction. +func assetToInteractiveVOutput(a asset.Asset, version asset.Version, + shutdownMsg cmsg.AuxShutdownMsg, + anchorOutputIndex uint32) (*tappsbt.VOutput, error) { + + scriptKey, ok := shutdownMsg.ScriptKeys.Val[a.ID()] + if !ok { + return nil, fmt.Errorf("no script key for asset %s", a.ID()) + } + + proofDeliveryUrl, err := lfn.MapOptionZ( + shutdownMsg.ProofDeliveryAddr.ValOpt(), + func(u []byte) lfn.Result[*url.URL] { + proofDeliveryUrl, err := url.Parse(string(u)) + if err != nil { + return lfn.Err[*url.URL](fmt.Errorf("unable "+ + "to parse proof delivery address: %w", + err)) + } + + return lfn.Ok(proofDeliveryUrl) + }, + ).Unpack() + if err != nil { + return nil, fmt.Errorf("unable to decode proof delivery "+ + "address: %w", err) + } + + outType := tappsbt.TypeSplitRoot + if a.SplitCommitmentRoot == nil { + outType = tappsbt.TypeSimple + } + + return &tappsbt.VOutput{ + Amount: a.Amount, + AssetVersion: version, + Type: outType, + Interactive: true, + AnchorOutputIndex: anchorOutputIndex, + AnchorOutputInternalKey: shutdownMsg.AssetInternalKey.Val, + ScriptKey: asset.NewScriptKey(&scriptKey), + ProofDeliveryAddress: proofDeliveryUrl, + }, nil +} + // InPlaceCustomCommitSort performs an in-place sort of a transaction, given a // list of allocations. The sort is applied to the transaction outputs, using // the allocation's OutputIndex. The transaction inputs are sorted by the From 83823c58662d9b5a101279fc33e87151c884fc29 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 2 May 2025 14:13:35 +0200 Subject: [PATCH 3/3] tapchannel: fix coop close asset distribution issue This commit fixes the actual issue. It makes sure that the way the asset pieces of different asset IDs (for example in grouped asset channels) are distributed matches exactly the last commitment transaction. See comment above call to CommitmentToPackets in this commit for more details. --- tapchannel/aux_closer.go | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/tapchannel/aux_closer.go b/tapchannel/aux_closer.go index 20f633fae..6d503fdcb 100644 --- a/tapchannel/aux_closer.go +++ b/tapchannel/aux_closer.go @@ -402,13 +402,43 @@ func (a *AuxChanCloser) AuxCloseOutputs( closeAllocs[idx].OutputIndex = uint32(idx) } - // Now that we have the complete set of allocations, we'll distribute - // them to create the vPackets we'll need to anchor everything. - vPackets, err := tapsend.DistributeCoins( - inputProofs, closeAllocs, a.cfg.ChainParams, true, tappsbt.V1, + // Now we know the deterministic ordering of the local/remote asset/btc + // outputs, we can extract the output indexes for the allocations. + var ( + localOutputIndex, remoteOutputIndex uint32 + ) + if localAlloc != nil { + localOutputIndex = localAlloc.OutputIndex + } + if remoteAlloc != nil { + remoteOutputIndex = remoteAlloc.OutputIndex + } + + // We don't use the normal allocation code here. This requires a bit of + // a lengthy explanation: When we close a channel, the output of the + // `lncli closedchannels` command will show the last commitment state of + // the channel as the closing asset balance. Which is correct in terms + // of balances. But if there are multiple different asset IDs (e.g., in + // a grouped asset channel), then _how_ those pieces are distributed + // within the commitment transaction depends on the order of the + // allocations. And the order of the allocations is dependent on the + // BTC amount and the pkScript of the BTC-level output. Both of which + // are different in the coop close output (we set the asset-level output + // BTC amount to the dummy amount, and the pkScript will be a newly + // derived internal key with no sibling script path). + // So, long story short: If we used the tapsend.DistributeCoins method + // here, it could happen that the actual asset output distribution shown + // in the `lncli closedchannels` command would be different from the + // actual distribution in the co-op close transaction. + // This could mostly be seen as an UX-only issue, but was actually + // discovered while attempting to assert the final closing balance of + // grouped asset channels in the litd integration test. + vPackets, err := CommitmentToPackets( + commitState, inputProofs, a.cfg.ChainParams, localShutdown, + remoteShutdown, localOutputIndex, remoteOutputIndex, tappsbt.V1, ) if err != nil { - return none, fmt.Errorf("unable to distribute coins: %w", err) + return none, fmt.Errorf("unable to create vPackets: %w", err) } // We can now add the witness for the OP_TRUE spend of the commitment