lnwallet/channel: tie-break HTLCs by cltv when locating

This commit fixes #4118 by properly sorting the HTLC signatures sent
over the wire to match the BOLT3 BIP69+CLTV sorting of the commitment
outputs.

To do so, we expose the slice of cltv deltas for HTLCs on the unsigned
commitment after applying the commitment sorting. This will be used to
locate the proper output index, as the CLTV serves as a tie breaker
between HTLCs that otherwise have the same payment hash and amount.

Note that #3412 fixed the issue partially by ensuring the commitment was
constructed properly (and the second-level prev outpoint's txid was
correct), but failed to address that the HTLC signatures were still sent
out in the incorrect order. With this, we pass the test case introduce
in the next commit.
This commit is contained in:
Conner Fromknecht 2020-03-30 15:50:10 -07:00
parent 294e13eefc
commit 3ea3b1d0b5
No known key found for this signature in database
GPG Key ID: E7D737B67FA592C7
2 changed files with 22 additions and 9 deletions

@ -549,7 +549,7 @@ type commitment struct {
// transition. This ensures that we don't assign multiple HTLC's to the same
// index within the commitment transaction.
func locateOutputIndex(p *PaymentDescriptor, tx *wire.MsgTx, ourCommit bool,
dups map[PaymentHash][]int32) (int32, error) {
dups map[PaymentHash][]int32, cltvs []uint32) (int32, error) {
// Checks to see if element (e) exists in slice (s).
contains := func(s []int32, e int32) bool {
@ -571,8 +571,11 @@ func locateOutputIndex(p *PaymentDescriptor, tx *wire.MsgTx, ourCommit bool,
}
for i, txOut := range tx.TxOut {
cltv := cltvs[i]
if bytes.Equal(txOut.PkScript, pkScript) &&
txOut.Value == int64(p.Amount.ToSatoshis()) {
txOut.Value == int64(p.Amount.ToSatoshis()) &&
cltv == p.Timeout {
// If this payment hash and index has already been
// found, then we'll continue in order to avoid any
@ -587,8 +590,8 @@ func locateOutputIndex(p *PaymentDescriptor, tx *wire.MsgTx, ourCommit bool,
}
}
return 0, fmt.Errorf("unable to find htlc: script=%x, value=%v",
pkScript, p.Amount)
return 0, fmt.Errorf("unable to find htlc: script=%x, value=%v, "+
"cltv=%v", pkScript, p.Amount, p.Timeout)
}
// populateHtlcIndexes modifies the set of HTLC's locked-into the target view
@ -596,7 +599,9 @@ func locateOutputIndex(p *PaymentDescriptor, tx *wire.MsgTx, ourCommit bool,
// we need to keep track of the indexes of each HTLC in order to properly write
// the current state to disk, and also to locate the PaymentDescriptor
// corresponding to HTLC outputs in the commitment transaction.
func (c *commitment) populateHtlcIndexes(chanType channeldb.ChannelType) error {
func (c *commitment) populateHtlcIndexes(chanType channeldb.ChannelType,
cltvs []uint32) error {
// First, we'll set up some state to allow us to locate the output
// index of the all the HTLC's within the commitment transaction. We
// must keep this index so we can validate the HTLC signatures sent to
@ -632,7 +637,7 @@ func (c *commitment) populateHtlcIndexes(chanType channeldb.ChannelType) error {
// signatures.
case c.isOurs:
htlc.localOutputIndex, err = locateOutputIndex(
htlc, c.txn, c.isOurs, dups,
htlc, c.txn, c.isOurs, dups, cltvs,
)
if err != nil {
return err
@ -653,7 +658,7 @@ func (c *commitment) populateHtlcIndexes(chanType channeldb.ChannelType) error {
// index within the HTLC index.
case !c.isOurs:
htlc.remoteOutputIndex, err = locateOutputIndex(
htlc, c.txn, c.isOurs, dups,
htlc, c.txn, c.isOurs, dups, cltvs,
)
if err != nil {
return err
@ -2440,8 +2445,11 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
}
// Finally, we'll populate all the HTLC indexes so we can track the
// locations of each HTLC in the commitment state.
if err := c.populateHtlcIndexes(lc.channelState.ChanType); err != nil {
// locations of each HTLC in the commitment state. We pass in the sorted
// slice of CLTV deltas in order to properly locate HTLCs that otherwise
// have the same payment hash and amount.
err = c.populateHtlcIndexes(lc.channelState.ChanType, commitTx.cltvs)
if err != nil {
return nil, err
}

@ -382,6 +382,10 @@ type unsignedCommitmentTx struct {
// balances before creating the commitment transaction as one party must
// pay the commitment fee.
theirBalance lnwire.MilliSatoshi
// cltvs is a sorted list of CLTV deltas for each HTLC on the commitment
// transaction. Any non-htlc outputs will have a CLTV delay of zero.
cltvs []uint32
}
// createUnsignedCommitmentTx generates the unsigned commitment transaction for
@ -562,6 +566,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance,
fee: commitFee,
ourBalance: ourBalance,
theirBalance: theirBalance,
cltvs: cltvs,
}, nil
}