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

@@ -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

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.
+	var (
-	hodlChan := make(chan interface{}, 1)
+		hodlChan       chan interface{}
-
+		witnessUpdates <-chan lntypes.Preimage
-	// 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,
 	)
-	if err != nil {
+	if payload.FwdInfo.NextHop == hop.Exit {
-		return nil, err
+		// Create a buffered hodl chan to prevent deadlock.
-	}
+		hodlChan = make(chan interface{}, 1)
 
-	defer h.Registry.HodlUnsubscribeAll(hodlChan)
+		// Notify registry that we are potentially resolving as an exit
-
+		// hop on-chain. If this HTLC indeed pays to an existing
-	// Take action based on the resolution we received. If the htlc was
+		// invoice, the invoice registry will tell us what to do with
-	// settled, or a htlc for a known invoice failed we can resolve it
+		// the HTLC. This is identical to HTLC resolution in the link.
-	// directly. If the resolution is nil, the htlc was neither accepted
+		circuitKey := channeldb.CircuitKey{
-	// nor failed, so we cannot take action yet.
+			ChanID: h.ShortChanID,
-	switch res := resolution.(type) {
+			HtlcID: h.htlc.HtlcIndex,
-	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.
+		resolution, err := h.Registry.NotifyExitHopHtlc(
-	case *invoices.HtlcSettleResolution:
+			h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, currentHeight,
-		return processHtlcResolution(resolution)
+			circuitKey, hodlChan, payload,
-
+		)
-	// If the resolution is nil, the htlc was neither settled nor failed so
+		if err != nil {
-	// 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 {
 			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:

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 := newIncomingResolverTestContext(t, false)
-	ctx.registry.notifyResolution = invoices.NewFailResolution(
-		testResCircuitKey, testHtlcExpiry,
-		invoices.ResultInvoiceNotFound,
-	)
 	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 := newIncomingResolverTestContext(t, false)
-	ctx.registry.notifyResolution = invoices.NewFailResolution(
-		testResCircuitKey, testHtlcExpiry,
-		invoices.ResultInvoiceNotFound,
-	)
 	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 := newIncomingResolverTestContext(t, false)
-	ctx.registry.notifyResolution = invoices.NewFailResolution(
+
-		testResCircuitKey, testHtlcExpiry,
+	// Replace our checkpoint with one which will push reports into a
-		invoices.ResultInvoiceNotFound,
+	// 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 := newIncomingResolverTestContext(t, true)
-	ctx.registry.notifyResolution = invoices.NewFailResolution(
-		testResCircuitKey, testHtlcExpiry,
-		invoices.ResultInvoiceNotFound,
-	)
 	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,
 			},