Skip to content

Commit c2ef059

Browse files
committed
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.
1 parent e9dd241 commit c2ef059

File tree

1 file changed

+35
-5
lines changed

1 file changed

+35
-5
lines changed

tapchannel/aux_closer.go

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -402,13 +402,43 @@ func (a *AuxChanCloser) AuxCloseOutputs(
402402
closeAllocs[idx].OutputIndex = uint32(idx)
403403
}
404404

405-
// Now that we have the complete set of allocations, we'll distribute
406-
// them to create the vPackets we'll need to anchor everything.
407-
vPackets, err := tapsend.DistributeCoins(
408-
inputProofs, closeAllocs, a.cfg.ChainParams, true, tappsbt.V1,
405+
// Now we know the deterministic ordering of the local/remote asset/btc
406+
// outputs, we can extract the output indexes for the allocations.
407+
var (
408+
localOutputIndex, remoteOutputIndex uint32
409+
)
410+
if localAlloc != nil {
411+
localOutputIndex = localAlloc.OutputIndex
412+
}
413+
if remoteAlloc != nil {
414+
remoteOutputIndex = remoteAlloc.OutputIndex
415+
}
416+
417+
// We don't use the normal allocation code here. This requires a bit of
418+
// a lengthy explanation: When we close a channel, the output of the
419+
// `lncli closedchannels` command will show the last commitment state of
420+
// the channel as the closing asset balance. Which is correct in terms
421+
// of balances. But if there are multiple different asset IDs (e.g., in
422+
// a grouped asset channel), then _how_ those pieces are distributed
423+
// within the commitment transaction depends on the order of the
424+
// allocations. And the order of the allocations is dependent on the
425+
// BTC amount and the pkScript of the BTC-level output. Both of which
426+
// are different in the coop close output (we set the asset-level output
427+
// BTC amount to the dummy amount, and the pkScript will be a newly
428+
// derived internal key with no sibling script path).
429+
// So, long story short: If we used the tapsend.DistributeCoins method
430+
// here, it could happen that the actual asset output distribution shown
431+
// in the `lncli closedchannels` command would be different from the
432+
// actual distribution in the co-op close transaction.
433+
// This could mostly be seen as an UX-only issue, but was actually
434+
// discovered while attempting to assert the final closing balance of
435+
// grouped asset channels in the litd integration test.
436+
vPackets, err := CommitmentToPackets(
437+
commitState, inputProofs, a.cfg.ChainParams, localShutdown,
438+
remoteShutdown, localOutputIndex, remoteOutputIndex, tappsbt.V1,
409439
)
410440
if err != nil {
411-
return none, fmt.Errorf("unable to distribute coins: %w", err)
441+
return none, fmt.Errorf("unable to create vPackets: %w", err)
412442
}
413443

414444
// We can now add the witness for the OP_TRUE spend of the commitment

0 commit comments

Comments
 (0)