fix: re-order onchain payment logic (#1871)

* fix: re-order onchain payment logic

* test: add test to support pending and not pending errors

* fix: update return for revert functions

src/app/wallets/send-on-chain.ts

@@ -1,40 +1,47 @@
-import { getCurrentPrice } from "@app/prices"
+import crypto from "crypto"
+
 import {
   BTC_NETWORK,
   getFeesConfig,
   getOnChainWalletConfig,
   ONCHAIN_SCAN_DEPTH_OUTGOING,
 } from "@config"
-import { checkedToSats, checkedToTargetConfs, toSats } from "@domain/bitcoin"
+
+import { getCurrentPrice } from "@app/prices"
+
+import { DisplayCurrency } from "@domain/fiat"
+import { WalletCurrency } from "@domain/shared"
 import { PaymentSendStatus } from "@domain/bitcoin/lightning"
-import { checkedToOnChainAddress, TxDecoder } from "@domain/bitcoin/onchain"
+import { ResourceExpiredLockServiceError } from "@domain/lock"
+import { DisplayCurrencyConverter } from "@domain/fiat/display-currency"
+import { ImbalanceCalculator } from "@domain/ledger/imbalance-calculator"
+import { checkedToSats, checkedToTargetConfs, toSats } from "@domain/bitcoin"
+import { PaymentInputValidator, WithdrawalFeeCalculator } from "@domain/wallets"
 import {
   InsufficientBalanceError,
   LessThanDustThresholdError,
   NotImplementedError,
   RebalanceNeededError,
   SelfPaymentError,
 } from "@domain/errors"
-import { DisplayCurrencyConverter } from "@domain/fiat/display-currency"
-import { PaymentInputValidator, WithdrawalFeeCalculator } from "@domain/wallets"
-import { WalletCurrency } from "@domain/shared"
+import {
+  checkedToOnChainAddress,
+  CPFPAncestorLimitReachedError,
+  InsufficientOnChainFundsError,
+  TxDecoder,
+} from "@domain/bitcoin/onchain"
+
 import { LockService } from "@services/lock"
+import { baseLogger } from "@services/logger"
 import { LedgerService } from "@services/ledger"
 import { OnChainService } from "@services/lnd/onchain-service"
-import { baseLogger } from "@services/logger"
+import { addAttributesToCurrentSpan } from "@services/tracing"
+import { NotificationsService } from "@services/notifications"
 import {
   AccountsRepository,
   UsersRepository,
   WalletsRepository,
 } from "@services/mongoose"
-import { NotificationsService } from "@services/notifications"
-import { addAttributesToCurrentSpan } from "@services/tracing"
-
-import { ImbalanceCalculator } from "@domain/ledger/imbalance-calculator"
-
-import { ResourceExpiredLockServiceError } from "@domain/lock"
-
-import { DisplayCurrency } from "@domain/fiat"
 
 import {
   checkIntraledgerLimits,
@@ -322,35 +329,6 @@ const executePaymentViaOnChain = async ({
       return new ResourceExpiredLockServiceError(signal.error?.message)
     }
 
-    const txHash = await onChainService.payToAddress({
-      address,
-      amount: amountToSend,
-      targetConfirmations,
-    })
-    if (txHash instanceof Error) {
-      logger.error(
-        { err: txHash, address, tokens: amountToSend, success: false },
-        "Impossible to sendToChainAddress",
-      )
-      return txHash
-    }
-
-    let minerFee: Satoshis
-
-    const minerFee_ = await onChainService.lookupOnChainFee({
-      txHash,
-      scanDepth: ONCHAIN_SCAN_DEPTH_OUTGOING,
-    })
-    if (minerFee_ instanceof Error) {
-      logger.error({ err: minerFee_ }, "impossible to get fee for onchain payment")
-      addAttributesToCurrentSpan({
-        "payOnChainByWalletId.errorGettingMinerFee": true,
-      })
-      minerFee = estimatedFee
-    } else {
-      minerFee = minerFee_
-    }
-
     const imbalanceCalculator = ImbalanceCalculator({
       method: feeConfig.withdrawMethod,
       volumeLightningFn: LedgerService().lightningTxBaseVolumeSince,
@@ -361,6 +339,13 @@ const executePaymentViaOnChain = async ({
     const imbalance = await imbalanceCalculator.getSwapOutImbalance(senderWallet.id)
     if (imbalance instanceof Error) return imbalance
 
+    const minerFee = await onChainService.getOnChainFeeEstimate({
+      amount: amountToSend,
+      address,
+      targetConfirmations,
+    })
+    if (minerFee instanceof Error) return minerFee
+
     const fees = withdrawFeeCalculator.onChainWithdrawalFee({
       amount: amountToSend,
       minerFee,
@@ -370,22 +355,22 @@ const executePaymentViaOnChain = async ({
 
     const totalFee = fees.totalFee
     const bankFee = fees.bankFee
+    const sats = toSats(amountToSend + totalFee)
+    const amountDisplayCurrency = dCConverter.fromSats(sats)
+    const totalFeeDisplayCurrency = dCConverter.fromSats(totalFee)
 
     addAttributesToCurrentSpan({
-      "payOnChainByWalletId.actualMinerFee": `${minerFee}`,
+      "payOnChainByWalletId.estimatedFee": `${estimatedFee}`,
+      "payOnChainByWalletId.estimatedMinerFee": `${minerFee}`,
       "payOnChainByWalletId.totalFee": `${totalFee}`,
       "payOnChainByWalletId.bankFee": `${bankFee}`,
     })
 
-    const sats = toSats(amountToSend + totalFee)
-
-    const amountDisplayCurrency = dCConverter.fromSats(sats)
-    const totalFeeDisplayCurrency = dCConverter.fromSats(totalFee)
-
     const journal = await ledgerService.addOnChainTxSend({
       walletId: senderWallet.id,
       walletCurrency: senderWallet.currency,
-      txHash,
+      // we need a temporary hash to be able to search in admin panel
+      txHash: crypto.randomBytes(32).toString("hex") as OnChainTxHash,
       description: memo || "",
       sats,
       totalFee,
@@ -397,6 +382,51 @@ const executePaymentViaOnChain = async ({
     })
     if (journal instanceof Error) return journal
 
+    const txHash = await onChainService.payToAddress({
+      address,
+      amount: amountToSend,
+      targetConfirmations,
+      description: `journal-${journal.journalId}`,
+    })
+    if (
+      txHash instanceof InsufficientOnChainFundsError ||
+      txHash instanceof CPFPAncestorLimitReachedError
+    ) {
+      const reverted = await ledgerService.revertOnChainPayment({
+        journalId: journal.journalId,
+      })
+      if (reverted instanceof Error) return reverted
+      return txHash
+    }
+    if (txHash instanceof Error) {
+      logger.error(
+        { err: txHash, address, tokens: amountToSend, success: false },
+        "Impossible to sendToChainAddress",
+      )
+      return txHash
+    }
+
+    const updated = await ledgerService.setOnChainTxSendHash({
+      journalId: journal.journalId,
+      newTxHash: txHash,
+    })
+    if (updated instanceof Error) return updated
+
+    const finalMinerFee = await onChainService.lookupOnChainFee({
+      txHash,
+      scanDepth: ONCHAIN_SCAN_DEPTH_OUTGOING,
+    })
+    if (finalMinerFee instanceof Error) {
+      logger.error({ err: finalMinerFee }, "impossible to get fee for onchain payment")
+      addAttributesToCurrentSpan({
+        "payOnChainByWalletId.errorGettingMinerFee": true,
+      })
+    }
+
+    addAttributesToCurrentSpan({
+      "payOnChainByWalletId.actualMinerFee": `${finalMinerFee}`,
+    })
+
     return PaymentSendStatus.Success
   })
 }

src/domain/bitcoin/onchain/index.types.d.ts

@@ -69,6 +69,7 @@ type PayToAddressArgs = {
   amount: Satoshis
   address: OnChainAddress
   targetConfirmations: TargetConfirmations
+  description?: string
 }
 
 type IncomingOnChainTxHandler = {

src/domain/ledger/index.types.d.ts

@@ -113,6 +113,11 @@ type AddOnChainTxSendArgs = OnChainTxArgs & {
   totalFeeDisplayCurrency: DisplayCurrencyBaseAmount
 }
 
+type SetOnChainTxSendHashArgs = {
+  journalId: LedgerJournalId
+  newTxHash: OnChainTxHash
+}
+
 type AddColdStorageTxReceiveArgs = {
   txHash: OnChainTxHash
   payeeAddress: OnChainAddress
@@ -235,6 +240,11 @@ type RevertLightningPaymentArgs = {
   paymentHash: PaymentHash
 }
 
+type RevertOnChainPaymentArgs = {
+  journalId: LedgerJournalId
+  description?: string
+}
+
 interface ILedgerService {
   updateMetadataByHash(
     ledgerTxMetadata:
@@ -335,6 +345,8 @@ interface ILedgerService {
     args: AddOnChainTxSendArgs,
   ): Promise<LedgerJournal | LedgerServiceError>
 
+  setOnChainTxSendHash(args: SetOnChainTxSendHashArgs): Promise<true | LedgerServiceError>
+
   addOnChainIntraledgerTxTransfer(
     args: AddOnChainIntraledgerTxTransferArgs,
   ): Promise<LedgerJournal | LedgerServiceError>
@@ -349,7 +361,9 @@ interface ILedgerService {
 
   revertLightningPayment(
     args: RevertLightningPaymentArgs,
-  ): Promise<void | LedgerServiceError>
+  ): Promise<true | LedgerServiceError>
+
+  revertOnChainPayment(args: RevertOnChainPaymentArgs): Promise<true | LedgerServiceError>
 
   getWalletIdByTransactionHash(
     hash: OnChainTxHash,

src/services/ledger/facade.ts

@@ -153,7 +153,7 @@ export const settlePendingLnSend = async (
 export const recordLnSendRevert = async ({
   journalId,
   paymentHash,
-}: RevertLightningPaymentArgs): Promise<void | LedgerServiceError> => {
+}: RevertLightningPaymentArgs): Promise<true | LedgerServiceError> => {
   const reason = "Payment canceled"
   try {
     const savedEntry = await MainBook.void(journalId, reason)
@@ -164,6 +164,7 @@ export const recordLnSendRevert = async ({
       hash: paymentHash,
     }))
     txMetadataRepo.persistAll(txsMetadataToPersist)
+    return true
   } catch (err) {
     return new UnknownLedgerError(err)
   }

src/services/ledger/send.ts

@@ -1,12 +1,14 @@
 import { LedgerTransactionType } from "@domain/ledger"
-import { NotImplementedError } from "@domain/errors"
+import { NotImplementedError, NoTransactionToUpdateError } from "@domain/errors"
 import {
   LedgerServiceError,
   NoTransactionToSettleError,
   UnknownLedgerError,
 } from "@domain/ledger/errors"
 import { WalletCurrency, paymentAmountFromNumber } from "@domain/shared"
 
+import { toObjectId } from "@services/mongoose/utils"
+
 import { LegacyEntryBuilder, toLedgerAccountId } from "./domain"
 
 import { MainBook, Transaction } from "./books"
@@ -63,6 +65,25 @@ export const send = {
     })
   },
 
+  setOnChainTxSendHash: async ({
+    journalId,
+    newTxHash,
+  }: SetOnChainTxSendHashArgs): Promise<true | LedgerServiceError> => {
+    try {
+      const result = await Transaction.updateMany(
+        { _journal: toObjectId(journalId) },
+        { hash: newTxHash },
+      )
+      const success = result.modifiedCount > 0
+      if (!success) {
+        return new NoTransactionToUpdateError()
+      }
+      return true
+    } catch (err) {
+      return new UnknownLedgerError(err)
+    }
+  },
+
   settlePendingLnPayment: async (
     paymentHash: PaymentHash,
   ): Promise<true | LedgerServiceError> => {
@@ -99,7 +120,7 @@ export const send = {
   revertLightningPayment: async ({
     journalId,
     paymentHash,
-  }: RevertLightningPaymentArgs): Promise<void | LedgerServiceError> => {
+  }: RevertLightningPaymentArgs): Promise<true | LedgerServiceError> => {
     const reason = "Payment canceled"
     try {
       const savedEntry = await MainBook.void(journalId, reason)
@@ -110,6 +131,26 @@ export const send = {
         hash: paymentHash,
       }))
       txMetadataRepo.persistAll(txsMetadataToPersist)
+      return true
+    } catch (err) {
+      return new UnknownLedgerError(err)
+    }
+  },
+
+  revertOnChainPayment: async ({
+    journalId,
+    description = "Protocol error",
+  }: RevertOnChainPaymentArgs): Promise<true | LedgerServiceError> => {
+    try {
+      // pending update must be before void to avoid pending voided records
+      await Transaction.updateMany(
+        { _journal: toObjectId(journalId) },
+        { pending: false },
+      )
+
+      await MainBook.void(journalId, description)
+      // TODO: persist to metadata
+      return true
     } catch (err) {
       return new UnknownLedgerError(err)
     }

src/services/lnd/onchain-service.ts

@@ -174,6 +174,7 @@ export const OnChainService = (
     amount,
     address,
     targetConfirmations,
+    description,
   }: PayToAddressArgs): Promise<OnChainTxHash | OnChainServiceError> => {
     try {
       const { id } = await sendToChainAddress({
@@ -182,13 +183,16 @@ export const OnChainService = (
         tokens: amount,
         utxo_confirmations: 1,
         target_confirmations: targetConfirmations,
+        description,
       })
 
       return id as OnChainTxHash
     } catch (err) {
       const errDetails = parseLndErrorDetails(err)
       const match = (knownErrDetail: RegExp): boolean => knownErrDetail.test(errDetails)
       switch (true) {
+        case match(KnownLndErrorDetails.InsufficientFunds):
+          return new InsufficientOnChainFundsError()
         case match(KnownLndErrorDetails.CPFPAncestorLimitReached):
           return new CPFPAncestorLimitReachedError()
         default: