From f073b42e326a64afc730cb2cfe5288ecdbb4c9c5 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Fri, 1 Dec 2023 12:22:22 -0400 Subject: [PATCH 1/9] Renames testRunner to multipleTestExecutionsRunner and clean env before each test execution --- testRunner.js => multipleTestExecutionsRunner.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) rename testRunner.js => multipleTestExecutionsRunner.js (72%) diff --git a/testRunner.js b/multipleTestExecutionsRunner.js similarity index 72% rename from testRunner.js rename to multipleTestExecutionsRunner.js index 430b5775..38c70649 100644 --- a/testRunner.js +++ b/multipleTestExecutionsRunner.js @@ -6,13 +6,22 @@ const timesArg = Number(process.argv[2]); const RUN_ALL_TESTS_THESE_TIMES = timesArg || Number(process.env.RUN_ALL_TESTS_THESE_TIMES) || 1; -console.info(`Will attempt to run tests ${RUN_ALL_TESTS_THESE_TIMES} times.`) +console.info(`Will attempt to run tests ${RUN_ALL_TESTS_THESE_TIMES} times.`); + +const cleanEnvCommand = "kill $(ps -A | grep -e java -e python -e bitcoind | awk '{print $1}')"; + +const ensureCleanEnv = () => { + console.info('Cleaning environment...'); + shell.exec(cleanEnvCommand); + console.info('Environment clean.'); +}; let fails = 0; let attempts = 1; for(let i = 0; i < RUN_ALL_TESTS_THESE_TIMES; i++) { console.info(`Running tests ${attempts} out of ${RUN_ALL_TESTS_THESE_TIMES} times.`); + ensureCleanEnv(); if (shell.exec('npm run test-fail-fast').code !== 0) { fails++; } From 8ec9b900adc652f354caeb5c97d76dec032de44f Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Fri, 1 Dec 2023 12:27:22 -0400 Subject: [PATCH 2/9] Updates package.json run-tests-multiple-times script --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1a9a95a2..fe0e285d 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "scripts": { "test": "mocha --timeout 600000", "test-fail-fast": "mocha -b --timeout 600000", - "run-tests-multiple-times": "node testRunner.js", + "run-tests-multiple-times": "node multipleTestExecutionsRunner.js", "run-single-test-file": "node singleTestFileRunner.js" }, "author": "", From 1d212ec8d6acfdcc8225e630f2512d6a15c117e4 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 15:42:17 -0400 Subject: [PATCH 3/9] Adds fix to wait for txs to get to the mempool before mining --- lib/2wp-utils.js | 30 ++--- lib/rsk-utils.js | 120 ++++++++++++++++-- lib/tests/2wp.js | 4 +- lib/utils.js | 88 +++++++++++++ tests/01_03_01-lock_whitelist_pre_papyrus.js | 8 +- ...03_54-post-papyrus_coinbase_information.js | 1 + 6 files changed, 217 insertions(+), 34 deletions(-) diff --git a/lib/2wp-utils.js b/lib/2wp-utils.js index 33e85aaf..c1bf2c08 100644 --- a/lib/2wp-utils.js +++ b/lib/2wp-utils.js @@ -1,6 +1,6 @@ const expect = require('chai').expect; -const { sendFromCow, mineAndSync, sendTxWithCheck, getUnlockedAddress } = require('./rsk-utils'); -const { wait, retryWithCheck } = require('./utils'); +const { sendFromCow, mineAndSync, sendTxWithCheck, getUnlockedAddress, waitForRskMempoolToGetTxs, waitAndUpdateBridge } = require('./rsk-utils'); +const { retryWithCheck, waitForBtcTxToBeInMempool, waitForBtcMempoolToGetTxs } = require('./utils'); const { getBridge, getLatestActiveForkName } = require('./precompiled-abi-forks-util'); const { getBridgeState } = require('@rsksmart/bridge-state-data-parser'); const btcEthUnitConverter = require('@rsksmart/btc-eth-unit-converter'); @@ -62,7 +62,10 @@ const sendTxToBridge = async (rskTxHelper, amountInRbtc, rskFromAddress, mine = if(!mine) { return; } - await wait(1000); + + // Wait for the rsk tx to be in the rsk mempool before mining + await waitForRskMempoolToGetTxs(rskTxHelper); + await mineAndSync(getRskTransactionHelpers()); const result = await txPromise; return result; @@ -103,9 +106,13 @@ const isUtxoRegisteredInBridge = async (rskTxHelper, peginTxHash, expectedUxtosC const mineForPeginRegistration = async (rskTxHelper, btcTxHelper) => { // Enough confirmations to register the coinbase but not the pegin. + // Wait for the pegin to be in the bitcoin mempool before mining + await waitForBtcMempoolToGetTxs(btcTxHelper); await btcTxHelper.mine(BTC_TO_RSK_MINIMUM_CONFIRMATIONS - 1); await waitAndUpdateBridge(rskTxHelper, 500); // One more confirmation to register the pegin. + // Wait for the pegin to be in the bitcoin mempool before mining + await waitForBtcMempoolToGetTxs(btcTxHelper); await btcTxHelper.mine(1); await waitAndUpdateBridge(rskTxHelper, 500); }; @@ -132,6 +139,9 @@ const sendPegin = async (rskTxHelper, btcTxHelper, btcSenderAddressInformation, const peginBtcTxHash = await btcTxHelper.transferBtc(btcSenderAddressInformation, recipientsTransferInformation, data); + // Wait for the pegin to be in the bitcoin mempool before mining + await waitForBtcTxToBeInMempool(btcTxHelper, peginBtcTxHash); + await mineForPeginRegistration(rskTxHelper, btcTxHelper); return peginBtcTxHash; @@ -177,18 +187,6 @@ const ensurePeginIsRegistered = async (rskTxHelper, peginBtcTxHash, expectedUtxo }; -/** - * Waits for the specified time, updates the bridge and mines 1 rsk block - * @param {RskTransactionHelper} rskTxHelper - * @param {number} timeInMilliseconds defaults to 1000 - * @returns {Promise} - */ -const waitAndUpdateBridge = async (rskTxHelper, timeInMilliseconds = 1000) => { - await wait(timeInMilliseconds); - await rskTxHelper.updateBridge(); - await mineAndSync(getRskTransactionHelpers()); -}; - /** * Creates a pegin v1 data for a user to indicate to which rsk address to receive their pegin funds. * @param {string} rskDestinationAddress @@ -273,5 +271,5 @@ module.exports = { createPeginV1TxData, mineForPeginRegistration, MIN_PEGOUT_VALUE_IN_RBTC, - disableWhitelisting + disableWhitelisting, }; diff --git a/lib/rsk-utils.js b/lib/rsk-utils.js index 5507d756..ba41ce86 100644 --- a/lib/rsk-utils.js +++ b/lib/rsk-utils.js @@ -5,7 +5,7 @@ const { getBridge, getLatestActiveForkName } = require('./precompiled-abi-forks- const hopBridgeTxParser = require('bridge-transaction-parser-hop400'); const fingerrootBridgeTxParser = require('bridge-transaction-parser-fingerroot500'); const { getRskTransactionHelpers } = require('../lib/rsk-tx-helper-provider'); -const { removePrefix0x } = require('./utils'); +const { removePrefix0x, waitForBtcMempoolToGetTxs } = require('./utils'); const BTC_TO_RSK_MINIMUM_ACCEPTABLE_CONFIRMATIONS = 3; const RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS = 3; @@ -127,6 +127,98 @@ const increaseBlockToNextPegoutHeight = async (rskTransactionHelpers) => { } }; +/** + * Waits for the specified time, updates the bridge and mines 1 rsk block + * @param {RskTransactionHelper} rskTxHelper + * @param {number} timeInMilliseconds defaults to 1000 + * @returns {Promise} + */ +const waitAndUpdateBridge = async (rskTxHelper, timeInMilliseconds = 1000) => { + await wait(timeInMilliseconds); + await rskTxHelper.updateBridge(); + + // Wait for the rsk `updateBridge` tx to be in the rsk mempool before mining + await waitForRskMempoolToGetTxs(rskTxHelper); + + await mineAndSync(getRskTransactionHelpers()); +}; + +/** + * + * @param {RskTransactionHelper} rskTxHelper + * @returns {Promise} array of tx hashes in the mempool + */ +const getRskMempoolTransactionHashes = async (rskTxHelper) => { + const mempoolBlock = await rskTxHelper.getClient().eth.getBlock('pending'); + return mempoolBlock.transactions; +}; + +/** + * + * @param {RskTransactionHelper} rskTxHelper + * @param {string} txHash + * @param {number} maxAttempts Defaults to 20 + * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds + * @returns {boolean} whether the tx is in the mempool or not + */ +const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5, checkEveryMilliseconds = 1000) => { + + const method = async () => { + const tx = await rskTxHelper.getClient().eth.getTransaction(txHash); + if(tx && !tx.blockNumber) { + console.debug(`The tx (${txHash}) is in the mempool`); + return true; + } else if(tx && tx.blockNumber) { + console.debug(`The tx (${txHash}) is already mined in a block`); + return true; + } else { + console.debug(`The tx (${txHash}) is not in the mempool nor in a block yet. Will keep retrying until it is in the mempool, block, or it reaches the max attempts to find it`); + return false; + } + }; + + const check = async (txIsInTheMempool, currentAttempts) => { + console.debug(`Attempting to find the tx ${txHash} in the mempool. Attempt ${currentAttempts} out of ${maxAttempts}`); + return txIsInTheMempool; + }; + + return await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + +}; + +/** + * + * @param {RskTransactionHelper} rskTxHelper + * @param {number} maxAttempts Defaults to 20 + * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds + * @returns + */ +const waitForRskMempoolToGetTxs = async (rskTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { + + const initialRskMempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); + + console.debug(`[waitForRskMempoolToGetTxs] initial mempool size: ${initialRskMempoolTxHashes.length}`); + + const method = async () => { + const mempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); + if(mempoolTxHashes.length > initialRskMempoolTxHashes.length) { + console.debug(`The mempool got new ${mempoolTxHashes.length - initialRskMempoolTxHashes.length} transactions`); + return true; + } else { + console.debug(`The mempool did not get new transactions yet. Will keep retrying until it gets new transactions or it reaches the max attempts`); + return false; + } + }; + + const check = async (mempoolHasTxs, currentAttempts) => { + console.debug(`Attempting to find new txs in the mempool. Attempt ${currentAttempts} out of ${maxAttempts}`); + return mempoolHasTxs; + }; + + return await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + + }; + /** * * @param {Array} rskTransactionHelpers RskTransactionHelper instances each belonging to one federator node to make calls to the rsk network @@ -141,12 +233,9 @@ const triggerRelease = async (rskTransactionHelpers, btcClient, callbacks = {}) await increaseBlockToNextPegoutHeight(rskTransactionHelpers); } - // Sync all nodes - await waitForSync(rskTransactionHelpers); - // Adds the pegout to the pegoutsWaitingForConfirmations structure with this 1 confirmation - await rskTxHelper.updateBridge(); - await mineAndSync(rskTransactionHelpers); // release_request_received and batch_pegout_created triggered here (if appropriate fork, RSKIP185 and RSKIP271, is/are active) + // release_request_received and batch_pegout_created triggered here (if appropriate fork, RSKIP185 and RSKIP271, is/are active) + await waitAndUpdateBridge(rskTxHelper); if(callbacks.pegoutCreatedCallback) { await callbacks.pegoutCreatedCallback(rskTxHelper); @@ -156,8 +245,8 @@ const triggerRelease = async (rskTransactionHelpers, btcClient, callbacks = {}) await mineAndSync(rskTransactionHelpers, BTC_TO_RSK_MINIMUM_ACCEPTABLE_CONFIRMATIONS - 1); // Moves the pegout from pegoutsWaitingForConfirmations to pegoutsWaitingForSignatures - await rskTxHelper.updateBridge(); // Makes an updateCollections call which is the tx that will move the pegout to pegoutsWaitingForSignatures - await mineAndSync(rskTransactionHelpers); // pegout_confirmed event triggered here (if appropriate fork, RSKIP326, is active) + // pegout_confirmed event triggered here (if appropriate fork, RSKIP326, is active) + await waitAndUpdateBridge(rskTxHelper); if(callbacks.pegoutConfirmedCallback) { await callbacks.pegoutConfirmedCallback(rskTxHelper); @@ -192,13 +281,16 @@ const triggerRelease = async (rskTransactionHelpers, btcClient, callbacks = {}) await callbacks.releaseBtcCallback(rskTxHelper); } + // Waiting to make sure that the pegout tx is in the bitcoin mempool before mining the required blocks for confirmation. + await waitForBtcMempoolToGetTxs(btcClient); + // From the btc network side, mine `RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS + 1` blocks, 1 for the pegout funds to be mined and reflected in the recipient address, // and `RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS` more to have enough confirmation for the change balance to be reflected back in the bridge (like a pegin) await btcClient.mine(RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS + 1); // Make pegnatories register the change utxo back in the bridge - await rskTxHelper.updateBridge(); - await mineAndSync(rskTransactionHelpers); // At this point the bridge should already have the change uxto registered + // After this point the bridge should already have the change uxto registered + await waitAndUpdateBridge(rskTxHelper); }; @@ -221,7 +313,7 @@ const sendTxWithCheck = async (rskTxHelper, method, from, checkCallback) => { const estimatedGas = await method.estimateGas({ from }); const txReceiptPromise = method.send({ from, value: 0, gasPrice: 0, gas: estimatedGas }); - await wait(1000); + await waitForRskMempoolToGetTxs(rskTxHelper); await mineAndSync(getRskTransactionHelpers()); return await txReceiptPromise; @@ -344,5 +436,9 @@ module.exports = { getUnlockedAddress, getFedsPubKeys, activateFork, - getLatestForkName + getLatestForkName, + getRskMempoolTransactionHashes, + waitForRskTxToBeInTheMempool, + waitForRskMempoolToGetTxs, + waitAndUpdateBridge, }; diff --git a/lib/tests/2wp.js b/lib/tests/2wp.js index 872ceb6e..25c5b40c 100644 --- a/lib/tests/2wp.js +++ b/lib/tests/2wp.js @@ -9,6 +9,7 @@ const { getRskTransactionHelpers, getRskTransactionHelper } = require('../rsk-tx const { getDerivedRSKAddressInformation } = require('@rsksmart/btc-rsk-derivation'); const btcEthUnitConverter = require('@rsksmart/btc-eth-unit-converter'); const { sendTxToBridge, sendPegin, ensurePeginIsRegistered, donateToBridge } = require('../2wp-utils'); +const { waitAndUpdateBridge } = require('../rsk-utils'); const DONATION_AMOUNT = 250; const REJECTED_REASON = 1; @@ -50,8 +51,7 @@ const execute = (description, getRskHost) => { rskTxHelpers = getRskTransactionHelpers(); // Update the bridge to sync btc blockchains - await rskTxHelper.updateBridge(); - await rskUtils.mineAndSync(rskTxHelpers); + await waitAndUpdateBridge(rskTxHelper); // At the moment there are a lot of pegout tests that depend on the bridge to have enough balance. // Those tests are not doing a pegin if needed, so we need to donate to the bridge to ensure it has enough balance. diff --git a/lib/utils.js b/lib/utils.js index cfc96921..abb1b7dd 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -99,6 +99,9 @@ const fundAddressAndGetData = async (btcTxHelper, addressToFund, amountToFundInB const txId = await btcTxHelper.transferBtc(btcSenderAddressInformation, recipientsTransactionInformation); + // Wait for the pegin to be in the bitcoin mempool before mining + await waitForBtcTxToBeInMempool(btcTxHelper, txId); + const rawTx = await btcTxHelper.nodeClient.getRawTransaction(txId); await btcTxHelper.importAddress(addressToFund); @@ -212,6 +215,88 @@ const retryWithCheck = async (method, check, maxAttempts = 5, delayInMillisecond return result; }; +/** + * + * @param {BtcTransactionHelper} btcTxHelper + * @returns {Array} the mempool tx ids + */ +const getBitcoinMempool = async (btcTxHelper) => { + const mempool = await btcTxHelper.nodeClient.execute('getrawmempool', []); + return mempool; +}; + +/** + * + * @param {BtcTransactionHelper} btcTxHelper + * @param {string} btcTxHash + */ +const waitForBtcTxToBeInMempool = async (btcTxHelper, btcTxHash) => { + + const bitcoinMempoolHasTx = async () => { + const bitcoinMempool = await getBitcoinMempool(btcTxHelper); + const txIsInMempool = bitcoinMempool.includes(btcTxHash); + if(!txIsInMempool) { + console.debug(`Attempting to check if the btc tx (${btcTxHash}) was already mined since it's not in the mempool yet.`); + const tx = await btcTransactionHelper.getTransaction(btcTxHash); + if(tx) { + console.debug(`The btc tx (${btcTxHash}) was already mined.`); + return true; + } + } + }; + + const checkBitcoinMempoolHasTx = async (btcTxAlreadyFound, currentAttempts) => { + if(btcTxAlreadyFound) { + console.debug(`The btc tx ${btcTxHash} was found in the mempool at attempt ${currentAttempts}.`); + } else { + console.log(`Attempting to get the btc tx ${btcTxHash} in the mempool. Attempt: ${currentAttempts}.`); + } + return btcTxAlreadyFound; + }; + + const onError = async (e) => { + if(e.message.includes('No such mempool or blockchain transaction')) { + console.debug(`The btc tx ${btcTxHash} is not in the mempool nor mined yet. Let's allow some more time before retrying to get it.`); + return true; + } + console.error(`Un expected error while trying to get the btc tx ${btcTxHash} in the mempool.`, e); + throw e; + }; + + await retryWithCheck(bitcoinMempoolHasTx, checkBitcoinMempoolHasTx, 5, 1000, onError); +}; + +/** + * Waits until the btc mempool has at least one tx. + * @param {BtcTransactionHelper} btcTxHelper + * @returns {boolean} + */ +const waitForBtcMempoolToGetTxs = async (btcTxHelper) => { + + const initialBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; + + console.debug(`The initial btc mempool size is ${initialBitcoinMempoolSize}.`); + + const getBitcoinMempoolSize = async () => { + const bitcoinMempool = await getBitcoinMempool(btcTxHelper); + const bitcoinMempoolSize = bitcoinMempool.length; + console.debug(`The btc mempool has ${bitcoinMempoolSize} txs.`); + return bitcoinMempoolSize; + }; + + const checkBtcMempoolIsNotEmpty = async (bitcoinMempoolSize, currentAttempts) => { + console.debug(`The btc mempool has ${bitcoinMempoolSize} txs at attempt ${currentAttempts}.`); + return bitcoinMempoolSize > 0; + }; + + const onError = async (e) => { + console.error(`Un expected error while trying to get the btc mempool.`, e); + throw e; + }; + + return await retryWithCheck(getBitcoinMempoolSize, checkBtcMempoolIsNotEmpty, 5, 1000, onError); +} + module.exports = { sequentialPromise: sequentialPromise, mapPromiseAll: mapPromiseAll, @@ -233,4 +318,7 @@ module.exports = { }, removePrefix0x, retryWithCheck, + getBitcoinMempool, + waitForBtcTxToBeInMempool, + waitForBtcMempoolToGetTxs, } diff --git a/tests/01_03_01-lock_whitelist_pre_papyrus.js b/tests/01_03_01-lock_whitelist_pre_papyrus.js index 57ede0a1..6d928799 100644 --- a/tests/01_03_01-lock_whitelist_pre_papyrus.js +++ b/tests/01_03_01-lock_whitelist_pre_papyrus.js @@ -15,6 +15,7 @@ const { sendPegin, ensurePeginIsRegistered } = require('../lib/2wp-utils'); const { getDerivedRSKAddressInformation } = require('@rsksmart/btc-rsk-derivation'); const { btcToWeis, btcToSatoshis, satoshisToBtc } = require('@rsksmart/btc-eth-unit-converter'); const { getBridge, getLatestActiveForkName } = require('../lib/precompiled-abi-forks-util'); +const { waitAndUpdateBridge } = require('../lib/rsk-utils'); let rskTxHelpers; let btcTxHelper; @@ -164,8 +165,7 @@ describe('Lock whitelisting', () => { const federationBalanceAfterPegin = await btcTxHelper.getAddressBalance(federationAddress); expect(Number(btcToSatoshis(federationBalanceAfterPegin))).to.equal(Number(btcToSatoshis(initialFederationBalance + AMOUNT_TO_TRY_TO_LOCK)), `Lock BTC federation ${federationAddress} credit`); - await rskTxHelper.updateBridge(); - await rskUtils.mineAndSync(rskTxHelpers); + await waitAndUpdateBridge(rskTxHelper); const initialBlockNumber = await rskTxHelper.getBlockNumber(); await rskUtils.mineAndSync(rskTxHelpers); @@ -231,8 +231,8 @@ describe('Lock whitelisting', () => { const federationBalanceAfterPegin = await btcTxHelper.getAddressBalance(federationAddress); expect(federationBalanceAfterPegin).to.equal(initialFederationBalance + PEGIN_VALUE_IN_BTC, 'The federation address should have its balance increased by the pegin amount'); - await rskTxHelper.updateBridge(); - await rskUtils.mineAndSync(rskTxHelpers); + // Update the bridge to sync + await waitAndUpdateBridge(rskTxHelper); const recipientRskAddressBalance = Number(await rskTxHelper.getBalance(recipientRskAddressInfo.address)); expect(recipientRskAddressBalance).to.equal(0, 'The recipient rsk address should not have any balance'); diff --git a/tests/01_03_54-post-papyrus_coinbase_information.js b/tests/01_03_54-post-papyrus_coinbase_information.js index fe5856af..cb16cf6b 100644 --- a/tests/01_03_54-post-papyrus_coinbase_information.js +++ b/tests/01_03_54-post-papyrus_coinbase_information.js @@ -31,6 +31,7 @@ describe('Calling coinbase information methods after papyrus', () => { const blockHash = await btcClient.mine(1); await wait(1000); await rskTxHelper.updateBridge(); + await rskUtils.waitForRskMempoolToGetTxs(rskTxHelper); const blockData = await btcClient.nodeClient.getBlock(blockHash[0], false); const block = bitcoinJs.Block.fromHex(blockData); From b02383310f43a630ae777d92bbd4f5a67cdbcee0 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 16:27:01 -0400 Subject: [PATCH 4/9] Returns early while checking bitcoin mempool --- lib/utils.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index abb1b7dd..11ad8c70 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -242,6 +242,8 @@ const waitForBtcTxToBeInMempool = async (btcTxHelper, btcTxHash) => { console.debug(`The btc tx (${btcTxHash}) was already mined.`); return true; } + } else { + return true; } }; @@ -275,12 +277,11 @@ const waitForBtcMempoolToGetTxs = async (btcTxHelper) => { const initialBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; - console.debug(`The initial btc mempool size is ${initialBitcoinMempoolSize}.`); + console.debug(`[waitForBtcMempoolToGetTxs] The initial btc mempool size is ${initialBitcoinMempoolSize}.`); const getBitcoinMempoolSize = async () => { const bitcoinMempool = await getBitcoinMempool(btcTxHelper); const bitcoinMempoolSize = bitcoinMempool.length; - console.debug(`The btc mempool has ${bitcoinMempoolSize} txs.`); return bitcoinMempoolSize; }; From ab9ec2cb04a8b010f4f0a863a037908c6b802c28 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 17:36:20 -0400 Subject: [PATCH 5/9] Returning false from waitForBtcTxToBeInMempool and removing else --- lib/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 11ad8c70..801efbb0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -242,9 +242,9 @@ const waitForBtcTxToBeInMempool = async (btcTxHelper, btcTxHash) => { console.debug(`The btc tx (${btcTxHash}) was already mined.`); return true; } - } else { - return true; + return false; } + return true; }; const checkBitcoinMempoolHasTx = async (btcTxAlreadyFound, currentAttempts) => { From 0434d1a06b908f2fd1346ba9192564795ab0e30d Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 19:02:24 -0400 Subject: [PATCH 6/9] Refactors. Removes unnecessary logs. --- lib/2wp-utils.js | 12 ++--- lib/rsk-utils.js | 45 ++++++++++--------- lib/utils.js | 34 +++++++------- ...03_54-post-papyrus_coinbase_information.js | 2 +- 4 files changed, 50 insertions(+), 43 deletions(-) diff --git a/lib/2wp-utils.js b/lib/2wp-utils.js index c1bf2c08..e380f03e 100644 --- a/lib/2wp-utils.js +++ b/lib/2wp-utils.js @@ -1,6 +1,6 @@ const expect = require('chai').expect; -const { sendFromCow, mineAndSync, sendTxWithCheck, getUnlockedAddress, waitForRskMempoolToGetTxs, waitAndUpdateBridge } = require('./rsk-utils'); -const { retryWithCheck, waitForBtcTxToBeInMempool, waitForBtcMempoolToGetTxs } = require('./utils'); +const { sendFromCow, mineAndSync, sendTxWithCheck, getUnlockedAddress, waitForRskMempoolToGetNewTxs, waitAndUpdateBridge } = require('./rsk-utils'); +const { retryWithCheck, waitForBitcoinTxToBeInMempool, waitForBitcoinMempoolToGetTxs } = require('./utils'); const { getBridge, getLatestActiveForkName } = require('./precompiled-abi-forks-util'); const { getBridgeState } = require('@rsksmart/bridge-state-data-parser'); const btcEthUnitConverter = require('@rsksmart/btc-eth-unit-converter'); @@ -64,7 +64,7 @@ const sendTxToBridge = async (rskTxHelper, amountInRbtc, rskFromAddress, mine = } // Wait for the rsk tx to be in the rsk mempool before mining - await waitForRskMempoolToGetTxs(rskTxHelper); + await waitForRskMempoolToGetNewTxs(rskTxHelper); await mineAndSync(getRskTransactionHelpers()); const result = await txPromise; @@ -107,12 +107,12 @@ const isUtxoRegisteredInBridge = async (rskTxHelper, peginTxHash, expectedUxtosC const mineForPeginRegistration = async (rskTxHelper, btcTxHelper) => { // Enough confirmations to register the coinbase but not the pegin. // Wait for the pegin to be in the bitcoin mempool before mining - await waitForBtcMempoolToGetTxs(btcTxHelper); + await waitForBitcoinMempoolToGetTxs(btcTxHelper); await btcTxHelper.mine(BTC_TO_RSK_MINIMUM_CONFIRMATIONS - 1); await waitAndUpdateBridge(rskTxHelper, 500); // One more confirmation to register the pegin. // Wait for the pegin to be in the bitcoin mempool before mining - await waitForBtcMempoolToGetTxs(btcTxHelper); + await waitForBitcoinMempoolToGetTxs(btcTxHelper); await btcTxHelper.mine(1); await waitAndUpdateBridge(rskTxHelper, 500); }; @@ -140,7 +140,7 @@ const sendPegin = async (rskTxHelper, btcTxHelper, btcSenderAddressInformation, const peginBtcTxHash = await btcTxHelper.transferBtc(btcSenderAddressInformation, recipientsTransferInformation, data); // Wait for the pegin to be in the bitcoin mempool before mining - await waitForBtcTxToBeInMempool(btcTxHelper, peginBtcTxHash); + await waitForBitcoinTxToBeInMempool(btcTxHelper, peginBtcTxHash); await mineForPeginRegistration(rskTxHelper, btcTxHelper); diff --git a/lib/rsk-utils.js b/lib/rsk-utils.js index ba41ce86..7f32a70c 100644 --- a/lib/rsk-utils.js +++ b/lib/rsk-utils.js @@ -5,7 +5,7 @@ const { getBridge, getLatestActiveForkName } = require('./precompiled-abi-forks- const hopBridgeTxParser = require('bridge-transaction-parser-hop400'); const fingerrootBridgeTxParser = require('bridge-transaction-parser-fingerroot500'); const { getRskTransactionHelpers } = require('../lib/rsk-tx-helper-provider'); -const { removePrefix0x, waitForBtcMempoolToGetTxs } = require('./utils'); +const { removePrefix0x, waitForBitcoinMempoolToGetTxs } = require('./utils'); const BTC_TO_RSK_MINIMUM_ACCEPTABLE_CONFIRMATIONS = 3; const RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS = 3; @@ -138,7 +138,7 @@ const waitAndUpdateBridge = async (rskTxHelper, timeInMilliseconds = 1000) => { await rskTxHelper.updateBridge(); // Wait for the rsk `updateBridge` tx to be in the rsk mempool before mining - await waitForRskMempoolToGetTxs(rskTxHelper); + await waitForRskMempoolToGetNewTxs(rskTxHelper); await mineAndSync(getRskTransactionHelpers()); }; @@ -159,7 +159,7 @@ const getRskMempoolTransactionHashes = async (rskTxHelper) => { * @param {string} txHash * @param {number} maxAttempts Defaults to 20 * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds - * @returns {boolean} whether the tx is in the mempool or not + * @returns {Promise} whether the tx is in the mempool or not */ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5, checkEveryMilliseconds = 1000) => { @@ -171,10 +171,9 @@ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5 } else if(tx && tx.blockNumber) { console.debug(`The tx (${txHash}) is already mined in a block`); return true; - } else { - console.debug(`The tx (${txHash}) is not in the mempool nor in a block yet. Will keep retrying until it is in the mempool, block, or it reaches the max attempts to find it`); - return false; } + console.debug(`The tx (${txHash}) is not in the mempool nor in a block yet. Will keep retrying until it is in the mempool, block, or it reaches the max attempts to find it`); + return false; }; const check = async (txIsInTheMempool, currentAttempts) => { @@ -182,7 +181,9 @@ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5 return txIsInTheMempool; }; - return await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + const txIsInTheMempool = await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + + console.debug(`Tx ${txHash} was found in the mempool or mined: ${txIsInTheMempool}`); }; @@ -191,32 +192,36 @@ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5 * @param {RskTransactionHelper} rskTxHelper * @param {number} maxAttempts Defaults to 20 * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds - * @returns + * @returns {Promise} whether the mempool has new txs or not */ -const waitForRskMempoolToGetTxs = async (rskTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { +const waitForRskMempoolToGetNewTxs = async (rskTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { const initialRskMempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); - console.debug(`[waitForRskMempoolToGetTxs] initial mempool size: ${initialRskMempoolTxHashes.length}`); + console.debug(`[waitForRskMempoolToGetNewTxs] initial rsk mempool size: ${initialRskMempoolTxHashes.length}`); + console.debug(`Will wait and attempt to check if the rsk mempool has received any new transactions ${maxAttempts} times.`); const method = async () => { const mempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); if(mempoolTxHashes.length > initialRskMempoolTxHashes.length) { - console.debug(`The mempool got new ${mempoolTxHashes.length - initialRskMempoolTxHashes.length} transactions`); + console.debug(`The mempool got ${mempoolTxHashes.length - initialRskMempoolTxHashes.length} new transactions`); return true; - } else { - console.debug(`The mempool did not get new transactions yet. Will keep retrying until it gets new transactions or it reaches the max attempts`); - return false; } + return false; }; - const check = async (mempoolHasTxs, currentAttempts) => { - console.debug(`Attempting to find new txs in the mempool. Attempt ${currentAttempts} out of ${maxAttempts}`); + const check = async (mempoolHasTxs) => { return mempoolHasTxs; }; - return await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + const retryResult = await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + const finalRskMempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); + + console.debug(`[waitForRskMempoolToGetNewTxs] final rsk mempool size: ${finalRskMempoolTxHashes.length}. Difference with initial mempool size: ${finalRskMempoolTxHashes.length - initialRskMempoolTxHashes.length}`); + + return retryResult; + }; /** @@ -282,7 +287,7 @@ const triggerRelease = async (rskTransactionHelpers, btcClient, callbacks = {}) } // Waiting to make sure that the pegout tx is in the bitcoin mempool before mining the required blocks for confirmation. - await waitForBtcMempoolToGetTxs(btcClient); + await waitForBitcoinMempoolToGetTxs(btcClient); // From the btc network side, mine `RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS + 1` blocks, 1 for the pegout funds to be mined and reflected in the recipient address, // and `RSK_TO_BTC_MINIMUM_ACCEPTABLE_CONFIRMATIONS` more to have enough confirmation for the change balance to be reflected back in the bridge (like a pegin) @@ -313,7 +318,7 @@ const sendTxWithCheck = async (rskTxHelper, method, from, checkCallback) => { const estimatedGas = await method.estimateGas({ from }); const txReceiptPromise = method.send({ from, value: 0, gasPrice: 0, gas: estimatedGas }); - await waitForRskMempoolToGetTxs(rskTxHelper); + await waitForRskMempoolToGetNewTxs(rskTxHelper); await mineAndSync(getRskTransactionHelpers()); return await txReceiptPromise; @@ -439,6 +444,6 @@ module.exports = { getLatestForkName, getRskMempoolTransactionHashes, waitForRskTxToBeInTheMempool, - waitForRskMempoolToGetTxs, + waitForRskMempoolToGetNewTxs, waitAndUpdateBridge, }; diff --git a/lib/utils.js b/lib/utils.js index 801efbb0..1172d5b6 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -100,7 +100,7 @@ const fundAddressAndGetData = async (btcTxHelper, addressToFund, amountToFundInB const txId = await btcTxHelper.transferBtc(btcSenderAddressInformation, recipientsTransactionInformation); // Wait for the pegin to be in the bitcoin mempool before mining - await waitForBtcTxToBeInMempool(btcTxHelper, txId); + await waitForBitcoinTxToBeInMempool(btcTxHelper, txId); const rawTx = await btcTxHelper.nodeClient.getRawTransaction(txId); @@ -218,7 +218,7 @@ const retryWithCheck = async (method, check, maxAttempts = 5, delayInMillisecond /** * * @param {BtcTransactionHelper} btcTxHelper - * @returns {Array} the mempool tx ids + * @returns {Promise>} the mempool tx ids */ const getBitcoinMempool = async (btcTxHelper) => { const mempool = await btcTxHelper.nodeClient.execute('getrawmempool', []); @@ -228,9 +228,9 @@ const getBitcoinMempool = async (btcTxHelper) => { /** * * @param {BtcTransactionHelper} btcTxHelper - * @param {string} btcTxHash + * @param {Promise} btcTxHash */ -const waitForBtcTxToBeInMempool = async (btcTxHelper, btcTxHash) => { +const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { const bitcoinMempoolHasTx = async () => { const bitcoinMempool = await getBitcoinMempool(btcTxHelper); @@ -269,15 +269,16 @@ const waitForBtcTxToBeInMempool = async (btcTxHelper, btcTxHash) => { }; /** - * Waits until the btc mempool has at least one tx. + * Waits until the bitcoin mempool has at least one tx. * @param {BtcTransactionHelper} btcTxHelper - * @returns {boolean} + * @returns {Promise} */ -const waitForBtcMempoolToGetTxs = async (btcTxHelper) => { +const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts, checkEveryMilliseconds = 1000) => { const initialBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; - console.debug(`[waitForBtcMempoolToGetTxs] The initial btc mempool size is ${initialBitcoinMempoolSize}.`); + console.debug(`[waitForBitcoinMempoolToGetTxs] The initial bitcoin mempool size is ${initialBitcoinMempoolSize}.`); + console.debug(`Will wait and attempt to check if the bitcoin mempool has received any new transactions ${maxAttempts} times.`); const getBitcoinMempoolSize = async () => { const bitcoinMempool = await getBitcoinMempool(btcTxHelper); @@ -286,16 +287,17 @@ const waitForBtcMempoolToGetTxs = async (btcTxHelper) => { }; const checkBtcMempoolIsNotEmpty = async (bitcoinMempoolSize, currentAttempts) => { - console.debug(`The btc mempool has ${bitcoinMempoolSize} txs at attempt ${currentAttempts}.`); return bitcoinMempoolSize > 0; }; - const onError = async (e) => { - console.error(`Un expected error while trying to get the btc mempool.`, e); - throw e; - }; + const bitcoinMempoolHasTx = await retryWithCheck(getBitcoinMempoolSize, checkBtcMempoolIsNotEmpty, maxAttempts, checkEveryMilliseconds); + + const finalBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; + + console.debug(`[waitForBitcoinMempoolToGetTxs] The final bitcoin mempool size is ${finalBitcoinMempoolSize}. Difference with initial mempool size: ${finalBitcoinMempoolSize - initialBitcoinMempoolSize}.`); + + return bitcoinMempoolHasTx; - return await retryWithCheck(getBitcoinMempoolSize, checkBtcMempoolIsNotEmpty, 5, 1000, onError); } module.exports = { @@ -320,6 +322,6 @@ module.exports = { removePrefix0x, retryWithCheck, getBitcoinMempool, - waitForBtcTxToBeInMempool, - waitForBtcMempoolToGetTxs, + waitForBitcoinTxToBeInMempool, + waitForBitcoinMempoolToGetTxs, } diff --git a/tests/01_03_54-post-papyrus_coinbase_information.js b/tests/01_03_54-post-papyrus_coinbase_information.js index cb16cf6b..90401da2 100644 --- a/tests/01_03_54-post-papyrus_coinbase_information.js +++ b/tests/01_03_54-post-papyrus_coinbase_information.js @@ -31,7 +31,7 @@ describe('Calling coinbase information methods after papyrus', () => { const blockHash = await btcClient.mine(1); await wait(1000); await rskTxHelper.updateBridge(); - await rskUtils.waitForRskMempoolToGetTxs(rskTxHelper); + await rskUtils.waitForRskMempoolToGetNewTxs(rskTxHelper); const blockData = await btcClient.nodeClient.getBlock(blockHash[0], false); const block = bitcoinJs.Block.fromHex(blockData); From 2997818eff9b563ef99b01cf167d702862f29016 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 19:52:48 -0400 Subject: [PATCH 7/9] Adds maxAttempts = 5 value. --- lib/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils.js b/lib/utils.js index 1172d5b6..5a9f65a4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -273,7 +273,7 @@ const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { * @param {BtcTransactionHelper} btcTxHelper * @returns {Promise} */ -const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts, checkEveryMilliseconds = 1000) => { +const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { const initialBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; From 87a2d382db3297c76187fc02bd276007c88d56af Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Mon, 11 Dec 2023 23:40:44 -0400 Subject: [PATCH 8/9] Refactors. Correct defaults in docs. --- lib/2wp-utils-legacy.js | 2 +- lib/2wp-utils.js | 2 +- lib/rsk-utils.js | 22 ++++++----- lib/utils.js | 37 +++++++++++-------- ...03_54-post-papyrus_coinbase_information.js | 8 +++- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/lib/2wp-utils-legacy.js b/lib/2wp-utils-legacy.js index 1e01c4d1..1743ff66 100644 --- a/lib/2wp-utils-legacy.js +++ b/lib/2wp-utils-legacy.js @@ -153,7 +153,7 @@ const ensurePeginIsRegistered = async (rskClient, peginBtcTxHash, expectedUxtosC return utxoIsRegistered; }; - const utxoIsRegisteredInTheBridge = await retryWithCheck(method, check, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); + const { result: utxoIsRegisteredInTheBridge } = await retryWithCheck(method, check, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); if(utxoIsRegisteredInTheBridge) { console.debug(`Found pegin ${peginBtcTxHash} registered in the bridge.`); diff --git a/lib/2wp-utils.js b/lib/2wp-utils.js index e380f03e..6561e3fa 100644 --- a/lib/2wp-utils.js +++ b/lib/2wp-utils.js @@ -173,7 +173,7 @@ const ensurePeginIsRegistered = async (rskTxHelper, peginBtcTxHash, expectedUtxo return utxoIsRegistered; }; - const utxoIsRegisteredInTheBridge = await retryWithCheck(method, check, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); + const { result: utxoIsRegisteredInTheBridge } = await retryWithCheck(method, check, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); if(utxoIsRegisteredInTheBridge) { console.debug(`Found pegin ${peginBtcTxHash} registered in the bridge.`); diff --git a/lib/rsk-utils.js b/lib/rsk-utils.js index 7f32a70c..ebddbd40 100644 --- a/lib/rsk-utils.js +++ b/lib/rsk-utils.js @@ -157,7 +157,7 @@ const getRskMempoolTransactionHashes = async (rskTxHelper) => { * * @param {RskTransactionHelper} rskTxHelper * @param {string} txHash - * @param {number} maxAttempts Defaults to 20 + * @param {number} maxAttempts Defaults to 5 * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds * @returns {Promise} whether the tx is in the mempool or not */ @@ -181,16 +181,18 @@ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5 return txIsInTheMempool; }; - const txIsInTheMempool = await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + const { result: isTxInTheMempool, attempts } = await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); - console.debug(`Tx ${txHash} was found in the mempool or mined: ${txIsInTheMempool}`); + console.debug(`Tx ${txHash} was found in the rsk mempool or mined: ${isTxInTheMempool}, after ${attempts} attempts.`); + + return isTxInTheMempool; }; /** * * @param {RskTransactionHelper} rskTxHelper - * @param {number} maxAttempts Defaults to 20 + * @param {number} maxAttempts Defaults to 5 * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds * @returns {Promise} whether the mempool has new txs or not */ @@ -201,7 +203,7 @@ const waitForRskMempoolToGetNewTxs = async (rskTxHelper, maxAttempts = 5, checkE console.debug(`[waitForRskMempoolToGetNewTxs] initial rsk mempool size: ${initialRskMempoolTxHashes.length}`); console.debug(`Will wait and attempt to check if the rsk mempool has received any new transactions ${maxAttempts} times.`); - const method = async () => { + const areThereNewTxsInTheMempool = async () => { const mempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); if(mempoolTxHashes.length > initialRskMempoolTxHashes.length) { console.debug(`The mempool got ${mempoolTxHashes.length - initialRskMempoolTxHashes.length} new transactions`); @@ -214,13 +216,13 @@ const waitForRskMempoolToGetNewTxs = async (rskTxHelper, maxAttempts = 5, checkE return mempoolHasTxs; }; - const retryResult = await retryWithCheck(method, check, maxAttempts, checkEveryMilliseconds); + const { result: newTxsWhereFoundInTheRskMempool, attempts } = await retryWithCheck(areThereNewTxsInTheMempool, check, maxAttempts, checkEveryMilliseconds); const finalRskMempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); - console.debug(`[waitForRskMempoolToGetNewTxs] final rsk mempool size: ${finalRskMempoolTxHashes.length}. Difference with initial mempool size: ${finalRskMempoolTxHashes.length - initialRskMempoolTxHashes.length}`); + console.debug(`[waitForRskMempoolToGetNewTxs] final rsk mempool size: ${finalRskMempoolTxHashes.length}, after ${attempts} attempts. Difference with initial mempool size: ${finalRskMempoolTxHashes.length - initialRskMempoolTxHashes.length}`); - return retryResult; + return newTxsWhereFoundInTheRskMempool; }; @@ -277,9 +279,9 @@ const triggerRelease = async (rskTransactionHelpers, btcClient, callbacks = {}) return false; // Returning false to make the retryWithCheck loop continue until this check returns true or it reaches the max attempts }; - const wasPegoutBroadcasted = await retryWithCheck(method, pegoutIsBroadcasted => pegoutIsBroadcasted, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); + const { result: wasPegoutBroadcasted, attempts } = await retryWithCheck(method, pegoutIsBroadcasted => pegoutIsBroadcasted, MAX_ATTEMPTS, CHECK_EVERY_MILLISECONDS); - console.debug(`Pegout broadcasted: ${wasPegoutBroadcasted}`); + console.debug(`Pegout broadcasted: ${wasPegoutBroadcasted}, after ${attempts} attempts.`); // Last add_signature and release_btc events emitted here at the block that just broadcasted the pegout to the btc network. if(callbacks.releaseBtcCallback) { diff --git a/lib/utils.js b/lib/utils.js index 5a9f65a4..8e85ccf6 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -199,7 +199,10 @@ const retryWithCheck = async (method, check, maxAttempts = 5, delayInMillisecond try { result = await method(); if(!check || (await check(result, currentAttempts))) { - return result; + return { + result, + attempts: currentAttempts + }; } await wait(delayInMilliseconds); currentAttempts++; @@ -212,7 +215,10 @@ const retryWithCheck = async (method, check, maxAttempts = 5, delayInMillisecond } } } - return result; + return { + result, + attempts: currentAttempts + }; }; /** @@ -220,9 +226,8 @@ const retryWithCheck = async (method, check, maxAttempts = 5, delayInMillisecond * @param {BtcTransactionHelper} btcTxHelper * @returns {Promise>} the mempool tx ids */ -const getBitcoinMempool = async (btcTxHelper) => { - const mempool = await btcTxHelper.nodeClient.execute('getrawmempool', []); - return mempool; +const getBitcoinTransactionsInMempool = async (btcTxHelper) => { + return await btcTxHelper.nodeClient.execute('getrawmempool', []); }; /** @@ -233,9 +238,9 @@ const getBitcoinMempool = async (btcTxHelper) => { const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { const bitcoinMempoolHasTx = async () => { - const bitcoinMempool = await getBitcoinMempool(btcTxHelper); - const txIsInMempool = bitcoinMempool.includes(btcTxHash); - if(!txIsInMempool) { + const bitcoinMempool = await getBitcoinTransactionsInMempool(btcTxHelper); + const isTxInMempool = bitcoinMempool.includes(btcTxHash); + if(!isTxInMempool) { console.debug(`Attempting to check if the btc tx (${btcTxHash}) was already mined since it's not in the mempool yet.`); const tx = await btcTransactionHelper.getTransaction(btcTxHash); if(tx) { @@ -275,26 +280,26 @@ const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { */ const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { - const initialBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; + const initialBitcoinMempoolSize = (await getBitcoinTransactionsInMempool(btcTxHelper)).length; console.debug(`[waitForBitcoinMempoolToGetTxs] The initial bitcoin mempool size is ${initialBitcoinMempoolSize}.`); console.debug(`Will wait and attempt to check if the bitcoin mempool has received any new transactions ${maxAttempts} times.`); - const getBitcoinMempoolSize = async () => { - const bitcoinMempool = await getBitcoinMempool(btcTxHelper); + const getCountOfTransactionsInMempool = async () => { + const bitcoinMempool = await getBitcoinTransactionsInMempool(btcTxHelper); const bitcoinMempoolSize = bitcoinMempool.length; return bitcoinMempoolSize; }; - const checkBtcMempoolIsNotEmpty = async (bitcoinMempoolSize, currentAttempts) => { + const checkBtcMempoolIsNotEmpty = async (bitcoinMempoolSize) => { return bitcoinMempoolSize > 0; }; - const bitcoinMempoolHasTx = await retryWithCheck(getBitcoinMempoolSize, checkBtcMempoolIsNotEmpty, maxAttempts, checkEveryMilliseconds); + const { result: bitcoinMempoolHasTx, attempts } = await retryWithCheck(getCountOfTransactionsInMempool, checkBtcMempoolIsNotEmpty, maxAttempts, checkEveryMilliseconds); - const finalBitcoinMempoolSize = (await getBitcoinMempool(btcTxHelper)).length; + const finalBitcoinMempoolSize = (await getBitcoinTransactionsInMempool(btcTxHelper)).length; - console.debug(`[waitForBitcoinMempoolToGetTxs] The final bitcoin mempool size is ${finalBitcoinMempoolSize}. Difference with initial mempool size: ${finalBitcoinMempoolSize - initialBitcoinMempoolSize}.`); + console.debug(`[waitForBitcoinMempoolToGetTxs] The final bitcoin mempool size is ${finalBitcoinMempoolSize}, after ${attempts} attempts. Difference with initial mempool size: ${finalBitcoinMempoolSize - initialBitcoinMempoolSize}.`); return bitcoinMempoolHasTx; @@ -321,7 +326,7 @@ module.exports = { }, removePrefix0x, retryWithCheck, - getBitcoinMempool, + getBitcoinTransactionsInMempool, waitForBitcoinTxToBeInMempool, waitForBitcoinMempoolToGetTxs, } diff --git a/tests/01_03_54-post-papyrus_coinbase_information.js b/tests/01_03_54-post-papyrus_coinbase_information.js index 90401da2..992a7ecb 100644 --- a/tests/01_03_54-post-papyrus_coinbase_information.js +++ b/tests/01_03_54-post-papyrus_coinbase_information.js @@ -64,10 +64,14 @@ describe('Calling coinbase information methods after papyrus', () => { const hash = ensure0x(blockHash[0]); - const hasBtcBlockCoinbaseInformation = await retryWithCheck(bridge.methods.hasBtcBlockCoinbaseTransactionInformation(hash).call, (resultSoFar, currentAttempts) => { + const hasBtcBlockCoinbaseTransactionInformationMethod = bridge.methods.hasBtcBlockCoinbaseTransactionInformation(hash).call; + + const check = (resultSoFar, currentAttempts) => { console.log(`Attempting to get the btc block coinbase information in the bridge for hash: ${hash}, attempt: ${currentAttempts}.`); return resultSoFar; - }); + }; + + const { result: hasBtcBlockCoinbaseInformation } = await retryWithCheck(hasBtcBlockCoinbaseTransactionInformationMethod, check); expect(hasBtcBlockCoinbaseInformation).to.be.true; From b4c467e6d1abecc52ba416cd5ba1b394f2b843a7 Mon Sep 17 00:00:00 2001 From: jeremy-then Date: Tue, 12 Dec 2023 12:52:01 -0400 Subject: [PATCH 9/9] Updates maxAttempts and delay. Fixes sonar suggestions. --- lib/rsk-utils.js | 27 +++++++++++++++++++-------- lib/utils.js | 24 ++++++++++++++++-------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/lib/rsk-utils.js b/lib/rsk-utils.js index ebddbd40..c8d19f85 100644 --- a/lib/rsk-utils.js +++ b/lib/rsk-utils.js @@ -157,23 +157,34 @@ const getRskMempoolTransactionHashes = async (rskTxHelper) => { * * @param {RskTransactionHelper} rskTxHelper * @param {string} txHash - * @param {number} maxAttempts Defaults to 5 - * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds + * @param {number} maxAttempts Defaults to 3 + * @param {number} checkEveryMilliseconds Defaults to 500 milliseconds * @returns {Promise} whether the tx is in the mempool or not */ -const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5, checkEveryMilliseconds = 1000) => { +const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 3, checkEveryMilliseconds = 500) => { const method = async () => { + const tx = await rskTxHelper.getClient().eth.getTransaction(txHash); - if(tx && !tx.blockNumber) { + + const isTxInTheMempool = tx && !tx.blockNumber; + + if(isTxInTheMempool) { console.debug(`The tx (${txHash}) is in the mempool`); return true; - } else if(tx && tx.blockNumber) { + } + + const isTxAlreadyMined = tx && tx.blockNumber; + + if(isTxAlreadyMined) { console.debug(`The tx (${txHash}) is already mined in a block`); return true; } + console.debug(`The tx (${txHash}) is not in the mempool nor in a block yet. Will keep retrying until it is in the mempool, block, or it reaches the max attempts to find it`); + return false; + }; const check = async (txIsInTheMempool, currentAttempts) => { @@ -192,11 +203,11 @@ const waitForRskTxToBeInTheMempool = async (rskTxHelper, txHash, maxAttempts = 5 /** * * @param {RskTransactionHelper} rskTxHelper - * @param {number} maxAttempts Defaults to 5 - * @param {number} checkEveryMilliseconds Defaults to 1000 milliseconds + * @param {number} maxAttempts Defaults to 3 + * @param {number} checkEveryMilliseconds Defaults to 500 milliseconds * @returns {Promise} whether the mempool has new txs or not */ -const waitForRskMempoolToGetNewTxs = async (rskTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { +const waitForRskMempoolToGetNewTxs = async (rskTxHelper, maxAttempts = 3, checkEveryMilliseconds = 500) => { const initialRskMempoolTxHashes = await getRskMempoolTransactionHashes(rskTxHelper); diff --git a/lib/utils.js b/lib/utils.js index 8e85ccf6..999be738 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -186,13 +186,13 @@ const removePrefix0x = hash => hash.substr(2); * @param {function} check callback function to check the result of the method. * If this callback returns true, then the method call is considered successful and the result is returned. * Otherwise, the method is executed again. - * @param {number} maxAttempts defaults to 5 - * @param {number} delayInMilliseconds defaults to 2000 milliseconds + * @param {number} maxAttempts defaults to 3 + * @param {number} delayInMilliseconds defaults to 500 milliseconds * @param {function} onError callback function for the caller to check the thrown error. If the callback returns true, then the function will stop executing. * If this callback is not provided, then the error will be thrown. * @returns {Promise} the result of the method call or the last value of `result` after the attempts. */ -const retryWithCheck = async (method, check, maxAttempts = 5, delayInMilliseconds = 2000, onError) => { +const retryWithCheck = async (method, check, maxAttempts = 3, delayInMilliseconds = 500, onError) => { let currentAttempts = 1; let result; while(currentAttempts <= maxAttempts) { @@ -232,10 +232,13 @@ const getBitcoinTransactionsInMempool = async (btcTxHelper) => { /** * - * @param {BtcTransactionHelper} btcTxHelper - * @param {Promise} btcTxHash + * @param {BtcTransactionHelper} btcTxHelper + * @param {string} btcTxHash + * @param {number} maxAttempts defaults to 3 + * @param {number} checkEveryMilliseconds defaults to 500 milliseconds + * @returns {Promise} whether the tx got to the mempool or not after the attempts */ -const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { +const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash, maxAttempts = 3, checkEveryMilliseconds = 500) => { const bitcoinMempoolHasTx = async () => { const bitcoinMempool = await getBitcoinTransactionsInMempool(btcTxHelper); @@ -270,15 +273,20 @@ const waitForBitcoinTxToBeInMempool = async (btcTxHelper, btcTxHash) => { throw e; }; - await retryWithCheck(bitcoinMempoolHasTx, checkBitcoinMempoolHasTx, 5, 1000, onError); + const { result: btcTxAlreadyFoundInMempool } = retryWithCheck(bitcoinMempoolHasTx, checkBitcoinMempoolHasTx, maxAttempts, checkEveryMilliseconds, onError); + + return btcTxAlreadyFoundInMempool; + }; /** * Waits until the bitcoin mempool has at least one tx. * @param {BtcTransactionHelper} btcTxHelper + * @param {number} maxAttempts defaults to 3 + * @param {number} checkEveryMilliseconds defaults to 500 milliseconds * @returns {Promise} */ -const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts = 5, checkEveryMilliseconds = 1000) => { +const waitForBitcoinMempoolToGetTxs = async (btcTxHelper, maxAttempts = 3, checkEveryMilliseconds = 500) => { const initialBitcoinMempoolSize = (await getBitcoinTransactionsInMempool(btcTxHelper)).length;