Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve find wallet for redemption logic #651

Merged
merged 7 commits into from
Jul 12, 2023
12 changes: 12 additions & 0 deletions typescript/src/bitcoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea for the future: we could add a parameter specifying the required number of confirmations.

The "confirmed transaction" is a fuzzy term. In this context, it's a transaction with 1 confirmation but in some other contexts, we do not consider the transaction as confirmed if it has less than 6 confirmations.


/**
* Gets the full transaction object for given transaction hash.
* @param transactionHash - Hash of the transaction.
Expand Down
55 changes: 54 additions & 1 deletion typescript/src/electrum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import bcoin from "bcoin"
import pTimeout from "p-timeout"
import {
Client as BitcoinClient,
createOutputScriptFromAddress,
RawTransaction,
Transaction,
TransactionHash,
Expand Down Expand Up @@ -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 }
Expand All @@ -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<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Instead of having await, I think we could use .then to chain promises together.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think async/await style leads to more readable code and is generally preferred over then. It is also widely used in our codebase and I think mixing both styles is not necessarily the best idea.

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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting blockchain_scripthash_getHistory to accept limit as a parameter but yeah, I see in the linked docs it is not possible. 😞


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}
Expand Down
57 changes: 13 additions & 44 deletions typescript/src/redemption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand All @@ -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.
Expand Down
124 changes: 123 additions & 1 deletion typescript/src/wallet.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
r-czajkowski marked this conversation as resolved.
Show resolved Hide resolved
}

// 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))
}
Loading