feat(backend): add Open Payments client service #595

wilsonianb · 2022-09-08T18:51:03Z

Changes proposed in this pull request

add Open Payments client service
replace calls to Pay.setupPayment
return correct ilpStreamConnection type
Update ilp-packet to pre-release version to match types of other interledgerjs dependencies.
downgrade axios

Context

fixes Query incoming payments directly #581
fixes Add a dev mode access token #586
fixes Downgrade axios #589
related to Make incoming payment ilpStreamConnection type explicit open-payments#160

Checklist

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

packages/backend/src/config/app.ts

@@ -50,6 +50,7 @@
  nonceRedisKey: envString('NONCE_REDIS_KEY', 'nonceToProject'),
  adminKey: envString('ADMIN_KEY', 'qwertyuiop1234567890'),
  sessionLength: envInt('SESSION_LENGTH', 30), // in minutes
+  devAccessToken: envString('DEV_ACCESS_TOKEN', 'dev-access-token'),


Update ilp-packet version to match types.

…tion

Replace calls to Pay.setupPayment Downgrade axios to v0.26.1

wilsonianb · 2022-09-09T21:07:54Z

packages/backend/src/open_payments/payment/incoming/model.ts

@@ -134,6 +141,13 @@ export class IncomingPayment
    return `${this.paymentPointer.url}/incoming-payments/${this.id}`
  }

+  public get hasConnection(): boolean {


An alternative to this would be to set connectionId to null when the incoming payment transitions to Completed/Expired

I think that would be cleaner.

👍 I'll change it.
My only reservation is if we don't want to lose record of the connectionId

Is there any admin purpose we can think of where it would be beneficial to get the ilp credentials after the payment is completed or expired?

Not returning the connection with a completed/expired payment is an Open Payments OpenAPI thing.
It would technically still be possible to generate ilp credentials for the incoming payment since the connectionId use only used for the connection url:
https://github.com/interledger/rafiki/pull/595/files#diff-b9b1a593e29f9fdc18a9826d0da73d506ccca8cca9e777165b4ac86b5a0bfdd5R72

I know it's an OP thing. I just checked the code again and remembered that it is something completely artificial that's not even used to create the STREAM credentials. In that case, set it to null.

sabineschaller · 2022-09-13T11:26:52Z

packages/backend/src/open_payments/client/service.ts

+  const validateResponse =
+    deps_.openApi.createResponseValidator<IncomingPaymentJSON>({
+      path: '/incoming-payments/{incomingPaymentId}',
+      method: HttpMethod.GET
+    })


Why does that have to be overwritten?

validateResponse isn't included on deps_.
Or are you asking why define a response validator that should already exist in a validator middleware?

Yes, the latter.

🤔
Nothing is being overwritten.
Both this and the auth middleware (only for the get incoming payment route) will each create the same response validator once.
However, the auth middleware does not create response validators when running in production:
https://github.com/interledger/rafiki/blob/main/packages/openapi/src/middleware.ts#L11-L12

Never mind, I misread.

sabineschaller · 2022-09-13T11:29:19Z

packages/backend/src/open_payments/client/service.ts

+  }
+}
+
+export async function getIncomingPayment(


Could we check whether the incoming payment is a resource on the same Rafiki instance and if so, we just use the service to get it instead of the axios call + all the validation?

I initially tried that but didn't like having two code paths, one dealing with IncomingPayments model and the other dealing with IncomingPaymentJSON, the former of which duplicated a lot of functionality in middleware and incoming payments routes.ts, including the solution for #598

Alright, we keep it simple then.

This looks up local incoming payments / connections and keeps it simpler by calling the route methods:

chore(backend): don't resolve local receiver over http #658

sabineschaller · 2022-09-13T11:33:53Z

packages/backend/src/open_payments/payment/incoming/model.ts

@@ -134,6 +141,13 @@ export class IncomingPayment
    return `${this.paymentPointer.url}/incoming-payments/${this.id}`
  }

+  public get hasConnection(): boolean {


I think that would be cleaner.

Remove publicHost from OutgoingPaymentService.

github-actions bot added pkg: backend Changes in the backend package. type: source Changes business logic type: tests Testing related labels Sep 8, 2022

github-advanced-security bot found potential problems Sep 8, 2022

View reviewed changes

This was referenced Sep 8, 2022

Add CreateIncomingPayment on the admin graphql #580

Closed

Query incoming payments directly #581

Closed

wilsonianb force-pushed the bw-client branch from 77d48a9 to 06351f0 Compare September 9, 2022 14:58

wilsonianb marked this pull request as ready for review September 9, 2022 15:07

wilsonianb requested review from sabineschaller, dclipp and matdehaast September 9, 2022 15:08

wilsonianb added 4 commits September 9, 2022 16:14

fix(backend): return correct ilpStreamConnection type

5051462

Update ilp-packet version to match types.

chore(backend): add configurable DEV_ACCESS_TOKEN to bypass introspec…

e7f71d1

…tion

feat(backend): add Open Payments client service

e78c60f

Replace calls to Pay.setupPayment Downgrade axios to v0.26.1

chore(backend): rename QuoteError.InvalidDestination to InvalidReceiver

5d7c193

wilsonianb force-pushed the bw-client branch from 06351f0 to 5d7c193 Compare September 9, 2022 21:58

wilsonianb commented Sep 9, 2022

View reviewed changes

wilsonianb mentioned this pull request Sep 12, 2022

Support quote/outgoing payment to STREAM connection #596

Closed

sabineschaller reviewed Sep 13, 2022

View reviewed changes

matdehaast previously approved these changes Sep 13, 2022

View reviewed changes

chore(backend): delete completed/expired incoming payment connectionId

d34228c

wilsonianb dismissed matdehaast’s stale review via d34228c September 13, 2022 13:56

wilsonianb requested a review from sabineschaller September 13, 2022 14:13

fix(backend): pass correct host to ConnectionService

8928c14

Remove publicHost from OutgoingPaymentService.

sabineschaller approved these changes Sep 15, 2022

View reviewed changes

wilsonianb merged commit e96d44e into main Sep 15, 2022

wilsonianb deleted the bw-client branch September 15, 2022 13:04

This was referenced Oct 4, 2022

Request grant at other Open Payments server(s) #583

Closed

Create Open Payments client library #663

Closed

wilsonianb mentioned this pull request Oct 25, 2022

fix(backend): do not import (nonexistent) test code #691

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(backend): add Open Payments client service #595

feat(backend): add Open Payments client service #595

wilsonianb commented Sep 8, 2022 •

edited

Loading

wilsonianb Sep 9, 2022

sabineschaller Sep 13, 2022

wilsonianb Sep 13, 2022

sabineschaller Sep 14, 2022

wilsonianb Sep 14, 2022

sabineschaller Sep 15, 2022

sabineschaller Sep 13, 2022

wilsonianb Sep 13, 2022

sabineschaller Sep 14, 2022

wilsonianb Sep 14, 2022

sabineschaller Sep 15, 2022

sabineschaller Sep 13, 2022

wilsonianb Sep 13, 2022

sabineschaller Sep 15, 2022 •

edited

Loading

wilsonianb Oct 5, 2022

sabineschaller Sep 13, 2022

feat(backend): add Open Payments client service #595

feat(backend): add Open Payments client service #595

Conversation

wilsonianb commented Sep 8, 2022 • edited Loading

Changes proposed in this pull request

Context

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabineschaller Sep 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb commented Sep 8, 2022 •

edited

Loading

sabineschaller Sep 15, 2022 •

edited

Loading