From 2569b4d08a0d11912c3f9cb88ececc6327360a32 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:10 +0200 Subject: [PATCH 1/6] multi: replace htlcResolution with an interface This commit repalces the htlcResolution struct with an interface. This interface is implemeted by failure, settle and accept resolution structs. Only settles and fails are exported because the existing code that handles htlc resolutions uses a nil resolution to indicate that a htlc was accepted. The accept resolution is used internally to report on the resolution result of the accepted htlc, but a nil resolution is surfaced. Further refactoring of all the functions that call NotifyExitHopHtlc to handle a htlc accept case (rather than having a nil check) is required. --- .../htlc_incoming_contest_resolver.go | 59 ++++-- contractcourt/htlc_incoming_resolver_test.go | 14 +- contractcourt/interfaces.go | 2 +- contractcourt/mock_registry_test.go | 4 +- htlcswitch/interfaces.go | 2 +- htlcswitch/link.go | 67 ++++--- htlcswitch/mock.go | 2 +- invoices/invoiceregistry.go | 184 ++++++++---------- invoices/invoiceregistry_test.go | 181 ++++++++++------- invoices/resolution.go | 124 ++++++++++++ invoices/update.go | 95 ++++++--- 11 files changed, 477 insertions(+), 257 deletions(-) create mode 100644 invoices/resolution.go diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 98e52500a..cc4c0f2b2 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/binary" "errors" + "fmt" "io" "github.com/btcsuite/btcutil" @@ -172,7 +173,23 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { processHtlcResolution := func(e invoices.HtlcResolution) ( ContractResolver, error) { - if e.Preimage == nil { + // Take action based on the type of resolution we have + // received. + switch resolution := e.(type) { + + // If the htlc resolution was a settle, apply the + // preimage and return a success resolver. + case *invoices.HtlcSettleResolution: + err := applyPreimage(resolution.Preimage) + if err != nil { + return nil, err + } + + return &h.htlcSuccessResolver, nil + + // If the htlc was failed, mark the htlc as + // resolved. + case *invoices.HtlcFailResolution: log.Infof("%T(%v): Exit hop HTLC canceled "+ "(expiry=%v, height=%v), abandoning", h, h.htlcResolution.ClaimOutpoint, @@ -180,13 +197,13 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { h.resolved = true return nil, h.Checkpoint(h) - } - if err := applyPreimage(*e.Preimage); err != nil { - return nil, err + // Error if the resolution type is unknown, we are only + // expecting settles and fails. + default: + return nil, fmt.Errorf("unknown resolution"+ + " type: %v", e) } - - return &h.htlcSuccessResolver, nil } // Create a buffered hodl chan to prevent deadlock. @@ -211,14 +228,29 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { defer h.Registry.HodlUnsubscribeAll(hodlChan) - // If the resolution is non-nil (indicating that a settle or cancel has - // occurred), and the invoice is known to the registry (indicating that - // the htlc is paying one of our invoices and is not a forward), try to - // resolve it directly. - if resolution != nil && - resolution.Outcome != invoices.ResultInvoiceNotFound { + // 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) + } - 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) } // With the epochs and preimage subscriptions initialized, we'll query @@ -256,7 +288,6 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { case hodlItem := <-hodlChan: htlcResolution := hodlItem.(invoices.HtlcResolution) - return processHtlcResolution(htlcResolution) case newBlock, ok := <-blockEpochs.Epochs: diff --git a/contractcourt/htlc_incoming_resolver_test.go b/contractcourt/htlc_incoming_resolver_test.go index 773b22c00..400662d73 100644 --- a/contractcourt/htlc_incoming_resolver_test.go +++ b/contractcourt/htlc_incoming_resolver_test.go @@ -35,7 +35,7 @@ func TestHtlcIncomingResolverFwdPreimageKnown(t *testing.T) { defer timeout(t)() ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailureResolution( + ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testHtlcExpiry, invoices.ResultInvoiceNotFound, ) @@ -52,7 +52,7 @@ func TestHtlcIncomingResolverFwdContestedSuccess(t *testing.T) { defer timeout(t)() ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailureResolution( + ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testHtlcExpiry, invoices.ResultInvoiceNotFound, ) @@ -72,7 +72,7 @@ func TestHtlcIncomingResolverFwdContestedTimeout(t *testing.T) { defer timeout(t)() ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailureResolution( + ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testHtlcExpiry, invoices.ResultInvoiceNotFound, ) @@ -91,7 +91,7 @@ func TestHtlcIncomingResolverFwdTimeout(t *testing.T) { defer timeout(t)() ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailureResolution( + ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testHtlcExpiry, invoices.ResultInvoiceNotFound, ) @@ -139,7 +139,7 @@ func TestHtlcIncomingResolverExitCancel(t *testing.T) { defer timeout(t)() ctx := newIncomingResolverTestContext(t) - ctx.registry.notifyResolution = invoices.NewFailureResolution( + ctx.registry.notifyResolution = invoices.NewFailResolution( testResCircuitKey, testAcceptHeight, invoices.ResultInvoiceAlreadyCanceled, ) @@ -158,7 +158,7 @@ func TestHtlcIncomingResolverExitSettleHodl(t *testing.T) { ctx.resolve() notifyData := <-ctx.registry.notifyChan - notifyData.hodlChan <- *invoices.NewSettleResolution( + notifyData.hodlChan <- invoices.NewSettleResolution( testResPreimage, testResCircuitKey, testAcceptHeight, invoices.ResultSettled, ) @@ -187,7 +187,7 @@ func TestHtlcIncomingResolverExitCancelHodl(t *testing.T) { ctx := newIncomingResolverTestContext(t) ctx.resolve() notifyData := <-ctx.registry.notifyChan - notifyData.hodlChan <- *invoices.NewFailureResolution( + notifyData.hodlChan <- invoices.NewFailResolution( testResCircuitKey, testAcceptHeight, invoices.ResultCanceled, ) diff --git a/contractcourt/interfaces.go b/contractcourt/interfaces.go index 086a2aee5..18dfbc0e8 100644 --- a/contractcourt/interfaces.go +++ b/contractcourt/interfaces.go @@ -27,7 +27,7 @@ type Registry interface { NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, - payload invoices.Payload) (*invoices.HtlcResolution, error) + payload invoices.Payload) (invoices.HtlcResolution, error) // HodlUnsubscribeAll unsubscribes from all htlc resolutions. HodlUnsubscribeAll(subscriber chan<- interface{}) diff --git a/contractcourt/mock_registry_test.go b/contractcourt/mock_registry_test.go index dca27d629..a7f430f23 100644 --- a/contractcourt/mock_registry_test.go +++ b/contractcourt/mock_registry_test.go @@ -18,13 +18,13 @@ type notifyExitHopData struct { type mockRegistry struct { notifyChan chan notifyExitHopData notifyErr error - notifyResolution *invoices.HtlcResolution + notifyResolution invoices.HtlcResolution } func (r *mockRegistry) NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, - payload invoices.Payload) (*invoices.HtlcResolution, error) { + payload invoices.Payload) (invoices.HtlcResolution, error) { r.notifyChan <- notifyExitHopData{ hodlChan: hodlChan, diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 1281e9515..f0eae99dd 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -27,7 +27,7 @@ type InvoiceDatabase interface { NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, - payload invoices.Payload) (*invoices.HtlcResolution, error) + payload invoices.Payload) (invoices.HtlcResolution, error) // CancelInvoice attempts to cancel the invoice corresponding to the // passed payment hash. diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 92ecba65c..dee3c588d 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1158,7 +1158,7 @@ loop: for { // Lookup all hodl htlcs that can be failed or settled with this event. // The hodl htlc must be present in the map. - circuitKey := htlcResolution.CircuitKey + circuitKey := htlcResolution.CircuitKey() hodlHtlc, ok := l.hodlMap[circuitKey] if !ok { return fmt.Errorf("hodl htlc not found: %v", circuitKey) @@ -1193,46 +1193,61 @@ loop: func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution, htlc hodlHtlc) error { - circuitKey := resolution.CircuitKey + circuitKey := resolution.CircuitKey() - // Determine required action for the resolution. If the event's preimage is - // non-nil, the htlc must be settled. Otherwise, it should be canceled. - if resolution.Preimage != nil { - l.log.Debugf("received settle resolution for %v", circuitKey) + // Determine required action for the resolution based on the type of + // resolution we have received. + switch res := resolution.(type) { + // Settle htlcs that returned a settle resolution using the preimage + // in the resolution. + case *invoices.HtlcSettleResolution: + l.log.Debugf("received settle resolution for %v"+ + "with outcome: %v", circuitKey, res.Outcome) return l.settleHTLC( - *resolution.Preimage, htlc.pd.HtlcIndex, + res.Preimage, htlc.pd.HtlcIndex, htlc.pd.SourceRef, ) + + // For htlc failures, we get the relevant failure message based + // on the failure resolution and then fail the htlc. + case *invoices.HtlcFailResolution: + l.log.Debugf("received cancel resolution for "+ + "%v with outcome: %v", circuitKey, res.Outcome) + + // Get the lnwire failure message based on the resolution + // result. + failure := getResolutionFailure(res, htlc.pd.Amount) + + l.sendHTLCError( + htlc.pd.HtlcIndex, failure, htlc.obfuscator, + htlc.pd.SourceRef, + ) + return nil + + // Fail if we do not get a settle of fail resolution, since we + // are only expecting to handle settles and fails. + default: + return fmt.Errorf("unknown htlc resolution type: %T", + resolution) } - - l.log.Debugf("received cancel resolution for %v with outcome: %v", - circuitKey, resolution.Outcome) - - // Get the lnwire failure message based on the resolution result. - failure := getResolutionFailure(resolution, htlc.pd.Amount) - - l.sendHTLCError( - htlc.pd.HtlcIndex, failure, htlc.obfuscator, - htlc.pd.SourceRef, - ) - return nil } // getResolutionFailure returns the wire message that a htlc resolution should // be failed with. -func getResolutionFailure(resolution invoices.HtlcResolution, +func getResolutionFailure(resolution *invoices.HtlcFailResolution, amount lnwire.MilliSatoshi) lnwire.FailureMessage { - // If the resolution has been resolved as part of a MPP timeout, we need - // to fail the htlc with lnwire.FailMppTimeout. + // If the resolution has been resolved as part of a MPP timeout, + // we need to fail the htlc with lnwire.FailMppTimeout. if resolution.Outcome == invoices.ResultMppTimeout { return &lnwire.FailMPPTimeout{} } - // If the htlc is not a MPP timeout, we fail it with FailIncorrectDetails - // This covers hodl cancels (which return it to avoid leaking information - // and other invoice failures such as underpayment or expiry too soon. + // If the htlc is not a MPP timeout, we fail it with + // FailIncorrectDetails. This error is sent for invoice payment + // failures such as underpayment/ expiry too soon and hodl invoices + // (which return FailIncorrectDetails to avoid leaking information). return lnwire.NewFailIncorrectDetails( amount, uint32(resolution.AcceptHeight), ) @@ -2863,7 +2878,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, } // Process the received resolution. - return l.processHtlcResolution(*event, htlc) + return l.processHtlcResolution(event, htlc) } // settleHTLC settles the HTLC on the channel. diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 02737a715..7cc9fb04a 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -816,7 +816,7 @@ func (i *mockInvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error func (i *mockInvoiceRegistry) NotifyExitHopHtlc(rhash lntypes.Hash, amt lnwire.MilliSatoshi, expiry uint32, currentHeight int32, circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, - payload invoices.Payload) (*invoices.HtlcResolution, error) { + payload invoices.Payload) (invoices.HtlcResolution, error) { event, err := i.registry.NotifyExitHopHtlc( rhash, amt, expiry, currentHeight, circuitKey, hodlChan, diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 115b38af7..7f62ae45e 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -36,48 +36,6 @@ const ( DefaultHtlcHoldDuration = 120 * time.Second ) -// HtlcResolution describes how an htlc should be resolved. If the preimage -// field is set, the event indicates a settle event. If Preimage is nil, it is -// a cancel event. -type HtlcResolution struct { - // Preimage is the htlc preimage. Its value is nil in case of a cancel. - Preimage *lntypes.Preimage - - // CircuitKey is the key of the htlc for which we have a resolution - // decision. - CircuitKey channeldb.CircuitKey - - // AcceptHeight is the original height at which the htlc was accepted. - AcceptHeight int32 - - // Outcome indicates the outcome of the invoice registry update. - Outcome ResolutionResult -} - -// NewFailureResolution returns a htlc failure resolution. -func NewFailureResolution(key channeldb.CircuitKey, - acceptHeight int32, outcome ResolutionResult) *HtlcResolution { - - return &HtlcResolution{ - CircuitKey: key, - AcceptHeight: acceptHeight, - Outcome: outcome, - } -} - -// NewSettleResolution returns a htlc resolution which is associated with a -// settle. -func NewSettleResolution(preimage lntypes.Preimage, key channeldb.CircuitKey, - acceptHeight int32, outcome ResolutionResult) *HtlcResolution { - - return &HtlcResolution{ - Preimage: &preimage, - CircuitKey: key, - AcceptHeight: acceptHeight, - Outcome: outcome, - } -} - // RegistryConfig contains the configuration parameters for invoice registry. type RegistryConfig struct { // FinalCltvRejectDelta defines the number of blocks before the expiry @@ -683,7 +641,7 @@ func (i *InvoiceRegistry) cancelSingleHtlc(hash lntypes.Hash, return fmt.Errorf("htlc %v not found", key) } if htlc.State == channeldb.HtlcStateCanceled { - resolution := *NewFailureResolution( + resolution := NewFailResolution( key, int32(htlc.AcceptHeight), result, ) @@ -777,7 +735,7 @@ func (i *InvoiceRegistry) processKeySend(ctx invoiceUpdateCtx) error { func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, amtPaid lnwire.MilliSatoshi, expiry uint32, currentHeight int32, circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, - payload Payload) (*HtlcResolution, error) { + payload Payload) (HtlcResolution, error) { mpp := payload.MultiPath() @@ -802,7 +760,7 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, if err != nil { updateCtx.log(fmt.Sprintf("keysend error: %v", err)) - return NewFailureResolution( + return NewFailResolution( circuitKey, currentHeight, ResultKeySendError, ), nil } @@ -817,15 +775,10 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, } switch r := resolution.(type) { - - // A direct resolution was received for this htlc. - case *HtlcResolution: - return r, nil - // The htlc is held. Start a timer outside the lock if the htlc should // be auto-released, because otherwise a deadlock may happen with the // main event loop. - case *acceptResolution: + case *htlcAcceptResolution: if r.autoRelease { err := i.startHtlcTimer(rHash, circuitKey, r.acceptTime) if err != nil { @@ -833,33 +786,31 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, } } + // We return a nil resolution because htlc acceptances are + // represented as nil resolutions externally. + // TODO(carla) update calling code to handle accept resolutions. return nil, nil + // A direct resolution was received for this htlc. + case HtlcResolution: + return r, nil + + // Fail if an unknown resolution type was received. default: return nil, errors.New("invalid resolution type") } } -// acceptResolution is returned when the htlc should be held. -type acceptResolution struct { - // autoRelease signals that the htlc should be automatically released - // after a timeout. - autoRelease bool - - // acceptTime is the time at which this htlc was accepted. - acceptTime time.Time -} - // notifyExitHopHtlcLocked is the internal implementation of NotifyExitHopHtlc // that should be executed inside the registry lock. func (i *InvoiceRegistry) notifyExitHopHtlcLocked( ctx *invoiceUpdateCtx, hodlChan chan<- interface{}) ( - interface{}, error) { + HtlcResolution, error) { // We'll attempt to settle an invoice matching this rHash on disk (if // one exists). The callback will update the invoice state and/or htlcs. var ( - result ResolutionResult + resolution HtlcResolution updateSubscribers bool ) invoice, err := i.cdb.UpdateInvoice( @@ -876,8 +827,8 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( updateSubscribers = updateDesc != nil && updateDesc.State != nil - // Assign result to outer scope variable. - result = res + // Assign resolution to outer scope variable. + resolution = res return updateDesc, nil }, @@ -886,7 +837,7 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( case channeldb.ErrInvoiceNotFound: // If the invoice was not found, return a failure resolution // with an invoice not found result. - return NewFailureResolution( + return NewFailResolution( ctx.circuitKey, ctx.currentHeight, ResultInvoiceNotFound, ), nil @@ -898,38 +849,40 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( return nil, err } - ctx.log(result.String()) - if updateSubscribers { i.notifyClients(ctx.hash, invoice, invoice.State) } - // Inspect latest htlc state on the invoice. - invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey] + switch res := resolution.(type) { + case *HtlcFailResolution: + // Inspect latest htlc state on the invoice. If it is found, + // we will update the accept height as it was recorded in the + // invoice database (which occurs in the case where the htlc + // reached the database in a previous call). If the htlc was + // not found on the invoice, it was immediately failed so we + // send the failure resolution as is, which has the current + // height set as the accept height. + invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey] + if ok { + res.AcceptHeight = int32(invoiceHtlc.AcceptHeight) + } - // If it isn't recorded, cancel htlc. - if !ok { - return NewFailureResolution( - ctx.circuitKey, ctx.currentHeight, result, - ), nil - } + ctx.log(fmt.Sprintf("failure resolution result "+ + "outcome: %v, at accept height: %v", + res.Outcome, res.AcceptHeight)) - // Determine accepted height of this htlc. If the htlc reached the - // invoice database (possibly in a previous call to the invoice - // registry), we'll take the original accepted height as it was recorded - // in the database. - acceptHeight := int32(invoiceHtlc.AcceptHeight) + return res, nil - switch invoiceHtlc.State { - case channeldb.HtlcStateCanceled: - return NewFailureResolution( - ctx.circuitKey, acceptHeight, result, - ), nil + // If the htlc was settled, we will settle any previously accepted + // htlcs and notify our peer to settle them. + case *HtlcSettleResolution: + ctx.log(fmt.Sprintf("settle resolution result "+ + "outcome: %v, at accept height: %v", + res.Outcome, res.AcceptHeight)) - case channeldb.HtlcStateSettled: - // Also settle any previously accepted htlcs. The invoice state - // is leading. If an htlc is marked as settled, we should follow - // now and settle the htlc with our peer. + // Also settle any previously accepted htlcs. If a htlc is + // marked as settled, we should follow now and settle the htlc + // with our peer. for key, htlc := range invoice.Htlcs { if htlc.State != channeldb.HtlcStateSettled { continue @@ -940,34 +893,49 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( // resolution is set based on the outcome of the single // htlc that we just settled, so may not be accurate // for all htlcs. - resolution := *NewSettleResolution( - invoice.Terms.PaymentPreimage, key, - acceptHeight, result, + htlcSettleResolution := NewSettleResolution( + res.Preimage, key, + int32(htlc.AcceptHeight), res.Outcome, ) - i.notifyHodlSubscribers(resolution) + // Notify subscribers that the htlc should be settled + // with our peer. + i.notifyHodlSubscribers(htlcSettleResolution) } - resolution := NewSettleResolution( - invoice.Terms.PaymentPreimage, ctx.circuitKey, - acceptHeight, result, - ) return resolution, nil - case channeldb.HtlcStateAccepted: - var resolution acceptResolution + // If we accepted the htlc, subscribe to the hodl invoice and return + // an accept resolution with the htlc's accept time on it. + case *htlcAcceptResolution: + invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey] + if !ok { + return nil, fmt.Errorf("accepted htlc: %v not"+ + " present on invoice: %x", ctx.circuitKey, + ctx.hash[:]) + } + + // Determine accepted height of this htlc. If the htlc reached + // the invoice database (possibly in a previous call to the + // invoice registry), we'll take the original accepted height + // as it was recorded in the database. + acceptHeight := int32(invoiceHtlc.AcceptHeight) + + ctx.log(fmt.Sprintf("accept resolution result "+ + "outcome: %v, at accept height: %v", + res.outcome, acceptHeight)) // Auto-release the htlc if the invoice is still open. It can // only happen for mpp payments that there are htlcs in state // Accepted while the invoice is Open. if invoice.State == channeldb.ContractOpen { - resolution.acceptTime = invoiceHtlc.AcceptTime - resolution.autoRelease = true + res.acceptTime = invoiceHtlc.AcceptTime + res.autoRelease = true } i.hodlSubscribe(hodlChan, ctx.circuitKey) - return &resolution, nil + return res, nil default: panic("unknown action") @@ -1022,7 +990,7 @@ func (i *InvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error { continue } - resolution := *NewSettleResolution( + resolution := NewSettleResolution( preimage, key, int32(htlc.AcceptHeight), ResultSettled, ) @@ -1102,7 +1070,7 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(payHash lntypes.Hash, } i.notifyHodlSubscribers( - *NewFailureResolution( + NewFailResolution( key, int32(htlc.AcceptHeight), ResultCanceled, ), ) @@ -1374,7 +1342,7 @@ func (i *InvoiceRegistry) SubscribeSingleInvoice( // notifyHodlSubscribers sends out the htlc resolution to all current // subscribers. func (i *InvoiceRegistry) notifyHodlSubscribers(htlcResolution HtlcResolution) { - subscribers, ok := i.hodlSubscriptions[htlcResolution.CircuitKey] + subscribers, ok := i.hodlSubscriptions[htlcResolution.CircuitKey()] if !ok { return } @@ -1391,11 +1359,11 @@ func (i *InvoiceRegistry) notifyHodlSubscribers(htlcResolution HtlcResolution) { delete( i.hodlReverseSubscriptions[subscriber], - htlcResolution.CircuitKey, + htlcResolution.CircuitKey(), ) } - delete(i.hodlSubscriptions, htlcResolution.CircuitKey) + delete(i.hodlSubscriptions, htlcResolution.CircuitKey()) } // hodlSubscribe adds a new invoice subscription. diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 9f360e267..319c30cf0 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -74,29 +74,38 @@ func TestSettleInvoice(t *testing.T) { if err != nil { t.Fatal(err) } - if resolution.Preimage != nil { - t.Fatal("expected cancel resolution") + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if resolution.AcceptHeight != testCurrentHeight { + if failResolution.AcceptHeight != testCurrentHeight { t.Fatalf("expected acceptHeight %v, but got %v", - testCurrentHeight, resolution.AcceptHeight) + testCurrentHeight, failResolution.AcceptHeight) } - if resolution.Outcome != ResultExpiryTooSoon { + if failResolution.Outcome != ResultExpiryTooSoon { t.Fatalf("expected expiry too soon, got: %v", - resolution.Outcome) + failResolution.Outcome) } // Settle invoice with a slightly higher amount. amtPaid := lnwire.MilliSatoshi(100500) resolution, err = ctx.registry.NotifyExitHopHtlc( - testInvoicePaymentHash, amtPaid, testHtlcExpiry, testCurrentHeight, - getCircuitKey(0), hodlChan, testPayload, + testInvoicePaymentHash, amtPaid, testHtlcExpiry, + testCurrentHeight, getCircuitKey(0), hodlChan, + testPayload, ) if err != nil { t.Fatal(err) } - if resolution.Outcome != ResultSettled { - t.Fatalf("expected settled, got: %v", resolution.Outcome) + settleResolution, ok := resolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + resolution) + } + if settleResolution.Outcome != ResultSettled { + t.Fatalf("expected settled, got: %v", + settleResolution.Outcome) } // We expect the settled state to be sent to the single invoice @@ -134,12 +143,14 @@ func TestSettleInvoice(t *testing.T) { if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) } - if resolution.Preimage == nil { - t.Fatal("expected settle resolution") + settleResolution, ok = resolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + resolution) } - if resolution.Outcome != ResultReplayToSettled { + if settleResolution.Outcome != ResultReplayToSettled { t.Fatalf("expected replay settled, got: %v", - resolution.Outcome) + settleResolution.Outcome) } // Try to settle again with a new higher-valued htlc. This payment @@ -152,12 +163,14 @@ func TestSettleInvoice(t *testing.T) { if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) } - if resolution.Preimage == nil { - t.Fatal("expected settle resolution") + settleResolution, ok = resolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + resolution) } - if resolution.Outcome != ResultDuplicateToSettled { + if settleResolution.Outcome != ResultDuplicateToSettled { t.Fatalf("expected duplicate settled, got: %v", - resolution.Outcome) + settleResolution.Outcome) } // Try to settle again with a lower amount. This should fail just as it @@ -169,12 +182,14 @@ func TestSettleInvoice(t *testing.T) { if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) } - if resolution.Preimage != nil { - t.Fatal("expected cancel resolution") + failResolution, ok = resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if resolution.Outcome != ResultAmountTooLow { + if failResolution.Outcome != ResultAmountTooLow { t.Fatalf("expected amount too low, got: %v", - resolution.Outcome) + failResolution.Outcome) } // Check that settled amount is equal to the sum of values of the htlcs @@ -298,17 +313,18 @@ func TestCancelInvoice(t *testing.T) { if err != nil { t.Fatal("expected settlement of a canceled invoice to succeed") } - - if resolution.Preimage != nil { - t.Fatal("expected cancel htlc resolution") + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if resolution.AcceptHeight != testCurrentHeight { + if failResolution.AcceptHeight != testCurrentHeight { t.Fatalf("expected acceptHeight %v, but got %v", - testCurrentHeight, resolution.AcceptHeight) + testCurrentHeight, failResolution.AcceptHeight) } - if resolution.Outcome != ResultInvoiceAlreadyCanceled { - t.Fatalf("expected invoice already canceled, got: %v", - resolution.Outcome) + if failResolution.Outcome != ResultInvoiceAlreadyCanceled { + t.Fatalf("expected expiry too soon, got: %v", + failResolution.Outcome) } } @@ -422,12 +438,14 @@ func TestSettleHoldInvoice(t *testing.T) { if err != nil { t.Fatalf("expected settle to succeed but got %v", err) } - if resolution == nil || resolution.Preimage != nil { - t.Fatalf("expected htlc to be canceled") + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if resolution.Outcome != ResultExpiryTooSoon { + if failResolution.Outcome != ResultExpiryTooSoon { t.Fatalf("expected expiry too soon, got: %v", - resolution.Outcome) + failResolution.Outcome) } // We expect the accepted state to be sent to the single invoice @@ -449,16 +467,21 @@ func TestSettleHoldInvoice(t *testing.T) { } htlcResolution := (<-hodlChan).(HtlcResolution) - if *htlcResolution.Preimage != testInvoicePreimage { + settleResolution, ok := htlcResolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + htlcResolution) + } + if settleResolution.Preimage != testInvoicePreimage { t.Fatal("unexpected preimage in hodl resolution") } - if htlcResolution.AcceptHeight != testCurrentHeight { + if settleResolution.AcceptHeight != testCurrentHeight { t.Fatalf("expected acceptHeight %v, but got %v", - testCurrentHeight, resolution.AcceptHeight) + testCurrentHeight, settleResolution.AcceptHeight) } - if htlcResolution.Outcome != ResultSettled { + if settleResolution.Outcome != ResultSettled { t.Fatalf("expected result settled, got: %v", - htlcResolution.Outcome) + settleResolution.Outcome) } // We expect a settled notification to be sent out for both all and @@ -545,8 +568,10 @@ func TestCancelHoldInvoice(t *testing.T) { } htlcResolution := (<-hodlChan).(HtlcResolution) - if htlcResolution.Preimage != nil { - t.Fatal("expected cancel htlc resolution") + _, ok := htlcResolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + htlcResolution) } // Offering the same htlc again at a higher height should still result @@ -559,16 +584,18 @@ func TestCancelHoldInvoice(t *testing.T) { if err != nil { t.Fatalf("expected settle to succeed but got %v", err) } - if resolution.Preimage != nil { - t.Fatalf("expected htlc to be canceled") + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if resolution.AcceptHeight != testCurrentHeight { + if failResolution.AcceptHeight != testCurrentHeight { t.Fatalf("expected acceptHeight %v, but got %v", - testCurrentHeight, resolution.AcceptHeight) + testCurrentHeight, failResolution.AcceptHeight) } - if resolution.Outcome != ResultReplayToCanceled { + if failResolution.Outcome != ResultReplayToCanceled { t.Fatalf("expected replay to canceled, got %v", - resolution.Outcome) + failResolution.Outcome) } } @@ -585,16 +612,21 @@ func TestUnknownInvoice(t *testing.T) { // succeed. hodlChan := make(chan interface{}) amt := lnwire.MilliSatoshi(100000) - result, err := ctx.registry.NotifyExitHopHtlc( + resolution, err := ctx.registry.NotifyExitHopHtlc( testInvoicePaymentHash, amt, testHtlcExpiry, testCurrentHeight, getCircuitKey(0), hodlChan, testPayload, ) if err != nil { t.Fatal("unexpected error") } - if result.Outcome != ResultInvoiceNotFound { + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) + } + if failResolution.Outcome != ResultInvoiceNotFound { t.Fatalf("expected ResultInvoiceNotFound, got: %v", - result.Outcome) + failResolution.Outcome) } } @@ -646,16 +678,17 @@ func testKeySend(t *testing.T, keySendEnabled bool) { if err != nil { t.Fatal(err) } - - // Expect a cancel resolution with the correct outcome. - if resolution.Preimage != nil { - t.Fatal("expected cancel resolution") + failResolution, ok := resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } + switch { - case !keySendEnabled && resolution.Outcome != ResultInvoiceNotFound: + case !keySendEnabled && failResolution.Outcome != ResultInvoiceNotFound: t.Fatal("expected invoice not found outcome") - case keySendEnabled && resolution.Outcome != ResultKeySendError: + case keySendEnabled && failResolution.Outcome != ResultKeySendError: t.Fatal("expected keysend error") } @@ -676,15 +709,25 @@ func testKeySend(t *testing.T, keySendEnabled bool) { // Expect a cancel resolution if keysend is disabled. if !keySendEnabled { - if resolution.Outcome != ResultInvoiceNotFound { + failResolution, ok = resolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) + } + if failResolution.Outcome != ResultInvoiceNotFound { t.Fatal("expected keysend payment not to be accepted") } return } // Otherwise we expect no error and a settle resolution for the htlc. - if resolution.Preimage == nil || *resolution.Preimage != preimage { - t.Fatal("expected valid settle event") + settleResolution, ok := resolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + resolution) + } + if settleResolution.Preimage != preimage { + t.Fatalf("expected settle with matching preimage") } // We expect a new invoice notification to be sent out. @@ -739,12 +782,14 @@ func TestMppPayment(t *testing.T) { ctx.clock.SetTime(testTime.Add(30 * time.Second)) htlcResolution := (<-hodlChan1).(HtlcResolution) - if htlcResolution.Preimage != nil { - t.Fatal("expected cancel resolution") + failResolution, ok := htlcResolution.(*HtlcFailResolution) + if !ok { + t.Fatalf("expected fail resolution, got: %T", + resolution) } - if htlcResolution.Outcome != ResultMppTimeout { + if failResolution.Outcome != ResultMppTimeout { t.Fatalf("expected mpp timeout, got: %v", - htlcResolution.Outcome) + failResolution.Outcome) } // Send htlc 2. @@ -771,12 +816,14 @@ func TestMppPayment(t *testing.T) { if err != nil { t.Fatal(err) } - if resolution == nil { - t.Fatal("expected a settle resolution") + settleResolution, ok := resolution.(*HtlcSettleResolution) + if !ok { + t.Fatalf("expected settle resolution, got: %T", + htlcResolution) } - if resolution.Outcome != ResultSettled { + if settleResolution.Outcome != ResultSettled { t.Fatalf("expected result settled, got: %v", - resolution.Outcome) + settleResolution.Outcome) } // Check that settled amount is equal to the sum of values of the htlcs diff --git a/invoices/resolution.go b/invoices/resolution.go new file mode 100644 index 000000000..483a57775 --- /dev/null +++ b/invoices/resolution.go @@ -0,0 +1,124 @@ +package invoices + +import ( + "time" + + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lntypes" +) + +// HtlcResolution describes how an htlc should be resolved. +type HtlcResolution interface { + // CircuitKey returns the circuit key for the htlc that we have a + // resolution for. + CircuitKey() channeldb.CircuitKey +} + +// HtlcFailResolution is an implementation of the HtlcResolution interface +// which is returned when a htlc is failed. +type HtlcFailResolution struct { + // circuitKey is the key of the htlc for which we have a resolution. + circuitKey channeldb.CircuitKey + + // AcceptHeight is the original height at which the htlc was accepted. + AcceptHeight int32 + + // outcome indicates the outcome of the invoice registry update. + Outcome ResolutionResult +} + +// NewFailResolution returns a htlc failure resolution. +func NewFailResolution(key channeldb.CircuitKey, + acceptHeight int32, outcome ResolutionResult) *HtlcFailResolution { + + return &HtlcFailResolution{ + circuitKey: key, + AcceptHeight: acceptHeight, + Outcome: outcome, + } +} + +// CircuitKey returns the circuit key for the htlc that we have a +// resolution for. +// +// Note: it is part of the HtlcResolution interface. +func (f *HtlcFailResolution) CircuitKey() channeldb.CircuitKey { + return f.circuitKey +} + +// HtlcSettleResolution is an implementation of the HtlcResolution interface +// which is returned when a htlc is settled. +type HtlcSettleResolution struct { + // Preimage is the htlc preimage. Its value is nil in case of a cancel. + Preimage lntypes.Preimage + + // circuitKey is the key of the htlc for which we have a resolution. + circuitKey channeldb.CircuitKey + + // acceptHeight is the original height at which the htlc was accepted. + AcceptHeight int32 + + // Outcome indicates the outcome of the invoice registry update. + Outcome ResolutionResult +} + +// NewSettleResolution returns a htlc resolution which is associated with a +// settle. +func NewSettleResolution(preimage lntypes.Preimage, key channeldb.CircuitKey, + acceptHeight int32, outcome ResolutionResult) *HtlcSettleResolution { + + return &HtlcSettleResolution{ + Preimage: preimage, + circuitKey: key, + AcceptHeight: acceptHeight, + Outcome: outcome, + } +} + +// CircuitKey returns the circuit key for the htlc that we have a +// resolution for. +// +// Note: it is part of the HtlcResolution interface. +func (s *HtlcSettleResolution) CircuitKey() channeldb.CircuitKey { + return s.circuitKey +} + +// htlcAcceptResolution is an implementation of the HtlcResolution interface +// which is returned when a htlc is accepted. This struct is not exported +// because the codebase uses a nil resolution to indicate that a htlc was +// accepted. This struct is used internally in the invoice registry to +// surface accept resolution results. When an invoice update returns an +// acceptResolution, a nil resolution should be surfaced. +type htlcAcceptResolution struct { + // circuitKey is the key of the htlc for which we have a resolution. + circuitKey channeldb.CircuitKey + + // autoRelease signals that the htlc should be automatically released + // after a timeout. + autoRelease bool + + // acceptTime is the time at which this htlc was accepted. + acceptTime time.Time + + // outcome indicates the outcome of the invoice registry update. + outcome ResolutionResult +} + +// newAcceptResolution returns a htlc resolution which is associated with a +// htlc accept. +func newAcceptResolution(key channeldb.CircuitKey, + outcome ResolutionResult) *htlcAcceptResolution { + + return &htlcAcceptResolution{ + circuitKey: key, + outcome: outcome, + } +} + +// CircuitKey returns the circuit key for the htlc that we have a +// resolution for. +// +// Note: it is part of the HtlcResolution interface. +func (a *htlcAcceptResolution) CircuitKey() channeldb.CircuitKey { + return a.circuitKey +} diff --git a/invoices/update.go b/invoices/update.go index e6feaba06..c747521e8 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -186,26 +186,53 @@ func (i *invoiceUpdateCtx) log(s string) { i.hash[:], s, i.amtPaid, i.expiry, i.circuitKey, i.mpp) } +// failRes is a helper function which creates a failure resolution with +// the information contained in the invoiceUpdateCtx and the outcome provided. +func (i invoiceUpdateCtx) failRes(outcome ResolutionResult) *HtlcFailResolution { + return NewFailResolution(i.circuitKey, i.currentHeight, outcome) +} + +// settleRes is a helper function which creates a settle resolution with +// the information contained in the invoiceUpdateCtx and the preimage and +// outcome provided. +func (i invoiceUpdateCtx) settleRes(preimage lntypes.Preimage, + outcome ResolutionResult) *HtlcSettleResolution { + + return NewSettleResolution( + preimage, i.circuitKey, i.currentHeight, outcome, + ) +} + +// acceptRes is a helper function which creates an accept resolution with +// the information contained in the invoiceUpdateCtx and the outcome provided. +func (i invoiceUpdateCtx) acceptRes(outcome ResolutionResult) *htlcAcceptResolution { + return newAcceptResolution(i.circuitKey, outcome) +} + // updateInvoice is a callback for DB.UpdateInvoice that contains the invoice -// settlement logic. +// settlement logic. It returns a hltc resolution that indicates what the +// outcome of the update was. func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( - *channeldb.InvoiceUpdateDesc, ResolutionResult, error) { + *channeldb.InvoiceUpdateDesc, HtlcResolution, error) { // Don't update the invoice when this is a replayed htlc. htlc, ok := inv.Htlcs[ctx.circuitKey] if ok { switch htlc.State { case channeldb.HtlcStateCanceled: - return nil, ResultReplayToCanceled, nil + return nil, ctx.failRes(ResultReplayToCanceled), nil case channeldb.HtlcStateAccepted: - return nil, ResultReplayToAccepted, nil + return nil, ctx.acceptRes(ResultReplayToAccepted), nil case channeldb.HtlcStateSettled: - return nil, ResultReplayToSettled, nil + return nil, ctx.settleRes( + inv.Terms.PaymentPreimage, + ResultReplayToSettled, + ), nil default: - return nil, 0, errors.New("unknown htlc state") + return nil, nil, errors.New("unknown htlc state") } } @@ -218,8 +245,9 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // updateMpp is a callback for DB.UpdateInvoice that contains the invoice // settlement logic for mpp payments. -func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( - *channeldb.InvoiceUpdateDesc, ResolutionResult, error) { +func updateMpp(ctx *invoiceUpdateCtx, + inv *channeldb.Invoice) (*channeldb.InvoiceUpdateDesc, + HtlcResolution, error) { // Start building the accept descriptor. acceptDesc := &channeldb.HtlcAcceptDesc{ @@ -235,23 +263,23 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // Because non-mpp payments don't have a payment address, this is needed // to thwart probing. if inv.State != channeldb.ContractOpen { - return nil, ResultInvoiceNotOpen, nil + return nil, ctx.failRes(ResultInvoiceNotOpen), nil } // Check the payment address that authorizes the payment. if ctx.mpp.PaymentAddr() != inv.Terms.PaymentAddr { - return nil, ResultAddressMismatch, nil + return nil, ctx.failRes(ResultAddressMismatch), nil } // Don't accept zero-valued sets. if ctx.mpp.TotalMsat() == 0 { - return nil, ResultHtlcSetTotalTooLow, nil + return nil, ctx.failRes(ResultHtlcSetTotalTooLow), nil } // Check that the total amt of the htlc set is high enough. In case this // is a zero-valued invoice, it will always be enough. if ctx.mpp.TotalMsat() < inv.Terms.Value { - return nil, ResultHtlcSetTotalTooLow, nil + return nil, ctx.failRes(ResultHtlcSetTotalTooLow), nil } // Check whether total amt matches other htlcs in the set. @@ -265,7 +293,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( } if ctx.mpp.TotalMsat() != htlc.MppTotalAmt { - return nil, ResultHtlcSetTotalMismatch, nil + return nil, ctx.failRes(ResultHtlcSetTotalMismatch), nil } newSetTotal += htlc.Amt @@ -276,16 +304,16 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // Make sure the communicated set total isn't overpaid. if newSetTotal > ctx.mpp.TotalMsat() { - return nil, ResultHtlcSetOverpayment, nil + return nil, ctx.failRes(ResultHtlcSetOverpayment), nil } // The invoice is still open. Check the expiry. if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) { - return nil, ResultExpiryTooSoon, nil + return nil, ctx.failRes(ResultExpiryTooSoon), nil } if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) { - return nil, ResultExpiryTooSoon, nil + return nil, ctx.failRes(ResultExpiryTooSoon), nil } // Record HTLC in the invoice database. @@ -300,7 +328,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // If the invoice cannot be settled yet, only record the htlc. setComplete := newSetTotal == ctx.mpp.TotalMsat() if !setComplete { - return &update, ResultPartialAccepted, nil + return &update, ctx.acceptRes(ResultPartialAccepted), nil } // Check to see if we can settle or this is an hold invoice and @@ -310,7 +338,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( update.State = &channeldb.InvoiceStateUpdateDesc{ NewState: channeldb.ContractAccepted, } - return &update, ResultAccepted, nil + return &update, ctx.acceptRes(ResultAccepted), nil } update.State = &channeldb.InvoiceStateUpdateDesc{ @@ -318,18 +346,20 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( Preimage: inv.Terms.PaymentPreimage, } - return &update, ResultSettled, nil + return &update, ctx.settleRes( + inv.Terms.PaymentPreimage, ResultSettled, + ), nil } // updateLegacy is a callback for DB.UpdateInvoice that contains the invoice // settlement logic for legacy payments. -func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( - *channeldb.InvoiceUpdateDesc, ResolutionResult, error) { +func updateLegacy(ctx *invoiceUpdateCtx, + inv *channeldb.Invoice) (*channeldb.InvoiceUpdateDesc, HtlcResolution, error) { // If the invoice is already canceled, there is no further // checking to do. if inv.State == channeldb.ContractCanceled { - return nil, ResultInvoiceAlreadyCanceled, nil + return nil, ctx.failRes(ResultInvoiceAlreadyCanceled), nil } // If an invoice amount is specified, check that enough is paid. Also @@ -337,7 +367,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // or accepted. In case this is a zero-valued invoice, it will always be // enough. if ctx.amtPaid < inv.Terms.Value { - return nil, ResultAmountTooLow, nil + return nil, ctx.failRes(ResultAmountTooLow), nil } // TODO(joostjager): Check invoice mpp required feature @@ -350,17 +380,17 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( if htlc.State == channeldb.HtlcStateAccepted && htlc.MppTotalAmt > 0 { - return nil, ResultMppInProgress, nil + return nil, ctx.failRes(ResultMppInProgress), nil } } // The invoice is still open. Check the expiry. if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) { - return nil, ResultExpiryTooSoon, nil + return nil, ctx.failRes(ResultExpiryTooSoon), nil } if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) { - return nil, ResultExpiryTooSoon, nil + return nil, ctx.failRes(ResultExpiryTooSoon), nil } // Record HTLC in the invoice database. @@ -381,10 +411,12 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( // We do accept or settle the HTLC. switch inv.State { case channeldb.ContractAccepted: - return &update, ResultDuplicateToAccepted, nil + return &update, ctx.acceptRes(ResultDuplicateToAccepted), nil case channeldb.ContractSettled: - return &update, ResultDuplicateToSettled, nil + return &update, ctx.settleRes( + inv.Terms.PaymentPreimage, ResultDuplicateToSettled, + ), nil } // Check to see if we can settle or this is an hold invoice and we need @@ -394,7 +426,8 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( update.State = &channeldb.InvoiceStateUpdateDesc{ NewState: channeldb.ContractAccepted, } - return &update, ResultAccepted, nil + + return &update, ctx.acceptRes(ResultAccepted), nil } update.State = &channeldb.InvoiceStateUpdateDesc{ @@ -402,5 +435,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( Preimage: inv.Terms.PaymentPreimage, } - return &update, ResultSettled, nil + return &update, ctx.settleRes( + inv.Terms.PaymentPreimage, ResultSettled, + ), nil } From 8cbed23f26ee208200865874ed18e03db6e6f046 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:16 +0200 Subject: [PATCH 2/6] invoices: split resolution result into settle, fail and accept enums This commit splits the resolution result enum into results divided by outcome (settled, failed or accepted). This allows us to more strictly control which resolution results can be used with which HtlcResolution structs, to prevent the combination of a settle resolution result with a failure resolution result, for example. --- invoices/invoiceregistry.go | 2 +- invoices/resolution.go | 17 +-- invoices/resolution_result.go | 200 ++++++++++++++++++++++++++++++++++ invoices/update.go | 182 +++---------------------------- 4 files changed, 223 insertions(+), 178 deletions(-) create mode 100644 invoices/resolution_result.go diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 7f62ae45e..00885edb9 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -565,7 +565,7 @@ func (i *InvoiceRegistry) startHtlcTimer(hash lntypes.Hash, // a resolution result which will be used to notify subscribed links and // resolvers of the details of the htlc cancellation. func (i *InvoiceRegistry) cancelSingleHtlc(hash lntypes.Hash, - key channeldb.CircuitKey, result ResolutionResult) error { + key channeldb.CircuitKey, result FailResolutionResult) error { i.Lock() defer i.Unlock() diff --git a/invoices/resolution.go b/invoices/resolution.go index 483a57775..e6f313303 100644 --- a/invoices/resolution.go +++ b/invoices/resolution.go @@ -23,13 +23,13 @@ type HtlcFailResolution struct { // AcceptHeight is the original height at which the htlc was accepted. AcceptHeight int32 - // outcome indicates the outcome of the invoice registry update. - Outcome ResolutionResult + // Outcome indicates the outcome of the invoice registry update. + Outcome FailResolutionResult } // NewFailResolution returns a htlc failure resolution. func NewFailResolution(key channeldb.CircuitKey, - acceptHeight int32, outcome ResolutionResult) *HtlcFailResolution { + acceptHeight int32, outcome FailResolutionResult) *HtlcFailResolution { return &HtlcFailResolution{ circuitKey: key, @@ -59,13 +59,14 @@ type HtlcSettleResolution struct { AcceptHeight int32 // Outcome indicates the outcome of the invoice registry update. - Outcome ResolutionResult + Outcome SettleResolutionResult } // NewSettleResolution returns a htlc resolution which is associated with a // settle. -func NewSettleResolution(preimage lntypes.Preimage, key channeldb.CircuitKey, - acceptHeight int32, outcome ResolutionResult) *HtlcSettleResolution { +func NewSettleResolution(preimage lntypes.Preimage, + key channeldb.CircuitKey, acceptHeight int32, + outcome SettleResolutionResult) *HtlcSettleResolution { return &HtlcSettleResolution{ Preimage: preimage, @@ -101,13 +102,13 @@ type htlcAcceptResolution struct { acceptTime time.Time // outcome indicates the outcome of the invoice registry update. - outcome ResolutionResult + outcome acceptResolutionResult } // newAcceptResolution returns a htlc resolution which is associated with a // htlc accept. func newAcceptResolution(key channeldb.CircuitKey, - outcome ResolutionResult) *htlcAcceptResolution { + outcome acceptResolutionResult) *htlcAcceptResolution { return &htlcAcceptResolution{ circuitKey: key, diff --git a/invoices/resolution_result.go b/invoices/resolution_result.go new file mode 100644 index 000000000..3952d11a5 --- /dev/null +++ b/invoices/resolution_result.go @@ -0,0 +1,200 @@ +package invoices + +// acceptResolutionResult provides metadata which about a htlc that was +// accepted by the registry. +type acceptResolutionResult uint8 + +const ( + resultInvalidAccept acceptResolutionResult = iota + + // resultReplayToAccepted is returned when we replay an accepted + // invoice. + resultReplayToAccepted + + // resultDuplicateToAccepted is returned when we accept a duplicate + // htlc. + resultDuplicateToAccepted + + // resultAccepted is returned when we accept a hodl invoice. + resultAccepted + + // resultPartialAccepted is returned when we have partially received + // payment. + resultPartialAccepted +) + +// String returns a string representation of the result. +func (a acceptResolutionResult) String() string { + switch a { + case resultInvalidAccept: + return "invalid accept result" + + case resultReplayToAccepted: + return "replayed htlc to accepted invoice" + + case resultDuplicateToAccepted: + return "accepting duplicate payment to accepted invoice" + + case resultAccepted: + return "accepted" + + case resultPartialAccepted: + return "partial payment accepted" + + default: + return "unknown accept resolution result" + } +} + +// FailResolutionResult provides metadata about a htlc that was failed by +// the registry. It can be used to take custom actions on resolution of the +// htlc. +type FailResolutionResult uint8 + +const ( + resultInvalidFailure FailResolutionResult = iota + + // ResultReplayToCanceled is returned when we replay a canceled invoice. + ResultReplayToCanceled + + // ResultInvoiceAlreadyCanceled is returned when trying to pay an + // invoice that is already canceled. + ResultInvoiceAlreadyCanceled + + // ResultAmountTooLow is returned when an invoice is underpaid. + ResultAmountTooLow + + // ResultExpiryTooSoon is returned when we do not accept an invoice + // payment because it expires too soon. + ResultExpiryTooSoon + + // ResultCanceled is returned when we cancel an invoice and its + // associated htlcs. + ResultCanceled + + // ResultInvoiceNotOpen is returned when a mpp invoice is not open. + ResultInvoiceNotOpen + + // ResultMppTimeout is returned when an invoice paid with multiple + // partial payments times out before it is fully paid. + ResultMppTimeout + + // ResultAddressMismatch is returned when the payment address for a mpp + // invoice does not match. + ResultAddressMismatch + + // ResultHtlcSetTotalMismatch is returned when the amount paid by a + // htlc does not match its set total. + ResultHtlcSetTotalMismatch + + // ResultHtlcSetTotalTooLow is returned when a mpp set total is too low + // for an invoice. + ResultHtlcSetTotalTooLow + + // ResultHtlcSetOverpayment is returned when a mpp set is overpaid. + ResultHtlcSetOverpayment + + // ResultInvoiceNotFound is returned when an attempt is made to pay an + // invoice that is unknown to us. + ResultInvoiceNotFound + + // ResultKeySendError is returned when we receive invalid keysend + // parameters. + ResultKeySendError + + // ResultMppInProgress is returned when we are busy receiving a mpp + // payment. + ResultMppInProgress +) + +// String returns a string representation of the result. +func (f FailResolutionResult) String() string { + switch f { + case resultInvalidFailure: + return "invalid failure result" + + case ResultReplayToCanceled: + return "replayed htlc to canceled invoice" + + case ResultInvoiceAlreadyCanceled: + return "invoice already canceled" + + case ResultAmountTooLow: + return "amount too low" + + case ResultExpiryTooSoon: + return "expiry too soon" + + case ResultCanceled: + return "canceled" + + case ResultInvoiceNotOpen: + return "invoice no longer open" + + case ResultMppTimeout: + return "mpp timeout" + + case ResultAddressMismatch: + return "payment address mismatch" + + case ResultHtlcSetTotalMismatch: + return "htlc total amt doesn't match set total" + + case ResultHtlcSetTotalTooLow: + return "set total too low for invoice" + + case ResultHtlcSetOverpayment: + return "mpp is overpaying set total" + + case ResultInvoiceNotFound: + return "invoice not found" + + case ResultKeySendError: + return "invalid keysend parameters" + + case ResultMppInProgress: + return "mpp reception in progress" + + default: + return "unknown failure resolution result" + } +} + +// SettleResolutionResult provides metadata which about a htlc that was failed +// by the registry. It can be used to take custom actions on resolution of the +// htlc. +type SettleResolutionResult uint8 + +const ( + resultInvalidSettle SettleResolutionResult = iota + + // ResultSettled is returned when we settle an invoice. + ResultSettled + + // ResultReplayToSettled is returned when we replay a settled invoice. + ResultReplayToSettled + + // ResultDuplicateToSettled is returned when we settle an invoice which + // has already been settled at least once. + ResultDuplicateToSettled +) + +// String returns a string representation of the result. +func (s SettleResolutionResult) String() string { + switch s { + case resultInvalidSettle: + return "invalid settle result" + + case ResultSettled: + return "settled" + + case ResultReplayToSettled: + return "replayed htlc to settled invoice" + + case ResultDuplicateToSettled: + return "accepting duplicate payment to settled invoice" + + default: + return "unknown settle resolution result" + } +} diff --git a/invoices/update.go b/invoices/update.go index c747521e8..3226779ce 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -9,164 +9,6 @@ import ( "github.com/lightningnetwork/lnd/record" ) -// ResolutionResult provides metadata which about an invoice update which can -// be used to take custom actions on resolution of the htlc. Only results which -// are actionable by the link are exported. -type ResolutionResult uint8 - -const ( - resultInvalid ResolutionResult = iota - - // ResultReplayToCanceled is returned when we replay a canceled invoice. - ResultReplayToCanceled - - // ResultReplayToAccepted is returned when we replay an accepted invoice. - ResultReplayToAccepted - - // ResultReplayToSettled is returned when we replay a settled invoice. - ResultReplayToSettled - - // ResultInvoiceAlreadyCanceled is returned when trying to pay an invoice - // that is already canceled. - ResultInvoiceAlreadyCanceled - - // ResultAmountTooLow is returned when an invoice is underpaid. - ResultAmountTooLow - - // ResultExpiryTooSoon is returned when we do not accept an invoice payment - // because it expires too soon. - ResultExpiryTooSoon - - // ResultDuplicateToAccepted is returned when we accept a duplicate htlc. - ResultDuplicateToAccepted - - // ResultDuplicateToSettled is returned when we settle an invoice which has - // already been settled at least once. - ResultDuplicateToSettled - - // ResultAccepted is returned when we accept a hodl invoice. - ResultAccepted - - // ResultSettled is returned when we settle an invoice. - ResultSettled - - // ResultCanceled is returned when we cancel an invoice and its associated - // htlcs. - ResultCanceled - - // ResultInvoiceNotOpen is returned when a mpp invoice is not open. - ResultInvoiceNotOpen - - // ResultPartialAccepted is returned when we have partially received - // payment. - ResultPartialAccepted - - // ResultMppInProgress is returned when we are busy receiving a mpp payment. - ResultMppInProgress - - // ResultMppTimeout is returned when an invoice paid with multiple partial - // payments times out before it is fully paid. - ResultMppTimeout - - // ResultAddressMismatch is returned when the payment address for a mpp - // invoice does not match. - ResultAddressMismatch - - // ResultHtlcSetTotalMismatch is returned when the amount paid by a htlc - // does not match its set total. - ResultHtlcSetTotalMismatch - - // ResultHtlcSetTotalTooLow is returned when a mpp set total is too low for - // an invoice. - ResultHtlcSetTotalTooLow - - // ResultHtlcSetOverpayment is returned when a mpp set is overpaid. - ResultHtlcSetOverpayment - - // ResultInvoiceNotFound is returned when an attempt is made to pay an - // invoice that is unknown to us. - ResultInvoiceNotFound - - // ResultKeySendError is returned when we receive invalid keysend - // parameters. - ResultKeySendError -) - -// String returns a human-readable representation of the invoice update result. -func (u ResolutionResult) String() string { - switch u { - - case resultInvalid: - return "invalid" - - case ResultReplayToCanceled: - return "replayed htlc to canceled invoice" - - case ResultReplayToAccepted: - return "replayed htlc to accepted invoice" - - case ResultReplayToSettled: - return "replayed htlc to settled invoice" - - case ResultInvoiceAlreadyCanceled: - return "invoice already canceled" - - case ResultAmountTooLow: - return "amount too low" - - case ResultExpiryTooSoon: - return "expiry too soon" - - case ResultDuplicateToAccepted: - return "accepting duplicate payment to accepted invoice" - - case ResultDuplicateToSettled: - return "accepting duplicate payment to settled invoice" - - case ResultAccepted: - return "accepted" - - case ResultSettled: - return "settled" - - case ResultCanceled: - return "canceled" - - case ResultInvoiceNotOpen: - return "invoice no longer open" - - case ResultPartialAccepted: - return "partial payment accepted" - - case ResultMppInProgress: - return "mpp reception in progress" - - case ResultMppTimeout: - return "mpp timeout" - - case ResultAddressMismatch: - return "payment address mismatch" - - case ResultHtlcSetTotalMismatch: - return "htlc total amt doesn't match set total" - - case ResultHtlcSetTotalTooLow: - return "set total too low for invoice" - - case ResultHtlcSetOverpayment: - return "mpp is overpaying set total" - - case ResultKeySendError: - return "invalid keysend parameters" - - case ResultInvoiceNotFound: - return "invoice not found" - - default: - return "unknown" - } -} - // invoiceUpdateCtx is an object that describes the context for the invoice // update to be carried out. type invoiceUpdateCtx struct { @@ -187,16 +29,17 @@ func (i *invoiceUpdateCtx) log(s string) { } // failRes is a helper function which creates a failure resolution with -// the information contained in the invoiceUpdateCtx and the outcome provided. -func (i invoiceUpdateCtx) failRes(outcome ResolutionResult) *HtlcFailResolution { +// the information contained in the invoiceUpdateCtx and the fail resolution +// result provided. +func (i invoiceUpdateCtx) failRes(outcome FailResolutionResult) *HtlcFailResolution { return NewFailResolution(i.circuitKey, i.currentHeight, outcome) } // settleRes is a helper function which creates a settle resolution with // the information contained in the invoiceUpdateCtx and the preimage and -// outcome provided. +// the settle resolution result provided. func (i invoiceUpdateCtx) settleRes(preimage lntypes.Preimage, - outcome ResolutionResult) *HtlcSettleResolution { + outcome SettleResolutionResult) *HtlcSettleResolution { return NewSettleResolution( preimage, i.circuitKey, i.currentHeight, outcome, @@ -204,8 +47,9 @@ func (i invoiceUpdateCtx) settleRes(preimage lntypes.Preimage, } // acceptRes is a helper function which creates an accept resolution with -// the information contained in the invoiceUpdateCtx and the outcome provided. -func (i invoiceUpdateCtx) acceptRes(outcome ResolutionResult) *htlcAcceptResolution { +// the information contained in the invoiceUpdateCtx and the accept resolution +// result provided. +func (i invoiceUpdateCtx) acceptRes(outcome acceptResolutionResult) *htlcAcceptResolution { return newAcceptResolution(i.circuitKey, outcome) } @@ -223,7 +67,7 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) ( return nil, ctx.failRes(ResultReplayToCanceled), nil case channeldb.HtlcStateAccepted: - return nil, ctx.acceptRes(ResultReplayToAccepted), nil + return nil, ctx.acceptRes(resultReplayToAccepted), nil case channeldb.HtlcStateSettled: return nil, ctx.settleRes( @@ -328,7 +172,7 @@ func updateMpp(ctx *invoiceUpdateCtx, // If the invoice cannot be settled yet, only record the htlc. setComplete := newSetTotal == ctx.mpp.TotalMsat() if !setComplete { - return &update, ctx.acceptRes(ResultPartialAccepted), nil + return &update, ctx.acceptRes(resultPartialAccepted), nil } // Check to see if we can settle or this is an hold invoice and @@ -338,7 +182,7 @@ func updateMpp(ctx *invoiceUpdateCtx, update.State = &channeldb.InvoiceStateUpdateDesc{ NewState: channeldb.ContractAccepted, } - return &update, ctx.acceptRes(ResultAccepted), nil + return &update, ctx.acceptRes(resultAccepted), nil } update.State = &channeldb.InvoiceStateUpdateDesc{ @@ -411,7 +255,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, // We do accept or settle the HTLC. switch inv.State { case channeldb.ContractAccepted: - return &update, ctx.acceptRes(ResultDuplicateToAccepted), nil + return &update, ctx.acceptRes(resultDuplicateToAccepted), nil case channeldb.ContractSettled: return &update, ctx.settleRes( @@ -427,7 +271,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, NewState: channeldb.ContractAccepted, } - return &update, ctx.acceptRes(ResultAccepted), nil + return &update, ctx.acceptRes(resultAccepted), nil } update.State = &channeldb.InvoiceStateUpdateDesc{ From bdd9411bbdc90b8b30cd47fd7f226a753c5e27fd Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:16 +0200 Subject: [PATCH 3/6] htlcswitch: rename FailureDetail to OutgoingFailure Rename FailureDetail in a separate commit so that a FailureDetail interface can be introduced in the following commit. OutgoingFailureOnionDecode is renamed to OutgoingFailureDecodeError to specifically indicate that we could not decode the wire failure that our payment experienced. --- htlcswitch/failure.go | 14 ++++----- htlcswitch/failure_detail.go | 58 ++++++++++++++++++------------------ htlcswitch/link.go | 4 +-- htlcswitch/switch.go | 14 ++++----- htlcswitch/switch_test.go | 6 ++-- 5 files changed, 48 insertions(+), 48 deletions(-) diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 35d78590b..c4ef73c5c 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -34,8 +34,8 @@ type LinkError struct { // node. msg lnwire.FailureMessage - // FailureDetail enriches the wire error with additional information. - FailureDetail + // OutgoingFailure enriches the wire error with additional information. + OutgoingFailure } // NewLinkError returns a LinkError with the failure message provided. @@ -48,11 +48,11 @@ func NewLinkError(msg lnwire.FailureMessage) *LinkError { // NewDetailedLinkError returns a link error that enriches a wire message with // a failure detail. func NewDetailedLinkError(msg lnwire.FailureMessage, - detail FailureDetail) *LinkError { + detail OutgoingFailure) *LinkError { return &LinkError{ - msg: msg, - FailureDetail: detail, + msg: msg, + OutgoingFailure: detail, } } @@ -72,11 +72,11 @@ func (l *LinkError) WireMessage() lnwire.FailureMessage { func (l *LinkError) Error() string { // If the link error has no failure detail, return the wire message's // error. - if l.FailureDetail == FailureDetailNone { + if l.OutgoingFailure == OutgoingFailureNone { return l.msg.Error() } - return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail) + return fmt.Sprintf("%v: %v", l.msg.Error(), l.OutgoingFailure) } // ForwardingError wraps an lnwire.FailureMessage in a struct that also diff --git a/htlcswitch/failure_detail.go b/htlcswitch/failure_detail.go index 976015b8c..95f43f118 100644 --- a/htlcswitch/failure_detail.go +++ b/htlcswitch/failure_detail.go @@ -1,63 +1,63 @@ package htlcswitch -// FailureDetail is an enum which is used to enrich failures with -// additional information. -type FailureDetail int +// OutgoingFailure is an enum which is used to enrich failures which occur in +// the switch or on our outgoing link with additional metadata. +type OutgoingFailure int const ( - // FailureDetailNone is returned when the wire message contains + // OutgoingFailureNone is returned when the wire message contains // sufficient information. - FailureDetailNone = iota + OutgoingFailureNone OutgoingFailure = iota - // FailureDetailOnionDecode indicates that we could not decode an - // onion error. - FailureDetailOnionDecode + // OutgoingFailureDecodeError indicates that we could not decode the + // failure reason provided for a failed payment. + OutgoingFailureDecodeError - // FailureDetailLinkNotEligible indicates that a routing attempt was + // OutgoingFailureLinkNotEligible indicates that a routing attempt was // made over a link that is not eligible for routing. - FailureDetailLinkNotEligible + OutgoingFailureLinkNotEligible - // FailureDetailOnChainTimeout indicates that a payment had to be timed - // out on chain before it got past the first hop by us or the remote - // party. - FailureDetailOnChainTimeout + // OutgoingFailureOnChainTimeout indicates that a payment had to be + // timed out on chain before it got past the first hop by us or the + // remote party. + OutgoingFailureOnChainTimeout - // FailureDetailHTLCExceedsMax is returned when a htlc exceeds our + // OutgoingFailureHTLCExceedsMax is returned when a htlc exceeds our // policy's maximum htlc amount. - FailureDetailHTLCExceedsMax + OutgoingFailureHTLCExceedsMax - // FailureDetailInsufficientBalance is returned when we cannot route a + // OutgoingFailureInsufficientBalance is returned when we cannot route a // htlc due to insufficient outgoing capacity. - FailureDetailInsufficientBalance + OutgoingFailureInsufficientBalance - // FailureDetailCircularRoute is returned when an attempt is made + // OutgoingFailureCircularRoute is returned when an attempt is made // to forward a htlc through our node which arrives and leaves on the // same channel. - FailureDetailCircularRoute + OutgoingFailureCircularRoute ) // String returns the string representation of a failure detail. -func (fd FailureDetail) String() string { +func (fd OutgoingFailure) String() string { switch fd { - case FailureDetailNone: + case OutgoingFailureNone: return "no failure detail" - case FailureDetailOnionDecode: - return "could not decode onion" + case OutgoingFailureDecodeError: + return "could not decode wire failure" - case FailureDetailLinkNotEligible: + case OutgoingFailureLinkNotEligible: return "link not eligible" - case FailureDetailOnChainTimeout: + case OutgoingFailureOnChainTimeout: return "payment was resolved on-chain, then canceled back" - case FailureDetailHTLCExceedsMax: + case OutgoingFailureHTLCExceedsMax: return "htlc exceeds maximum policy amount" - case FailureDetailInsufficientBalance: + case OutgoingFailureInsufficientBalance: return "insufficient bandwidth to route htlc" - case FailureDetailCircularRoute: + case OutgoingFailureCircularRoute: return "same incoming and outgoing channel" default: diff --git a/htlcswitch/link.go b/htlcswitch/link.go index dee3c588d..94773f2cf 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2284,7 +2284,7 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, return lnwire.NewTemporaryChannelFailure(upd) }, ) - return NewDetailedLinkError(failure, FailureDetailHTLCExceedsMax) + return NewDetailedLinkError(failure, OutgoingFailureHTLCExceedsMax) } // We want to avoid offering an HTLC which will expire in the near @@ -2319,7 +2319,7 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy, }, ) return NewDetailedLinkError( - failure, FailureDetailInsufficientBalance, + failure, OutgoingFailureInsufficientBalance, ) } diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 760aadf00..5db9dac55 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -771,7 +771,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error { // will be returned back to the router. return NewDetailedLinkError( lnwire.NewTemporaryChannelFailure(nil), - FailureDetailLinkNotEligible, + OutgoingFailureLinkNotEligible, ) } @@ -919,11 +919,11 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, // need to apply an update here since it goes // directly to the router. lnwire.NewTemporaryChannelFailure(nil), - FailureDetailOnionDecode, + OutgoingFailureDecodeError, ) log.Errorf("%v: (hash=%v, pid=%d): %v", - linkError.FailureDetail, paymentHash, paymentID, + linkError.OutgoingFailure, paymentHash, paymentID, err) return linkError @@ -939,10 +939,10 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, case isResolution && htlc.Reason == nil: linkError := NewDetailedLinkError( &lnwire.FailPermanentChannelFailure{}, - FailureDetailOnChainTimeout, + OutgoingFailureOnChainTimeout, ) - log.Info("%v: hash=%v, pid=%d", linkError.FailureDetail, + log.Info("%v: hash=%v, pid=%d", linkError.OutgoingFailure, paymentHash, paymentID) return linkError @@ -1041,7 +1041,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { if !link.EligibleToForward() { failure = NewDetailedLinkError( &lnwire.FailUnknownNextPeer{}, - FailureDetailLinkNotEligible, + OutgoingFailureLinkNotEligible, ) } else { // We'll ensure that the HTLC satisfies the @@ -1217,7 +1217,7 @@ func checkCircularForward(incoming, outgoing lnwire.ShortChannelID, // node, so we do not include a channel update. return NewDetailedLinkError( lnwire.NewTemporaryChannelFailure(nil), - FailureDetailCircularRoute, + OutgoingFailureCircularRoute, ) } diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 828ef232c..a098967b6 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -1348,7 +1348,7 @@ func TestCircularForwards(t *testing.T) { allowCircularPayment: false, expectedErr: NewDetailedLinkError( lnwire.NewTemporaryChannelFailure(nil), - FailureDetailCircularRoute, + OutgoingFailureCircularRoute, ), }, } @@ -1465,7 +1465,7 @@ func TestCheckCircularForward(t *testing.T) { outgoingLink: lnwire.NewShortChanIDFromInt(123), expectedErr: NewDetailedLinkError( lnwire.NewTemporaryChannelFailure(nil), - FailureDetailCircularRoute, + OutgoingFailureCircularRoute, ), }, } @@ -1527,7 +1527,7 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) { eligible1: true, failure1: NewDetailedLinkError( lnwire.NewTemporaryChannelFailure(nil), - FailureDetailInsufficientBalance, + OutgoingFailureInsufficientBalance, ), eligible2: true, failure2: NewLinkError( From 9390d3bbfd2db5706b8a383a06bacf51526f6ed2 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:16 +0200 Subject: [PATCH 4/6] htlcswitch: replace outgoing failure with interface Add a FailureDetail interface which allows us have different kinds of failures for link errors. This interface will be used to cover failures that occur when on invoice payment, because the errors have already been enumerated in the invoices package. --- htlcswitch/failure.go | 14 +++++++------- htlcswitch/failure_detail.go | 14 ++++++++++++-- htlcswitch/switch.go | 7 ++++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index c4ef73c5c..9734a2e02 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -34,8 +34,8 @@ type LinkError struct { // node. msg lnwire.FailureMessage - // OutgoingFailure enriches the wire error with additional information. - OutgoingFailure + // FailureDetail enriches the wire error with additional information. + FailureDetail } // NewLinkError returns a LinkError with the failure message provided. @@ -48,11 +48,11 @@ func NewLinkError(msg lnwire.FailureMessage) *LinkError { // NewDetailedLinkError returns a link error that enriches a wire message with // a failure detail. func NewDetailedLinkError(msg lnwire.FailureMessage, - detail OutgoingFailure) *LinkError { + detail FailureDetail) *LinkError { return &LinkError{ - msg: msg, - OutgoingFailure: detail, + msg: msg, + FailureDetail: detail, } } @@ -72,11 +72,11 @@ func (l *LinkError) WireMessage() lnwire.FailureMessage { func (l *LinkError) Error() string { // If the link error has no failure detail, return the wire message's // error. - if l.OutgoingFailure == OutgoingFailureNone { + if l.FailureDetail == nil { return l.msg.Error() } - return fmt.Sprintf("%v: %v", l.msg.Error(), l.OutgoingFailure) + return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail) } // ForwardingError wraps an lnwire.FailureMessage in a struct that also diff --git a/htlcswitch/failure_detail.go b/htlcswitch/failure_detail.go index 95f43f118..9c068cebf 100644 --- a/htlcswitch/failure_detail.go +++ b/htlcswitch/failure_detail.go @@ -1,5 +1,13 @@ package htlcswitch +// FailureDetail is an interface implemented by failures that occur on +// our incoming or outgoing link, or within the switch itself. +type FailureDetail interface { + // FailureString returns the string representation of a failure + // detail. + FailureString() string +} + // OutgoingFailure is an enum which is used to enrich failures which occur in // the switch or on our outgoing link with additional metadata. type OutgoingFailure int @@ -36,8 +44,10 @@ const ( OutgoingFailureCircularRoute ) -// String returns the string representation of a failure detail. -func (fd OutgoingFailure) String() string { +// FailureString returns the string representation of a failure detail. +// +// Note: it is part of the FailureDetail interface. +func (fd OutgoingFailure) FailureString() string { switch fd { case OutgoingFailureNone: return "no failure detail" diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 5db9dac55..7ca56223a 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -923,8 +923,8 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, ) log.Errorf("%v: (hash=%v, pid=%d): %v", - linkError.OutgoingFailure, paymentHash, paymentID, - err) + linkError.FailureDetail.FailureString(), + paymentHash, paymentID, err) return linkError } @@ -942,7 +942,8 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter, OutgoingFailureOnChainTimeout, ) - log.Info("%v: hash=%v, pid=%d", linkError.OutgoingFailure, + log.Info("%v: hash=%v, pid=%d", + linkError.FailureDetail.FailureString(), paymentHash, paymentID) return linkError From 74e0d545fe83c3acea95d1eaea4c232d4c5b0951 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:17 +0200 Subject: [PATCH 5/6] htlcswitch: add linkError field to htlcpacket This commit adds a linkError field to track the value of failures which occur at our node. This field is set when local payments or multi hop htlcs fail in the switch or on our outgoing link. This addition is required for the addition of a htlc notifier which will notify these failures in handleDownstreamPacket. The passing of link error to failAddPacket removes the need for an additional error field, because the link error's failure detail will contain any additional metadata. In the places where the failure detail does not cover all the metadata that was previously supplied by addr err, the error is logged before calling failAddPacket so that this change does not reduce the amount of information we log. --- htlcswitch/failure.go | 2 +- htlcswitch/failure_detail.go | 21 ++++++++++++ htlcswitch/link.go | 32 +++++++++++++++--- htlcswitch/packet.go | 5 +++ htlcswitch/switch.go | 65 +++++++++++++++++++----------------- htlcswitch/switch_test.go | 21 ++++++++---- 6 files changed, 102 insertions(+), 44 deletions(-) diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 9734a2e02..373263381 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -76,7 +76,7 @@ func (l *LinkError) Error() string { return l.msg.Error() } - return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail) + return l.FailureDetail.FailureString() } // ForwardingError wraps an lnwire.FailureMessage in a struct that also diff --git a/htlcswitch/failure_detail.go b/htlcswitch/failure_detail.go index 9c068cebf..341688d12 100644 --- a/htlcswitch/failure_detail.go +++ b/htlcswitch/failure_detail.go @@ -42,6 +42,18 @@ const ( // to forward a htlc through our node which arrives and leaves on the // same channel. OutgoingFailureCircularRoute + + // OutgoingFailureIncompleteForward is returned when we cancel an incomplete + // forward. + OutgoingFailureIncompleteForward + + // OutgoingFailureDownstreamHtlcAdd is returned when we fail to add a + // downstream htlc to our outgoing link. + OutgoingFailureDownstreamHtlcAdd + + // OutgoingFailureForwardsDisabled is returned when the switch is + // configured to disallow forwards. + OutgoingFailureForwardsDisabled ) // FailureString returns the string representation of a failure detail. @@ -70,6 +82,15 @@ func (fd OutgoingFailure) FailureString() string { case OutgoingFailureCircularRoute: return "same incoming and outgoing channel" + case OutgoingFailureIncompleteForward: + return "failed after detecting incomplete forward" + + case OutgoingFailureDownstreamHtlcAdd: + return "could not add downstream htlc" + + case OutgoingFailureForwardsDisabled: + return "node configured to disallow forwards" + default: return "unknown failure detail" } diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 94773f2cf..484d0e07e 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1314,6 +1314,10 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { reason lnwire.OpaqueReason ) + // Create a temporary channel failure which we + // will send back to our peer if this is a + // forward, or report to the user if the failed + // payment was locally initiated. failure := l.createFailureWithUpdate( func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage { return lnwire.NewTemporaryChannelFailure( @@ -1322,28 +1326,43 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { }, ) - // Encrypt the error back to the source unless - // the payment was generated locally. + // If the payment was locally initiated (which + // is indicated by a nil obfuscator), we do + // not need to encrypt it back to the sender. if pkt.obfuscator == nil { var b bytes.Buffer err := lnwire.EncodeFailure(&b, failure, 0) if err != nil { - l.log.Errorf("unable to encode failure: %v", err) + l.log.Errorf("unable to "+ + "encode failure: %v", err) l.mailBox.AckPacket(pkt.inKey()) return } reason = lnwire.OpaqueReason(b.Bytes()) localFailure = true } else { + // If the packet is part of a forward, + // (identified by a non-nil obfuscator) + // we need to encrypt the error back to + // the source. var err error reason, err = pkt.obfuscator.EncryptFirstHop(failure) if err != nil { - l.log.Errorf("unable to obfuscate error: %v", err) + l.log.Errorf("unable to "+ + "obfuscate error: %v", err) l.mailBox.AckPacket(pkt.inKey()) return } } + // Create a link error containing the temporary + // channel failure and a detail which indicates + // the we failed to add the htlc. + linkError := NewDetailedLinkError( + failure, + OutgoingFailureDownstreamHtlcAdd, + ) + failPkt := &htlcPacket{ incomingChanID: pkt.incomingChanID, incomingHTLCID: pkt.incomingHTLCID, @@ -1351,6 +1370,7 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { sourceRef: pkt.sourceRef, hasSource: true, localFailure: localFailure, + linkFailure: linkError, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -2461,7 +2481,9 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg, } // Fetch the reason the HTLC was canceled so we can - // continue to propagate it. + // continue to propagate it. This failure originated + // from another node, so the linkFailure field is not + // set on the packet. failPacket := &htlcPacket{ outgoingChanID: l.ShortChanID(), outgoingHTLCID: pd.ParentIndex, diff --git a/htlcswitch/packet.go b/htlcswitch/packet.go index 3e0816e6e..e0aa75275 100644 --- a/htlcswitch/packet.go +++ b/htlcswitch/packet.go @@ -54,6 +54,11 @@ type htlcPacket struct { // encrypted with any shared secret. localFailure bool + // linkFailure is non-nil for htlcs that fail at our node. This may + // occur for our own payments which fail on the outgoing link, + // or for forwards which fail in the switch or on the outgoing link. + linkFailure *LinkError + // convertedError is set to true if this is an HTLC fail that was // created using an UpdateFailMalformedHTLC from the remote party. If // this is true, then when forwarding this failure packet, we'll need diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 7ca56223a..f4370635d 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -45,11 +45,6 @@ var ( // through the switch and is locked into another commitment txn. ErrDuplicateAdd = errors.New("duplicate add HTLC detected") - // ErrIncompleteForward is used when an htlc was already forwarded - // through the switch, but did not get locked into another commitment - // txn. - ErrIncompleteForward = errors.New("incomplete forward detected") - // ErrUnknownErrorDecryptor signals that we were unable to locate the // error decryptor for this payment. This is likely due to restarting // the daemon. @@ -496,9 +491,12 @@ func (s *Switch) forward(packet *htlcPacket) error { } else { failure = lnwire.NewTemporaryChannelFailure(update) } - addErr := ErrIncompleteForward - return s.failAddPacket(packet, failure, addErr) + linkError := NewDetailedLinkError( + failure, OutgoingFailureIncompleteForward, + ) + + return s.failAddPacket(packet, linkError) } packet.circuit = circuit @@ -647,14 +645,14 @@ func (s *Switch) ForwardPackets(linkQuit chan struct{}, } else { failure = lnwire.NewTemporaryChannelFailure(update) } + linkError := NewDetailedLinkError( + failure, OutgoingFailureIncompleteForward, + ) for _, packet := range failedPackets { - addErr := errors.New("failing packet after " + - "detecting incomplete forward") - // We don't handle the error here since this method // always returns an error. - s.failAddPacket(packet, failure, addErr) + _ = s.failAddPacket(packet, linkError) } } @@ -979,10 +977,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // Check if the node is set to reject all onward HTLCs and also make // sure that HTLC is not from the source node. if s.cfg.RejectHTLC && packet.incomingChanID != hop.Source { - failure := &lnwire.FailChannelDisabled{} - addErr := fmt.Errorf("unable to forward any htlcs") + failure := NewDetailedLinkError( + &lnwire.FailChannelDisabled{}, + OutgoingFailureForwardsDisabled, + ) - return s.failAddPacket(packet, failure, addErr) + return s.failAddPacket(packet, failure) } if packet.incomingChanID == hop.Source { @@ -1002,9 +1002,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { s.cfg.AllowCircularRoute, htlc.PaymentHash, ) if linkErr != nil { - return s.failAddPacket( - packet, linkErr.WireMessage(), linkErr, - ) + return s.failAddPacket(packet, linkErr) } s.indexMtx.RLock() @@ -1012,14 +1010,17 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { if err != nil { s.indexMtx.RUnlock() + log.Debugf("unable to find link with "+ + "destination %v", packet.outgoingChanID) + // If packet was forwarded from another channel link // than we should notify this link that some error // occurred. - failure := &lnwire.FailUnknownNextPeer{} - addErr := fmt.Errorf("unable to find link with "+ - "destination %v", packet.outgoingChanID) + linkError := NewLinkError( + &lnwire.FailUnknownNextPeer{}, + ) - return s.failAddPacket(packet, failure, addErr) + return s.failAddPacket(packet, linkError) } targetPeerKey := targetLink.Peer().PubKey() interfaceLinks, _ := s.getLinks(targetPeerKey) @@ -1088,12 +1089,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { })) } - addErr := fmt.Errorf("incoming HTLC(%x) violated "+ + log.Tracef("incoming HTLC(%x) violated "+ "target outgoing link (id=%v) policy: %v", htlc.PaymentHash[:], packet.outgoingChanID, linkErr) - return s.failAddPacket(packet, linkErr.WireMessage(), addErr) + return s.failAddPacket(packet, linkErr) } // Send the packet to the destination channel link which @@ -1225,13 +1226,11 @@ func checkCircularForward(incoming, outgoing lnwire.ShortChannelID, // failAddPacket encrypts a fail packet back to an add packet's source. // The ciphertext will be derived from the failure message proivded by context. // This method returns the failErr if all other steps complete successfully. -func (s *Switch) failAddPacket(packet *htlcPacket, - failure lnwire.FailureMessage, failErr error) error { - +func (s *Switch) failAddPacket(packet *htlcPacket, failure *LinkError) error { // Encrypt the failure so that the sender will be able to read the error // message. Since we failed this packet, we use EncryptFirstHop to // obfuscate the failure for their eyes only. - reason, err := packet.obfuscator.EncryptFirstHop(failure) + reason, err := packet.obfuscator.EncryptFirstHop(failure.WireMessage()) if err != nil { err := fmt.Errorf("unable to obfuscate "+ "error: %v", err) @@ -1239,13 +1238,14 @@ func (s *Switch) failAddPacket(packet *htlcPacket, return err } - log.Error(failErr) + log.Error(failure.Error()) failPkt := &htlcPacket{ sourceRef: packet.sourceRef, incomingChanID: packet.incomingChanID, incomingHTLCID: packet.incomingHTLCID, circuit: packet.circuit, + linkFailure: failure, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -1261,7 +1261,7 @@ func (s *Switch) failAddPacket(packet *htlcPacket, return err } - return failErr + return failure } // closeCircuit accepts a settle or fail htlc and the associated htlc packet and @@ -1869,8 +1869,11 @@ func (s *Switch) reforwardSettleFails(fwdPkgs []*channeldb.FwdPkg) { // commitment state, so we'll forward this to the switch so the // backwards undo can continue. case lnwallet.Fail: - // Fetch the reason the HTLC was canceled so we can - // continue to propagate it. + // Fetch the reason the HTLC was canceled so + // we can continue to propagate it. This + // failure originated from another node, so + // the linkFailure field is not set on this + // packet. failPacket := &htlcPacket{ outgoingChanID: fwdPkg.Source, outgoingHTLCID: pd.ParentIndex, diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index a098967b6..adc196499 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -211,10 +211,13 @@ func TestSwitchSendPending(t *testing.T) { // Send the ADD packet, this should not be forwarded out to the link // since there are no eligible links. err = s.forward(packet) - expErr := fmt.Sprintf("unable to find link with destination %v", - aliceChanID) - if err != nil && err.Error() != expErr { - t.Fatalf("expected forward failure: %v", err) + linkErr, ok := err.(*LinkError) + if !ok { + t.Fatalf("expected link error, got: %T", err) + } + if linkErr.WireMessage().Code() != lnwire.CodeUnknownNextPeer { + t.Fatalf("expected fail unknown next peer, got: %T", + linkErr.WireMessage().Code()) } // No message should be sent, since the packet was failed. @@ -1070,9 +1073,13 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) { // Resend the failed htlc, it should be returned to alice since the // switch will detect that it has been half added previously. err = s2.forward(ogPacket) - if err != ErrIncompleteForward { - t.Fatal("unexpected error when reforwarding a "+ - "failed packet", err) + linkErr, ok := err.(*LinkError) + if !ok { + t.Fatalf("expected link error, got: %T", err) + } + if linkErr.FailureDetail != OutgoingFailureIncompleteForward { + t.Fatalf("expected incomplete forward, got: %v", + linkErr.FailureDetail) } // After detecting an incomplete forward, the fail packet should have From 1ad395ec3ff8293f516279490433d60d541988ee Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 6 Feb 2020 19:35:17 +0200 Subject: [PATCH 6/6] htlcswitch: add failure details to incoming failures This commit adds LinkErrors with failure details to htlcs which fail on our incoming link. This change is made with the intention of notifying detailed htlc failure reasons in sendHTLCError. The FailureDetail interface is implemented on FailureResolutionResults so that they can directly be used to enrich LinkErrors. sendHtlcError is updated to take a LinkError in preparation for the addition of a htlcnotifier which will notify the detail of the error. --- htlcswitch/link.go | 34 +++++++++++++++++++++++----------- invoices/resolution_result.go | 6 ++++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 484d0e07e..80b90f483 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1220,8 +1220,8 @@ func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution, failure := getResolutionFailure(res, htlc.pd.Amount) l.sendHTLCError( - htlc.pd.HtlcIndex, failure, htlc.obfuscator, - htlc.pd.SourceRef, + htlc.pd.HtlcIndex, failure, + htlc.obfuscator, htlc.pd.SourceRef, ) return nil @@ -1236,21 +1236,25 @@ func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution, // getResolutionFailure returns the wire message that a htlc resolution should // be failed with. func getResolutionFailure(resolution *invoices.HtlcFailResolution, - amount lnwire.MilliSatoshi) lnwire.FailureMessage { + amount lnwire.MilliSatoshi) *LinkError { // If the resolution has been resolved as part of a MPP timeout, // we need to fail the htlc with lnwire.FailMppTimeout. if resolution.Outcome == invoices.ResultMppTimeout { - return &lnwire.FailMPPTimeout{} + return NewDetailedLinkError( + &lnwire.FailMPPTimeout{}, resolution.Outcome, + ) } // If the htlc is not a MPP timeout, we fail it with // FailIncorrectDetails. This error is sent for invoice payment // failures such as underpayment/ expiry too soon and hodl invoices // (which return FailIncorrectDetails to avoid leaking information). - return lnwire.NewFailIncorrectDetails( + incorrectDetails := lnwire.NewFailIncorrectDetails( amount, uint32(resolution.AcceptHeight), ) + + return NewDetailedLinkError(incorrectDetails, resolution.Outcome) } // randomFeeUpdateTimeout returns a random timeout between the bounds defined @@ -2650,9 +2654,10 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg, // for TLV payloads that also supports injecting invalid // payloads. Deferring this non-trival effort till a // later date + failure := lnwire.NewInvalidOnionPayload(failedType, 0) l.sendHTLCError( pd.HtlcIndex, - lnwire.NewInvalidOnionPayload(failedType, 0), + NewLinkError(failure), obfuscator, pd.SourceRef, ) @@ -2766,7 +2771,10 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg, ) l.sendHTLCError( - pd.HtlcIndex, failure, obfuscator, pd.SourceRef, + pd.HtlcIndex, + NewLinkError(failure), + obfuscator, + pd.SourceRef, ) continue } @@ -2849,7 +2857,9 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, "value: expected %v, got %v", pd.RHash, pd.Amount, fwdInfo.AmountToForward) - failure := lnwire.NewFinalIncorrectHtlcAmount(pd.Amount) + failure := NewLinkError( + lnwire.NewFinalIncorrectHtlcAmount(pd.Amount), + ) l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef) return nil @@ -2862,7 +2872,9 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, "time-lock: expected %v, got %v", pd.RHash[:], pd.Timeout, fwdInfo.OutgoingCTLV) - failure := lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout) + failure := NewLinkError( + lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout), + ) l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef) return nil @@ -2979,10 +2991,10 @@ func (l *channelLink) handleBatchFwdErrs(errChan chan error) { // sendHTLCError functions cancels HTLC and send cancel message back to the // peer from which HTLC was received. -func (l *channelLink) sendHTLCError(htlcIndex uint64, failure lnwire.FailureMessage, +func (l *channelLink) sendHTLCError(htlcIndex uint64, failure *LinkError, e hop.ErrorEncrypter, sourceRef *channeldb.AddRef) { - reason, err := e.EncryptFirstHop(failure) + reason, err := e.EncryptFirstHop(failure.WireMessage()) if err != nil { l.log.Errorf("unable to obfuscate error: %v", err) return diff --git a/invoices/resolution_result.go b/invoices/resolution_result.go index 3952d11a5..41da902a6 100644 --- a/invoices/resolution_result.go +++ b/invoices/resolution_result.go @@ -107,8 +107,10 @@ const ( ResultMppInProgress ) -// String returns a string representation of the result. -func (f FailResolutionResult) String() string { +// FailureString returns a string representation of the result. +// +// Note: it is part of the FailureDetail interface. +func (f FailResolutionResult) FailureString() string { switch f { case resultInvalidFailure: return "invalid failure result"