663/mk/create outgoing payment

[mkurapov]

Changes proposed in this pull request

• Adding outgoingPayment.create to open payments client

Context

• Progress towards Create Open Payments client library #663

Checklist

• Related issues linked using fixes #number
• Tests added/updated
• Documentation added
• Make sure that all checks pass

[mkurapov]

This is a guess about the desired behaviour

the main issue here was using || instead of ??. When we call this function just above:

rafiki/packages/backend/src/open_payments/payment/outgoing/service.ts

Line 160 in cbbc416

return await addSentAmount(deps, payment, BigInt(0))

we pass in value: BigInt(0) . BigInt(0) is falsy, so we end up calling deps.accountingService.getTotalSent(payment.id)) even though we actually just wanted to set sent to be 0.

[mkurapov]

Here the only other validation I could think of doing on a newly created outgoing payment (that wasn't already covered in validateOutgoingPayment) was checking whether sentAmount.value === 0. Feels like a weird behaviour to already start processing a payment before returning it, but does the caller care?

[wilsonianb]

I agree that does somehow seem less concerning than a non-zero receivedAmount on a new incoming payment.

[mkurapov]

was missing previously

[wilsonianb]

The properly POSTs request test is failing with the addition of headers: {}

[mkurapov]

Yup, saw that, fixing.

[wilsonianb]

quoteId and id are supposed to be uris
https://github.com/interledger/open-payments/blob/02a6d505d79124cce776054df89e28f02c8e5d59/openapi/resource-server.yaml#L1072-L1084
Am I correct that we shouldn't be concerned about the quality of our OpenAPI validation because this package is only testing against successfulValidator/failedValidator?

[mkurapov]

Good point, I'll make the test case a bit more realistic. But yes, in general, IMO the actual OpenAPI spec validation is a bit out of scope of the package, that responsibility is openapis

[wilsonianb]

The properly POSTs request test is failing with the addition of headers: {}

[wilsonianb]

I agree that does somehow seem less concerning than a non-zero receivedAmount on a new incoming payment.