[WIP] Async sendMoney

[nkramer44]

addresses #15 and #16

This PR converts sendMoney from a blocking call to an async call. A request to sendMoney will now return almost immediately without waiting for the payment to complete. Instead of returning a SendPaymentResult or a GRPC analogue, Hermes will register the payment with a PENDING status and return a Payment object immediately. Users can then poll the getPayment endpoint to see the status of the Payment.

This doesn't solve all of our resiliency issues, but does solve for the situation in which the client disconnects from Hermes. We could have done idempotent sendMoney without async, but async seems to provide a better user experience than blocking during a potentially long ILP payment.

Typical flow would look like:

sendMoney:

curl --location --request PUT 'http://{hermes-host}/accounts/{accountId}/payments/{UUID paymentId}' \
--header 'Content-Type: application/json' \
--header 'Authorization: Bearer {auth_token}' \
--data-raw '{
	"amount" : {sendAmount},
	"destinationPaymentPointer": "{receiverPaymentPointer}"
}'

returns:

{
    "paymentId": "{paymentId}",
    "senderAccountId": "{senderAccountId}",
    "originalAmount": "{sendAmount}",
    "amountDelivered": "0",
    "amountSent": "0",
    "amountLeftToSend": "{sendAmount}",
    "destination": "{receiverPaymentPointer}",
    "status": "PENDING"
}

getPayment:

curl --location --request GET 'http://{hermes_host}/payments/{paymentId}'

returns:

{
    "paymentId": "{paymentId}",
    "senderAccountId": "{senderUserId}",
    "originalAmount": "{sendAmount}",
    "amountDelivered": "{amount delivered by stream sender}",
    "amountSent": "{amount sent by stream sender}",
    "amountLeftToSend": "{amount left to send by stream sender}",
    "destination": "{receiverPaymentPointer}",
    "status": "{SUCCESSFUL or FAILED}"
}

NOTE: PENDING and SUCCESSFUL/FAILED payments are stored in Redis, but these entries are only updated with once a STREAM payment has completed or an error occurred in STREAM sender, and thus are not real-time figures.

[theotherian]

Looking very good thus far. Added a few comments regarding some design decisions we should discuss.

[theotherian]

I've been thinking about this, and part of me is thinking that both SendMoney and GetPayment (along with their REST counterparts) should probably return the same response.

I'm trying to think if there's anything that would be different in a way between the two responses that would necessitate having them be separate entities.

My rationale for this is that it makes it a little easier on the client side if you want to have a blocking vs non-blocking wrapper for sending a payment if the response types are the same, because then the interface can be common for both implementations.

[nkramer44]

Agree. Having two protobufs that are identical makes me uncomfortable. We should check on this, but in our original meeting with Keefer to discuss GRPC, I think he said that best practice is to have a different protobuf for each request/response, even if they are identical.

[theotherian]

I think he said that best practice is to have a different protobuf for each request/response, even if they are identical.

😕

I'm not sure I understand why that's considered a best practice. I'm fine with pushing back on that one, especially considering that it likely makes his life harder when it comes to the new behavior.

I'm also curious to see what the GRPC best practices are and why they're best practices because this is at least the second one I've heard of that left me scratching my head.

[nkramer44]

Yeah, we should do some research on this. I just remember that in my original protobufs, I returned a protobuf called AccountSettings that mirrored the connector AccountSettings for both createAccount and getAccount, but I was told to separate them....

[nkramer44]

I would love to get rid of the redundant protobufs... maybe I could actually reuse some code if we did that

[theotherian]

Per my previous comment, I'm wondering if this and GetPaymentResponse can be the same.

[theotherian]

Per the previous comments, these should probably end up being the same type.

[theotherian]

This value obviously shouldn't be hard coded.

[nkramer44]

Also agree. This should be the address of the connector, right?

If so, we can get that from the Metadata api on the connector.

[theotherian]

I would assume that's correct.