chore(backend): remove wallet address middleware #2708

mkurapov · 2024-05-07T16:11:43Z

Changes proposed in this pull request

To finalize the standardization of Open Payments route errors in backend, this PR removes walletAddressMiddleware.
The benefits:

We now handle all possible errors properly in the openPaymentsServerErrorMiddleware (i.e. we don't use ctx.throw at all in other places)
walletAddressUrl is determined as an inline-middleware per route, meaning the complex logic walletAddressMiddleware can be removed
We don't need to fetch the walletAddress until each route needs it. This is nice for a few reasons:
- we can control whether a route will poll (or not) the wallet address creation at the ASE (i.e. we can choose whether a route will call walletAddressService.getOrPollByUrl or just getByUrl)
- we can eliminate a request to get the wallet address before other checks are ran (like token introspection, http signature middleware, Open Api validation, etc)

Context

Completion of backend changes for #1905

Checklist

… middleware

…-middleware # Conflicts: # packages/backend/src/open_payments/wallet_address/model.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.ts

mkurapov · 2024-05-07T16:16:38Z

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

@@ -181,12 +188,13 @@ async function getWalletAddress(

 async function getOrPollByUrl(
  deps: ServiceDependencies,
-  url: string
+  url: string,
+  options: GetOrPollByUrlOptions = { mustBeActive: true }


Since we primarily use this method when resolving the wallet address in Open Payments routes, we should expect it to always be active

…-middleware

mkurapov · 2024-05-07T17:02:18Z

packages/backend/src/app.ts

+        const incomingPaymentService = await ctx.container.use(
+          'incomingPaymentService'
+        )
+        const incomingPayment = await incomingPaymentService.get({
+          id: ctx.params.id
+        })
+
+        if (!incomingPayment?.walletAddress) {
+          throw new OpenPaymentsServerRouteError(401, 'Unauthorized', {
+            description: 'Failed to get wallet address from incoming payment',
+            id: ctx.params.id
+          })
+        }
+
+        ctx.walletAddressUrl = incomingPayment.walletAddress.url


We do not get a specific wallet address in the following OP requests (either in the query params or request body):

GET /incoming-payments/:id
GET /outgoing-payments/:id
GET /quotes/:id
POST /incoming-payments/:id/complete

We need the walletAddressUrl in order to pass down into createTokenIntrospectionMiddleware, as the middleware needs to determine whether the possible identifier on the grant matches up to the resource for the particular wallet address.

This is why we still need to get the resource first, before we can proceed with handling the request. The status quo is returning a 401/Unauthorized response code (to not give away information about whether resource exists or not). Maybe this should be 400, but I don't have a strong opinion here.

I need to add tests for this, but I would need to do some work about splitting apart these in-line middlewares into separate functions. I can handle it in a separate PR where I do a little more refactoring, since I don't want to balloon this one.

For the following:

GET /incoming-payments/:id
GET /outgoing-payments/:id
GET /quotes/:id

we could just store these on ctx.state and not have to call the <resource>Routes.get/fetch the resource from the DB again, but a) could be done in separate PR, or b) not worth doing at all for the consistency of having the routes as they are

mkurapov · 2024-05-07T17:20:05Z

packages/backend/src/open_payments/payment/outgoing/routes.ts

@@ -100,9 +102,18 @@ async function createOutgoingPayment(
  deps: ServiceDependencies,
  ctx: CreateContext<CreateBody>
 ): Promise<void> {
+  const walletAddress = await deps.walletAddressService.getOrPollByUrl(


Do we still want to poll here? Typically, the polling probably necessary only on the receiving side, i.e. incoming payment creation only. (since that is the most likely place where we would need to allow the ASE to make a request to their backend to create the corresponding wallet addresses)

mkurapov · 2024-05-07T17:21:07Z

packages/backend/src/payment-method/ilp/spsp/middleware.ts

-    const walletAddress = await walletAddressService.getByUrl(
+    const walletAddress = await walletAddressService.getOrPollByUrl(
      ctx.walletAddressUrl
    )

-    if (!walletAddress?.isActive) {


already handled in getOrPollByUrl by default. However, this comment still stands: https://github.com/interledger/rafiki/pull/2708/files#r1592829255

# Conflicts: # packages/backend/src/open_payments/wallet_address/model.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.ts

netlify · 2024-05-10T17:16:01Z

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

Name	Link
🔨 Latest commit	`2082015`
🔍 Latest deploy log	https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/663e689bc9a532000837568f

mkurapov · 2024-05-10T17:17:24Z

packages/backend/src/payment-method/ilp/spsp/middleware.test.ts

@@ -107,27 +107,4 @@ describe('SPSP Middleware', (): void => {
      expect(spspGetRouteSpy).not.toHaveBeenCalled()
    }
  })
-
-  test('throws error if inactive wallet address', async () => {


Already handled: https://github.com/interledger/rafiki/pull/2708/files#r1592830278

mkurapov · 2024-05-10T18:36:03Z

packages/backend/src/shared/pagination.ts

@@ -20,14 +20,12 @@ export function parsePaginationQueryParameters({
 type GetPageInfoArgs<T extends BaseModel> = {
  getPage: (pagination: Pagination, sortOrder?: SortOrder) => Promise<T[]>
  page: T[]
-  walletAddress?: string


mkurapov added 7 commits May 6, 2024 17:45

feat(backend): SPSPRouteError

4936272

chore(backend): remove wallet address middleware, replace with inline…

c5bfc9f

… middleware

feat(backend): SPSPRouteError

6cf1e7f

feat(backend): SPSPRouteError

b0b6e13

chore(backend): use getOrPollByUrl in SPSP middleware

18a4ba5

github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels May 7, 2024

mkurapov commented May 7, 2024

View reviewed changes

mkurapov added 3 commits May 7, 2024 18:20

Merge branch 'main' into 1905/mk/spsp-errors

a123c69

Merge branch '1905/mk/spsp-errors' into 1905/mk/remove-wallet-address…

221f55f

…-middleware

chore(backend): update log during wallet address polling

9307437

mkurapov changed the title ~~1905/mk/remove wallet address middleware~~ chore(backend): remove wallet address middleware May 7, 2024

mkurapov mentioned this pull request May 7, 2024

feat(backend): SPSPRouteError #2705

Merged

5 tasks

chore(backend): minor cleanup in app.ts

62b3e9a

mkurapov commented May 7, 2024

View reviewed changes

mkurapov added 2 commits May 7, 2024 19:16

chore(backend): convert error log to debug statement in getOrPollByUrl

6725937

chore(backend): updating wallet address get route

d657a8d

mkurapov commented May 7, 2024

View reviewed changes

mkurapov linked an issue May 8, 2024 that may be closed by this pull request

Standardize Open Payment errors #1905

Closed

Base automatically changed from 1905/mk/spsp-errors to main May 10, 2024 17:12

Merge branch 'main' into 1905/mk/remove-wallet-address-middleware

5b59c71

# Conflicts: # packages/backend/src/open_payments/wallet_address/model.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.test.ts # packages/backend/src/payment-method/ilp/spsp/middleware.ts

mkurapov commented May 10, 2024

View reviewed changes

chore(backend): remove unused walletAddress arg

2082015

mkurapov commented May 10, 2024

View reviewed changes

mkurapov marked this pull request as ready for review May 10, 2024 18:36

mkurapov marked this pull request as draft May 10, 2024 19:22

mkurapov closed this May 10, 2024

mkurapov removed a link to an issue May 21, 2024

Standardize Open Payment errors #1905

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(backend): remove wallet address middleware #2708

chore(backend): remove wallet address middleware #2708

mkurapov commented May 7, 2024 •

edited

Loading

mkurapov May 7, 2024

mkurapov May 7, 2024 •

edited

Loading

mkurapov May 7, 2024 •

edited

Loading

mkurapov May 10, 2024

mkurapov May 7, 2024 •

edited

Loading

mkurapov May 7, 2024

netlify bot commented May 10, 2024 •

edited

Loading

mkurapov May 10, 2024 •

edited

Loading

mkurapov May 10, 2024

chore(backend): remove wallet address middleware #2708

chore(backend): remove wallet address middleware #2708

Conversation

mkurapov commented May 7, 2024 • edited Loading

Changes proposed in this pull request

Context

Checklist

mkurapov May 7, 2024

Choose a reason for hiding this comment

mkurapov May 7, 2024 • edited Loading

Choose a reason for hiding this comment

mkurapov May 7, 2024 • edited Loading

Choose a reason for hiding this comment

mkurapov May 10, 2024

Choose a reason for hiding this comment

mkurapov May 7, 2024 • edited Loading

Choose a reason for hiding this comment

mkurapov May 7, 2024

Choose a reason for hiding this comment

netlify bot commented May 10, 2024 • edited Loading

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

mkurapov May 10, 2024 • edited Loading

Choose a reason for hiding this comment

mkurapov May 10, 2024

Choose a reason for hiding this comment

mkurapov commented May 7, 2024 •

edited

Loading

mkurapov May 7, 2024 •

edited

Loading

mkurapov May 7, 2024 •

edited

Loading

mkurapov May 7, 2024 •

edited

Loading

netlify bot commented May 10, 2024 •

edited

Loading

mkurapov May 10, 2024 •

edited

Loading