invoices: add subscribesingleinvoice #2356

joostjager · 2018-12-20T11:11:03Z

This PR allows listening to a notification stream for a single invoice.

Usage

SubscribeSingleInvoice is only available for applications integrating directly with the lnd invoices sub server grpc interface. It is not accessible through lncli.

After registering for single invoice notifications, the caller will immediately receive the current invoice state via the stream. From there onwards, lnd will continue pushing updates as they happen.

If the invoice that is subscribed to doesn't exist yet, the call will still block and start streaming when the invoice is created.

Build

Requires lnd to be built with the invoicesrpc build tag:

make tags="invoicesrpc"

lnrpc/invoicesrpc/config_default.go

lnrpc/invoicesrpc/invoices_server.go

lnrpc/invoicesrpc/log.go

invoices/invoiceregistry.go

lnrpc/utils.go

lnrpc/invoicesrpc/config_active.go

cfromknecht · 2019-01-10T23:17:25Z

lnrpc/invoicesrpc/invoices_server.go

+
+	// Now that we know the full path of the invoices macaroon, we can
+	// check to see if we need to create it or not.
+	if !fileExists(macFilePath) && cfg.MacService != nil {


preferably this logic would also be made into a helper method in lnrpc as well. would you be opposed to doing so and then we can defer a refactoring of other subservers to a later PR?

Gave it a try, but it gets many parameters. I think it may be better to, in a separate pr, have subservers expose some macaroon info and bake the macaroon somewhere else.

sounds good, thanks!

cmd/lncli/invoicesrpc_default.go

cmd/lncli/invoicesrpc_active.go

joostjager · 2019-01-11T09:57:06Z

@cfromknecht PTAL

cfromknecht

Thanks @joostjager, few last nits. Otherwise i'd say this about ready to land! 🎉

invoices/invoiceregistry.go

lnrpc/invoicesrpc/invoices_server.go

cfromknecht · 2019-01-12T03:34:33Z

lnrpc/invoicesrpc/invoices_server.go

+//
+// NOTE: This is part of the lnrpc.SubServer interface.
+func (s *Server) Stop() error {
+	close(s.quit)


i think it'd be worth protecting this with a compare and swap to prevent us from accidentally closing the same channel twice, as it would lead us to panic during shutdown. Start should be fine though since it's a NOP

Calling Stop() twice is invalid use of the subserver and adding the compare/swap would camouflage this. We would loose the opportunity to discover the bug of the caller. In general I'd like to avoid defensive code, unless we are dealing with legacy that has become too complicate to reason about and we want to take no risk.

lntypes/hash.go

lntypes/preimage.go

invoices/invoiceregistry.go

invoices/invoiceregistry_test.go

joostjager · 2019-01-27T19:17:29Z

Leaktest removed. CLI command removed.

PTAL @halseth @Roasbeef @cfromknecht

Roasbeef

Needs a go mod tidy, but other than that, LGTM 🐣

go.sum

joostjager · 2019-01-29T10:20:09Z

Stale review dismissed, ptal

Roasbeef

LGTM 🌶

cfromknecht

LGTM 📈

invoices/invoiceregistry_test.go

This commit adds new hash and preimage types. These types are similar to chainhash.Hash, except for that string representations are not reversed. The reason for adding dedicated types and not use [32]byte, is to facilitate logging (%v displays as hex string) and have standard methods to convert from byte slice and string with a length check.

Previously chainhash.Hash was used, which converts to/from string in reversed format. Payment hashes and preimages are supposed to be non-reversed.

Sub server implementation is still empty. This is a preparatory step for adding invoice functionality.

halseth · 2019-02-01T08:42:43Z

Needs a rebase!

Without this addition, sub servers cannot import google annotations in their proto files.

As a preparation for subscribing to single invoices, InvoiceRegistry needs to become aware of settling a settled invoice.

As a preparation for reusing the marshall code in the invoices sub server.

joostjager · 2019-02-01T09:54:33Z

@halseth @cfromknecht @Roasbeef

Rebased, ptal

Roasbeef

LGTM 🥘

joostjager force-pushed the invoices-subserver branch 5 times, most recently from ff5e889 to 6225ced Compare December 20, 2018 22:42

Roasbeef added database Related to the database/storage of LND payments Related to invoices/payments incomplete WIP PR, not fully finalized, but light review possible enhancement Improvements to existing features / behaviour labels Dec 21, 2018

joostjager force-pushed the invoices-subserver branch 4 times, most recently from 6154c5e to 6fe9e32 Compare December 27, 2018 14:52

joostjager force-pushed the invoices-subserver branch 3 times, most recently from 3340c88 to 3eedbcf Compare January 3, 2019 18:34

joostjager changed the title ~~invoices: add subscribesingleinvoice [no review]~~ invoices: add subscribesingleinvoice Jan 3, 2019

joostjager force-pushed the invoices-subserver branch 4 times, most recently from 44baf90 to dedf7fe Compare January 8, 2019 10:38

Roasbeef requested a review from cfromknecht January 9, 2019 02:16

cfromknecht reviewed Jan 10, 2019

View reviewed changes

joostjager force-pushed the invoices-subserver branch 3 times, most recently from 522ad59 to 970984a Compare January 11, 2019 09:56

joostjager mentioned this pull request Jan 11, 2019

invoices: CancelInvoice #2457

Merged

cfromknecht reviewed Jan 12, 2019

View reviewed changes

joostjager force-pushed the invoices-subserver branch from 970984a to 07b561e Compare January 14, 2019 09:10

halseth reviewed Jan 25, 2019

View reviewed changes

joostjager force-pushed the invoices-subserver branch from df424fe to 0e291b2 Compare January 27, 2019 19:09

Roasbeef previously approved these changes Jan 29, 2019

View reviewed changes

go.sum Outdated Show resolved Hide resolved

joostjager dismissed Roasbeef’s stale review via 5c73328 January 29, 2019 09:35

joostjager force-pushed the invoices-subserver branch from 0e291b2 to 5c73328 Compare January 29, 2019 09:35

joostjager force-pushed the invoices-subserver branch 2 times, most recently from 0f2ab5b to b640b48 Compare January 30, 2019 07:46

Roasbeef previously approved these changes Jan 30, 2019

View reviewed changes

cfromknecht previously approved these changes Feb 1, 2019

View reviewed changes

invoices/invoiceregistry_test.go Outdated Show resolved Hide resolved

joostjager added 4 commits February 1, 2019 09:42

invoices: use lntypes.Hash and lntypes.Preimage

bacd924

Previously chainhash.Hash was used, which converts to/from string in reversed format. Payment hashes and preimages are supposed to be non-reversed.

invoicesrpc: create sub server

3545685

Sub server implementation is still empty. This is a preparatory step for adding invoice functionality.

invoiceregistry: extract dispatch to method

78cd075

joostjager added 5 commits February 1, 2019 09:42

lnrpc: add search path to gen_protos.sh

2a4c93c

Without this addition, sub servers cannot import google annotations in their proto files.

channeldb: move idempotency up the call tree

436dd41

As a preparation for subscribing to single invoices, InvoiceRegistry needs to become aware of settling a settled invoice.

invoices: subscribe single invoice

acb0162

lnrpc: move invoice marshall code to package

4c4536a

As a preparation for reusing the marshall code in the invoices sub server.

invoicesrpc: add SubscribeSingleInvoice rpc

70c874b

joostjager dismissed stale reviews from cfromknecht and Roasbeef via c049173 February 1, 2019 08:46

joostjager force-pushed the invoices-subserver branch from b640b48 to c049173 Compare February 1, 2019 08:46

joostjager added 2 commits February 1, 2019 09:47

lnrpc: reuse common FileExists function

8996a14

invoices: add SubscribeSingleInvoice test

b163571

joostjager force-pushed the invoices-subserver branch from c049173 to b163571 Compare February 1, 2019 08:47

Roasbeef approved these changes Feb 2, 2019

View reviewed changes

Roasbeef merged commit 4af857f into lightningnetwork:master Feb 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

invoices: add subscribesingleinvoice #2356

invoices: add subscribesingleinvoice #2356

joostjager commented Dec 20, 2018 •

edited

Loading

cfromknecht Jan 10, 2019

joostjager Jan 11, 2019

cfromknecht Jan 14, 2019

joostjager commented Jan 11, 2019

cfromknecht left a comment

cfromknecht Jan 12, 2019

joostjager Jan 14, 2019

joostjager commented Jan 27, 2019

Roasbeef left a comment

joostjager commented Jan 29, 2019

Roasbeef left a comment

cfromknecht left a comment

halseth commented Feb 1, 2019

joostjager commented Feb 1, 2019

Roasbeef left a comment

invoices: add subscribesingleinvoice #2356

invoices: add subscribesingleinvoice #2356

Conversation

joostjager commented Dec 20, 2018 • edited Loading

Usage

Build

cfromknecht Jan 10, 2019

Choose a reason for hiding this comment

joostjager Jan 11, 2019

Choose a reason for hiding this comment

cfromknecht Jan 14, 2019

Choose a reason for hiding this comment

joostjager commented Jan 11, 2019

cfromknecht left a comment

Choose a reason for hiding this comment

cfromknecht Jan 12, 2019

Choose a reason for hiding this comment

joostjager Jan 14, 2019

Choose a reason for hiding this comment

joostjager commented Jan 27, 2019

Roasbeef left a comment

Choose a reason for hiding this comment

joostjager commented Jan 29, 2019

Roasbeef left a comment

Choose a reason for hiding this comment

cfromknecht left a comment

Choose a reason for hiding this comment

halseth commented Feb 1, 2019

joostjager commented Feb 1, 2019

Roasbeef left a comment

Choose a reason for hiding this comment

joostjager commented Dec 20, 2018 •

edited

Loading