From cf739f3f87fdcb28ab45dfd48e3d18adf26e45b3 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 7 Jul 2020 19:49:54 +0200 Subject: [PATCH] contractcourt: persist timed out incoming htlc resolver reports Incoming htlcs that are timed out or failed (invalid htlc or invoice condition not met), save a single on chain resolution because we don't need to take any actions on them ourselves (we don't need to worry about 2 stage claims since this is the success path for our peer). --- channeldb/reports.go | 13 ++ .../htlc_incoming_contest_resolver.go | 173 +++++++++++------- contractcourt/htlc_incoming_resolver_test.go | 132 ++++++++++--- 3 files changed, 221 insertions(+), 97 deletions(-) diff --git a/channeldb/reports.go b/channeldb/reports.go index 5781e5ba..00a7468f 100644 --- a/channeldb/reports.go +++ b/channeldb/reports.go @@ -49,6 +49,9 @@ type ResolverType uint8 const ( // ResolverTypeAnchor represents a resolver for an anchor output. ResolverTypeAnchor ResolverType = 0 + + // ResolverTypeIncomingHtlc represents resolution of an incoming htlc. + ResolverTypeIncomingHtlc ResolverType = 1 ) // ResolverOutcome indicates the outcome for the resolver that that the contract @@ -64,6 +67,16 @@ const ( // chain. This may be the case for anchors that we did not sweep, or // outputs that were not economical to sweep. ResolverOutcomeUnclaimed ResolverOutcome = 1 + + // ResolverOutcomeAbandoned indicates that we did not attempt to claim + // an output on chain. This is the case for htlcs that we could not + // decode to claim, or invoice which we fail when an attempt is made + // to settle them on chain. + ResolverOutcomeAbandoned ResolverOutcome = 2 + + // ResolverOutcomeTimeout indicates that a contract was timed out on + // chain. + ResolverOutcomeTimeout ResolverOutcome = 3 ) // ResolverReport provides an account of the outcome of a resolver. This differs diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 2bb02ec6..442121a6 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -81,7 +81,14 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // present itself when we crash before processRemoteAdds in the // link has ran. h.resolved = true - return nil, nil + + // We write a report to disk that indicates we could not decode + // the htlc. + resReport := h.report().resolverReport( + nil, channeldb.ResolverTypeIncomingHtlc, + channeldb.ResolverOutcomeAbandoned, + ) + return nil, h.PutResolverReport(nil, resReport) } // Register for block epochs. After registration, the current height @@ -120,7 +127,14 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { "abandoning", h, h.htlcResolution.ClaimOutpoint, h.htlcExpiry, currentHeight) h.resolved = true - return nil, h.Checkpoint(h) + + // Finally, get our report and checkpoint our resolver with a + // timeout outcome report. + report := h.report().resolverReport( + nil, channeldb.ResolverTypeIncomingHtlc, + channeldb.ResolverOutcomeTimeout, + ) + return nil, h.Checkpoint(h, report) } // applyPreimage is a helper function that will populate our internal @@ -158,16 +172,6 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { return nil } - // If the HTLC hasn't expired yet, then we may still be able to claim - // it if we learn of the pre-image, so we'll subscribe to the preimage - // database to see if it turns up, or the HTLC times out. - // - // NOTE: This is done BEFORE opportunistically querying the db, to - // ensure the preimage can't be delivered between querying and - // registering for the preimage subscription. - preimageSubscription := h.PreimageDB.SubscribeUpdates() - defer preimageSubscription.CancelSubscription() - // Define a closure to process htlc resolutions either directly or // triggered by future notifications. processHtlcResolution := func(e invoices.HtlcResolution) ( @@ -196,7 +200,14 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { h.htlcExpiry, currentHeight) h.resolved = true - return nil, h.Checkpoint(h) + + // Checkpoint our resolver with an abandoned outcome + // because we take no further action on this htlc. + report := h.report().resolverReport( + nil, channeldb.ResolverTypeIncomingHtlc, + channeldb.ResolverOutcomeAbandoned, + ) + return nil, h.Checkpoint(h, report) // Error if the resolution type is unknown, we are only // expecting settles and fails. @@ -206,71 +217,91 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { } } - // Create a buffered hodl chan to prevent deadlock. - hodlChan := make(chan interface{}, 1) - - // Notify registry that we are potentially resolving as an exit hop - // on-chain. If this HTLC indeed pays to an existing invoice, the - // invoice registry will tell us what to do with the HTLC. This is - // identical to HTLC resolution in the link. - circuitKey := channeldb.CircuitKey{ - ChanID: h.ShortChanID, - HtlcID: h.htlc.HtlcIndex, - } - - resolution, err := h.Registry.NotifyExitHopHtlc( - h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, currentHeight, - circuitKey, hodlChan, payload, + var ( + hodlChan chan interface{} + witnessUpdates <-chan lntypes.Preimage ) - if err != nil { - return nil, err - } + if payload.FwdInfo.NextHop == hop.Exit { + // Create a buffered hodl chan to prevent deadlock. + hodlChan = make(chan interface{}, 1) - defer h.Registry.HodlUnsubscribeAll(hodlChan) - - // Take action based on the resolution we received. If the htlc was - // settled, or a htlc for a known invoice failed we can resolve it - // directly. If the resolution is nil, the htlc was neither accepted - // nor failed, so we cannot take action yet. - switch res := resolution.(type) { - case *invoices.HtlcFailResolution: - // In the case where the htlc failed, but the invoice was known - // to the registry, we can directly resolve the htlc. - if res.Outcome != invoices.ResultInvoiceNotFound { - return processHtlcResolution(resolution) + // Notify registry that we are potentially resolving as an exit + // hop on-chain. If this HTLC indeed pays to an existing + // invoice, the invoice registry will tell us what to do with + // the HTLC. This is identical to HTLC resolution in the link. + circuitKey := channeldb.CircuitKey{ + ChanID: h.ShortChanID, + HtlcID: h.htlc.HtlcIndex, } - // If we settled the htlc, we can resolve it. - case *invoices.HtlcSettleResolution: - return processHtlcResolution(resolution) - - // If the resolution is nil, the htlc was neither settled nor failed so - // we cannot take action at present. - case nil: - - default: - return nil, fmt.Errorf("unknown htlc resolution type: %T", - resolution) - } - - // With the epochs and preimage subscriptions initialized, we'll query - // to see if we already know the preimage. - preimage, ok := h.PreimageDB.LookupPreimage(h.htlc.RHash) - if ok { - // If we do, then this means we can claim the HTLC! However, - // we don't know how to ourselves, so we'll return our inner - // resolver which has the knowledge to do so. - if err := applyPreimage(preimage); err != nil { + resolution, err := h.Registry.NotifyExitHopHtlc( + h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, currentHeight, + circuitKey, hodlChan, payload, + ) + if err != nil { return nil, err } - return &h.htlcSuccessResolver, nil + defer h.Registry.HodlUnsubscribeAll(hodlChan) + + // Take action based on the resolution we received. If the htlc + // was settled, or a htlc for a known invoice failed we can + // resolve it directly. If the resolution is nil, the htlc was + // neither accepted nor failed, so we cannot take action yet. + switch res := resolution.(type) { + case *invoices.HtlcFailResolution: + // In the case where the htlc failed, but the invoice + // was known to the registry, we can directly resolve + // the htlc. + if res.Outcome != invoices.ResultInvoiceNotFound { + return processHtlcResolution(resolution) + } + + // If we settled the htlc, we can resolve it. + case *invoices.HtlcSettleResolution: + return processHtlcResolution(resolution) + + // If the resolution is nil, the htlc was neither settled nor + // failed so we cannot take action at present. + case nil: + + default: + return nil, fmt.Errorf("unknown htlc resolution type: %T", + resolution) + } + } else { + // If the HTLC hasn't expired yet, then we may still be able to + // claim it if we learn of the pre-image, so we'll subscribe to + // the preimage database to see if it turns up, or the HTLC + // times out. + // + // NOTE: This is done BEFORE opportunistically querying the db, + // to ensure the preimage can't be delivered between querying + // and registering for the preimage subscription. + preimageSubscription := h.PreimageDB.SubscribeUpdates() + defer preimageSubscription.CancelSubscription() + + // With the epochs and preimage subscriptions initialized, we'll + // query to see if we already know the preimage. + preimage, ok := h.PreimageDB.LookupPreimage(h.htlc.RHash) + if ok { + // If we do, then this means we can claim the HTLC! + // However, we don't know how to ourselves, so we'll + // return our inner resolver which has the knowledge to + // do so. + if err := applyPreimage(preimage); err != nil { + return nil, err + } + + return &h.htlcSuccessResolver, nil + } + + witnessUpdates = preimageSubscription.WitnessUpdates } for { - select { - case preimage := <-preimageSubscription.WitnessUpdates: + case preimage := <-witnessUpdates: // We received a new preimage, but we need to ignore // all except the preimage we are waiting for. if !preimage.Matches(h.htlc.RHash) { @@ -305,7 +336,13 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { h.htlcResolution.ClaimOutpoint, h.htlcExpiry, currentHeight) h.resolved = true - return nil, h.Checkpoint(h) + + report := h.report().resolverReport( + nil, + channeldb.ResolverTypeIncomingHtlc, + channeldb.ResolverOutcomeTimeout, + ) + return nil, h.Checkpoint(h, report) } case <-h.quit: diff --git a/contractcourt/htlc_incoming_resolver_test.go b/contractcourt/htlc_incoming_resolver_test.go index 059e1ccc..55bc33b5 100644 --- a/contractcourt/htlc_incoming_resolver_test.go +++ b/contractcourt/htlc_incoming_resolver_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "testing" + sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/kvdb" @@ -13,6 +14,7 @@ import ( "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwire" ) const ( @@ -26,6 +28,7 @@ var ( testResCircuitKey = channeldb.CircuitKey{} testOnionBlob = []byte{4, 5, 6} testAcceptHeight int32 = 1234 + testHtlcAmount = 2300 ) // TestHtlcIncomingResolverFwdPreimageKnown tests resolution of a forwarded htlc @@ -34,11 +37,7 @@ func TestHtlcIncomingResolverFwdPreimageKnown(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailResolution( - testResCircuitKey, testHtlcExpiry, - invoices.ResultInvoiceNotFound, - ) + ctx := newIncomingResolverTestContext(t, false) ctx.witnessBeacon.lookupPreimage[testResHash] = testResPreimage ctx.resolve() ctx.waitForResult(true) @@ -51,11 +50,7 @@ func TestHtlcIncomingResolverFwdContestedSuccess(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailResolution( - testResCircuitKey, testHtlcExpiry, - invoices.ResultInvoiceNotFound, - ) + ctx := newIncomingResolverTestContext(t, false) ctx.resolve() // Simulate a new block coming in. HTLC is not yet expired. @@ -71,16 +66,36 @@ func TestHtlcIncomingResolverFwdContestedTimeout(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailResolution( - testResCircuitKey, testHtlcExpiry, - invoices.ResultInvoiceNotFound, - ) + ctx := newIncomingResolverTestContext(t, false) + + // Replace our checkpoint with one which will push reports into a + // channel for us to consume. We replace this function on the resolver + // itself because it is created by the test context. + reportChan := make(chan *channeldb.ResolverReport) + ctx.resolver.Checkpoint = func(_ ContractResolver, + reports ...*channeldb.ResolverReport) error { + + // Send all of our reports into the channel. + for _, report := range reports { + reportChan <- report + } + + return nil + } + ctx.resolve() // Simulate a new block coming in. HTLC expires. ctx.notifyEpoch(testHtlcExpiry) + // Assert that we have a failure resolution because our invoice was + // cancelled. + assertResolverReport(t, reportChan, &channeldb.ResolverReport{ + Amount: lnwire.MilliSatoshi(testHtlcAmount).ToSatoshis(), + ResolverType: channeldb.ResolverTypeIncomingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeTimeout, + }) + ctx.waitForResult(false) } @@ -90,11 +105,7 @@ func TestHtlcIncomingResolverFwdTimeout(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailResolution( - testResCircuitKey, testHtlcExpiry, - invoices.ResultInvoiceNotFound, - ) + ctx := newIncomingResolverTestContext(t, true) ctx.witnessBeacon.lookupPreimage[testResHash] = testResPreimage ctx.resolver.htlcExpiry = 90 ctx.resolve() @@ -107,7 +118,7 @@ func TestHtlcIncomingResolverExitSettle(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) + ctx := newIncomingResolverTestContext(t, true) ctx.registry.notifyResolution = invoices.NewSettleResolution( testResPreimage, testResCircuitKey, testAcceptHeight, invoices.ResultReplayToSettled, @@ -138,7 +149,7 @@ func TestHtlcIncomingResolverExitCancel(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) + ctx := newIncomingResolverTestContext(t, true) ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testAcceptHeight, invoices.ResultInvoiceAlreadyCanceled, @@ -154,7 +165,7 @@ func TestHtlcIncomingResolverExitSettleHodl(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) + ctx := newIncomingResolverTestContext(t, true) ctx.resolve() notifyData := <-ctx.registry.notifyChan @@ -172,9 +183,34 @@ func TestHtlcIncomingResolverExitTimeoutHodl(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) + ctx := newIncomingResolverTestContext(t, true) + + // Replace our checkpoint with one which will push reports into a + // channel for us to consume. We replace this function on the resolver + // itself because it is created by the test context. + reportChan := make(chan *channeldb.ResolverReport) + ctx.resolver.Checkpoint = func(_ ContractResolver, + reports ...*channeldb.ResolverReport) error { + + // Send all of our reports into the channel. + for _, report := range reports { + reportChan <- report + } + + return nil + } + ctx.resolve() ctx.notifyEpoch(testHtlcExpiry) + + // Assert that we have a failure resolution because our invoice was + // cancelled. + assertResolverReport(t, reportChan, &channeldb.ResolverReport{ + Amount: lnwire.MilliSatoshi(testHtlcAmount).ToSatoshis(), + ResolverType: channeldb.ResolverTypeIncomingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeTimeout, + }) + ctx.waitForResult(false) } @@ -184,25 +220,62 @@ func TestHtlcIncomingResolverExitCancelHodl(t *testing.T) { t.Parallel() defer timeout(t)() - ctx := newIncomingResolverTestContext(t) + ctx := newIncomingResolverTestContext(t, true) + + // Replace our checkpoint with one which will push reports into a + // channel for us to consume. We replace this function on the resolver + // itself because it is created by the test context. + reportChan := make(chan *channeldb.ResolverReport) + ctx.resolver.Checkpoint = func(_ ContractResolver, + reports ...*channeldb.ResolverReport) error { + + // Send all of our reports into the channel. + for _, report := range reports { + reportChan <- report + } + + return nil + } + ctx.resolve() notifyData := <-ctx.registry.notifyChan notifyData.hodlChan <- invoices.NewFailResolution( testResCircuitKey, testAcceptHeight, invoices.ResultCanceled, ) + // Assert that we have a failure resolution because our invoice was + // cancelled. + assertResolverReport(t, reportChan, &channeldb.ResolverReport{ + Amount: lnwire.MilliSatoshi(testHtlcAmount).ToSatoshis(), + ResolverType: channeldb.ResolverTypeIncomingHtlc, + ResolverOutcome: channeldb.ResolverOutcomeAbandoned, + }) + ctx.waitForResult(false) } type mockHopIterator struct { + isExit bool hop.Iterator } func (h *mockHopIterator) HopPayload() (*hop.Payload, error) { - return nil, nil + var nextAddress [8]byte + if !h.isExit { + nextAddress = [8]byte{0x01} + } + + return hop.NewLegacyPayload(&sphinx.HopData{ + Realm: [1]byte{}, + NextAddress: nextAddress, + ForwardAmount: 100, + OutgoingCltv: 40, + ExtraBytes: [12]byte{}, + }), nil } type mockOnionProcessor struct { + isExit bool offeredOnionBlob []byte } @@ -215,7 +288,7 @@ func (o *mockOnionProcessor) ReconstructHopIterator(r io.Reader, rHash []byte) ( } o.offeredOnionBlob = data - return &mockHopIterator{}, nil + return &mockHopIterator{isExit: o.isExit}, nil } type incomingResolverTestContext struct { @@ -229,7 +302,7 @@ type incomingResolverTestContext struct { t *testing.T } -func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext { +func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolverTestContext { notifier := &mockNotifier{ epochChan: make(chan *chainntnfs.BlockEpoch), spendChan: make(chan *chainntnfs.SpendDetail), @@ -240,7 +313,7 @@ func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext { notifyChan: make(chan notifyExitHopData, 1), } - onionProcessor := &mockOnionProcessor{} + onionProcessor := &mockOnionProcessor{isExit: isExit} checkPointChan := make(chan struct{}, 1) @@ -272,6 +345,7 @@ func newIncomingResolverTestContext(t *testing.T) *incomingResolverTestContext { contractResolverKit: *newContractResolverKit(cfg), htlcResolution: lnwallet.IncomingHtlcResolution{}, htlc: channeldb.HTLC{ + Amt: lnwire.MilliSatoshi(testHtlcAmount), RHash: testResHash, OnionBlob: testOnionBlob, },