feat(backend): rename "payment pointer" tables & columns to use "wallet address"

[njlie]

Changes proposed in this pull request

• Rename all tables & columns that refer to 'payment pointers' to refer to 'wallet addresses' instead.

Context

Fixes #1934.

Checklist

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

[njlie]

The backend service isn't going to build in this branch (yet), but the migration should run successfully.

[BlairCurrey]

This concern spans this PR as well as the code changes. There are payment pointer webhook events which are being renamed to wallet address. I think there will be webhook events with JSON payloads with a paymentPointer key in existing deployments (e.g. testnet). Can we rule out that this will cause any problems? Are we never referencing the walletAddress key in the event payload read from the db? Otherwise I think we'd need to update the keys to walletAddress.

This is the same potential issue I ran into when renaming sendAmount -> debitAmount here: #1755 (comment)

This is what the migration to rename the keys looked like: https://github.com/interledger/rafiki/blob/main/packages/auth/migrations/20230907201006_rename-limits-jsonb-keys.js

[mkurapov]

I'm not actually sure, what happens to the existing paymentPointer.id foreign key here? Does it get properly handled in the rename, or dropped?

[BlairCurrey]

This also applies the indexes (not pictured above) in line 16: table.index(['walletAddressId', 'createdAt', 'id']).

Looks like the migration is duplicating these. Two fkey constraints with different fkey names but the same relationship. I think we can just remove the table.foreign('walletAddressId').references('walletAddresses.id') line (and the indexes). This is what I see in Indexes and Foreign-key constraints: from doing a \d quotes after this migration.

quote:

Indexes:
    "quotes_pkey" PRIMARY KEY, btree (id)
    "quotes_paymentpointerid_createdat_id_index" btree ("walletAddressId", "createdAt", id)
    "quotes_walletaddressid_createdat_id_index" btree ("walletAddressId", "createdAt", id)
Foreign-key constraints:
    "quotes_assetid_foreign" FOREIGN KEY ("assetId") REFERENCES assets(id)
    "quotes_feeid_foreign" FOREIGN KEY ("feeId") REFERENCES fees(id)
    "quotes_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    "quotes_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)

and here is some of \d "walletAddresses" for good measure (see the duplicate quote fkey):

Referenced by:
    TABLE ""incomingPayments"" CONSTRAINT "incomingpayments_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE ""incomingPayments"" CONSTRAINT "incomingpayments_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE ""outgoingPayments"" CONSTRAINT "outgoingpayments_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE ""outgoingPayments"" CONSTRAINT "outgoingpayments_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE ""walletAddressKeys"" CONSTRAINT "paymentpointerkeys_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE "quotes" CONSTRAINT "quotes_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE "quotes" CONSTRAINT "quotes_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)
    TABLE ""walletAddressKeys"" CONSTRAINT "walletaddresskeys_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id)

[BlairCurrey]

Actually instead of removing these lines we might want to just drop the old one. This way we end up with "quotes_walletaddressid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id) instead of "quotes_paymentpointerid_foreign" FOREIGN KEY ("walletAddressId") REFERENCES "walletAddresses"(id).

So doing like:

    knex.schema.alterTable('quotes', function (table) {
      table.dropForeign(['paymentPointerId'])
      table.dropIndex(['paymentPointerId', 'createdAt', 'id'])
      table.renameColumn('paymentPointerId', 'walletAddressId')
      table.foreign('walletAddressId').references('walletAddresses.id')
      table.index(['walletAddressId', 'createdAt', 'id'])
    }),

[BlairCurrey]

i inserted some dummy data before this migration and this and similar all seem to be working correctly

[njlie]

The indexes are not aggregated because the original indexes weren't either. Aggregating them produces, for example:

Indexes:
"outgoingpayments_walletaddressid_createdat_id_index" btree ("walletAddressId", "createdAt", id)

Whereas originally the indexes were not aggregated:

"outgoingpayments_paymentpointerid__index" btree ("paymentPointerId")
"outgoingpayments_createdat_index" btree ("createdAt")
"outgoingpayments_id_index" btree (id)

So table.index is called separately each time to preserve this.

[BlairCurrey]

The indexes are not aggregated because the original indexes weren't either.

It seems like they are originally 1 composite index to me but maybe I'm missing something... without running this migration I see "outgoingpayments_paymentpointerid_createdat_id_index" btree ("paymentPointerId", "createdAt", id) (same for quotes, did not check the rest). It's also what I would expect from the original migration:

rafiki/packages/backend/migrations/20221129213751_create_outgoing_payments_table.js

Line 28 in 07cff89

table.index(['paymentPointerId', 'createdAt', 'id'])

If they were seperate indexes I also don't think we'd need to do table.dropIndex(['paymentPointerId', 'createdAt', 'id']), but instead would just drop the payment pointer one (and thus not have to re-add the others).

[njlie]

Oh yeah, they're composite. A misreading on my part earlier. So we'll keep the composite dropping & adding of these indexes.