Merge pull request #1505 from lightninglabs/coop-close-allocation-sort

channels: fix coop close asset distribution for grouped asset channels

Author: ffranr

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

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

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
+}