Add webhook event for a created incoming payment

[AlexLakatos]

When an incoming payment is created, the AP should decide if that's accepted or rejected, and return that status to the client making the incoming payment.

The Open Payments Spec needs to be changed in order to be able to handle this new behaviour.

[wilsonianb]

It seems like this would resemble the call to the account provider that happens during quote creation which is distinct from webhook events: https://github.com/interledger/rafiki/blob/main/packages/backend/src/config/app.ts#L89-L93 https://github.com/interledger/rafiki/blob/main/packages/backend/src/open_payments/quote/service.ts#L262 https://github.com/interledger/rafiki/blob/main/docs/transaction-api.md#quote

I've actually been considering doing this for outgoing payments as well to replace the asynchronous flow of:

1. outgoing_payment.created webhook event
2. account provider calls Mutation.depositEventLiquidity

https://github.com/interledger/rafiki/blob/main/docs/transaction-api.md#outgoing_paymentcreated (I'm not it makes sense though, because it'd require being able to withdraw liquidity from an outgoing payment that may have failed to successfully be created)

The Open Payments Spec needs to be changed in order to be able to handle this new behaviour.

RS <> Account provider interaction isn't defined in Open Payments.

[wilsonianb]

RS <> Account provider interaction isn't defined in Open Payments.

yet:

• https://github.com/interledger/rafiki/issues/626#issue-1381270487

[mkurapov]

@wilsonianb

Thanks for the notes. A few questions (and please correct any of my assumptions):

1. What is the route for the backend <> account provider? Based on your comment

during quote creation which is distinct from webhook events:

it seems that I would not reuse the /webhooks route? Or am I misunderstanding here.

2. In the logic for the account provider, what do I actually need to validate there? I'm guessing no balance needs to be check if the incoming payment is a credit to the user. Do I just need to check for the existence of the account by paymentPointerId?
3. In the webhook section of the transaction api doc, the incoming_payment.completed and incoming_payment.expired are describing the same behaviour (credit the receiver). This seems wrong, I feel like there should be no liquidity movement in the expired case?

[wilsonianb]

it seems that I would not reuse the /webhooks route?

Correct, if we want to follow the quote creation model, in which the backend currently sends a "synchronous" request to the account provider via the quoteUrl:

OTOH, the backend does not wait for account provider interaction when creating an outgoing payment:

If we go the quote route, it seems like this might require a corresponding incoming payments specific url.

2. I'm not entirely clear on the motivation for this issue, so I'm not sure what you could put in the mock-account-provider. If we expect an account provider to perform any kind of RO/user interaction, then I worry doing the "synchronous" quote creation route will cause the create incoming payment request to time out. Which is why I'm wondering if we can accomplish this via grant authorization: https://github.com/interledger/rafiki/issues/648#issuecomment-1265457863

3. An incoming payment can (partially) receive funds and then expire. (Given that Rafiki sets expiresAt even if one isn't specified,) the only way an incoming payment doesn't expire is if it has an incomingAmount that is fully paid (before the expiration). Upon further inspection, the backend currently doesn't send an incoming_payment.expired webhook event if the expired incoming payment hasn't received any funds: https://github.com/interledger/rafiki/blob/80861115fb1f5d69bbe2485254d94f16946f8a39/packages/backend/src/open_payments/payment/incoming/service.ts#L222-L238

[mkurapov]

1. Makes sense, that is what I had assumed. Thank you for the clear diagrams!
2. I can see that adding an extra hop could cause an issue where there wasn't originally. @AlexLakatos would you be able to provide some more context about this issue?
3. Also confirmed by no handlers on the AP side: https://github.com/interledger/rafiki/blob/80861115fb1f5d69bbe2485254d94f16946f8a39/packages/mock-account-provider/app/routes/webhooks.ts#L28-L43

[wilsonianb]

3. Also confirmed by no handlers on the AP side:

It should be handling incoming_payment.expired. I just opened:

• https://github.com/interledger/rafiki/issues/659

[matdehaast]

@wilsonianb @mkurapov Just for clarity the high level issue we are solving for here is that there are cases where an incoming payment will be attempted to be created at a payment pointer, and for business rules (account closed, not setup etc.), the account provider may want to decide not to accept that. So the goal for this is that when an incoming payment is created on Rafiki, it would synchronously call out to the Account Provider which would then allow it to tell Rafiki whether it's fine with creating it or now.

[mkurapov]

@matdehaast that makes sense to me - thanks to the clarification. I'm imagining some sort of /incoming-payment route that on the account provider side just does a simple payment pointer validation for now, but later on could be used to add different business rules as mentioned.

[wilsonianb]

for business rules (account closed, not setup etc.), the account provider may want to decide not to accept that.

At first, I was going to suggest that the Account Provider tell the AS to revoke grants for such accounts. But since there'll likely be grants to create incoming payments for any payment pointer (no specified identifier), I'm wondering if this can be addressed by allowing the Account Provider to deactivate a payment pointer.

[matdehaast]

So that's what I thought initially but if I think of the current way our product is working. We have people who can have send only accounts or receive only accounts. Now we would need to have flags on the PP on what is and isn't activated/deactivated. Also how do we deal with it when people start to hit limits and need to do stuff?

Another example is a user can receive up to $1000/month but anything after that is required to do some extra KYC.

Happy for some other options if you think they are easier

[wilsonianb]

:thinking: If we did want to support send only / receive only payment pointers in Rafiki, I'm wondering if that should be reflected in Open Payments (as in, GET or POST requests to /incoming-payments of a valid payment pointer returning 404). (I seem to recall one version of Open Payments payment pointers specifying the endpoints for incoming payments, mandates, etc. But the endpoints have been subsequently standardized. Maybe we could return to that (still have standardized endpoints), but exclude the specified incoming payments endpoint in the payment pointer response.)

How are you thinking about handling existing incoming payments for an account that enters such a send only state? Is it that you disallow both creation of new incoming payments and withdrawals (from Rafiki) from existing incoming payments? It seem like an account could have however many open incoming payments whose received amounts would put the account over whatever limit(s). The Account Provider could track the incomingAmount creating incoming payments, but that wouldn't help with incoming payments with no specified incomingAmount.

If we do decide to add the validate created incoming payment endpoint on the account provider, maybe we make it an optional value in the backend config, and maybe we make the existing quoteUrl optional as well (no default), for Account Providers who don't need to validate/adjust new incoming payments / quotes.

[mkurapov]

What if we did the same flow as outgoing payments, where we publish a webhook on incoming payment creation, and then we have a GraphQL call to rafiki to void the incoming payment if necessary? It is a extra jump but that way there doesn't need to be any changes to Open Payments spec, and if there are changes to the account on the account provider side at any point, we could use that mutation (or a separate mutation) to void any outstanding incoming payments?

Just brainstorming ideas with my limited knowledge here

[wilsonianb]

I'm guessing the concern there would be that (without changing the Open Payments spec) the incoming payment could receive funds between creation and the account provider voiding the incoming payment.

:thinking: I guess you could internally add a new inactive(?) state to incoming payments in which it cannot receive funds. The account provider would use a new Mutation.activateIncomingPayment(?) upon processing an incomiing_payment.created webhoook event at which point it could receive funds. So the STREAM connection would still be returned for every incoming payment request (create, read, etc), but ILP packets would be rejected as long as the incoming payment is "inactive". The backend could have a requireIncomingPaymentActivation config value that determines the state in which incoming payments are created.

[mkurapov]

That makes sense. In that case, it still feels like the sync call to the account provider during incoming payment creation like the straightforward solution:

• No need to track payment pointer "state" on the rafiki side, full control remains with account provider. No "out-of-sync" issues between rafiki & AP.
• No issues about (unnecessarily) rejecting of ILP packets for "inactive" incoming payments. The incoming payment creation could have been prevented earlier in the flow.

This:

How are you thinking about handling existing incoming payments for an account that enters such a send only state?

remains a good outstanding question. How would that look like from a business logic standpoint @matdehaast ?

[adrianhopebailie]

So the STREAM connection would still be returned for every incoming payment request (create, read, etc), but ILP packets would be rejected as long as the incoming payment is "inactive".

Seems like a lot of machinery to get moving if the incoming payment may still be rejected. I think we may be overstating the issues with making this synchronous and should just stick with that.

How are you thinking about handling existing incoming payments for an account that enters such a send only state?

An account is very unlikely to "enter" a send only state all of a sudden. There are basically two scenarios:

1. The receive account is closed
2. The receive account has hit some limit

1 - is mostly mitigated by the fact that it happens very seldom and when it does can be prevented if there are pending incoming payments. Part of the rationale for all incoming payments having an expiry is to allow multi-phase processes like account closing to block new incoming payments, wait for all existing incoming payments to complete or expire, and then complete the process.

2 - is mostly mitigated by the fact that the limits SHOULD be considered when creating the incoming payment. If a client attempts to create an incoming payment without a receive amount and the account is already close to the receive limit then the AP can take steps to mitigate this such as rejecting the request or asynchronously attempting to raise the limit.

[wilsonianb]

Seems like a lot of machinery to get moving if the incoming payment may still be rejected.

fwiw the connector is already checking incoming payment state https://github.com/interledger/rafiki/blob/f17fdb9981d57d6199062a94ef032d47339b5459/packages/backend/src/connector/core/middleware/account.ts#L49-L57

2 - is mostly mitigated by the fact that the limits SHOULD be considered when creating the incoming payment. If a client attempts to create an incoming payment without a receive amount and the account is already close to the receive limit then the AP can take steps to mitigate this such as rejecting the request or asynchronously attempting to raise the limit.

It seems like allowing any incoming payment without incomingAmount (regardless of how close an account is to a limit) introduces the possibility of the limit being obliterated (even with an expiry). In which case, a synchronous call could give the account provider the option of disallowing all incoming payments with incomingAmount, which brings me back to wondering if that should be restricted at the grant level. (As in, issue grants requiring incoming payments to have incomingAmount and use the new synchronous call to validate that the incomingAmount of created incoming payments don't put the account past a limit.)

[mkurapov]

@adrianhopebailie please correct me if I'm wrong but basically the proposed solution from the chat during the call is a combination of

1. add this synchronous call to the account provider on incoming payment creation in order to "fail early" if a limit is hit
2. if incomingAmount is not provided, create the incoming payment anyway, but reject on packet receive instead.This will likely will result in a good amount of partial/rejected payments, but at least a lot of these scenarios were mitigated by the sync call to the AP.

[wilsonianb]

This incomingAmount of a created incoming payment came up in this pull request discussion:

• https://github.com/interledger/rafiki/pull/796#discussion_r1039576791

Should any of these be allowed in a create incoming payment response, and would any be helpful for what's been discussed in this issue?

1. request has no specified incomingAmount, but the response does have an incomingAmount
2. request has a specified incomingAmount, but
   1. response has no incomingAmount
   2. response has incomingAmount with a lower value
   3. response has incomingAmount with a higher value

[mkurapov]

@sabineschaller Last week we discussed that we should add this webhook. Even though it would be a no-op on the mock ASE side, should we still add this, just to support any side effects the ASE could possibly want to have (i.e. sending a notification)?

I edited the description already