From c3b2791158de6c188d0fa1f82888d0b60fb15fac Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 20 Apr 2021 08:46:23 +0200 Subject: [PATCH] breacharbiter: don't transition to second level if own spend --- breacharbiter.go | 19 ++++++++ breacharbiter_test.go | 105 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 4 deletions(-) diff --git a/breacharbiter.go b/breacharbiter.go index db792aedf..fdd5cd163 100644 --- a/breacharbiter.go +++ b/breacharbiter.go @@ -460,11 +460,30 @@ func updateBreachInfo(breachInfo *retributionInfo, spends []spend) { for _, s := range spends { breachedOutput := &inputs[s.index] + txIn := s.detail.SpendingTx.TxIn[s.detail.SpenderInputIndex] switch breachedOutput.witnessType { case input.HtlcAcceptedRevoke: fallthrough case input.HtlcOfferedRevoke: + // If the HTLC output was spent using the revocation + // key, it is our own spend, and we can forget the + // output. Otherwise it has been taken to the second + // level. + signDesc := &breachedOutput.signDesc + ok, err := input.IsHtlcSpendRevoke(txIn, signDesc) + if err != nil { + brarLog.Errorf("Unable to determine if "+ + "revoke spend: %v", err) + break + } + + if ok { + brarLog.Debugf("HTLC spend was our own " + + "revocation spend") + break + } + brarLog.Infof("Spend on second-level "+ "%s(%v) for ChannelPoint(%v) "+ "transitions to second-level output", diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 7072250a1..d7818fe6f 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -36,6 +36,7 @@ import ( "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/shachain" + "github.com/stretchr/testify/require" ) var ( @@ -1231,6 +1232,10 @@ type breachTest struct { // htlc is in effect "readded" to the set of inputs. spend2ndLevel bool + // sweepHtlc tests that the HTLC output is swept using the revocation + // path in a separate tx. + sweepHtlc bool + // sendFinalConf informs the test to send a confirmation for the justice // transaction before asserting the arbiter is cleaned up. sendFinalConf bool @@ -1248,10 +1253,11 @@ type spendTxs struct { commitSpendTx *wire.MsgTx htlc2ndLevlTx *wire.MsgTx htlc2ndLevlSpend *wire.MsgTx + htlcSweep *wire.MsgTx } -func getSpendTransactions(_ input.Signer, _ *wire.OutPoint, - retribution *lnwallet.BreachRetribution) *spendTxs { +func getSpendTransactions(signer input.Signer, chanPoint *wire.OutPoint, + retribution *lnwallet.BreachRetribution) (*spendTxs, error) { localOutpoint := retribution.LocalOutpoint remoteOutpoint := retribution.RemoteOutpoint @@ -1303,11 +1309,50 @@ func getSpendTransactions(_ input.Signer, _ *wire.OutPoint, }, } + // htlcSweep is used to spend the HTLC output directly using the + // revocation key. + htlcSweep := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: htlcOutpoint, + }, + }, + TxOut: []*wire.TxOut{ + {Value: 21000}, + }, + } + + // In order for the breacharbiter to detect that it is being spent + // using the revocation key, it will inspect the witness. Therefore + // sign and add the witness to the HTLC sweep. + retInfo := newRetributionInfo(chanPoint, retribution) + + hashCache := txscript.NewTxSigHashes(htlcSweep) + for i := range retInfo.breachedOutputs { + inp := &retInfo.breachedOutputs[i] + + // Find the HTLC output. so we can add the witness. + switch inp.witnessType { + case input.HtlcAcceptedRevoke: + fallthrough + case input.HtlcOfferedRevoke: + inputScript, err := inp.CraftInputScript( + signer, htlcSweep, hashCache, 0, + ) + if err != nil { + return nil, err + } + + htlcSweep.TxIn[0].Witness = inputScript.Witness + } + } + return &spendTxs{ commitSpendTx: commitSpendTx, htlc2ndLevlTx: htlc2ndLevlTx, htlc2ndLevlSpend: htlcSpendTx, - } + htlcSweep: htlcSweep, + }, nil } var breachTests = []breachTest{ @@ -1422,6 +1467,50 @@ var breachTests = []breachTest{ } }, }, + { // nolint: dupl + // Test that if the HTLC output is swept via the revoke path + // (by us) in a separate tx, it will be handled correctly. + name: "sweep htlc", + sweepHtlc: true, + whenNonZeroInputs: func(t *testing.T, + inputs map[wire.OutPoint]struct{}, + publTx chan *wire.MsgTx, _ chainhash.Hash) { + + var tx *wire.MsgTx + select { + case tx = <-publTx: + case <-time.After(5 * time.Second): + t.Fatalf("tx was not published") + } + + // The justice transaction should have the same number + // of inputs as we are tracking in the test. + if len(tx.TxIn) != len(inputs) { + t.Fatalf("expected justice txn to have %d "+ + "inputs, found %d", len(inputs), + len(tx.TxIn)) + } + + // Ensure that each input exists on the justice + // transaction. + for in := range inputs { + findInputIndex(t, in, tx) + } + }, + whenZeroInputs: func(t *testing.T, + inputs map[wire.OutPoint]struct{}, + publTx chan *wire.MsgTx, _ chainhash.Hash) { + + // Sanity check to ensure the brar doesn't try to + // broadcast another sweep, since all outputs have been + // spent externally. + select { + case <-publTx: + t.Fatalf("tx published unexpectedly") + case <-time.After(50 * time.Millisecond): + } + }, + }, } // TestBreachSpends checks the behavior of the breach arbiter in response to @@ -1543,9 +1632,10 @@ func testBreachSpends(t *testing.T, test breachTest) { remoteOutpoint := retribution.RemoteOutpoint htlcOutpoint := retribution.HtlcRetributions[0].OutPoint - spendTxs := getSpendTransactions( + spendTxs, err := getSpendTransactions( brar.cfg.Signer, chanPoint, retribution, ) + require.NoError(t, err) // Construct a map from outpoint on the force close to the transaction // we want it to be spent by. As the test progresses, this map will be @@ -1567,6 +1657,13 @@ func testBreachSpends(t *testing.T, test breachTest) { htlc2ndLevlTx := spendTxs.htlc2ndLevlTx htlcSpendTx := spendTxs.htlc2ndLevlSpend + + // If the test is checking sweep of the HTLC directly without the + // second level, insert the sweep tx instead. + if test.sweepHtlc { + spentBy[htlcOutpoint] = spendTxs.htlcSweep + } + // Until no more inputs to spend remain, deliver the spend events and // process the assertions prescribed by the test case. for len(spentBy) > 0 {