multi: use separate cltv expiry field from invoice

Now that the Invoice struct contains the decoded final cltv delta value,
the decoding of payment requests can be removed from the invoice
registry.
This commit is contained in:
Joost Jager 2019-08-20 14:54:39 +02:00
parent 4105142c96
commit c1345a4117
No known key found for this signature in database
GPG Key ID: A61B9D4C393C59C7
12 changed files with 34 additions and 83 deletions

@ -1348,7 +1348,7 @@ func (c *ChannelArbitrator) isPreimageAvailable(hash lntypes.Hash) (bool,
// than the invoice cltv delta. We don't want to go to chain only to // than the invoice cltv delta. We don't want to go to chain only to
// have the incoming contest resolver decide that we don't want to // have the incoming contest resolver decide that we don't want to
// settle this invoice. // settle this invoice.
invoice, _, err := c.cfg.Registry.LookupInvoice(hash) invoice, err := c.cfg.Registry.LookupInvoice(hash)
switch err { switch err {
case nil: case nil:
case channeldb.ErrInvoiceNotFound, channeldb.ErrNoInvoicesCreated: case channeldb.ErrInvoiceNotFound, channeldb.ErrNoInvoicesCreated:

@ -10,10 +10,8 @@ import (
// Registry is an interface which represents the invoice registry. // Registry is an interface which represents the invoice registry.
type Registry interface { type Registry interface {
// LookupInvoice attempts to look up an invoice according to its 32 // LookupInvoice attempts to look up an invoice according to its 32
// byte payment hash. This method should also reutrn the min final CLTV // byte payment hash.
// delta for this invoice. We'll use this to ensure that the HTLC LookupInvoice(lntypes.Hash) (channeldb.Invoice, error)
// extended to us gives us enough time to settle as we prescribe.
LookupInvoice(lntypes.Hash) (channeldb.Invoice, uint32, error)
// NotifyExitHopHtlc attempts to mark an invoice as settled. If the // NotifyExitHopHtlc attempts to mark an invoice as settled. If the
// invoice is a debug invoice, then this method is a noop as debug // invoice is a debug invoice, then this method is a noop as debug

@ -39,8 +39,8 @@ func (r *mockRegistry) NotifyExitHopHtlc(payHash lntypes.Hash,
func (r *mockRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) {} func (r *mockRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) {}
func (r *mockRegistry) LookupInvoice(lntypes.Hash) (channeldb.Invoice, uint32, func (r *mockRegistry) LookupInvoice(lntypes.Hash) (channeldb.Invoice,
error) { error) {
return channeldb.Invoice{}, 0, channeldb.ErrInvoiceNotFound return channeldb.Invoice{}, channeldb.ErrInvoiceNotFound
} }

@ -14,10 +14,8 @@ import (
// which may search, lookup and settle invoices. // which may search, lookup and settle invoices.
type InvoiceDatabase interface { type InvoiceDatabase interface {
// LookupInvoice attempts to look up an invoice according to its 32 // LookupInvoice attempts to look up an invoice according to its 32
// byte payment hash. This method should also reutrn the min final CLTV // byte payment hash.
// delta for this invoice. We'll use this to ensure that the HTLC LookupInvoice(lntypes.Hash) (channeldb.Invoice, error)
// extended to us gives us enough time to settle as we prescribe.
LookupInvoice(lntypes.Hash) (channeldb.Invoice, uint32, error)
// NotifyExitHopHtlc attempts to mark an invoice as settled. If the // NotifyExitHopHtlc attempts to mark an invoice as settled. If the
// invoice is a debug invoice, then this method is a noop as debug // invoice is a debug invoice, then this method is a noop as debug

@ -238,7 +238,7 @@ func TestChannelLinkSingleHopPayment(t *testing.T) {
// Check that alice invoice was settled and bandwidth of HTLC // Check that alice invoice was settled and bandwidth of HTLC
// links was changed. // links was changed.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -498,7 +498,7 @@ func testChannelLinkMultiHopPayment(t *testing.T,
// Check that Carol invoice was settled and bandwidth of HTLC // Check that Carol invoice was settled and bandwidth of HTLC
// links were changed. // links were changed.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -912,7 +912,7 @@ func TestUpdateForwardingPolicy(t *testing.T) {
// Carol's invoice should now be shown as settled as the payment // Carol's invoice should now be shown as settled as the payment
// succeeded. // succeeded.
invoice, _, err := n.carolServer.registry.LookupInvoice(payResp) invoice, err := n.carolServer.registry.LookupInvoice(payResp)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -1030,7 +1030,7 @@ func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) {
// Check that alice invoice wasn't settled and bandwidth of htlc // Check that alice invoice wasn't settled and bandwidth of htlc
// links hasn't been changed. // links hasn't been changed.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -1215,7 +1215,7 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) {
// Check that alice invoice wasn't settled and bandwidth of htlc // Check that alice invoice wasn't settled and bandwidth of htlc
// links hasn't been changed. // links hasn't been changed.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -1330,7 +1330,7 @@ func TestChannelLinkMultiHopDecodeError(t *testing.T) {
// Check that alice invoice wasn't settled and bandwidth of htlc // Check that alice invoice wasn't settled and bandwidth of htlc
// links hasn't been changed. // links hasn't been changed.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }
@ -3455,7 +3455,7 @@ func TestChannelRetransmission(t *testing.T) {
// Check that alice invoice wasn't settled and // Check that alice invoice wasn't settled and
// bandwidth of htlc links hasn't been changed. // bandwidth of htlc links hasn't been changed.
invoice, _, err = receiver.registry.LookupInvoice(rhash) invoice, err = receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
err = errors.Errorf("unable to get invoice: %v", err) err = errors.Errorf("unable to get invoice: %v", err)
continue continue
@ -3974,7 +3974,7 @@ func TestChannelLinkAcceptOverpay(t *testing.T) {
// Even though we sent 2x what was asked for, Carol should still have // Even though we sent 2x what was asked for, Carol should still have
// accepted the payment and marked it as settled. // accepted the payment and marked it as settled.
invoice, _, err := receiver.registry.LookupInvoice(rhash) invoice, err := receiver.registry.LookupInvoice(rhash)
if err != nil { if err != nil {
t.Fatalf("unable to get invoice: %v", err) t.Fatalf("unable to get invoice: %v", err)
} }

@ -768,13 +768,9 @@ func newMockRegistry(minDelta uint32) *mockInvoiceRegistry {
panic(err) panic(err)
} }
decodeExpiry := func(invoice string) (uint32, error) {
return testInvoiceCltvExpiry, nil
}
finalCltvRejectDelta := int32(5) finalCltvRejectDelta := int32(5)
registry := invoices.NewRegistry(cdb, decodeExpiry, finalCltvRejectDelta) registry := invoices.NewRegistry(cdb, finalCltvRejectDelta)
registry.Start() registry.Start()
return &mockInvoiceRegistry{ return &mockInvoiceRegistry{
@ -783,7 +779,9 @@ func newMockRegistry(minDelta uint32) *mockInvoiceRegistry {
} }
} }
func (i *mockInvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice, uint32, error) { func (i *mockInvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (
channeldb.Invoice, error) {
return i.registry.LookupInvoice(rHash) return i.registry.LookupInvoice(rHash)
} }

@ -557,6 +557,7 @@ func generatePaymentWithPreimage(invoiceAmt, htlcAmt lnwire.MilliSatoshi,
Value: invoiceAmt, Value: invoiceAmt,
PaymentPreimage: preimage, PaymentPreimage: preimage,
}, },
FinalCltvDelta: testInvoiceCltvExpiry,
} }
htlc := &lnwire.UpdateAddHTLC{ htlc := &lnwire.UpdateAddHTLC{

@ -58,10 +58,6 @@ type InvoiceRegistry struct {
// new single invoice subscriptions are carried. // new single invoice subscriptions are carried.
invoiceEvents chan interface{} invoiceEvents chan interface{}
// decodeFinalCltvExpiry is a function used to decode the final expiry
// value from the payment request.
decodeFinalCltvExpiry func(invoice string) (uint32, error)
// subscriptions is a map from a payment hash to a list of subscribers. // subscriptions is a map from a payment hash to a list of subscribers.
// It is used for efficient notification of links. // It is used for efficient notification of links.
hodlSubscriptions map[lntypes.Hash]map[chan<- interface{}]struct{} hodlSubscriptions map[lntypes.Hash]map[chan<- interface{}]struct{}
@ -85,8 +81,7 @@ type InvoiceRegistry struct {
// wraps the persistent on-disk invoice storage with an additional in-memory // wraps the persistent on-disk invoice storage with an additional in-memory
// layer. The in-memory layer is in place such that debug invoices can be added // layer. The in-memory layer is in place such that debug invoices can be added
// which are volatile yet available system wide within the daemon. // which are volatile yet available system wide within the daemon.
func NewRegistry(cdb *channeldb.DB, decodeFinalCltvExpiry func(invoice string) ( func NewRegistry(cdb *channeldb.DB, finalCltvRejectDelta int32) *InvoiceRegistry {
uint32, error), finalCltvRejectDelta int32) *InvoiceRegistry {
return &InvoiceRegistry{ return &InvoiceRegistry{
cdb: cdb, cdb: cdb,
@ -97,7 +92,6 @@ func NewRegistry(cdb *channeldb.DB, decodeFinalCltvExpiry func(invoice string) (
invoiceEvents: make(chan interface{}, 100), invoiceEvents: make(chan interface{}, 100),
hodlSubscriptions: make(map[lntypes.Hash]map[chan<- interface{}]struct{}), hodlSubscriptions: make(map[lntypes.Hash]map[chan<- interface{}]struct{}),
hodlReverseSubscriptions: make(map[chan<- interface{}]map[lntypes.Hash]struct{}), hodlReverseSubscriptions: make(map[chan<- interface{}]map[lntypes.Hash]struct{}),
decodeFinalCltvExpiry: decodeFinalCltvExpiry,
finalCltvRejectDelta: finalCltvRejectDelta, finalCltvRejectDelta: finalCltvRejectDelta,
quit: make(chan struct{}), quit: make(chan struct{}),
} }
@ -404,26 +398,15 @@ func (i *InvoiceRegistry) AddInvoice(invoice *channeldb.Invoice,
} }
// LookupInvoice looks up an invoice by its payment hash (R-Hash), if found // LookupInvoice looks up an invoice by its payment hash (R-Hash), if found
// then we're able to pull the funds pending within an HTLC. We'll also return // then we're able to pull the funds pending within an HTLC.
// what the expected min final CLTV delta is, pre-parsed from the payment
// request. This may be used by callers to determine if an HTLC is well formed
// according to the cltv delta.
// //
// TODO(roasbeef): ignore if settled? // TODO(roasbeef): ignore if settled?
func (i *InvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice, uint32, error) { func (i *InvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice,
error) {
// We'll check the database to see if there's an existing matching // We'll check the database to see if there's an existing matching
// invoice. // invoice.
invoice, err := i.cdb.LookupInvoice(rHash) return i.cdb.LookupInvoice(rHash)
if err != nil {
return channeldb.Invoice{}, 0, err
}
expiry, err := i.decodeFinalCltvExpiry(string(invoice.PaymentRequest))
if err != nil {
return channeldb.Invoice{}, 0, err
}
return invoice, expiry, nil
} }
// checkHtlcParameters is a callback used inside invoice db transactions to // checkHtlcParameters is a callback used inside invoice db transactions to
@ -454,17 +437,11 @@ func (i *InvoiceRegistry) checkHtlcParameters(invoice *channeldb.Invoice,
return channeldb.ErrInvoiceAlreadySettled return channeldb.ErrInvoiceAlreadySettled
} }
// The invoice is still open. Check the expiry.
expiry, err := i.decodeFinalCltvExpiry(string(invoice.PaymentRequest))
if err != nil {
return err
}
if htlcExpiry < uint32(currentHeight+i.finalCltvRejectDelta) { if htlcExpiry < uint32(currentHeight+i.finalCltvRejectDelta) {
return ErrInvoiceExpiryTooSoon return ErrInvoiceExpiryTooSoon
} }
if htlcExpiry < uint32(currentHeight)+expiry { if htlcExpiry < uint32(currentHeight+invoice.FinalCltvDelta) {
return ErrInvoiceExpiryTooSoon return ErrInvoiceExpiryTooSoon
} }

@ -30,10 +30,6 @@ var (
testCurrentHeight = int32(1) testCurrentHeight = int32(1)
) )
func decodeExpiry(payReq string) (uint32, error) {
return uint32(testInvoiceCltvDelta), nil
}
var ( var (
testInvoice = &channeldb.Invoice{ testInvoice = &channeldb.Invoice{
Terms: channeldb.ContractTerm{ Terms: channeldb.ContractTerm{
@ -50,7 +46,7 @@ func newTestContext(t *testing.T) (*InvoiceRegistry, func()) {
} }
// Instantiate and start the invoice registry. // Instantiate and start the invoice registry.
registry := NewRegistry(cdb, decodeExpiry, testFinalCltvRejectDelta) registry := NewRegistry(cdb, testFinalCltvRejectDelta)
err = registry.Start() err = registry.Start()
if err != nil { if err != nil {
@ -204,7 +200,7 @@ func TestSettleInvoice(t *testing.T) {
} }
// Check that settled amount remains unchanged. // Check that settled amount remains unchanged.
inv, _, err := registry.LookupInvoice(hash) inv, err := registry.LookupInvoice(hash)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -337,7 +333,7 @@ func TestHoldInvoice(t *testing.T) {
defer cleanup() defer cleanup()
// Instantiate and start the invoice registry. // Instantiate and start the invoice registry.
registry := NewRegistry(cdb, decodeExpiry, testFinalCltvRejectDelta) registry := NewRegistry(cdb, testFinalCltvRejectDelta)
err = registry.Start() err = registry.Start()
if err != nil { if err != nil {

@ -36,13 +36,6 @@ func CreateRPCInvoice(invoice *channeldb.Invoice,
settleDate = invoice.SettleDate.Unix() settleDate = invoice.SettleDate.Unix()
} }
// Expiry time will default to 3600 seconds if not specified
// explicitly.
expiry := int64(decoded.Expiry().Seconds())
// The expiry will default to 9 blocks if not specified explicitly.
cltvExpiry := decoded.MinFinalCLTVExpiry()
// Convert between the `lnrpc` and `routing` types. // Convert between the `lnrpc` and `routing` types.
routeHints := CreateRPCRouteHints(decoded.RouteHints) routeHints := CreateRPCRouteHints(decoded.RouteHints)
@ -77,8 +70,8 @@ func CreateRPCInvoice(invoice *channeldb.Invoice,
Settled: isSettled, Settled: isSettled,
PaymentRequest: paymentRequest, PaymentRequest: paymentRequest,
DescriptionHash: descHash, DescriptionHash: descHash,
Expiry: expiry, Expiry: int64(invoice.Expiry.Seconds()),
CltvExpiry: cltvExpiry, CltvExpiry: uint64(invoice.FinalCltvDelta),
FallbackAddr: fallbackAddr, FallbackAddr: fallbackAddr,
RouteHints: routeHints, RouteHints: routeHints,
AddIndex: invoice.AddIndex, AddIndex: invoice.AddIndex,

@ -3493,7 +3493,7 @@ func (r *rpcServer) LookupInvoice(ctx context.Context,
rpcsLog.Tracef("[lookupinvoice] searching for invoice %x", payHash[:]) rpcsLog.Tracef("[lookupinvoice] searching for invoice %x", payHash[:])
invoice, _, err := r.server.invoices.LookupInvoice(payHash) invoice, err := r.server.invoices.LookupInvoice(payHash)
if err != nil { if err != nil {
return nil, err return nil, err
} }

@ -55,7 +55,6 @@ import (
"github.com/lightningnetwork/lnd/watchtower/wtclient" "github.com/lightningnetwork/lnd/watchtower/wtclient"
"github.com/lightningnetwork/lnd/watchtower/wtdb" "github.com/lightningnetwork/lnd/watchtower/wtdb"
"github.com/lightningnetwork/lnd/watchtower/wtpolicy" "github.com/lightningnetwork/lnd/watchtower/wtpolicy"
"github.com/lightningnetwork/lnd/zpay32"
) )
const ( const (
@ -347,14 +346,6 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB,
readBufferPool, cfg.Workers.Read, pool.DefaultWorkerTimeout, readBufferPool, cfg.Workers.Read, pool.DefaultWorkerTimeout,
) )
decodeFinalCltvExpiry := func(payReq string) (uint32, error) {
invoice, err := zpay32.Decode(payReq, activeNetParams.Params)
if err != nil {
return 0, err
}
return uint32(invoice.MinFinalCLTVExpiry()), nil
}
s := &server{ s := &server{
chanDB: chanDB, chanDB: chanDB,
cc: cc, cc: cc,
@ -364,8 +355,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB,
chansToRestore: chansToRestore, chansToRestore: chansToRestore,
invoices: invoices.NewRegistry( invoices: invoices.NewRegistry(
chanDB, decodeFinalCltvExpiry, chanDB, defaultFinalCltvRejectDelta,
defaultFinalCltvRejectDelta,
), ),
channelNotifier: channelnotifier.New(chanDB), channelNotifier: channelnotifier.New(chanDB),