Merge pull request #182 from LN-Zap/payment-description

Include outgoing payment description in audit memo

Author: guggero

accounting/entries.go

@@ -401,11 +401,18 @@ func paymentReference(sequenceNumber uint64, preimage lntypes.Preimage) string {
 
 // paymentNote creates a note for payments from our node.
 // nolint: interfacer
-func paymentNote(dest *route.Vertex) string {
-	if dest == nil {
-		return ""
+func paymentNote(dest *route.Vertex, memo *string) string {
+	var notes []string
+
+	if memo != nil && *memo != "" {
+		notes = append(notes, fmt.Sprintf("memo: %v", *memo))
 	}
-	return dest.String()
+
+	if dest != nil {
+		notes = append(notes, fmt.Sprintf("destination: %v", dest))
+	}
+
+	return strings.Join(notes, "/")
 }
 
 // paymentEntry creates an entry for an off chain payment, including fee entries
@@ -432,7 +439,7 @@ func paymentEntry(payment paymentInfo, paidToSelf bool,
 
 	// Create a note for our payment. Since we have already checked that our
 	// payment is settled, we will not have a nil preimage.
-	note := paymentNote(payment.destination)
+	note := paymentNote(payment.destination, payment.description)
 	ref := paymentReference(payment.SequenceNumber, *payment.Preimage)
 
 	// Payment values are expressed as positive values over rpc, but they

accounting/entries_test.go

@@ -83,10 +83,6 @@ var (
 		Tx:        &wire.MsgTx{},
 	}
 
-	paymentRequest = "lnbcrt10n1p0t6nmypp547evsfyrakg0nmyw59ud9cegkt99yccn5nnp4suq3ac4qyzzgevsdqqcqzpgsp54hvffpajcyddm20k3ptu53930425hpnv8m06nh5jrd6qhq53anrq9qy9qsqphhzyenspf7kfwvm3wyu04fa8cjkmvndyexlnrmh52huwa4tntppjmak703gfln76rvswmsx2cz3utsypzfx40dltesy8nj64ttgemgqtwfnj9"
-
-	invoiceMemo = "memo"
-
 	invoiceAmt = lnwire.MilliSatoshi(300)
 
 	invoiceOverpaidAmt = lnwire.MilliSatoshi(400)
@@ -112,7 +108,7 @@ var (
 
 	paymentTime = time.Unix(1590399649, 0)
 
-	paymentHash = "11f414479f0a0c2762492c71c58dded5dce99d56d65c3fa523f73513605bebb3"
+	paymentHash = "0001020304050607080900010203040506070809000102030405060708090102"
 	pmtHash, _  = lntypes.MakeHashFromStr(paymentHash)
 
 	paymentPreimage = "adfef20b24152accd4ed9a05257fb77203d90a8bbbe6d4069a75c5320f0538d9"
@@ -134,11 +130,13 @@ var (
 		Fee:            lnwire.MilliSatoshi(paymentFeeMsat),
 		Htlcs:          []*lnrpc.HTLCAttempt{{}},
 		SequenceNumber: uint64(paymentIndex),
+		PaymentRequest: paymentRequest,
 	}
 
 	payInfo = paymentInfo{
 		Payment:     payment,
 		destination: &otherPubkey,
+		description: &invoiceMemo,
 		settleTime:  paymentTime,
 	}
 
@@ -758,7 +756,7 @@ func TestPaymentEntry(t *testing.T) {
 			FiatValue: fiat.MsatToFiat(mockBTCPrice.Price, amtMsat),
 			TxID:      paymentHash,
 			Reference: paymentRef,
-			Note:      paymentNote(&otherPubkey),
+			Note:      paymentNote(&otherPubkey, &invoiceMemo),
 			Type:      EntryTypePayment,
 			OnChain:   false,
 			Credit:    false,
@@ -773,7 +771,7 @@ func TestPaymentEntry(t *testing.T) {
 			FiatValue: fiat.MsatToFiat(mockBTCPrice.Price, feeMsat),
 			TxID:      paymentHash,
 			Reference: FeeReference(paymentRef),
-			Note:      paymentNote(&otherPubkey),
+			Note:      paymentNote(&otherPubkey, &invoiceMemo),
 			Type:      EntryTypeFee,
 			OnChain:   false,
 			Credit:    false,

accounting/filter.go

@@ -109,15 +109,16 @@ func filterInvoices(startTime, endTime time.Time,
 	return filtered
 }
 
-// paymentInfo wraps a lndclient payment struct with a destination, if it is
-// available from the information we have available, and its settle time.
-// Since we now allow multi-path payments, a single payment may have multiple
-// htlcs resolved over a period of time. We use the most recent settle time for
-// payment because payments are not considered settled until all the htlcs are
-// resolved.
+// paymentInfo wraps a lndclient payment struct with a destination, and
+// description if available from the information we have available, and its
+// settle time. Since we now allow multi-path payments, a single payment may
+// have multiple htlcs resolved over a period of time. We use the most recent
+// settle time for payment because payments are not considered settled until
+// all the htlcs are resolved.
 type paymentInfo struct {
 	lndclient.Payment
 	destination *route.Vertex
+	description *string
 	settleTime  time.Time
 }
 
@@ -135,24 +136,31 @@ func preProcessPayments(payments []lndclient.Payment,
 	paymentList := make([]paymentInfo, len(payments))
 
 	for i, payment := range payments {
-		// Try to get our payment destination from our set of htlcs.
-		// If we cannot get it from our htlcs (which is the case for
-		// legacy payments that did not store htlcs), we try to get it
-		// from our payment request. This value may not be present for
-		// all payments, so we do not error if it is not.
+		// Attempt to obtain the payment destination and description
+		// from our payment request. If this is not possible (which
+		// can be the case for legacy payments that did not store
+		// payment requests, or payments that pay directly to a
+		// payment hash), then try to get it from our HTLCs. Note
+		// that HTLCs may also not be available for legacy payments
+		// that did not store HTLCs. In the event that we get a
+		// destination from both sources, we prefer the destination
+		// from the HTLCs.
+		payReqDestination, description, err := paymentRequestDetails(
+			payment.PaymentRequest, decode,
+		)
+		if err != nil && err != errNoPaymentRequest {
+			return nil, err
+		}
+
 		destination, err := paymentHtlcDestination(payment)
 		if err != nil {
-			destination, err = paymentRequestDestination(
-				payment.PaymentRequest, decode,
-			)
-			if err != nil && err != errNoPaymentRequest {
-				return nil, err
-			}
+			destination = payReqDestination
 		}
 
 		pmt := paymentInfo{
 			Payment:     payment,
 			destination: destination,
+			description: description,
 		}
 
 		// If the payment did not succeed, we can add it to our list
@@ -212,21 +220,23 @@ func paymentHtlcDestination(payment lndclient.Payment) (*route.Vertex, error) {
 	return &lastHopPubkey, nil
 }
 
-// paymentRequestDestination attempts to decode a payment address, and returns
-// the destination.
-func paymentRequestDestination(paymentRequest string,
-	decode decodePaymentRequest) (*route.Vertex, error) {
+// paymentRequestDetails attempts to decode a payment address, and returns
+// the destination and the description.
+func paymentRequestDetails(paymentRequest string,
+	decode decodePaymentRequest) (*route.Vertex, *string, error) {
 
 	if paymentRequest == "" {
-		return nil, errNoPaymentRequest
+		return nil, nil, errNoPaymentRequest
 	}
 
 	payReq, err := decode(paymentRequest)
 	if err != nil {
-		return nil, fmt.Errorf("decode payment request failed: %w", err)
+		return nil, nil, fmt.Errorf(
+			"decode payment request failed: %w", err,
+		)
 	}
 
-	return &payReq.Destination, nil
+	return &payReq.Destination, &payReq.Description, nil
 }
 
 // filterPayments filters out unsuccessful payments and those which did not

accounting/filter_test.go

@@ -378,6 +378,7 @@ func decode(toSelf bool) func(_ string) (*lndclient.PaymentRequest,
 
 		return &lndclient.PaymentRequest{
 			Destination: pubkey,
+			Description: invoiceMemo,
 		}, nil
 	}
 }
@@ -437,35 +438,39 @@ func TestPaymentHtlcDestination(t *testing.T) {
 	}
 }
 
-// TestPaymentRequestDestination tests getting of payment destinations from our
+// TestPaymentRequestDestination tests getting of payment details from our
 // payment request.
-func TestPaymentRequestDestination(t *testing.T) {
+func TestPaymentRequestDetails(t *testing.T) {
 	tests := []struct {
 		name           string
 		paymentRequest string
 		decode         decodePaymentRequest
-		dest           *route.Vertex
+		destination    *route.Vertex
+		description    *string
 		err            error
 	}{
 		{
 			name:           "no payment request",
 			decode:         decode(true),
 			paymentRequest: "",
-			dest:           nil,
+			destination:    nil,
+			description:    nil,
 			err:            errNoPaymentRequest,
 		},
 		{
 			name:           "to self",
 			decode:         decode(true),
 			paymentRequest: paymentRequest,
-			dest:           &ourPubKey,
+			destination:    &ourPubKey,
+			description:    &invoiceMemo,
 			err:            nil,
 		},
 		{
 			name:           "not to self",
 			decode:         decode(false),
 			paymentRequest: paymentRequest,
-			dest:           &otherPubkey,
+			destination:    &otherPubkey,
+			description:    &invoiceMemo,
 			err:            nil,
 		},
 	}
@@ -476,11 +481,13 @@ func TestPaymentRequestDestination(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
 
-			dest, err := paymentRequestDestination(
+			destination, description, err := paymentRequestDetails(
 				test.paymentRequest, test.decode,
 			)
+
 			require.Equal(t, test.err, err)
-			require.Equal(t, test.dest, dest)
+			require.Equal(t, test.destination, destination)
+			require.Equal(t, test.description, description)
 		})
 	}
 }

accounting/off_chain_test.go

@@ -24,6 +24,9 @@ var (
 	paymentHash2 = "a5530c5930b9eb7ea4284bcff39da52c6bca3103fc790749eb632911edc7143b"
 	hash2, _     = lntypes.MakeHashFromStr(paymentHash2)
 
+	paymentRequest = "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpuaztrnwngzn3kdzw5hydlzf03qdgm2hdq27cqv3agm2awhz5se903vruatfhq77w3ls4evs3ch9zw97j25emudupq63nyw24cg27h2rspfj9srp"
+	invoiceMemo    = "1 cup coffee"
+
 	hopToUs = &lnrpc.Hop{
 		PubKey: ourPK,
 	}