Lnd: Invoices can be paid after expiration

[NicolasDorier]

On version v0.7.1.

We got report of the possibility of paying an LND invoice after it got expired, in violation with BOLT11.

A payee after the timestamp plus expiry has passed SHOULD NOT accept a payment

This cause significant problem for merchant because:

1. BTCPay won't register the payment. This happen because BTCPay invoice is expired and the server stop listening LND.
2. Even if BTCPay would register the payment, it is painful for the merchant as he will need to manually contact the customer for reimbursing him.

There exist a workaround via hodl invoices, but implementing it put the burden of accepting an incoming payment on the shoulder of the LND client.

Because BTCPay is using an abstraction layer for lightning implementation and that hodl invoices can't fit into such abstraction, we can't really use this alternative.

[junderw]

ahh nvm you mean that LND will receive payments via expired payment requests.

Yeah, it does seem like LND will not send to an expired payment request, but it will receive a payment through a payment request that it sets as expired.

IMO, this is a bug of the sending side. If everyone refuses to send, then no problem.

Rejecting based on the receiving end means you will have to roll back an HTLC which is really bad and can cause issues.

So TLDR:

1. Receiving side should do sanity check.
2. Sending side MUST refuse to send when payment request is expired. Since there are many problems that can arise from rejecting incoming HTLCs in large numbers.

[NicolasDorier]

BOLT11:

Payer / Payee Requirements
A payer:

after the timestamp plus expiry has passed:
SHOULD NOT attempt a payment.
otherwise:
if a Lightning payment fails:
MAY attempt to use the address given in the first f field that it understands for payment.
MAY use the sequence of channels, specified by the r field, to route to the payee.
SHOULD consider the fee amount and payment timeout before initiating payment.
SHOULD use the first p field that it did NOT skip as the payment hash.
A payee:

after the timestamp plus expiry has passed:
SHOULD NOT accept a payment.

* after the timestamp plus expiry has passed SHOULD NOT accept a payment. *

Eclair wallet acknowledged the issue (ACINQ/eclair-mobile#209) and fixed it for next release. @jackeveritt tested other implementations, those reject payment correctly.

[junderw]

According to RFC2119: https://www.ipa.go.jp/security/rfc/RFC2119EN.html

4. SHOULD NOT   This phrase, or the phrase "NOT RECOMMENDED" mean that
   there may exist valid reasons in particular circumstances when the
   particular behavior is acceptable or even useful, but the full
   implications should be understood and the case carefully weighed
   before implementing any behavior described with this label.

[junderw]

That being said, I think LND should reject incoming payment to requests they generated with an expiry if that expiry is passed.

[junderw]

In fact, this is pretty much necessary for exchanges to support lightning using LND.

If a user uses a wallet that sends to expired requests, and we accept it even though it's required... then we need to add extra logic to double-check the invoice just in case forever. Which is not realistic.

[bhandras]

This is fixed in PR #3694 and waiting for review.

[Roasbeef]

Fixed by #3694.