Check quote expiry in outgoing payment worker

[mkurapov]

Context

Currently, we check the expiresAt of the quote during outgoing payment creation. If it is valid, we create the outgoing payment, store it in the DB.

Once the payment is funded, the outgoing payments worker starts processing the payment, whether that is local or over ILP. If the payment fails, it will stay in the SENDING state and the worker will keep retrying it (up to some maximum number of times).

In the worker, however, we do not check the expiry of the quote. The quote could expire between the time when we created the payment, and when it is being processed (or retried) in the worker. We should prevent processing an outgoing payment for an expired quote. (good spot would be in lifecycle.ts > handleSending)

Todos

• Prevent outgoing payment being processed for expired quotes (mark it as FAILED). Make sure the outgoing_payment.failed webhook is correctly sent.

[CollinsMunene]

Hi @mkurapov , I have made a change where if the quote expiresAt time is the same as the current time, then it handles the payment failure. Is this the desired effect?

[mkurapov]

@CollinsMunene yes 👍

and to confirm, outgoing payment should not be retried.

[CollinsMunene]

@mkurapov that's noted, so here is the implementation attached,
The payment is only retried if the payment state is "SENDING", so by throwing the PaymentFailed event, it changes the payment state to FAILED as well as sends the outgoing_payment.failed webhook.

Kindly confirm if this is okay.
The implementation on(lifecycle.ts > handleSending function):

The Webhook:

The outgoing payment body after:

[CollinsMunene]

Hi @mkurapov, I also noticed on the webhooks, can we also add an error description. For example, when the outgoing_payment.failed is raised when the quote expiry time has reached. The webhook does not really say why it has failed. It would be nice to see a reason like "quote has expired"

or even in any other instance of failure, makes it easy to debug. So same thing as the payment, we can have metadata for webhooks

[mkurapov]

Hi @CollinsMunene,

The webhook does not really say why it has failed. It would be nice to see a reason like "quote has expired"

Yes, agreed with this, would be much more helpful. I think by adding a new enum value to LifecycleError (something like QuoteExpired), and throw that when the quote expiry check happens, we should be able to see that in the webhook event data.

[CollinsMunene]

Hi @mkurapov , exactly my point. I have pushed this changes under PR (#3173)