fix(api): idempotency bug in registerBroadcastHandler (#3558)

* test: add idempotency runs for registerBroadcastedPayout handler

* chore: add new 'onchain-reconcile' module to facade

* chore: add onChainReconciliationTxn check methods

* refactor: change tx filter when setting payoutId

* fix: add reconciliation-txn-recorded check to register-broadcast handler

This is to fix the idempotency bug introduced in:
#3550

core/api/src/app/wallets/register-broadcasted-payout-txn.ts

@@ -21,6 +21,9 @@ export const registerBroadcastedPayout = async ({
   const { estimatedProtocolFee } = setTxIdResult
   if (estimatedProtocolFee.amount === proportionalFee.amount) return true
 
+  const isRecorded = await LedgerFacade.isOnChainFeeReconciliationRecorded(payoutId)
+  if (isRecorded !== false) return isRecorded
+
   const { metadata } = LedgerFacade.OnChainFeeReconciliationLedgerMetadata({
     payoutId,
     txHash: txId,

core/api/src/services/ledger/domain/errors.types.d.ts

@@ -0,0 +1 @@
+type LedgerFacadeError = import("./errors").LedgerFacadeError

core/api/src/services/ledger/facade/index.ts

@@ -2,6 +2,7 @@ export * from "./intraledger"
 export * from "./offchain-receive"
 export * from "./offchain-send"
 export * from "./onchain-receive"
+export * from "./onchain-reconcile"
 export * from "./onchain-send"
 export * from "./reconciliation"
 export * from "./static-account-ids"

core/api/src/services/ledger/facade/onchain-receive.ts

@@ -1,7 +1,6 @@
 import { MainBook } from "../books"
 
 import { EntryBuilder, toLedgerAccountDescriptor } from "../domain"
-import { FeeOnlyEntryBuilder } from "../domain/fee-only-entry-builder"
 import { persistAndReturnEntry } from "../helpers"
 
 import { staticAccountIds } from "./static-account-ids"
@@ -48,42 +47,3 @@ export const recordReceiveOnChain = async ({
 
   return persistAndReturnEntry({ entry, ...txMetadata })
 }
-
-export const recordReceiveOnChainFeeReconciliation = async ({
-  estimatedFee,
-  actualFee,
-  metadata,
-}: {
-  estimatedFee: BtcPaymentAmount
-  actualFee: BtcPaymentAmount
-  metadata: AddOnChainFeeReconciliationLedgerMetadata
-}) => {
-  const accountIds = await staticAccountIds()
-  if (accountIds instanceof Error) return accountIds
-
-  let entry = MainBook.entry("")
-  if (actualFee.amount > estimatedFee.amount) {
-    const btcFeeDifference = calc.sub(actualFee, estimatedFee)
-    const builder = FeeOnlyEntryBuilder({
-      staticAccountIds: accountIds,
-      entry,
-      metadata,
-      btcFee: btcFeeDifference,
-    })
-    entry = builder.debitBankOwner().creditOnChain()
-  } else {
-    const btcFeeDifference = calc.sub(estimatedFee, actualFee)
-    const builder = FeeOnlyEntryBuilder({
-      staticAccountIds: accountIds,
-      entry,
-      metadata,
-      btcFee: btcFeeDifference,
-    })
-    entry = builder.debitOnChain().creditBankOwner()
-  }
-
-  return persistAndReturnEntry({
-    entry,
-    hash: metadata.hash,
-  })
-}

core/api/src/services/ledger/facade/onchain-reconcile.ts

@@ -0,0 +1,76 @@
+import { MainBook } from "../books"
+
+import { translateToLedgerTx } from ".."
+import { getBankOwnerWalletId } from "../caching"
+import { UnknownLedgerError } from "../domain/errors"
+import { persistAndReturnEntry } from "../helpers"
+import { FeeOnlyEntryBuilder } from "../domain/fee-only-entry-builder"
+
+import { staticAccountIds } from "./static-account-ids"
+
+import { LedgerTransactionType, toLiabilitiesWalletId } from "@/domain/ledger"
+import { AmountCalculator } from "@/domain/shared"
+
+const calc = AmountCalculator()
+
+export const recordReceiveOnChainFeeReconciliation = async ({
+  estimatedFee,
+  actualFee,
+  metadata,
+}: {
+  estimatedFee: BtcPaymentAmount
+  actualFee: BtcPaymentAmount
+  metadata: AddOnChainFeeReconciliationLedgerMetadata
+}) => {
+  const accountIds = await staticAccountIds()
+  if (accountIds instanceof Error) return accountIds
+
+  let entry = MainBook.entry("")
+  if (actualFee.amount > estimatedFee.amount) {
+    const btcFeeDifference = calc.sub(actualFee, estimatedFee)
+    const builder = FeeOnlyEntryBuilder({
+      staticAccountIds: accountIds,
+      entry,
+      metadata,
+      btcFee: btcFeeDifference,
+    })
+    entry = builder.debitBankOwner().creditOnChain()
+  } else {
+    const btcFeeDifference = calc.sub(estimatedFee, actualFee)
+    const builder = FeeOnlyEntryBuilder({
+      staticAccountIds: accountIds,
+      entry,
+      metadata,
+      btcFee: btcFeeDifference,
+    })
+    entry = builder.debitOnChain().creditBankOwner()
+  }
+
+  return persistAndReturnEntry({
+    entry,
+    hash: metadata.hash,
+  })
+}
+
+export const isOnChainFeeReconciliationTxn = (
+  txn: LedgerTransaction<WalletCurrency>,
+): boolean =>
+  txn.type === LedgerTransactionType.OnchainPayment && txn.address === undefined
+
+export const isOnChainFeeReconciliationRecorded = async (
+  payoutId: PayoutId,
+): Promise<boolean | LedgerFacadeError> => {
+  try {
+    const bankOwnerWalletId = await getBankOwnerWalletId()
+    const { results } = await MainBook.ledger({
+      payout_id: payoutId,
+      account: toLiabilitiesWalletId(bankOwnerWalletId),
+    })
+    const txns = results.map((tx) => translateToLedgerTx(tx))
+
+    const reconciliationTxn = txns.find((txn) => isOnChainFeeReconciliationTxn(txn))
+    return reconciliationTxn !== undefined
+  } catch (err) {
+    return new UnknownLedgerError(err)
+  }
+}

core/api/src/services/ledger/facade/onchain-send.ts

@@ -14,6 +14,7 @@ import { TransactionsMetadataRepository } from "../services"
 
 import { translateToLedgerTx } from ".."
 
+import { isOnChainFeeReconciliationTxn } from "./onchain-reconcile"
 import { staticAccountIds } from "./static-account-ids"
 
 import {
@@ -168,7 +169,7 @@ export const setOnChainTxIdByPayoutId = async ({
 
   const bankOwnerWalletId = await getBankOwnerWalletId()
   const bankOwnerTxns = txns.filter(
-    (txn) => txn.satsFee && txn.walletId === bankOwnerWalletId,
+    (txn) => txn.walletId === bankOwnerWalletId && !isOnChainFeeReconciliationTxn(txn),
   )
   if (bankOwnerTxns.length !== 1) {
     return new InvalidLedgerTransactionStateError(`payoutId: ${payoutId}`)

core/api/test/legacy-integration/02-user-wallet/02-bria-handlers.spec.ts

@@ -514,6 +514,15 @@ describe("Bria Event Handlers", () => {
       })
       expect(res).toBe(true)
 
+      // Idempotency check
+      const resRerun = await registerBroadcastedPayout({
+        payoutId,
+        proportionalFee,
+        txId,
+        vout,
+      })
+      expect(resRerun).toBe(true)
+
       // Run after-broadcast checks
       const txnsAfter = await LedgerFacade.getTransactionsByPayoutId(payoutId)
       if (txnsAfter instanceof Error) throw txnsAfter
@@ -562,6 +571,15 @@ describe("Bria Event Handlers", () => {
       })
       expect(res).toBe(true)
 
+      // Idempotency check
+      const resRerun = await registerBroadcastedPayout({
+        payoutId,
+        proportionalFee,
+        txId,
+        vout,
+      })
+      expect(resRerun).toBe(true)
+
       // Run after-broadcast checks
       const txnsAfter = await LedgerFacade.getTransactionsByPayoutId(payoutId)
       if (txnsAfter instanceof Error) throw txnsAfter
@@ -617,6 +635,15 @@ describe("Bria Event Handlers", () => {
       })
       expect(res).toBe(true)
 
+      // Idempotency check
+      const resRerun = await registerBroadcastedPayout({
+        payoutId,
+        proportionalFee,
+        txId,
+        vout,
+      })
+      expect(resRerun).toBe(true)
+
       // Run after-broadcast checks
       const txnsAfter = await LedgerFacade.getTransactionsByPayoutId(payoutId)
       if (txnsAfter instanceof Error) throw txnsAfter
@@ -677,6 +704,15 @@ describe("Bria Event Handlers", () => {
       })
       expect(res).toBeInstanceOf(UnknownLedgerError)
 
+      // Idempotency check
+      const resRerun = await registerBroadcastedPayout({
+        payoutId,
+        proportionalFee,
+        txId,
+        vout,
+      })
+      expect(resRerun).toBeInstanceOf(UnknownLedgerError)
+
       spy.mockRestore()
 
       // Run after-broadcast checks