From c262d38609ebfac2cfce3f8de4ce64e6afc3de14 Mon Sep 17 00:00:00 2001 From: Peter John Bushnell Date: Thu, 3 Feb 2022 11:07:33 +0000 Subject: [PATCH] Fix restore UTXO test (#1080) * Sync empty mempool * Cat zap TX failure. Add wallet/mempool locks. Loop clearmempool in test. * lint: Add missing include * Add more checks to test and break down signing error message * Comment out assert to see error from SignTransaction * Log UTXO set * Fix node message in functional tests Signed-off-by: Anthony Fieroni * Give time for CCoinsViewCache to update after rollback Co-authored-by: Anthony Fieroni --- src/masternodes/mn_rpc.cpp | 25 +++++++++---- src/rpc/rawtransaction_util.cpp | 7 ++-- test/functional/feature_restore_utxo.py | 47 ++++++++++++++++++++++--- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/masternodes/mn_rpc.cpp b/src/masternodes/mn_rpc.cpp index 1f7f845c65..3637c929f1 100644 --- a/src/masternodes/mn_rpc.cpp +++ b/src/masternodes/mn_rpc.cpp @@ -836,19 +836,30 @@ static UniValue clearmempool(const JSONRPCRequest& request) std::vector vtxid; mempool.queryHashes(vtxid); - UniValue removed(UniValue::VARR); - for (const uint256& hash : vtxid) - removed.push_back(hash.ToString()); + { + auto locked_chain = pwallet->chain().lock(); + LOCK2(pwallet->cs_wallet, locked_chain->mutex()); - LOCK(cs_main); - mempool.clear(); + std::vector vHashOut; + if (pwallet->ZapSelectTx(vtxid, vHashOut) != DBErrors::LOAD_OK) { + throw JSONRPCError(RPC_WALLET_ERROR, "Could not delete mempool transactions from wallet"); + } + } - std::vector vHashOut; - pwallet->ZapSelectTx(vtxid, vHashOut); + { + LOCK(mempool.cs); + mempool.clear(); + } + + UniValue removed(UniValue::VARR); + for (const uint256& hash : vtxid) { + removed.push_back(hash.ToString()); + } return removed; } + static const CRPCCommand commands[] = { // category name actor (function) params diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 846fa8c492..df678bc092 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -383,8 +383,11 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival for (unsigned int i = 0; i < mtx.vin.size(); i++) { CTxIn& txin = mtx.vin[i]; auto coin = coins.find(txin.prevout); - if (coin == coins.end() || coin->second.IsSpent()) { - TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); + if (coin == coins.end()) { + TxInErrorToJSON(txin, vErrors, "Input not found"); + continue; + } else if (coin->second.IsSpent()) { + TxInErrorToJSON(txin, vErrors, "Input already spent"); continue; } const CScript& prevPubKey = coin->second.out.scriptPubKey; diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index ccf9f7b3ac..43b00a2520 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -7,7 +7,9 @@ from test_framework.test_framework import DefiTestFramework +from test_framework.authproxy import JSONRPCException from test_framework.util import assert_equal +import time class TestRestoreUTXOs(DefiTestFramework): def set_test_params(self): @@ -16,12 +18,45 @@ def set_test_params(self): self.extra_args = [['-txnotokens=0', '-amkheight=1', '-bayfrontheight=1'], ['-txnotokens=0', '-amkheight=1', '-bayfrontheight=1']] + def clearmempool(self, node: int): + try: + self.nodes[node].clearmempool() + except JSONRPCException as e: + return False + return True + + def account_to_account(self, node: int, soure, destination): + try: + self.nodes[node].accounttoaccount(soure, {destination: "1@BTC"}) + except JSONRPCException as e: + return False + return True + + def account_to_account_loop(self, node: int, soure, destination): + count = 0 + while not self.account_to_account(node, soure, destination): + if count == 5: + return False + else: + count += 1 + time.sleep(1) + return True + + def rollback(self, count): block = self.nodes[0].getblockhash(count) self.nodes[0].invalidateblock(block) self.nodes[1].invalidateblock(block) - self.nodes[0].clearmempool() - self.nodes[1].clearmempool() + assert(len(self.nodes[0].getrawmempool()) > 0) + assert(len(self.nodes[1].getrawmempool()) > 0) + while not self.clearmempool(0): + pass + while not self.clearmempool(1): + pass + assert_equal(len(self.nodes[0].getrawmempool()), 0) + assert_equal(len(self.nodes[1].getrawmempool()), 0) + assert_equal(self.nodes[0].getblockcount(), count - 1) + assert_equal(self.nodes[1].getblockcount(), count - 1) self.sync_blocks() def run_test(self): @@ -74,16 +109,20 @@ def run_test(self): self.nodes[0].generate(1) self.sync_blocks() block = self.nodes[0].getblockcount() + 1 + node0_utxos = len(self.nodes[0].listunspent()) + node1_utxos = len(self.nodes[1].listunspent()) # Test rollbacks for x in range(50): - self.nodes[0].accounttoaccount(node0_source, {node0_destination: "1@BTC"}) + assert(self.account_to_account_loop(0, node0_source, node0_destination)) self.nodes[0].generate(1) self.sync_blocks() - self.nodes[1].accounttoaccount(node1_source, {node1_destination: "1@BTC"}) + assert(self.account_to_account_loop(1, node1_source, node1_destination)) self.nodes[1].generate(1) self.sync_blocks() self.rollback(block) + assert_equal(len(self.nodes[0].listunspent()), node0_utxos) + assert_equal(len(self.nodes[1].listunspent()), node1_utxos) if __name__ == '__main__': TestRestoreUTXOs().main()