Invoice rpc metadata support #10316
Invoice rpc metadata support #10316
Conversation
This commit exposes the zpay32 metadata field through gRPC by adding a metadata field to the following proto messages: - Invoice (used by AddInvoice) - PayReq (returned by DecodePayReq) - AddHoldInvoiceRequest (used by AddHoldInvoice) The metadata field allows arbitrary data to be embedded in invoices and sent along with payments to the payee. This commit also includes the generated protobuf Go code for these new fields.
This commit implements metadata support for AddInvoice, AddHoldInvoice, and DecodePayReq by: - Adding Metadata field to AddInvoiceData struct - Passing metadata to zpay32.Invoice creation via zpay32.Metadata option - Wiring metadata from gRPC requests to AddInvoiceData in both AddInvoice and AddHoldInvoice handlers - Extracting and returning metadata in DecodePayReq responses The zpay32 package already supports encoding/decoding metadata in BOLT-11 invoices (field type 27), so this change simply exposes that functionality through the gRPC API.
This commit adds comprehensive integration tests to verify that metadata can be properly set and retrieved through AddInvoice, AddHoldInvoice, and DecodePayReq. The test cases cover: - Creating a regular invoice with metadata and verifying it encodes/decodes - Creating a hold invoice with metadata and verifying it encodes/decodes - Creating an invoice without metadata to ensure backwards compatibility
Summary of ChangesHello @sputn1ck, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the lnrpc system by integrating comprehensive support for invoice metadata. This feature allows users to embed arbitrary byte data directly into invoices when they are created, and subsequently retrieve this data when interacting with payment requests or querying invoice details. This provides a flexible and extensible mechanism for associating custom information with Lightning Network invoices, enabling a wider range of application-specific use cases. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for invoice metadata, allowing it to be added and fetched via lnrpc. The changes are well-implemented across the different layers of the RPC interface, and the new functionality is covered by a comprehensive integration test. My review includes one suggestion to refactor the new test for improved readability and maintainability.
| func testInvoiceMetadata(ht *lntest.HarnessTest) { | ||
| // Open a channel with 100k satoshis between Alice and Bob with Alice | ||
| // being the sole funder of the channel. | ||
| chanAmt := btcutil.Amount(100000) | ||
| chanPoints, nodes := ht.CreateSimpleNetwork( | ||
| [][]string{nil, nil}, lntest.OpenChannelParams{Amt: chanAmt}, | ||
| ) | ||
| cp := chanPoints[0] | ||
| alice, bob := nodes[0], nodes[1] | ||
|
|
||
| // assertAmountPaid is a helper closure that asserts the amount paid by | ||
| // Alice and received by Bob are expected. | ||
| assertAmountPaid := func(expected int64) { | ||
| ht.AssertAmountPaid("alice -> bob", alice, cp, expected, 0) | ||
| ht.AssertAmountPaid("bob <- alice", bob, cp, 0, expected) | ||
| } | ||
|
|
||
| const paymentAmt = 1000 | ||
|
|
||
| // Test 1: Create a regular invoice with metadata using AddInvoice. | ||
| testMetadata := []byte{0x01, 0xfa, 0xfa, 0xf0} | ||
| preimage := bytes.Repeat([]byte("B"), 32) | ||
| invoice := &lnrpc.Invoice{ | ||
| Memo: "metadata test", | ||
| RPreimage: preimage, | ||
| Value: paymentAmt, | ||
| Metadata: testMetadata, | ||
| } | ||
| invoiceResp := bob.RPC.AddInvoice(invoice) | ||
|
|
||
| // Decode the payment request and verify metadata is present. | ||
| payReq := bob.RPC.DecodePayReq(invoiceResp.PaymentRequest) | ||
| require.Equal(ht, testMetadata, payReq.Metadata, | ||
| "decoded metadata should match") | ||
|
|
||
| // Pay the invoice and verify it settles. | ||
| ht.CompletePaymentRequests(alice, []string{invoiceResp.PaymentRequest}) | ||
|
|
||
| // Verify the invoice is marked as settled and metadata persists in DB. | ||
| dbInvoice := bob.RPC.LookupInvoice(invoiceResp.RHash) | ||
| require.Equal(ht, lnrpc.Invoice_SETTLED, dbInvoice.State, | ||
| "bob's invoice should be marked as settled") | ||
| require.Equal(ht, testMetadata, dbInvoice.Metadata, | ||
| "metadata should persist when fetching invoice from DB") | ||
|
|
||
| // Verify balances are updated. | ||
| assertAmountPaid(paymentAmt) | ||
|
|
||
| // Test 2: Create a hold invoice with metadata using AddHoldInvoice. | ||
| testMetadata2 := []byte{0xde, 0xad, 0xbe, 0xef} | ||
| preimage2, err := lntypes.MakePreimage(bytes.Repeat([]byte("C"), 32)) | ||
| require.NoError(ht, err, "unable to make preimage") | ||
| hash := preimage2.Hash() | ||
|
|
||
| holdInvoice := &invoicesrpc.AddHoldInvoiceRequest{ | ||
| Memo: "hold invoice metadata test", | ||
| Hash: hash[:], | ||
| Value: paymentAmt, | ||
| Metadata: testMetadata2, | ||
| } | ||
| holdInvoiceResp := bob.RPC.AddHoldInvoice(holdInvoice) | ||
|
|
||
| // Decode the hold invoice payment request and verify metadata. | ||
| holdPayReq := bob.RPC.DecodePayReq(holdInvoiceResp.PaymentRequest) | ||
| require.Equal(ht, testMetadata2, holdPayReq.Metadata, | ||
| "decoded hold invoice metadata should match") | ||
|
|
||
| // Lookup the hold invoice from DB and verify metadata persists. | ||
| dbHoldInvoice := bob.RPC.LookupInvoice(hash[:]) | ||
| require.Equal(ht, testMetadata2, dbHoldInvoice.Metadata, | ||
| "hold invoice metadata should persist when fetching from DB") | ||
|
|
||
| // Test 3: Create an invoice without metadata and verify it's empty. | ||
| invoice3 := &lnrpc.Invoice{ | ||
| Memo: "no metadata", | ||
| Value: paymentAmt, | ||
| } | ||
| invoiceResp3 := bob.RPC.AddInvoice(invoice3) | ||
|
|
||
| // Decode and verify metadata is nil or empty. | ||
| payReq3 := bob.RPC.DecodePayReq(invoiceResp3.PaymentRequest) | ||
| require.Empty(ht, payReq3.Metadata, | ||
| "invoice without metadata should have empty metadata field") | ||
|
|
||
| // Lookup from DB and verify metadata remains empty. | ||
| dbInvoice3 := bob.RPC.LookupInvoice(invoiceResp3.RHash) | ||
| require.Empty(ht, dbInvoice3.Metadata, | ||
| "invoice without metadata should have empty metadata in DB") | ||
| } |
There was a problem hiding this comment.
This test function is quite long and contains multiple distinct test cases. For better readability, test isolation, and clearer test output, consider refactoring it to use sub-tests with ht.Run.1 This will make it easier to identify which specific part of the functionality fails if the test breaks in the future.
Here's an example of how it could be refactored:
func testInvoiceMetadata(ht *lntest.HarnessTest) {
// Open a channel with 100k satoshis between Alice and Bob with Alice
// being the sole funder of the channel.
chanAmt := btcutil.Amount(100000)
chanPoints, nodes := ht.CreateSimpleNetwork(
[][]string{nil, nil}, lntest.OpenChannelParams{Amt: chanAmt},
)
cp := chanPoints[0]
alice, bob := nodes[0], nodes[1]
// assertAmountPaid is a helper closure that asserts the amount paid by
// Alice and received by Bob are expected.
assertAmountPaid := func(expected int64) {
ht.AssertAmountPaid("alice -> bob", alice, cp, expected, 0)
ht.AssertAmountPaid("bob <- alice", bob, cp, 0, expected)
}
const paymentAmt = 1000
ht.Run("AddInvoice with metadata", func(st *lntest.HarnessTest) {
// Test 1: Create a regular invoice with metadata using AddInvoice.
testMetadata := []byte{0x01, 0xfa, 0xfa, 0xf0}
preimage := bytes.Repeat([]byte("B"), 32)
invoice := &lnrpc.Invoice{
Memo: "metadata test",
RPreimage: preimage,
Value: paymentAmt,
Metadata: testMetadata,
}
invoiceResp := bob.RPC.AddInvoice(invoice)
// Decode the payment request and verify metadata is present.
payReq := bob.RPC.DecodePayReq(invoiceResp.PaymentRequest)
require.Equal(st, testMetadata, payReq.Metadata,
"decoded metadata should match")
// Pay the invoice and verify it settles.
st.CompletePaymentRequests(alice, []string{invoiceResp.PaymentRequest})
// Verify the invoice is marked as settled and metadata persists in DB.
dbInvoice := bob.RPC.LookupInvoice(invoiceResp.RHash)
require.Equal(st, lnrpc.Invoice_SETTLED, dbInvoice.State,
"bob's invoice should be marked as settled")
require.Equal(st, testMetadata, dbInvoice.Metadata,
"metadata should persist when fetching invoice from DB")
// Verify balances are updated.
assertAmountPaid(paymentAmt)
})
ht.Run("AddHoldInvoice with metadata", func(st *lntest.HarnessTest) {
// Test 2: Create a hold invoice with metadata using AddHoldInvoice.
testMetadata2 := []byte{0xde, 0xad, 0xbe, 0xef}
preimage2, err := lntypes.MakePreimage(bytes.Repeat([]byte("C"), 32))
require.NoError(st, err, "unable to make preimage")
hash := preimage2.Hash()
holdInvoice := &invoicesrpc.AddHoldInvoiceRequest{
Memo: "hold invoice metadata test",
Hash: hash[:],
Value: paymentAmt,
Metadata: testMetadata2,
}
holdInvoiceResp := bob.RPC.AddHoldInvoice(holdInvoice)
// Decode the hold invoice payment request and verify metadata.
holdPayReq := bob.RPC.DecodePayReq(holdInvoiceResp.PaymentRequest)
require.Equal(st, testMetadata2, holdPayReq.Metadata,
"decoded hold invoice metadata should match")
// Lookup the hold invoice from DB and verify metadata persists.
dbHoldInvoice := bob.RPC.LookupInvoice(hash[:])
require.Equal(st, testMetadata2, dbHoldInvoice.Metadata,
"hold invoice metadata should persist when fetching from DB")
})
ht.Run("AddInvoice without metadata", func(st *lntest.HarnessTest) {
// Test 3: Create an invoice without metadata and verify it's empty.
invoice3 := &lnrpc.Invoice{
Memo: "no metadata",
Value: paymentAmt,
}
invoiceResp3 := bob.RPC.AddInvoice(invoice3)
// Decode and verify metadata is nil or empty.
payReq3 := bob.RPC.DecodePayReq(invoiceResp3.PaymentRequest)
require.Empty(st, payReq3.Metadata,
"invoice without metadata should have empty metadata field")
// Lookup from DB and verify metadata remains empty.
dbInvoice3 := bob.RPC.LookupInvoice(invoiceResp3.RHash)
require.Empty(st, dbInvoice3.Metadata,
"invoice without metadata should have empty metadata in DB")
})
}Style Guide References
Footnotes
|
Seems like the most straight-forward way to do it, just wanted to point out that now DB changes are needed because we pack the metadata into the invoice request which is saved as a blob in the SQL case. |
Do you think db changes are neccessary? The metadata is encoded in the payreq, thus is able to be fetched by the lookupinvoice call (as seen in the itest). |
2 participants
Change Description
This PR adds support for adding and fetching invoice metadata via lnrpc.