From 5b786c204cc5f0551dd89c549db973ea625ce405 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 12 Jul 2023 14:39:07 +0200 Subject: [PATCH 1/2] Improve errors in `findWalletForRedemption` fn Throw error when currently, there are no active wallets in the network and when the user requested redemption for all active wallets in the network using the same Bitcoin address - in that case, a user should use another Bitcoin address. --- typescript/src/redemption.ts | 14 ++++++++ typescript/test/redemption.test.ts | 57 +++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/typescript/src/redemption.ts b/typescript/src/redemption.ts index f8a0665cf..e1a631101 100644 --- a/typescript/src/redemption.ts +++ b/typescript/src/redemption.ts @@ -441,6 +441,7 @@ export async function findWalletForRedemption( } | undefined = undefined let maxAmount = BigNumber.from(0) + let activeWalletsCounter = 0 for (const wallet of wallets) { const { walletPublicKeyHash } = wallet @@ -456,6 +457,7 @@ export async function findWalletForRedemption( ) continue } + activeWalletsCounter++ // Wallet must have a main UTXO that can be determined. const mainUtxo = await determineWalletMainUtxo( @@ -511,6 +513,18 @@ export async function findWalletForRedemption( ) } + if (activeWalletsCounter === 0) { + throw new Error("Currently, there are no active wallets in the network.") + } + + // Cover a corner case when the user requested redemption for all active + // wallets in the network using the same Bitcoin address. + if (!walletData && activeWalletsCounter > 0 && maxAmount.eq(0)) { + throw new Error( + "All active wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." + ) + } + if (!walletData) throw new Error( `Could not find a wallet with enough funds. Maximum redemption amount is ${maxAmount} Satoshi.` diff --git a/typescript/test/redemption.test.ts b/typescript/test/redemption.test.ts index e2efa36d1..21af6263d 100644 --- a/typescript/test/redemption.test.ts +++ b/typescript/test/redemption.test.ts @@ -1468,7 +1468,7 @@ describe("Redemption", () => { bitcoinClient ) ).to.be.rejectedWith( - "Could not find a wallet with enough funds. Maximum redemption amount is 0 Satoshi." + "Currently, there are no active wallets in the network." ) }) } @@ -1676,6 +1676,61 @@ describe("Redemption", () => { }) } ) + + context( + "when all active wallets has pending redemption for a given Bitcoin address", + () => { + const amount: BigNumber = BigNumber.from("1000000") // 0.01 BTC + const redeemerOutputScript = + findWalletForRedemptionData.pendingRedemption.redeemerOutputScript + + beforeEach(async () => { + const walletPublicKeyHash = + findWalletForRedemptionData.walletWithPendingRedemption.event + .walletPublicKeyHash + + const pendingRedemptions = new Map< + BigNumberish, + RedemptionRequest + >() + + const pendingRedemption1 = MockBridge.buildRedemptionKey( + walletPublicKeyHash.toString(), + redeemerOutputScript + ) + + const pendingRedemption2 = MockBridge.buildRedemptionKey( + findWalletForRedemptionData.liveWallet.event.walletPublicKeyHash.toString(), + redeemerOutputScript + ) + + pendingRedemptions.set( + pendingRedemption1, + findWalletForRedemptionData.pendingRedemption + ) + + pendingRedemptions.set( + pendingRedemption2, + findWalletForRedemptionData.pendingRedemption + ) + bridge.setPendingRedemptions(pendingRedemptions) + }) + + it("should throw an error", async () => { + await expect( + findWalletForRedemption( + amount, + redeemerOutputScript, + BitcoinNetwork.Testnet, + bridge, + bitcoinClient + ) + ).to.be.rejectedWith( + "All active wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." + ) + }) + } + ) }) }) }) From 19e2dfff19b8bed26b98fe9e94a302ab1f183470 Mon Sep 17 00:00:00 2001 From: Rafal Czajkowski Date: Wed, 12 Jul 2023 15:08:43 +0200 Subject: [PATCH 2/2] Adjust a nomenclature in `findWalletForRedemption` There is always only one "active" wallet in the system. This is the one that accepts new deposits. What we are talking about here are "live" wallets that are operable from the Bridge's standpoint. Here we rename the `activeWalletsCounter` variable to `liveWalletsCounter` and adjust the nomenclature everywherre accordingly. --- typescript/src/redemption.ts | 25 +++++++++++++------------ typescript/test/redemption.test.ts | 4 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/typescript/src/redemption.ts b/typescript/src/redemption.ts index e1a631101..f3b2474a0 100644 --- a/typescript/src/redemption.ts +++ b/typescript/src/redemption.ts @@ -412,11 +412,12 @@ export async function getRedemptionRequest( } /** - * Finds the oldest active wallet that has enough BTC to handle a redemption request. + * Finds the oldest live wallet that has enough BTC to handle a redemption + * request. * @param amount The amount to be redeemed in satoshis. - * @param redeemerOutputScript The redeemer output script the redeemed funds - * are supposed to be locked on. Must be un-prefixed and not prepended - * with length. + * @param redeemerOutputScript The redeemer output script the redeemed funds are + * supposed to be locked on. Must be un-prefixed and not prepended with + * length. * @param bitcoinNetwork Bitcoin network. * @param bridge The handle to the Bridge on-chain contract. * @param bitcoinClient Bitcoin client used to interact with the network. @@ -441,7 +442,7 @@ export async function findWalletForRedemption( } | undefined = undefined let maxAmount = BigNumber.from(0) - let activeWalletsCounter = 0 + let liveWalletsCounter = 0 for (const wallet of wallets) { const { walletPublicKeyHash } = wallet @@ -457,7 +458,7 @@ export async function findWalletForRedemption( ) continue } - activeWalletsCounter++ + liveWalletsCounter++ // Wallet must have a main UTXO that can be determined. const mainUtxo = await determineWalletMainUtxo( @@ -513,15 +514,15 @@ export async function findWalletForRedemption( ) } - if (activeWalletsCounter === 0) { - throw new Error("Currently, there are no active wallets in the network.") + if (liveWalletsCounter === 0) { + throw new Error("Currently, there are no live wallets in the network.") } - // Cover a corner case when the user requested redemption for all active - // wallets in the network using the same Bitcoin address. - if (!walletData && activeWalletsCounter > 0 && maxAmount.eq(0)) { + // Cover a corner case when the user requested redemption for all live wallets + // in the network using the same Bitcoin address. + if (!walletData && liveWalletsCounter > 0 && maxAmount.eq(0)) { throw new Error( - "All active wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." + "All live wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." ) } diff --git a/typescript/test/redemption.test.ts b/typescript/test/redemption.test.ts index 21af6263d..58bac10cc 100644 --- a/typescript/test/redemption.test.ts +++ b/typescript/test/redemption.test.ts @@ -1468,7 +1468,7 @@ describe("Redemption", () => { bitcoinClient ) ).to.be.rejectedWith( - "Currently, there are no active wallets in the network." + "Currently, there are no live wallets in the network." ) }) } @@ -1726,7 +1726,7 @@ describe("Redemption", () => { bitcoinClient ) ).to.be.rejectedWith( - "All active wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." + "All live wallets in the network have the pending redemption for a given Bitcoin address. Please use another Bitcoin address." ) }) }