chore(core): add (externalId, accountId) invoices index (#4377)

* chore(core): create unique (externalId, accountId) invoices index * refactor(core): remove PartialWalletDescriptor type * test(core): add account-level external id uniqueness checks
GaloyMoney · May 9, 2024 · ed12289 · ed12289
1 parent 1d79fd9
commit ed12289
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 30 deletions.
diff --git a/bats/core/api/invoices.bats b/bats/core/api/invoices.bats
@@ -7,6 +7,7 @@ setup_file() {
   clear_cache
 
   create_user 'alice'
+  create_user 'bob'
   seed_invoices
 }
 
@@ -69,29 +70,75 @@ seed_invoices() {
   [[ "$num_errors" == "0" ]] || exit 1
 }
 
-@test "invoices: adding multiple invoices with same external id fails" {
-  token_name='alice'
-  btc_wallet_name="$token_name.btc_wallet_id"
-  btc_amount="1000"
+@test "invoices: adding multiple invoices with same external id fails for same account" {
   external_id="external-id-$RANDOM"
 
+  alice_btc_wallet_name="alice.btc_wallet_id"
+  alice_usd_wallet_name="alice.usd_wallet_id"
+  bob_btc_wallet_name="bob.btc_wallet_id"
+  bob_usd_wallet_name="bob.usd_wallet_id"
+
+  btc_amount="1000"
+  usd_amount="20"
+
   variables=$(
     jq -n \
-    --arg wallet_id "$(read_value $btc_wallet_name)" \
+    --arg wallet_id "$(read_value $alice_btc_wallet_name)" \
     --arg amount "$btc_amount" \
     --arg external_id "$external_id" \
     '{input: {walletId: $wallet_id, amount: $amount, externalId: $external_id}}'
   )
 
-  exec_graphql "$token_name" 'ln-invoice-create' "$variables"
+  exec_graphql 'alice' 'ln-invoice-create' "$variables"
   num_errors="$(graphql_output '.data.lnInvoiceCreate.errors | length')"
   [[ "$num_errors" == "0" ]] || exit 1
 
-  exec_graphql "$token_name" 'ln-invoice-create' "$variables"
+  # Check 'alice' can't re-use externalId for same wallet
+  exec_graphql 'alice' 'ln-invoice-create' "$variables"
   invoice="$(graphql_output '.data.lnInvoiceCreate.invoice')"
   [[ "$invoice" == "null" ]] || exit 1
   error_msg="$(graphql_output '.data.lnInvoiceCreate.errors[0].message')"
   [[ "${error_msg}" =~ "already exists" ]] || exit 1
+
+  # Check 'alice' can't re-use externalId for different wallet
+  variables=$(
+    jq -n \
+    --arg wallet_id "$(read_value $alice_usd_wallet_name)" \
+    --arg amount "$usd_amount" \
+    --arg external_id "$external_id" \
+    '{input: {walletId: $wallet_id, amount: $amount, externalId: $external_id}}'
+  )
+  exec_graphql 'alice' 'ln-usd-invoice-create' "$variables"
+  invoice="$(graphql_output '.data.lnUsdInvoiceCreate.invoice')"
+  [[ "$invoice" == "null" ]] || exit 1
+  error_msg="$(graphql_output '.data.lnUsdInvoiceCreate.errors[0].message')"
+  [[ "${error_msg}" =~ "already exists" ]] || exit 1
+
+  # Check 'bob' can re-use externalId once
+  variables=$(
+    jq -n \
+    --arg wallet_id "$(read_value $bob_btc_wallet_name)" \
+    --arg amount "$btc_amount" \
+    --arg external_id "$external_id" \
+    '{input: {walletId: $wallet_id, amount: $amount, externalId: $external_id}}'
+  )
+  exec_graphql 'bob' 'ln-invoice-create' "$variables"
+  num_errors="$(graphql_output '.data.lnInvoiceCreate.errors | length')"
+  [[ "$num_errors" == "0" ]] || exit 1
+
+  # Check 'bob' cant re-use externalId again
+  variables=$(
+    jq -n \
+    --arg wallet_id "$(read_value $bob_usd_wallet_name)" \
+    --arg amount "$usd_amount" \
+    --arg external_id "$external_id" \
+    '{input: {walletId: $wallet_id, amount: $amount, externalId: $external_id}}'
+  )
+  exec_graphql 'bob' 'ln-usd-invoice-create' "$variables"
+  invoice="$(graphql_output '.data.lnUsdInvoiceCreate.invoice')"
+  [[ "$invoice" == "null" ]] || exit 1
+  error_msg="$(graphql_output '.data.lnUsdInvoiceCreate.errors[0].message')"
+  [[ "${error_msg}" =~ "already exists" ]] || exit 1
 }
 
 @test "invoices: get invoices for account" {

diff --git a/core/api/src/domain/shared/index.types.d.ts b/core/api/src/domain/shared/index.types.d.ts
@@ -38,12 +38,9 @@ type BalanceAmount<T extends WalletCurrency> = Amount<T> & {
   readonly brand?: unique symbol
 }
 
-type PartialWalletDescriptor<T extends WalletCurrency> = {
+type WalletDescriptor<T extends WalletCurrency> = {
   id: WalletId
   currency: T
-  accountId?: AccountId // TODO: unify when we migrate accountId to invoices collection
-}
-type WalletDescriptor<T extends WalletCurrency> = PartialWalletDescriptor<T> & {
   accountId: AccountId
 }
 

diff --git a/core/api/src/domain/wallet-invoices/index.types.d.ts b/core/api/src/domain/wallet-invoices/index.types.d.ts
@@ -98,7 +98,7 @@ type WalletInvoiceWithOptionalLnInvoice = {
   selfGenerated: boolean
   pubkey: Pubkey
   usdAmount?: UsdPaymentAmount
-  recipientWalletDescriptor: PartialWalletDescriptor<WalletCurrency>
+  recipientWalletDescriptor: WalletDescriptor<WalletCurrency>
   paid: boolean
   createdAt: Date
   processingCompleted: boolean

diff --git a/core/api/src/services/mongoose/schema.ts b/core/api/src/services/mongoose/schema.ts
@@ -37,6 +37,7 @@ const walletInvoiceSchema = new Schema<WalletInvoiceRecord>({
   },
 
   accountId: {
+    required: true,
     type: String,
     validate: {
       validator: function (v: string) {
@@ -98,14 +99,15 @@ const walletInvoiceSchema = new Schema<WalletInvoiceRecord>({
   },
 
   externalId: {
+    required: true,
     type: String,
-    unique: true,
     validator: (v: string) => !(checkedToLedgerExternalId(v) instanceof Error),
   },
 })
 
 walletInvoiceSchema.index({ walletId: 1, paid: 1 })
 walletInvoiceSchema.index({ paid: 1, processingCompleted: 1 })
+walletInvoiceSchema.index({ accountId: 1, externalId: 1 }, { unique: true })
 
 export const WalletInvoice = mongoose.model<WalletInvoiceRecord>(
   "InvoiceUser",

diff --git a/core/api/test/helpers/wallet-invoices.ts b/core/api/test/helpers/wallet-invoices.ts
@@ -3,10 +3,9 @@ import { randomLedgerExternalId } from "./random"
 import { decodeInvoice, getSecretAndPaymentHash } from "@/domain/bitcoin/lightning"
 import { WalletCurrency } from "@/domain/shared"
 
-export const createMockWalletInvoice = (recipientWalletDescriptor: {
-  currency: WalletCurrency
-  id: WalletId
-}): WalletInvoice => {
+export const createMockWalletInvoice = (
+  recipientWalletDescriptor: WalletDescriptor<WalletCurrency>,
+): WalletInvoice => {
   const externalId = randomLedgerExternalId()
   if (externalId instanceof Error) throw externalId
 

diff --git a/core/api/test/integration/app/transactions/get-invoice-preimage-by-hash.spec.ts b/core/api/test/integration/app/transactions/get-invoice-preimage-by-hash.spec.ts
@@ -13,6 +13,7 @@ describe("getInvoicePreImageByHash", () => {
   it("returns a valid preimage when invoice has been paid", async () => {
     const invoice = createMockWalletInvoice({
       id: crypto.randomUUID() as WalletId,
+      accountId: crypto.randomUUID() as AccountId,
       currency: WalletCurrency.Btc,
     })
     await WalletInvoicesRepository().persistNew({ ...invoice, paid: true })
@@ -27,6 +28,7 @@ describe("getInvoicePreImageByHash", () => {
   it("returns error if invoice has not been paid", async () => {
     const invoice = createMockWalletInvoice({
       id: crypto.randomUUID() as WalletId,
+      accountId: crypto.randomUUID() as AccountId,
       currency: WalletCurrency.Btc,
     })
     await WalletInvoicesRepository().persistNew({ ...invoice, paid: false })

diff --git a/core/api/test/integration/app/wallets/update-pending-invoices.spec.ts b/core/api/test/integration/app/wallets/update-pending-invoices.spec.ts
@@ -72,6 +72,7 @@ describe("update pending invoices", () => {
         recipientWalletDescriptor: {
           id: randomUUID() as WalletId,
           currency: WalletCurrency.Usd,
+          accountId: randomUUID() as AccountId,
         },
         paid: false,
         lnInvoice: mockLnInvoice,
@@ -130,6 +131,7 @@ describe("update pending invoices", () => {
         recipientWalletDescriptor: {
           id: randomUUID() as WalletId,
           currency: WalletCurrency.Btc,
+          accountId: randomUUID() as AccountId,
         },
         paid: false,
         lnInvoice: mockLnInvoice,

diff --git a/core/api/test/integration/services/wallet-invoices-repository.spec.ts b/core/api/test/integration/services/wallet-invoices-repository.spec.ts
@@ -8,6 +8,7 @@ const walletInvoices = WalletInvoicesRepository()
 const recipientWalletDescriptor = {
   currency: WalletCurrency.Btc,
   id: crypto.randomUUID() as WalletId,
+  accountId: crypto.randomUUID() as AccountId,
 }
 
 let createdInvoices: WalletInvoice[] = []
@@ -32,6 +33,7 @@ beforeAll(async () => {
     createMockWalletInvoice({
       currency: WalletCurrency.Btc,
       id: crypto.randomUUID() as WalletId,
+      accountId: crypto.randomUUID() as AccountId,
     }),
   )
 })

diff --git a/core/api/test/unit/domain/wallet-invoices/wallet-invoice-checker.spec.ts b/core/api/test/unit/domain/wallet-invoices/wallet-invoice-checker.spec.ts
@@ -8,7 +8,11 @@ const goodWalletInvoice: WalletInvoiceWithOptionalLnInvoice = {
   secret: "secretPreImage" as SecretPreImage,
   selfGenerated: true,
   pubkey: "pubkey" as Pubkey,
-  recipientWalletDescriptor: { id: "walletId" as WalletId, currency: WalletCurrency.Usd },
+  recipientWalletDescriptor: {
+    id: "walletId" as WalletId,
+    accountId: crypto.randomUUID() as AccountId,
+    currency: WalletCurrency.Usd,
+  },
   paid: false,
   createdAt: new Date(Date.now()),
   processingCompleted: false,

diff --git a/core/api/test/unit/domain/wallet-invoices/wallet-invoice-receiver.spec.ts b/core/api/test/unit/domain/wallet-invoices/wallet-invoice-receiver.spec.ts
@@ -30,21 +30,15 @@ describe("WalletInvoiceReceiver", () => {
 
   const recipientAccountId = "recipientAccountId" as AccountId
 
-  const partialRecipientBtcWalletDescriptor = {
+  const recipientBtcWalletDescriptor = {
     id: "recipientBtcWalletId" as WalletId,
     currency: WalletCurrency.Btc,
-  }
-  const recipientBtcWalletDescriptor = {
-    ...partialRecipientBtcWalletDescriptor,
     accountId: recipientAccountId,
   }
 
-  const partialRecipientUsdWalletDescriptor = {
+  const recipientUsdWalletDescriptor = {
     id: "recipientUsdWalletId" as WalletId,
     currency: WalletCurrency.Usd,
-  }
-  const recipientUsdWalletDescriptor = {
-    ...partialRecipientUsdWalletDescriptor,
     accountId: recipientAccountId,
   }
 
@@ -77,7 +71,7 @@ describe("WalletInvoiceReceiver", () => {
       pubkey: "pubkey" as Pubkey,
       usdAmount: undefined,
       paid: false,
-      recipientWalletDescriptor: partialRecipientBtcWalletDescriptor,
+      recipientWalletDescriptor: recipientBtcWalletDescriptor,
       createdAt: new Date(),
       lnInvoice: mockLnInvoice,
       processingCompleted: false,
@@ -116,7 +110,7 @@ describe("WalletInvoiceReceiver", () => {
       const amountUsdInvoice: WalletInvoice = {
         paymentHash: "paymentHash" as PaymentHash,
         secret: "secret" as SecretPreImage,
-        recipientWalletDescriptor: partialRecipientUsdWalletDescriptor,
+        recipientWalletDescriptor: recipientUsdWalletDescriptor,
         selfGenerated: false,
         pubkey: "pubkey" as Pubkey,
         usdAmount: UsdPaymentAmount(BigInt(100)),
@@ -153,7 +147,7 @@ describe("WalletInvoiceReceiver", () => {
       const noAmountUsdInvoice: WalletInvoice = {
         paymentHash: "paymentHash" as PaymentHash,
         secret: "secret" as SecretPreImage,
-        recipientWalletDescriptor: partialRecipientUsdWalletDescriptor,
+        recipientWalletDescriptor: recipientUsdWalletDescriptor,
         selfGenerated: false,
         pubkey: "pubkey" as Pubkey,
         paid: false,

diff --git a/core/api/test/unit/domain/wallet-invoices/wallet-invoice-status-checker.spec.ts b/core/api/test/unit/domain/wallet-invoices/wallet-invoice-status-checker.spec.ts
@@ -9,7 +9,11 @@ const baseInvoice: WalletInvoice = {
   secret: "secretPreImage" as SecretPreImage,
   selfGenerated: true,
   pubkey: "pubkey" as Pubkey,
-  recipientWalletDescriptor: { id: "walletId" as WalletId, currency: WalletCurrency.Usd },
+  recipientWalletDescriptor: {
+    id: "walletId" as WalletId,
+    currency: WalletCurrency.Usd,
+    accountId: "accountId" as AccountId,
+  },
   paid: false,
   createdAt: new Date(Date.now()),
   processingCompleted: false,