From a627c65d65f8acfe9f36e995142bbd3a8ecaab10 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 5 Jun 2019 08:34:45 +0200 Subject: [PATCH 1/2] lnwallet/commit_sort: add commit sorting with htlc tie breaker --- lnwallet/commit_sort.go | 114 ++++++++++++++++ lnwallet/commit_sort_test.go | 253 +++++++++++++++++++++++++++++++++++ 2 files changed, 367 insertions(+) create mode 100644 lnwallet/commit_sort.go create mode 100644 lnwallet/commit_sort_test.go diff --git a/lnwallet/commit_sort.go b/lnwallet/commit_sort.go new file mode 100644 index 00000000..6fc93151 --- /dev/null +++ b/lnwallet/commit_sort.go @@ -0,0 +1,114 @@ +package lnwallet + +import ( + "bytes" + "sort" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" +) + +// InPlaceCommitSort performs an in-place sort of a commitment transaction, +// given an unsorted transaction and a list of CLTV values for the HTLCs. +// +// The sort applied is a modified BIP69 sort, that uses the CLTV values of HTLCs +// as a tie breaker in case two HTLC outputs have an identical amount and +// pkscript. The pkscripts can be the same if they share the same payment hash, +// but since the CLTV is enforced via the nLockTime of the second-layer +// transactions, the script does not directly commit to them. Instead, the CLTVs +// must be supplied separately to act as a tie-breaker, otherwise we may produce +// invalid HTLC signatures if the receiver produces an alternative ordering +// during verification. +// +// NOTE: Commitment outputs should have a 0 CLTV corresponding to their index on +// the unsorted commitment transaction. +func InPlaceCommitSort(tx *wire.MsgTx, cltvs []uint32) { + if len(tx.TxOut) != len(cltvs) { + panic("output and cltv list size mismatch") + } + + sort.Sort(sortableInputSlice(tx.TxIn)) + sort.Sort(sortableCommitOutputSlice{tx.TxOut, cltvs}) +} + +// sortableInputSlice is a slice of transaction inputs that supports sorting via +// BIP69. +type sortableInputSlice []*wire.TxIn + +// Len returns the length of the sortableInputSlice. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableInputSlice) Len() int { return len(s) } + +// Swap exchanges the position of inputs i and j. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableInputSlice) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +// Less is the BIP69 input comparison function. The sort is first applied on +// input hash (reversed / rpc-style), then index. This logic is copied from +// btcutil/txsort. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableInputSlice) Less(i, j int) bool { + // Input hashes are the same, so compare the index. + ihash := s[i].PreviousOutPoint.Hash + jhash := s[j].PreviousOutPoint.Hash + if ihash == jhash { + return s[i].PreviousOutPoint.Index < s[j].PreviousOutPoint.Index + } + + // At this point, the hashes are not equal, so reverse them to + // big-endian and return the result of the comparison. + const hashSize = chainhash.HashSize + for b := 0; b < hashSize/2; b++ { + ihash[b], ihash[hashSize-1-b] = ihash[hashSize-1-b], ihash[b] + jhash[b], jhash[hashSize-1-b] = jhash[hashSize-1-b], jhash[b] + } + return bytes.Compare(ihash[:], jhash[:]) == -1 +} + +// sortableCommitOutputSlice is a slice of transaction outputs on a commitment +// transaction and the corresponding CLTV values of any HTLCs. Commitment +// outputs should have a CLTV of 0 and the same index in cltvs. +type sortableCommitOutputSlice struct { + txouts []*wire.TxOut + cltvs []uint32 +} + +// Len returns the length of the sortableCommitOutputSlice. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableCommitOutputSlice) Len() int { + return len(s.txouts) +} + +// Swap exchanges the position of outputs i and j, as well as their +// corresponding CLTV values. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableCommitOutputSlice) Swap(i, j int) { + s.txouts[i], s.txouts[j] = s.txouts[j], s.txouts[i] + s.cltvs[i], s.cltvs[j] = s.cltvs[j], s.cltvs[i] +} + +// Less is a modified BIP69 output comparison, that sorts based on value, then +// pkscript, then CLTV value. +// +// NOTE: Part of the sort.Interface interface. +func (s sortableCommitOutputSlice) Less(i, j int) bool { + outi, outj := s.txouts[i], s.txouts[j] + + if outi.Value != outj.Value { + return outi.Value < outj.Value + } + + pkScriptCmp := bytes.Compare(outi.PkScript, outj.PkScript) + if pkScriptCmp != 0 { + return pkScriptCmp < 0 + } + + return s.cltvs[i] < s.cltvs[j] +} diff --git a/lnwallet/commit_sort_test.go b/lnwallet/commit_sort_test.go new file mode 100644 index 00000000..fd4ca6b4 --- /dev/null +++ b/lnwallet/commit_sort_test.go @@ -0,0 +1,253 @@ +package lnwallet_test + +import ( + "reflect" + "testing" + + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/lnwallet" +) + +type commitSortTest struct { + name string + tx *wire.MsgTx + cltvs []uint32 + intrans map[int]int // input transformation + outtrans map[int]int // output transformation +} + +func (t *commitSortTest) expTxIns() []*wire.TxIn { + if len(t.intrans) == 0 { + return nil + } + + expTxIns := make([]*wire.TxIn, len(t.intrans)) + for start, end := range t.intrans { + expTxIns[end] = t.tx.TxIn[start] + } + + return expTxIns +} + +func (t *commitSortTest) expTxOuts() []*wire.TxOut { + if len(t.outtrans) == 0 { + return nil + } + + expTxOuts := make([]*wire.TxOut, len(t.outtrans)) + for start, end := range t.outtrans { + expTxOuts[end] = t.tx.TxOut[start] + } + + return expTxOuts +} + +var commitSortTests = []commitSortTest{ + { + name: "sort inputs on prevoutpoint txid", + tx: &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: wire.OutPoint{ + Hash: [32]byte{0x01}, + }, + }, + { + PreviousOutPoint: wire.OutPoint{ + Hash: [32]byte{0x00}, + }, + }, + }, + }, + intrans: map[int]int{ + 0: 1, + 1: 0, + }, + }, + { + name: "sort inputs on prevoutpoint index", + tx: &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: wire.OutPoint{ + Index: 1, + }, + }, + { + PreviousOutPoint: wire.OutPoint{ + Index: 0, + }, + }, + }, + }, + intrans: map[int]int{ + 0: 1, + 1: 0, + }, + }, + { + name: "inputs already sorted", + tx: &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: wire.OutPoint{ + Index: 0, + }, + }, + { + PreviousOutPoint: wire.OutPoint{ + Index: 1, + }, + }, + }, + }, + intrans: map[int]int{ + 0: 0, + 1: 1, + }, + }, + { + name: "sort outputs on value", + tx: &wire.MsgTx{ + TxOut: []*wire.TxOut{ + { + Value: 2, + PkScript: []byte{0x0}, + }, + { + Value: 1, + PkScript: []byte{0x0}, + }, + }, + }, + cltvs: []uint32{0, 0}, + outtrans: map[int]int{ + 0: 1, + 1: 0, + }, + }, + { + name: "sort outputs on pkscript", + tx: &wire.MsgTx{ + TxOut: []*wire.TxOut{ + { + Value: 1, + PkScript: []byte{0x2}, + }, + { + Value: 1, + PkScript: []byte{0x1}, + }, + }, + }, + cltvs: []uint32{0, 0}, + outtrans: map[int]int{ + 0: 1, + 1: 0, + }, + }, + { + name: "sort outputs on cltv", + tx: &wire.MsgTx{ + TxOut: []*wire.TxOut{ + { + Value: 1, + PkScript: []byte{0x1}, + }, + { + Value: 1, + PkScript: []byte{0x1}, + }, + }, + }, + cltvs: []uint32{2, 1}, + outtrans: map[int]int{ + 0: 1, + 1: 0, + }, + }, + { + name: "sort complex outputs", + tx: &wire.MsgTx{ + TxOut: []*wire.TxOut{ + { + Value: 100000, + PkScript: []byte{0x01, 0x02}, + }, + { + Value: 200000, + PkScript: []byte{0x03, 0x02}, + }, + { + Value: 1000, + PkScript: []byte{0x03}, + }, + { + Value: 1000, + PkScript: []byte{0x02}, + }, + { + Value: 1000, + PkScript: []byte{0x1}, + }, + { + Value: 1000, + PkScript: []byte{0x1}, + }, + }, + }, + cltvs: []uint32{0, 0, 100, 90, 70, 80}, + outtrans: map[int]int{ + 0: 4, + 1: 5, + 2: 3, + 3: 2, + 4: 0, + 5: 1, + }, + }, + { + name: "outputs already sorted", + tx: &wire.MsgTx{ + TxOut: []*wire.TxOut{ + { + Value: 1, + PkScript: []byte{0x1}, + }, + { + Value: 1, + PkScript: []byte{0x1}, + }, + }, + }, + cltvs: []uint32{1, 2}, + outtrans: map[int]int{ + 0: 0, + 1: 1, + }, + }, +} + +// TestCommitSort asserts that the outputs of a transaction are properly sorted +// using InPlaceCommitSort. The outputs should always be sorted by value, then +// lexicographically by pkscript, and then CLTV value. +func TestCommitSort(t *testing.T) { + for _, test := range commitSortTests { + t.Run(test.name, func(t *testing.T) { + expTxIns := test.expTxIns() + expTxOuts := test.expTxOuts() + + lnwallet.InPlaceCommitSort(test.tx, test.cltvs) + + if !reflect.DeepEqual(test.tx.TxIn, expTxIns) { + t.Fatalf("commit inputs mismatch, want: %v, "+ + "got: %v", expTxIns, test.tx.TxIn) + } + + if !reflect.DeepEqual(test.tx.TxOut, expTxOuts) { + t.Fatalf("commit outputs mismatch, want: %v, "+ + "got: %v", expTxOuts, test.tx.TxOut) + } + }) + } +} From b26b88a1e21c54bb110bd71cf01760004764a888 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 5 Jun 2019 08:35:06 +0200 Subject: [PATCH 2/2] lnwallet/channel: use proper commitment output sorting --- lnwallet/channel.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index dddbe7e7..7cf16dd9 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2411,7 +2411,13 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // We'll now add all the HTLC outputs to the commitment transaction. // Each output includes an off-chain 2-of-2 covenant clause, so we'll // need the objective local/remote keys for this particular commitment - // as well. + // as well. For any non-dust HTLCs that are manifested on the commitment + // transaction, we'll also record its CLTV which is required to sort the + // commitment transaction below. The slice is initially sized to the + // number of existing outputs, since any outputs already added are + // commitment outputs and should correspond to zero values for the + // purposes of sorting. + cltvs := make([]uint32, len(commitTx.TxOut)) for _, htlc := range filteredHTLCView.ourUpdates { if htlcIsDust(false, c.isOurs, c.feePerKw, htlc.Amount.ToSatoshis(), c.dustLimit) { @@ -2422,6 +2428,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, if err != nil { return err } + cltvs = append(cltvs, htlc.Timeout) } for _, htlc := range filteredHTLCView.theirUpdates { if htlcIsDust(true, c.isOurs, c.feePerKw, @@ -2433,6 +2440,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, if err != nil { return err } + cltvs = append(cltvs, htlc.Timeout) } // Set the state hint of the commitment transaction to facilitate @@ -2446,7 +2454,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // Sort the transactions according to the agreed upon canonical // ordering. This lets us skip sending the entire transaction over, // instead we'll just send signatures. - txsort.InPlaceSort(commitTx) + InPlaceCommitSort(commitTx, cltvs) // Next, we'll ensure that we don't accidentally create a commitment // transaction which would be invalid by consensus.