Improve find wallet for redemption logic (#651)

Refs: threshold-network/token-dashboard#553

Here we improve the logic of `findWalletForRedemption` function to
handle the corner case when a wallet's main UTXO known to the `Bridge`
was actually spent on Bitcoin but that change was not yet reflected to
the `Bridge`. So far, `findWalletForRedemption` relied on
`findAllUnspentTransactionOutputs` which does not return spent UTXOs so
determining the actual main UTXO was not possible in the aforementioned
corner case. Now, the `findWalletForRedemption` looks for recent wallet
transaction history on Bitcoin and tries to match the actual main UTXO
based on that. This change will allow submitting new redemption requests
without downtime, even if the target wallet is performing an action at
the moment.

typescript/src/bitcoin.ts

@@ -333,6 +333,18 @@ export interface Client {
     address: string
   ): Promise<UnspentTransactionOutput[]>
 
+  /**
+   * Gets the history of confirmed transactions for given Bitcoin address.
+   * Returned transactions are sorted from oldest to newest. The returned
+   * result does not contain unconfirmed transactions living in the mempool
+   * at the moment of request.
+   * @param address - Bitcoin address transaction history should be determined for.
+   * @param limit - Optional parameter that can limit the resulting list to
+   *        a specific number of last transaction. For example, limit = 5 will
+   *        return only the last 5 transactions for the given address.
+   */
+  getTransactionHistory(address: string, limit?: number): Promise<Transaction[]>
+
   /**
    * Gets the full transaction object for given transaction hash.
    * @param transactionHash - Hash of the transaction.

typescript/src/electrum.ts

@@ -2,6 +2,7 @@ import bcoin from "bcoin"
 import pTimeout from "p-timeout"
 import {
   Client as BitcoinClient,
+  createOutputScriptFromAddress,
   RawTransaction,
   Transaction,
   TransactionHash,
@@ -232,7 +233,7 @@ export class Client implements BitcoinClient {
   ): Promise<UnspentTransactionOutput[]> {
     return this.withElectrum<UnspentTransactionOutput[]>(
       async (electrum: Electrum) => {
-        const script = bcoin.Script.fromAddress(address).toRaw().toString("hex")
+        const script = createOutputScriptFromAddress(address).toString()
 
         // eslint-disable-next-line camelcase
         type UnspentOutput = { tx_pos: number; value: number; tx_hash: string }
@@ -253,6 +254,58 @@ export class Client implements BitcoinClient {
     )
   }
 
+  // eslint-disable-next-line valid-jsdoc
+  /**
+   * @see {BitcoinClient#getTransactionHistory}
+   */
+  getTransactionHistory(
+    address: string,
+    limit?: number
+  ): Promise<Transaction[]> {
+    return this.withElectrum<Transaction[]>(async (electrum: Electrum) => {
+      const script = createOutputScriptFromAddress(address).toString()
+
+      // eslint-disable-next-line camelcase
+      type HistoryItem = { height: number; tx_hash: string }
+
+      let historyItems: HistoryItem[] = await this.withBackoffRetrier<
+        HistoryItem[]
+      >()(async () => {
+        return await electrum.blockchain_scripthash_getHistory(
+          computeScriptHash(script)
+        )
+      })
+
+      // According to https://electrumx.readthedocs.io/en/latest/protocol-methods.html#blockchain-scripthash-get-history
+      // unconfirmed items living in the mempool are appended at the end of the
+      // returned list and their height value is either -1 or 0. That means
+      // we need to take all items with height >0 to obtain a confirmed txs
+      // history.
+      historyItems = historyItems.filter((item) => item.height > 0)
+
+      // The list returned from blockchain.scripthash.get_history is sorted by
+      // the block height in the ascending order though we are sorting it
+      // again just in case (e.g. API contract changes).
+      historyItems = historyItems.sort(
+        (item1, item2) => item1.height - item2.height
+      )
+
+      if (
+        typeof limit !== "undefined" &&
+        limit > 0 &&
+        historyItems.length > limit
+      ) {
+        historyItems = historyItems.slice(-limit)
+      }
+
+      const transactions = historyItems.map((item) =>
+        this.getTransaction(TransactionHash.from(item.tx_hash))
+      )
+
+      return Promise.all(transactions)
+    })
+  }
+
   // eslint-disable-next-line valid-jsdoc
   /**
    * @see {BitcoinClient#getTransaction}

typescript/src/redemption.ts

@@ -7,11 +7,10 @@ import {
   UnspentTransactionOutput,
   Client as BitcoinClient,
   TransactionHash,
-  encodeToBitcoinAddress,
 } from "./bitcoin"
 import { Bridge, Identifier, TBTCToken } from "./chain"
 import { assembleTransactionProof } from "./proof"
-import { WalletState } from "./wallet"
+import { determineWalletMainUtxo, WalletState } from "./wallet"
 import { BitcoinNetwork } from "./bitcoin-network"
 import { Hex } from "./hex"
 
@@ -445,7 +444,7 @@ export async function findWalletForRedemption(
 
   for (const wallet of wallets) {
     const { walletPublicKeyHash } = wallet
-    const { state, mainUtxoHash, walletPublicKey, pendingRedemptionsValue } =
+    const { state, walletPublicKey, pendingRedemptionsValue } =
       await bridge.wallets(walletPublicKeyHash)
 
     // Wallet must be in Live state.
@@ -458,18 +457,20 @@ export async function findWalletForRedemption(
       continue
     }
 
-    if (
-      mainUtxoHash.equals(
-        Hex.from(
-          "0x0000000000000000000000000000000000000000000000000000000000000000"
-        )
-      )
-    ) {
+    // Wallet must have a main UTXO that can be determined.
+    const mainUtxo = await determineWalletMainUtxo(
+      walletPublicKeyHash,
+      bridge,
+      bitcoinClient,
+      bitcoinNetwork
+    )
+    if (!mainUtxo) {
       console.debug(
-        `Main utxo not set for wallet public ` +
-          `key hash(${walletPublicKeyHash.toString()}). ` +
+        `Could not find matching UTXO on chains ` +
+          `for wallet public key hash (${walletPublicKeyHash.toString()}). ` +
           `Continue the loop execution to the next wallet...`
       )
+      continue
     }
 
     const pendingRedemption = await bridge.pendingRedemptions(
@@ -489,38 +490,6 @@ export async function findWalletForRedemption(
       continue
     }
 
-    const walletBitcoinAddress = encodeToBitcoinAddress(
-      wallet.walletPublicKeyHash.toString(),
-      true,
-      bitcoinNetwork
-    )
-
-    // TODO: In case a wallet is working on something (e.g. redemption) and a
-    // Bitcoin transaction was already submitted by the wallet to the bitcoin
-    // chain (new utxo returned from bitcoin client), but proof hasn't been
-    // submitted yet to the Bridge (old main utxo returned from the Bridge) the
-    // `findWalletForRedemption` function will not find such a wallet. To cover
-    // this case, we should take, for example, the last 5 transactions made by
-    // the wallet into account. We will address this issue in a follow-up work.
-    const utxos = await bitcoinClient.findAllUnspentTransactionOutputs(
-      walletBitcoinAddress
-    )
-
-    // We need to find correct utxo- utxo components must point to the recent
-    // main UTXO of the given wallet, as currently known on the chain.
-    const mainUtxo = utxos.find((utxo) =>
-      mainUtxoHash.equals(bridge.buildUtxoHash(utxo))
-    )
-
-    if (!mainUtxo) {
-      console.debug(
-        `Could not find matching UTXO on chains ` +
-          `for wallet public key hash(${walletPublicKeyHash.toString()}). ` +
-          `Continue the loop execution to the next wallet...`
-      )
-      continue
-    }
-
     const walletBTCBalance = mainUtxo.value.sub(pendingRedemptionsValue)
 
     // Save the max possible redemption amount.

typescript/src/wallet.ts

@@ -1,6 +1,14 @@
 import { BigNumber } from "ethers"
 import { Hex } from "./hex"
-import { Event, Identifier } from "./chain"
+import { Bridge, Event, Identifier } from "./chain"
+import {
+  Client as BitcoinClient,
+  createOutputScriptFromAddress,
+  encodeToBitcoinAddress,
+  TransactionOutput,
+  UnspentTransactionOutput,
+} from "./bitcoin"
+import { BitcoinNetwork } from "./bitcoin-network"
 
 /* eslint-disable no-unused-vars */
 export enum WalletState {
@@ -209,3 +217,117 @@ type DkgResult = {
    */
   membersHash: Hex
 }
+
+/**
+ * Determines the plain-text wallet main UTXO currently registered in the
+ * Bridge on-chain contract. The returned main UTXO can be undefined if the
+ * wallet does not have a main UTXO registered in the Bridge at the moment.
+ *
+ * WARNING: THIS FUNCTION CANNOT DETERMINE THE MAIN UTXO IF IT COMES FROM A
+ * BITCOIN TRANSACTION THAT IS NOT ONE OF THE LATEST FIVE TRANSACTIONS
+ * TARGETING THE GIVEN WALLET PUBLIC KEY HASH. HOWEVER, SUCH A CASE IS
+ * VERY UNLIKELY.
+ *
+ * @param walletPublicKeyHash - Public key hash of the wallet.
+ * @param bridge - The handle to the Bridge on-chain contract.
+ * @param bitcoinClient - Bitcoin client used to interact with the network.
+ * @param bitcoinNetwork - Bitcoin network.
+ * @returns Promise holding the wallet main UTXO or undefined value.
+ */
+export async function determineWalletMainUtxo(
+  walletPublicKeyHash: Hex,
+  bridge: Bridge,
+  bitcoinClient: BitcoinClient,
+  bitcoinNetwork: BitcoinNetwork
+): Promise<UnspentTransactionOutput | undefined> {
+  const { mainUtxoHash } = await bridge.wallets(walletPublicKeyHash)
+
+  // Valid case when the wallet doesn't have a main UTXO registered into
+  // the Bridge.
+  if (
+    mainUtxoHash.equals(
+      Hex.from(
+        "0x0000000000000000000000000000000000000000000000000000000000000000"
+      )
+    )
+  ) {
+    return undefined
+  }
+
+  // Declare a helper function that will try to determine the main UTXO for
+  // the given wallet address type.
+  const determine = async (
+    witnessAddress: boolean
+  ): Promise<UnspentTransactionOutput | undefined> => {
+    // Build the wallet Bitcoin address based on its public key hash.
+    const walletAddress = encodeToBitcoinAddress(
+      walletPublicKeyHash.toString(),
+      witnessAddress,
+      bitcoinNetwork
+    )
+
+    // Get the wallet transaction history. The wallet main UTXO registered in the
+    // Bridge almost always comes from the latest BTC transaction made by the wallet.
+    // However, there may be cases where the BTC transaction was made but their
+    // SPV proof is not yet submitted to the Bridge thus the registered main UTXO
+    // points to the second last BTC transaction. In theory, such a gap between
+    // the actual latest BTC transaction and the registered main UTXO in the
+    // Bridge may be even wider. The exact behavior is a wallet implementation
+    // detail and not a protocol invariant so, it may be subject of changes.
+    // To cover the worst possible cases, we always take the five latest
+    // transactions made by the wallet for consideration.
+    const walletTransactions = await bitcoinClient.getTransactionHistory(
+      walletAddress,
+      5
+    )
+
+    // Get the wallet script based on the wallet address. This is required
+    // to find transaction outputs that lock funds on the wallet.
+    const walletScript = createOutputScriptFromAddress(walletAddress)
+    const isWalletOutput = (output: TransactionOutput) =>
+      walletScript.equals(output.scriptPubKey)
+
+    // Start iterating from the latest transaction as the chance it matches
+    // the wallet main UTXO is the highest.
+    for (let i = walletTransactions.length - 1; i >= 0; i--) {
+      const walletTransaction = walletTransactions[i]
+
+      // Find the output that locks the funds on the wallet. Only such an output
+      // can be a wallet main UTXO.
+      const outputIndex = walletTransaction.outputs.findIndex(isWalletOutput)
+
+      // Should never happen as all transactions come from wallet history. Just
+      // in case check whether the wallet output was actually found.
+      if (outputIndex < 0) {
+        console.error(
+          `wallet output for transaction ${walletTransaction.transactionHash.toString()} not found`
+        )
+        continue
+      }
+
+      // Build a candidate UTXO instance based on the detected output.
+      const utxo: UnspentTransactionOutput = {
+        transactionHash: walletTransaction.transactionHash,
+        outputIndex: outputIndex,
+        value: walletTransaction.outputs[outputIndex].value,
+      }
+
+      // Check whether the candidate UTXO hash matches the main UTXO hash stored
+      // on the Bridge.
+      if (mainUtxoHash.equals(bridge.buildUtxoHash(utxo))) {
+        return utxo
+      }
+    }
+
+    return undefined
+  }
+
+  // The most common case is that the wallet uses a witness address for all
+  // operations. Try to determine the main UTXO for that address first as the
+  // chance for success is the highest here.
+  const mainUtxo = await determine(true)
+
+  // In case the main UTXO was not found for witness address, there is still
+  // a chance it exists for the legacy wallet address.
+  return mainUtxo ?? (await determine(false))
+}