From b078cea83f3516aecaeb80216f6276d1847a9bab Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 18 Mar 2019 16:59:48 -0700 Subject: [PATCH 1/5] contractcourt: move claimCleanUp from contest resolver to timeout --- .../htlc_outgoing_contest_resolver.go | 75 +---------------- contractcourt/htlc_timeout_resolver.go | 81 +++++++++++++++++++ 2 files changed, 83 insertions(+), 73 deletions(-) diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index ad52668c..3aa47b8d 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -4,13 +4,7 @@ import ( "fmt" "io" - "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" - "github.com/davecgh/go-spew/spew" - - "github.com/lightningnetwork/lnd/chainntnfs" - "github.com/lightningnetwork/lnd/input" - "github.com/lightningnetwork/lnd/lntypes" ) // htlcOutgoingContestResolver is a ContractResolver that's able to resolve an @@ -44,70 +38,6 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { return nil, nil } - // claimCleanUp is a helper function that's called once the HTLC output - // is spent by the remote party. It'll extract the preimage, add it to - // the global cache, and finally send the appropriate clean up message. - claimCleanUp := func(commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { - // Depending on if this is our commitment or not, then we'll be - // looking for a different witness pattern. - spenderIndex := commitSpend.SpenderInputIndex - spendingInput := commitSpend.SpendingTx.TxIn[spenderIndex] - - log.Infof("%T(%v): extracting preimage! remote party spent "+ - "HTLC with tx=%v", h, h.htlcResolution.ClaimOutpoint, - spew.Sdump(commitSpend.SpendingTx)) - - // If this is the remote party's commitment, then we'll be - // looking for them to spend using the second-level success - // transaction. - var preimageBytes []byte - if h.htlcResolution.SignedTimeoutTx == nil { - // The witness stack when the remote party sweeps the - // output to them looks like: - // - // * - preimageBytes = spendingInput.Witness[3] - } else { - // Otherwise, they'll be spending directly from our - // commitment output. In which case the witness stack - // looks like: - // - // * - preimageBytes = spendingInput.Witness[1] - } - - preimage, err := lntypes.MakePreimage(preimageBytes) - if err != nil { - return nil, err - } - - log.Infof("%T(%v): extracting preimage=%v from on-chain "+ - "spend!", h, h.htlcResolution.ClaimOutpoint, - preimage) - - // With the preimage obtained, we can now add it to the global - // cache. - if err := h.PreimageDB.AddPreimages(preimage); err != nil { - log.Errorf("%T(%v): unable to add witness to cache", - h, h.htlcResolution.ClaimOutpoint) - } - - var pre [32]byte - copy(pre[:], preimage[:]) - - // Finally, we'll send the clean up message, mark ourselves as - // resolved, then exit. - if err := h.DeliverResolutionMsg(ResolutionMsg{ - SourceChan: h.ShortChanID, - HtlcIndex: h.htlcIndex, - PreImage: &pre, - }); err != nil { - return nil, err - } - h.resolved = true - return nil, h.Checkpoint(h) - } - // Otherwise, we'll watch for two external signals to decide if we'll // morph into another resolver, or fully resolve the contract. @@ -161,7 +91,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { } // TODO(roasbeef): Checkpoint? - return claimCleanUp(commitSpend) + return h.claimCleanUp(commitSpend) // If it hasn't, then we'll watch for both the expiration, and the // sweeping out this output. @@ -190,7 +120,6 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // // Source: // https://github.com/btcsuite/btcd/blob/991d32e72fe84d5fbf9c47cd604d793a0cd3a072/blockchain/validate.go#L154 - if uint32(currentHeight) >= h.htlcResolution.Expiry-1 { log.Infof("%T(%v): HTLC has expired (height=%v, expiry=%v), "+ "transforming into timeout resolver", h, @@ -242,7 +171,7 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // party is by revealing the preimage. So we'll perform // our duties to clean up the contract once it has been // claimed. - return claimCleanUp(commitSpend) + return h.claimCleanUp(commitSpend) case <-h.Quit: return nil, fmt.Errorf("resolver cancelled") diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index 55e9cc00..fa51463a 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -6,6 +6,10 @@ import ( "io" "github.com/btcsuite/btcd/wire" + "github.com/davecgh/go-spew/spew" + "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" ) @@ -67,6 +71,83 @@ func (h *htlcTimeoutResolver) ResolverKey() []byte { return key[:] } +const ( + // expectedRemoteWitnessSuccessSize is the expected size of the witness + // on the remote commitment transaction for an outgoing HTLC that is + // swept on-chain by them with pre-image. + expectedRemoteWitnessSuccessSize = 5 + + // remotePreimageIndex index within the witness on the remote + // commitment transaction that will hold they pre-image if they go to + // sweep it on chain. + remotePreimageIndex = 3 + + // localPreimageIndex is the index within the witness on the local + // commitment transaction for an outgoing HTLC that will hold the + // pre-image if the remote party sweeps it. + localPreimageIndex = 1 +) + +// claimCleanUp is a helper method that's called once the HTLC output is spent +// by the remote party. It'll extract the preimage, add it to the global cache, +// and finally send the appropriate clean up message. +func (h *htlcTimeoutResolver) claimCleanUp(commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { + // Depending on if this is our commitment or not, then we'll be looking + // for a different witness pattern. + spenderIndex := commitSpend.SpenderInputIndex + spendingInput := commitSpend.SpendingTx.TxIn[spenderIndex] + + log.Infof("%T(%v): extracting preimage! remote party spent "+ + "HTLC with tx=%v", h, h.htlcResolution.ClaimOutpoint, + spew.Sdump(commitSpend.SpendingTx)) + + // If this is the remote party's commitment, then we'll be looking for + // them to spend using the second-level success transaction. + var preimageBytes []byte + if h.htlcResolution.SignedTimeoutTx == nil { + // The witness stack when the remote party sweeps the output to + // them looks like: + // + // * <0> + preimageBytes = spendingInput.Witness[remotePreimageIndex] + } else { + // Otherwise, they'll be spending directly from our commitment + // output. In which case the witness stack looks like: + // + // * + preimageBytes = spendingInput.Witness[localPreimageIndex] + } + + preimage, err := lntypes.MakePreimage(preimageBytes) + if err != nil { + return nil, fmt.Errorf("unable to create pre-image from "+ + "witness: %v", err) + } + + log.Infof("%T(%v): extracting preimage=%v from on-chain "+ + "spend!", h, h.htlcResolution.ClaimOutpoint, preimage) + + // With the preimage obtained, we can now add it to the global cache. + if err := h.PreimageDB.AddPreimages(preimage); err != nil { + log.Errorf("%T(%v): unable to add witness to cache", + h, h.htlcResolution.ClaimOutpoint) + } + + var pre [32]byte + copy(pre[:], preimage[:]) + + // Finally, we'll send the clean up message, mark ourselves as + // resolved, then exit. + if err := h.DeliverResolutionMsg(ResolutionMsg{ + SourceChan: h.ShortChanID, + HtlcIndex: h.htlcIndex, + PreImage: &pre, + }); err != nil { + return nil, err + } + h.resolved = true + return nil, h.Checkpoint(h) +} // Resolve kicks off full resolution of an outgoing HTLC output. If it's our // commitment, it isn't resolved until we see the second level HTLC txn // confirmed. If it's the remote party's commitment, we don't resolve until we From e1a07b68e84e79148dd1f263c3cf38cd42cbaf32 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 18 Mar 2019 17:00:43 -0700 Subject: [PATCH 2/5] contractcourt: extract which timeout HTLC output to watch into new method --- .../htlc_outgoing_contest_resolver.go | 32 +++--------------- contractcourt/htlc_timeout_resolver.go | 33 +++++++++++++++++++ 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/contractcourt/htlc_outgoing_contest_resolver.go b/contractcourt/htlc_outgoing_contest_resolver.go index 3aa47b8d..222fd1f3 100644 --- a/contractcourt/htlc_outgoing_contest_resolver.go +++ b/contractcourt/htlc_outgoing_contest_resolver.go @@ -40,42 +40,20 @@ func (h *htlcOutgoingContestResolver) Resolve() (ContractResolver, error) { // Otherwise, we'll watch for two external signals to decide if we'll // morph into another resolver, or fully resolve the contract. - + // // The output we'll be watching for is the *direct* spend from the HTLC // output. If this isn't our commitment transaction, it'll be right on // the resolution. Otherwise, we fetch this pointer from the input of // the time out transaction. - var ( - outPointToWatch wire.OutPoint - scriptToWatch []byte - err error - ) - - // TODO(joostjager): output already set properly in - // lnwallet.newOutgoingHtlcResolution? And script too? - if h.htlcResolution.SignedTimeoutTx == nil { - outPointToWatch = h.htlcResolution.ClaimOutpoint - scriptToWatch = h.htlcResolution.SweepSignDesc.Output.PkScript - } else { - // If this is the remote party's commitment, then we'll need to - // grab watch the output that our timeout transaction points - // to. We can directly grab the outpoint, then also extract the - // witness script (the last element of the witness stack) to - // re-construct the pkScipt we need to watch. - outPointToWatch = h.htlcResolution.SignedTimeoutTx.TxIn[0].PreviousOutPoint - witness := h.htlcResolution.SignedTimeoutTx.TxIn[0].Witness - scriptToWatch, err = input.WitnessScriptHash( - witness[len(witness)-1], - ) - if err != nil { - return nil, err - } + outPointToWatch, scriptToWatch, err := h.chainDetailsToWatch() + if err != nil { + return nil, err } // First, we'll register for a spend notification for this output. If // the remote party sweeps with the pre-image, we'll be notified. spendNtfn, err := h.Notifier.RegisterSpendNtfn( - &outPointToWatch, scriptToWatch, h.broadcastHeight, + outPointToWatch, scriptToWatch, h.broadcastHeight, ) if err != nil { return nil, err diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index fa51463a..d0fab2fb 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -148,6 +148,39 @@ func (h *htlcTimeoutResolver) claimCleanUp(commitSpend *chainntnfs.SpendDetail) h.resolved = true return nil, h.Checkpoint(h) } + +// chainDetailsToWatch returns the output and script which we use to watch for +// spends from the direct HTLC output on the commitment transaction. +// +// TODO(joostjager): output already set properly in +// lnwallet.newOutgoingHtlcResolution? And script too? +func (h *htlcTimeoutResolver) chainDetailsToWatch() (*wire.OutPoint, []byte, error) { + // If there's no timeout transaction, then the claim output is the + // output directly on the commitment transaction, so we'll just use + // that. + if h.htlcResolution.SignedTimeoutTx == nil { + outPointToWatch := h.htlcResolution.ClaimOutpoint + scriptToWatch := h.htlcResolution.SweepSignDesc.Output.PkScript + + return &outPointToWatch, scriptToWatch, nil + } + + // If this is the remote party's commitment, then we'll need to grab + // watch the output that our timeout transaction points to. We can + // directly grab the outpoint, then also extract the witness script + // (the last element of the witness stack) to re-construct the pkScript + // we need to watch. + outPointToWatch := h.htlcResolution.SignedTimeoutTx.TxIn[0].PreviousOutPoint + witness := h.htlcResolution.SignedTimeoutTx.TxIn[0].Witness + scriptToWatch, err := input.WitnessScriptHash( + witness[len(witness)-1], + ) + if err != nil { + return nil, nil, err + } + + return &outPointToWatch, scriptToWatch, nil +} // Resolve kicks off full resolution of an outgoing HTLC output. If it's our // commitment, it isn't resolved until we see the second level HTLC txn // confirmed. If it's the remote party's commitment, we don't resolve until we From 6b24b6dabd06820932da2d5417f02cdab7f44949 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 18 Mar 2019 17:04:19 -0700 Subject: [PATCH 3/5] contractcourt: simplify htlcTimeoutResolver, unify with HTLC contest logic In this commit, we simplify the existing `htlcTImeoutResolver` with some newly refactored out methods from the `htlcTimeoutContestResolver`. The resulting logic is easier to follow as it's more linear, and only deals with spend notifications rather than both spend _and_ confirmation notifications. --- contractcourt/channel_arbitrator_test.go | 35 ++++-- contractcourt/htlc_timeout_resolver.go | 139 ++++++++++++++--------- 2 files changed, 113 insertions(+), 61 deletions(-) diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index ed64c203..a072773a 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/contractcourt/channel_arbitrator_test.go @@ -511,8 +511,10 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { // The force close request should trigger broadcast of the commitment // transaction. - assertStateTransitions(t, arbLog.newStates, - StateBroadcastCommit, StateCommitmentBroadcasted) + assertStateTransitions( + t, arbLog.newStates, StateBroadcastCommit, + StateCommitmentBroadcasted, + ) select { case <-respChan: case <-time.After(5 * time.Second): @@ -529,7 +531,17 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { } // Now notify about the local force close getting confirmed. - closeTx := &wire.MsgTx{} + closeTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: wire.OutPoint{}, + Witness: [][]byte{ + {0x1}, + {0x2}, + }, + }, + }, + } htlcOp := wire.OutPoint{ Hash: closeTx.TxHash(), @@ -569,8 +581,10 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { &channeldb.ChannelCloseSummary{}, } - assertStateTransitions(t, arbLog.newStates, StateContractClosed, - StateWaitingFullResolution) + assertStateTransitions( + t, arbLog.newStates, StateContractClosed, + StateWaitingFullResolution, + ) // htlcOutgoingContestResolver is now active and waiting for the HTLC to // expire. It should not yet have passed it on for incubation. @@ -592,12 +606,13 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { t.Fatalf("no response received") } - // Notify resolver that output of the commitment has been spent. - notifier.confChan <- &chainntnfs.TxConfirmation{} + // Notify resolver that the HTLC output of the commitment has been + // spent. + notifier.spendChan <- &chainntnfs.SpendDetail{SpendingTx: closeTx} // As this is our own commitment transaction, the HTLC will go through - // to the second level. Channel arbitrator should still not be marked as - // resolved. + // to the second level. Channel arbitrator should still not be marked + // as resolved. select { case <-resolved: t.Fatalf("channel resolved prematurely") @@ -605,7 +620,7 @@ func TestChannelArbitratorLocalForceClosePendingHtlc(t *testing.T) { } // Notify resolver that the second level transaction is spent. - notifier.spendChan <- &chainntnfs.SpendDetail{} + notifier.spendChan <- &chainntnfs.SpendDetail{SpendingTx: closeTx} // At this point channel should be marked as resolved. assertStateTransitions(t, arbLog.newStates, StateFullyResolved) diff --git a/contractcourt/htlc_timeout_resolver.go b/contractcourt/htlc_timeout_resolver.go index d0fab2fb..f7954fe6 100644 --- a/contractcourt/htlc_timeout_resolver.go +++ b/contractcourt/htlc_timeout_resolver.go @@ -91,7 +91,9 @@ const ( // claimCleanUp is a helper method that's called once the HTLC output is spent // by the remote party. It'll extract the preimage, add it to the global cache, // and finally send the appropriate clean up message. -func (h *htlcTimeoutResolver) claimCleanUp(commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { +func (h *htlcTimeoutResolver) claimCleanUp( + commitSpend *chainntnfs.SpendDetail) (ContractResolver, error) { + // Depending on if this is our commitment or not, then we'll be looking // for a different witness pattern. spenderIndex := commitSpend.SpenderInputIndex @@ -172,15 +174,51 @@ func (h *htlcTimeoutResolver) chainDetailsToWatch() (*wire.OutPoint, []byte, err // we need to watch. outPointToWatch := h.htlcResolution.SignedTimeoutTx.TxIn[0].PreviousOutPoint witness := h.htlcResolution.SignedTimeoutTx.TxIn[0].Witness - scriptToWatch, err := input.WitnessScriptHash( - witness[len(witness)-1], - ) + scriptToWatch, err := input.WitnessScriptHash(witness[len(witness)-1]) if err != nil { return nil, nil, err } return &outPointToWatch, scriptToWatch, nil } + +// isSuccessSpend returns true if the passed spend on the specified commitment +// is a success spend that reveals the pre-image or not. +func isSuccessSpend(spend *chainntnfs.SpendDetail, localCommit bool) bool { + // Based on the spending input index and transaction, obtain the + // witness that tells us what type of spend this is. + spenderIndex := spend.SpenderInputIndex + spendingInput := spend.SpendingTx.TxIn[spenderIndex] + spendingWitness := spendingInput.Witness + + // If this is the remote commitment then the only possible spends for + // outgoing HTLCs are: + // + // RECVR: <0> (2nd level success spend) + // REVOK: + // SENDR: 0 + // + // In this case, if 5 witness elements are present (factoring the + // witness script), and the 3rd element is the size of the pre-image, + // then this is a remote spend. If not, then we swept it ourselves, or + // revoked their output. + if !localCommit { + return len(spendingWitness) == expectedRemoteWitnessSuccessSize && + len(spendingWitness[remotePreimageIndex]) == lntypes.HashSize + } + + // Otherwise, for our commitment, the only possible spends for an + // outgoing HTLC are: + // + // SENDR: <0> <0> (2nd level timeout) + // RECVR: + // REVOK: + // + // So the only success case has the pre-image as the 2nd (index 1) + // element in the witness. + return len(spendingWitness[localPreimageIndex]) == lntypes.HashSize +} + // Resolve kicks off full resolution of an outgoing HTLC output. If it's our // commitment, it isn't resolved until we see the second level HTLC txn // confirmed. If it's the remote party's commitment, we don't resolve until we @@ -243,60 +281,59 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { return nil } - // With the output sent to the nursery, we'll now wait until the output - // has been fully resolved before sending the clean up message. - // - // TODO(roasbeef): need to be able to cancel nursery? - // * if they pull on-chain while we're waiting - - // If we don't have a second layer transaction, then this is a remote - // party's commitment, so we'll watch for a direct spend. - if h.htlcResolution.SignedTimeoutTx == nil { - // We'll block until: the HTLC output has been spent, and the - // transaction spending that output is sufficiently confirmed. - log.Infof("%T(%v): waiting for nursery to spend CLTV-locked "+ - "output", h, h.htlcResolution.ClaimOutpoint) - if err := waitForOutputResolution(); err != nil { - return nil, err - } - } else { - // Otherwise, this is our commitment, so we'll watch for the - // second-level transaction to be sufficiently confirmed. - secondLevelTXID := h.htlcResolution.SignedTimeoutTx.TxHash() - sweepScript := h.htlcResolution.SignedTimeoutTx.TxOut[0].PkScript - confNtfn, err := h.Notifier.RegisterConfirmationsNtfn( - &secondLevelTXID, sweepScript, 1, h.broadcastHeight, - ) - if err != nil { - return nil, err - } - - log.Infof("%T(%v): waiting second-level tx (txid=%v) to be "+ - "fully confirmed", h, h.htlcResolution.ClaimOutpoint, - secondLevelTXID) - - select { - case _, ok := <-confNtfn.Confirmed: - if !ok { - return nil, fmt.Errorf("quitting") - } - - case <-h.Quit: - return nil, fmt.Errorf("quitting") - } + // Now that we've handed off the HTLC to the nursery, we'll watch for a + // spend of the output, and make our next move off of that. Depending + // on if this is our commitment, or the remote party's commitment, + // we'll be watching a different outpoint and script. + outpointToWatch, scriptToWatch, err := h.chainDetailsToWatch() + if err != nil { + return nil, err + } + spendNtfn, err := h.Notifier.RegisterSpendNtfn( + outpointToWatch, scriptToWatch, h.broadcastHeight, + ) + if err != nil { + return nil, err } - // TODO(roasbeef): need to watch for remote party sweeping with pre-image? - // * have another waiting on spend above, will check the type, if it's - // pre-image, then we'll cancel, and send a clean up back with - // pre-image, also add to preimage cache + log.Infof("%T(%v): waiting for HTLC output %v to be spent"+ + "fully confirmed", h, h.htlcResolution.ClaimOutpoint, + outpointToWatch) + + // We'll block here until either we exit, or the HTLC output on the + // commitment transaction has been spent. + var ( + spend *chainntnfs.SpendDetail + ok bool + ) + select { + case spend, ok = <-spendNtfn.Spend: + if !ok { + return nil, fmt.Errorf("quitting") + } + + case <-h.Quit: + return nil, fmt.Errorf("quitting") + } + + // If the spend reveals the pre-image, then we'll enter the clean up + // workflow to pass the pre-image back to the incoming link, add it to + // the witness cache, and exit. + if isSuccessSpend(spend, h.htlcResolution.SignedTimeoutTx != nil) { + log.Infof("%T(%v): HTLC has been swept with pre-image by "+ + "remote party during timeout flow! Adding pre-image to "+ + "witness cache", h.htlcResolution.ClaimOutpoint) + + return h.claimCleanUp(spend) + } log.Infof("%T(%v): resolving htlc with incoming fail msg, fully "+ "confirmed", h, h.htlcResolution.ClaimOutpoint) // At this point, the second-level transaction is sufficiently // confirmed, or a transaction directly spending the output is. - // Therefore, we can now send back our clean up message. + // Therefore, we can now send back our clean up message, failing the + // HTLC on the incoming link. failureMsg := &lnwire.FailPermanentChannelFailure{} if err := h.DeliverResolutionMsg(ResolutionMsg{ SourceChan: h.ShortChanID, @@ -307,7 +344,7 @@ func (h *htlcTimeoutResolver) Resolve() (ContractResolver, error) { } // Finally, if this was an output on our commitment transaction, we'll - // for the second-level HTLC output to be spent, and for that + // wait for the second-level HTLC output to be spent, and for that // transaction itself to confirm. if h.htlcResolution.SignedTimeoutTx != nil { log.Infof("%T(%v): waiting for nursery to spend CSV delayed "+ From db411c244eec2c49dff719b34b963330a26163bf Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 18 Mar 2019 20:55:10 -0700 Subject: [PATCH 4/5] contractcourt: add new set of tests in htlcTimeoutResolver to exercise refactorings --- contractcourt/contract_resolvers_test.go | 1 - contractcourt/htlc_timeout_resolver_test.go | 364 ++++++++++++++++++++ 2 files changed, 364 insertions(+), 1 deletion(-) delete mode 100644 contractcourt/contract_resolvers_test.go create mode 100644 contractcourt/htlc_timeout_resolver_test.go diff --git a/contractcourt/contract_resolvers_test.go b/contractcourt/contract_resolvers_test.go deleted file mode 100644 index 236a3336..00000000 --- a/contractcourt/contract_resolvers_test.go +++ /dev/null @@ -1 +0,0 @@ -package contractcourt diff --git a/contractcourt/htlc_timeout_resolver_test.go b/contractcourt/htlc_timeout_resolver_test.go new file mode 100644 index 00000000..c83d66c9 --- /dev/null +++ b/contractcourt/htlc_timeout_resolver_test.go @@ -0,0 +1,364 @@ +package contractcourt + +import ( + "bytes" + "fmt" + "sync" + "testing" + "time" + + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet" +) + +type mockSigner struct { +} + +func (m *mockSigner) SignOutputRaw(tx *wire.MsgTx, + signDesc *input.SignDescriptor) ([]byte, error) { + return nil, nil +} + +func (m *mockSigner) ComputeInputScript(tx *wire.MsgTx, + signDesc *input.SignDescriptor) (*input.Script, error) { + return nil, nil +} + +type mockWitnessBeacon struct { + preImageUpdates chan lntypes.Preimage + + newPreimages chan []lntypes.Preimage +} + +func (m *mockWitnessBeacon) SubscribeUpdates() *WitnessSubscription { + return &WitnessSubscription{ + WitnessUpdates: m.preImageUpdates, + CancelSubscription: func() {}, + } +} + +func (m *mockWitnessBeacon) LookupPreimage(payhash lntypes.Hash) (lntypes.Preimage, bool) { + return lntypes.Preimage{}, false +} + +func (m *mockWitnessBeacon) AddPreimages(preimages ...lntypes.Preimage) error { + m.newPreimages <- preimages + return nil +} + +// TestHtlcTimeoutResolver tests that the timeout resolver properly handles all +// variations of possible local+remote spends. +func TestHtlcTimeoutResolver(t *testing.T) { + t.Parallel() + + fakePreimageBytes := bytes.Repeat([]byte{1}, lntypes.HashSize) + + var ( + htlcOutpoint wire.OutPoint + fakePreimage lntypes.Preimage + ) + fakeSignDesc := &input.SignDescriptor{ + Output: &wire.TxOut{}, + } + + copy(fakePreimage[:], fakePreimageBytes) + + signer := &mockSigner{} + sweepTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: htlcOutpoint, + Witness: [][]byte{{0x01}}, + }, + }, + } + fakeTimeout := int32(5) + + templateTx := &wire.MsgTx{ + TxIn: []*wire.TxIn{ + { + PreviousOutPoint: htlcOutpoint, + }, + }, + } + + testCases := []struct { + // name is a human readable description of the test case. + name string + + // remoteCommit denotes if the commitment broadcast was the + // remote commitment or not. + remoteCommit bool + + // timeout denotes if the HTLC should be let timeout, or if the + // "remote" party should sweep it on-chain. This also affects + // what type of resolution message we expect. + timeout bool + + // txToBroadcast is a function closure that should generate the + // transaction that should spend the HTLC output. Test authors + // can use this to customize the witness used when spending to + // trigger various redemption cases. + txToBroadcast func() (*wire.MsgTx, error) + }{ + // Remote commitment is broadcast, we time out the HTLC on + // chain, and should expect a fail HTLC resolution. + { + name: "timeout remote tx", + remoteCommit: true, + timeout: true, + txToBroadcast: func() (*wire.MsgTx, error) { + witness, err := input.ReceiverHtlcSpendTimeout( + signer, fakeSignDesc, sweepTx, + fakeTimeout, + ) + if err != nil { + return nil, err + } + + templateTx.TxIn[0].Witness = witness + return templateTx, nil + }, + }, + + // Our local commitment is broadcast, we timeout the HTLC and + // still expect an HTLC fail resolution. + { + name: "timeout local tx", + remoteCommit: false, + timeout: true, + txToBroadcast: func() (*wire.MsgTx, error) { + witness, err := input.SenderHtlcSpendTimeout( + nil, signer, fakeSignDesc, sweepTx, + ) + if err != nil { + return nil, err + } + + templateTx.TxIn[0].Witness = witness + return templateTx, nil + }, + }, + + // The remote commitment is broadcast, they sweep with the + // pre-image, we should get a settle HTLC resolution. + { + name: "success remote tx", + remoteCommit: true, + timeout: false, + txToBroadcast: func() (*wire.MsgTx, error) { + witness, err := input.ReceiverHtlcSpendRedeem( + nil, fakePreimageBytes, signer, + fakeSignDesc, sweepTx, + ) + if err != nil { + return nil, err + } + + templateTx.TxIn[0].Witness = witness + return templateTx, nil + }, + }, + + // The local commitment is broadcast, they sweep it with a + // timeout from the output, and we should still get the HTLC + // settle resolution back. + { + name: "success local tx", + remoteCommit: false, + timeout: false, + txToBroadcast: func() (*wire.MsgTx, error) { + witness, err := input.SenderHtlcSpendRedeem( + signer, fakeSignDesc, sweepTx, + fakePreimageBytes, + ) + if err != nil { + return nil, err + } + + templateTx.TxIn[0].Witness = witness + return templateTx, nil + }, + }, + } + + notifier := &mockNotifier{ + epochChan: make(chan *chainntnfs.BlockEpoch), + spendChan: make(chan *chainntnfs.SpendDetail), + confChan: make(chan *chainntnfs.TxConfirmation), + } + witnessBeacon := &mockWitnessBeacon{ + preImageUpdates: make(chan lntypes.Preimage, 1), + newPreimages: make(chan []lntypes.Preimage), + } + + for _, testCase := range testCases { + t.Logf("Running test case: %v", testCase.name) + + checkPointChan := make(chan struct{}, 1) + incubateChan := make(chan struct{}, 1) + resolutionChan := make(chan ResolutionMsg, 1) + + chainCfg := ChannelArbitratorConfig{ + ChainArbitratorConfig: ChainArbitratorConfig{ + Notifier: notifier, + PreimageDB: witnessBeacon, + IncubateOutputs: func(wire.OutPoint, + *lnwallet.CommitOutputResolution, + *lnwallet.OutgoingHtlcResolution, + *lnwallet.IncomingHtlcResolution, + uint32) error { + + incubateChan <- struct{}{} + return nil + }, + DeliverResolutionMsg: func(msgs ...ResolutionMsg) error { + if len(msgs) != 1 { + return fmt.Errorf("expected 1 "+ + "resolution msg, instead got %v", + len(msgs)) + } + + resolutionChan <- msgs[0] + return nil + }, + }, + } + + resolver := &htlcTimeoutResolver{ + ResolverKit: ResolverKit{ + ChannelArbitratorConfig: chainCfg, + Checkpoint: func(_ ContractResolver) error { + checkPointChan <- struct{}{} + return nil + }, + }, + } + resolver.htlcResolution.SweepSignDesc = *fakeSignDesc + + // If the test case needs the remote commitment to be + // broadcast, then we'll set the timeout commit to a fake + // transaction to force the code path. + if !testCase.remoteCommit { + resolver.htlcResolution.SignedTimeoutTx = sweepTx + } + + // With all the setup above complete, we can initiate the + // resolution process, and the bulk of our test. + var wg sync.WaitGroup + resolveErr := make(chan error, 1) + wg.Add(1) + go func() { + defer wg.Done() + + _, err := resolver.Resolve() + if err != nil { + resolveErr <- err + } + }() + + // At the output isn't yet in the nursery, we expect that we + // should receive an incubation request. + select { + case <-incubateChan: + case err := <-resolveErr: + t.Fatalf("unable to resolve HTLC: %v", err) + case <-time.After(time.Second * 5): + t.Fatalf("failed to receive incubation request") + } + + // Next, the resolver should request a spend notification for + // the direct HTLC output. We'll use the txToBroadcast closure + // for the test case to generate the transaction that we'll + // send to the resolver. + spendingTx, err := testCase.txToBroadcast() + if err != nil { + t.Fatalf("unable to generate tx: %v", err) + } + select { + case notifier.spendChan <- &chainntnfs.SpendDetail{ + SpendingTx: spendingTx, + }: + case <-time.After(time.Second * 5): + t.Fatalf("failed to request spend ntfn") + } + + if !testCase.timeout { + // If the resolver should settle now, then we'll + // extract the pre-image to be extracted and the + // resolution message sent. + select { + case newPreimage := <-witnessBeacon.newPreimages: + if newPreimage[0] != fakePreimage { + t.Fatalf("wrong pre-image: "+ + "expected %v, got %v", + fakePreimage, newPreimage) + } + + case <-time.After(time.Second * 5): + t.Fatalf("pre-image not added") + } + + // Finally, we should get a resolution message with the + // pre-image set within the message. + select { + case resolutionMsg := <-resolutionChan: + // Once again, the pre-images should match up. + if *resolutionMsg.PreImage != fakePreimage { + t.Fatalf("wrong pre-image: "+ + "expected %v, got %v", + fakePreimage, resolutionMsg.PreImage) + } + case <-time.After(time.Second * 5): + t.Fatalf("resolution not sent") + } + } else { + + // Otherwise, the HTLC should now timeout. First, we + // should get a resolution message with a populated + // failure message. + select { + case resolutionMsg := <-resolutionChan: + if resolutionMsg.Failure == nil { + t.Fatalf("expected failure resolution msg") + } + case <-time.After(time.Second * 5): + t.Fatalf("resolution not sent") + } + + // We should also get another request for the spend + // notification of the second-level transaction to + // indicate that it's been swept by the nursery, but + // only if this is a local commitment transaction. + if !testCase.remoteCommit { + select { + case notifier.spendChan <- &chainntnfs.SpendDetail{ + SpendingTx: spendingTx, + }: + case <-time.After(time.Second * 5): + t.Fatalf("failed to request spend ntfn") + } + } + } + + // In any case, before the resolver exits, it should checkpoint + // its final state. + select { + case <-checkPointChan: + case err := <-resolveErr: + t.Fatalf("unable to resolve HTLC: %v", err) + case <-time.After(time.Second * 5): + t.Fatalf("check point not received") + } + + wg.Wait() + + // Finally, the resolver should be marked as resolved. + if !resolver.resolved { + t.Fatalf("resolver should be marked as resolved") + } + } +} From 1cea8d98c9abbc9f99d406ba5ac1dd97df9e02c5 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 18 Mar 2019 20:56:44 -0700 Subject: [PATCH 5/5] utoxnursery: allow nursery to start up if timeout spend happens Fixes #2793. --- utxonursery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utxonursery.go b/utxonursery.go index 829d4ba0..36b97815 100644 --- a/utxonursery.go +++ b/utxonursery.go @@ -894,7 +894,7 @@ func (u *utxoNursery) sweepCribOutput(classHeight uint32, baby *babyOutput) erro // We'll now broadcast the HTLC transaction, then wait for it to be // confirmed before transitioning it to kindergarten. err := u.cfg.PublishTransaction(baby.timeoutTx) - if err != nil { + if err != nil && err != lnwallet.ErrDoubleSpend { utxnLog.Errorf("Unable to broadcast baby tx: "+ "%v, %v", err, spew.Sdump(baby.timeoutTx)) return err