From 05e6b62cb20af56c06544e4702fef3e804214c2c Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 8 Aug 2019 15:48:31 +0200 Subject: [PATCH] cnct+htlcswitch+invoices: report circuit key to invoice registry Currently the invoice registry cannot tell apart the htlcs that pay to an invoice. Because htlcs may also be replayed on startup, it isn't possible to determine the total amount paid to an invoice. This commit is a first step towards fixing that. It reports the circuit keys of htlcs to the invoice registry, which forms the basis for accurate invoice accounting. --- contractcourt/channel_arbitrator.go | 28 ++++++++++-- .../htlc_incoming_contest_resolver.go | 5 ++- contractcourt/interfaces.go | 2 +- contractcourt/mock_registry_test.go | 3 +- htlcswitch/interfaces.go | 2 +- htlcswitch/link.go | 7 ++- htlcswitch/mock.go | 5 ++- invoices/invoiceregistry.go | 7 +-- invoices/invoiceregistry_test.go | 45 ++++++++++++------- 9 files changed, 76 insertions(+), 28 deletions(-) diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 34d75b336..e05a5863b 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -498,9 +498,7 @@ func (c *ChannelArbitrator) supplementResolver(resolver ContractResolver, return c.supplementSuccessResolver(r, htlcMap) case *htlcIncomingContestResolver: - return c.supplementSuccessResolver( - &r.htlcSuccessResolver, htlcMap, - ) + return c.supplementIncomingContestResolver(r, htlcMap) case *htlcTimeoutResolver: return c.supplementTimeoutResolver(r, htlcMap) @@ -514,6 +512,30 @@ func (c *ChannelArbitrator) supplementResolver(resolver ContractResolver, return nil } +// supplementSuccessResolver takes a htlcIncomingContestResolver as it is +// restored from the log and fills in missing data from the htlcMap. +func (c *ChannelArbitrator) supplementIncomingContestResolver( + r *htlcIncomingContestResolver, + htlcMap map[wire.OutPoint]*channeldb.HTLC) error { + + res := r.htlcResolution + htlcPoint := res.HtlcPoint() + htlc, ok := htlcMap[htlcPoint] + if !ok { + return errors.New( + "htlc for incoming contest resolver unavailable", + ) + } + + r.htlcAmt = htlc.Amt + r.circuitKey = channeldb.CircuitKey{ + ChanID: c.cfg.ShortChanID, + HtlcID: htlc.HtlcIndex, + } + + return nil +} + // supplementSuccessResolver takes a htlcSuccessResolver as it is restored from // the log and fills in missing data from the htlcMap. func (c *ChannelArbitrator) supplementSuccessResolver(r *htlcSuccessResolver, diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 54bccab52..bda680987 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/contractcourt/htlc_incoming_contest_resolver.go @@ -27,6 +27,9 @@ type htlcIncomingContestResolver struct { // successfully. htlcExpiry uint32 + // circuitKey describes the incoming htlc that is being resolved. + circuitKey channeldb.CircuitKey + // htlcSuccessResolver is the inner resolver that may be utilized if we // learn of the preimage. htlcSuccessResolver @@ -166,7 +169,7 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) { // identical to HTLC resolution in the link. event, err := h.Registry.NotifyExitHopHtlc( h.payHash, h.htlcAmt, h.htlcExpiry, currentHeight, - hodlChan, nil, + h.circuitKey, hodlChan, nil, ) switch err { case channeldb.ErrInvoiceNotFound: diff --git a/contractcourt/interfaces.go b/contractcourt/interfaces.go index 3333e6b6b..e8c22c97d 100644 --- a/contractcourt/interfaces.go +++ b/contractcourt/interfaces.go @@ -22,7 +22,7 @@ type Registry interface { // the resolution is sent on the passed in hodlChan later. NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, - hodlChan chan<- interface{}, + circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, eob []byte) (*invoices.HodlEvent, error) // HodlUnsubscribeAll unsubscribes from all hodl events. diff --git a/contractcourt/mock_registry_test.go b/contractcourt/mock_registry_test.go index 288ea5ba6..071651c67 100644 --- a/contractcourt/mock_registry_test.go +++ b/contractcourt/mock_registry_test.go @@ -23,7 +23,8 @@ type mockRegistry struct { func (r *mockRegistry) NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, - hodlChan chan<- interface{}, eob []byte) (*invoices.HodlEvent, error) { + circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, + eob []byte) (*invoices.HodlEvent, error) { r.notifyChan <- notifyExitHopData{ hodlChan: hodlChan, diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 52a4c194b..379ef674b 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -28,7 +28,7 @@ type InvoiceDatabase interface { // for decoding purposes. NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32, - hodlChan chan<- interface{}, + circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, eob []byte) (*invoices.HodlEvent, error) // CancelInvoice attempts to cancel the invoice corresponding to the diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 7d14b7af6..3b4d10920 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2878,9 +2878,14 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, // receive back a resolution event. invoiceHash := lntypes.Hash(pd.RHash) + circuitKey := channeldb.CircuitKey{ + ChanID: l.ShortChanID(), + HtlcID: pd.HtlcIndex, + } + event, err := l.cfg.Registry.NotifyExitHopHtlc( invoiceHash, pd.Amount, pd.Timeout, int32(heightNow), - l.hodlQueue.ChanIn(), eob, + circuitKey, l.hodlQueue.ChanIn(), eob, ) switch err { diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 073e825dd..46be963b9 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -793,10 +793,11 @@ func (i *mockInvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error func (i *mockInvoiceRegistry) NotifyExitHopHtlc(rhash lntypes.Hash, amt lnwire.MilliSatoshi, expiry uint32, currentHeight int32, - hodlChan chan<- interface{}, eob []byte) (*invoices.HodlEvent, error) { + circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, + eob []byte) (*invoices.HodlEvent, error) { event, err := i.registry.NotifyExitHopHtlc( - rhash, amt, expiry, currentHeight, hodlChan, eob, + rhash, amt, expiry, currentHeight, circuitKey, hodlChan, eob, ) if err != nil { return nil, err diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 2cbba30eb..d1c3da0ca 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -484,14 +484,15 @@ func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice, // prevent deadlock. func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, amtPaid lnwire.MilliSatoshi, expiry uint32, currentHeight int32, - hodlChan chan<- interface{}, eob []byte) (*HodlEvent, error) { + circuitKey channeldb.CircuitKey, hodlChan chan<- interface{}, + eob []byte) (*HodlEvent, error) { i.Lock() defer i.Unlock() debugLog := func(s string) { - log.Debugf("Invoice(%x): %v, amt=%v, expiry=%v", - rHash[:], s, amtPaid, expiry) + log.Debugf("Invoice(%x): %v, amt=%v, expiry=%v, circuit=%v", + rHash[:], s, amtPaid, expiry, circuitKey) } // If this isn't a debug invoice, then we'll attempt to settle an diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index ed3ec0771..0394b8bfd 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -64,6 +64,15 @@ func newTestContext(t *testing.T) (*InvoiceRegistry, func()) { } } +func getCircuitKey(htlcID uint64) channeldb.CircuitKey { + return channeldb.CircuitKey{ + ChanID: lnwire.ShortChannelID{ + BlockHeight: 1, TxIndex: 2, TxPosition: 3, + }, + HtlcID: htlcID, + } +} + // TestSettleInvoice tests settling of an invoice and related notifications. func TestSettleInvoice(t *testing.T) { registry, cleanup := newTestContext(t) @@ -121,7 +130,8 @@ func TestSettleInvoice(t *testing.T) { // Settle invoice with a slightly higher amount. amtPaid := lnwire.MilliSatoshi(100500) _, err = registry.NotifyExitHopHtlc( - hash, amtPaid, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amtPaid, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatal(err) @@ -153,10 +163,11 @@ func TestSettleInvoice(t *testing.T) { t.Fatal("no update received") } - // Try to settle again. We need this idempotent behaviour after a - // restart. + // Try to settle again with the same htlc id. We need this idempotent + // behaviour after a restart. event, err := registry.NotifyExitHopHtlc( - hash, amtPaid, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amtPaid, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -165,12 +176,12 @@ func TestSettleInvoice(t *testing.T) { t.Fatal("expected settle event") } - // Try to settle again with a higher amount. This payment should also be - // accepted, to prevent any change in behaviour for a paid invoice that - // may open up a probe vector. + // Try to settle again with a new higher-valued htlc. This payment + // should also be accepted, to prevent any change in behaviour for a + // paid invoice that may open up a probe vector. event, err = registry.NotifyExitHopHtlc( hash, amtPaid+600, testHtlcExpiry, testCurrentHeight, - hodlChan, nil, + getCircuitKey(1), hodlChan, nil, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -183,7 +194,7 @@ func TestSettleInvoice(t *testing.T) { // would have failed if it were the first payment. event, err = registry.NotifyExitHopHtlc( hash, amtPaid-600, testHtlcExpiry, testCurrentHeight, - hodlChan, nil, + getCircuitKey(2), hodlChan, nil, ) if err != nil { t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err) @@ -306,7 +317,8 @@ func TestCancelInvoice(t *testing.T) { // succeed. hodlChan := make(chan interface{}) event, err := registry.NotifyExitHopHtlc( - hash, amt, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amt, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatal("expected settlement of a canceled invoice to succeed") @@ -382,7 +394,8 @@ func TestHoldInvoice(t *testing.T) { // NotifyExitHopHtlc without a preimage present in the invoice registry // should be possible. event, err := registry.NotifyExitHopHtlc( - hash, amtPaid, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amtPaid, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -393,7 +406,8 @@ func TestHoldInvoice(t *testing.T) { // Test idempotency. event, err = registry.NotifyExitHopHtlc( - hash, amtPaid, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amtPaid, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -406,7 +420,7 @@ func TestHoldInvoice(t *testing.T) { // is a replay. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, testHtlcExpiry, testCurrentHeight+10, - getCircuitKey(0), hodlChan, + getCircuitKey(0), hodlChan, nil, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -420,7 +434,7 @@ func TestHoldInvoice(t *testing.T) { // doesn't track individual htlcs it is accepted. event, err = registry.NotifyExitHopHtlc( hash, amtPaid, 1, testCurrentHeight, - getCircuitKey(1), hodlChan, + getCircuitKey(1), hodlChan, nil, ) if err != nil { t.Fatalf("expected settle to succeed but got %v", err) @@ -520,7 +534,8 @@ func TestUnknownInvoice(t *testing.T) { hodlChan := make(chan interface{}) amt := lnwire.MilliSatoshi(100000) _, err := registry.NotifyExitHopHtlc( - hash, amt, testHtlcExpiry, testCurrentHeight, hodlChan, nil, + hash, amt, testHtlcExpiry, testCurrentHeight, + getCircuitKey(0), hodlChan, nil, ) if err != channeldb.ErrInvoiceNotFound { t.Fatal("expected invoice not found error")