lnd: only set payment address if not empty in PaymentRequest

[bjarnemagnussen]

Background

This PR addresses an issue where not defining a payment address with the SendPaymentRequest results in a payment_incorrect_details error for:

• keysend payments, and
• in LND versions <=0.11 when defining the payment directly using the pubkey in Dest and PaymentHash instead of using an invoice with PaymentRequest.

The error occurs as not defining a payment address with the SendPaymentRequest results in an empty payment address byte array in the paymentIntent instead of a nil-value.

fixes #5413

[Roasbeef]

Solid fix! IMO the only thing missing here is updating the itests (testSingleHopInvoice) to account for this change. Looking at the existing test case for keysend we have I don't see how this wasn't already covered, given the request is identical to the proto sample you posted in the original issue.

[Roasbeef]

Ahh, so this ended up copying a zero valued payment address, which was rejected as it's used as a shibboleth elsewhere in the codebase.

[Roasbeef]

I don't see how this wasn't already covered, given the request is identical to the proto sample you posted in the original issue.

Ah I think the difference here is all our itests use SendPaymentV2 and not the deprecated variant on the main Lightning service.

[bjarnemagnussen]

Right, I was also wondering why the itest hadn't picked this up but just as you mention it is not tested for this SendPayment (probably due to deprecation?).

I should have probably made it more clear in the PR!

[Roasbeef]

NP, it all makes sense now, and also explains why we didn't catch this when testing on the CLI as the CLI uses all the v2 calls. Given that we worked to move all the integration tests over to using the v2 APIs, I think this can land as is.

[Roasbeef]

LGTM 🛰

Ran the itests locally, it's hitting some failures due to a regression (after 0.13) that #5429 should fix.