From 38a718f30a704e47da81946701752cf9df311674 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Fri, 28 Jan 2022 07:26:24 +0000 Subject: [PATCH 1/8] Sync empty mempool --- test/functional/feature_restore_utxo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index ccf9f7b3ac..98ad6162ef 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -22,7 +22,7 @@ def rollback(self, count): self.nodes[1].invalidateblock(block) self.nodes[0].clearmempool() self.nodes[1].clearmempool() - self.sync_blocks() + self.sync_all() def run_test(self): self.nodes[0].generate(101) From bfa0a6867f1e61a61edcc8a07f2ea728f8159371 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Fri, 28 Jan 2022 11:05:42 +0000 Subject: [PATCH 2/8] Cat zap TX failure. Add wallet/mempool locks. Loop clearmempool in test. --- src/masternodes/mn_rpc.cpp | 25 ++++++++++++++++++------- test/functional/feature_restore_utxo.py | 19 ++++++++++++++++--- 2 files changed, 34 insertions(+), 10 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/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index 98ad6162ef..d28e738266 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -16,13 +16,26 @@ 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 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() - self.sync_all() + 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) + self.sync_blocks() def run_test(self): self.nodes[0].generate(101) From d19b0f2a29f5ae413c7f01de8f3464fc3056709a Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Fri, 28 Jan 2022 11:17:02 +0000 Subject: [PATCH 3/8] lint: Add missing include --- test/functional/feature_restore_utxo.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index d28e738266..bc891060d0 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -7,6 +7,7 @@ from test_framework.test_framework import DefiTestFramework +from test_framework.authproxy import JSONRPCException from test_framework.util import assert_equal class TestRestoreUTXOs(DefiTestFramework): From 51ed6704cc72ee7dc5629699ed79023ced780d69 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Fri, 28 Jan 2022 13:27:20 +0000 Subject: [PATCH 4/8] Add more checks to test and break down signing error message --- src/rpc/rawtransaction_util.cpp | 7 +++++-- test/functional/feature_restore_utxo.py | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) 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 bc891060d0..4fe3b6edf2 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -36,6 +36,8 @@ def rollback(self, count): 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): @@ -88,6 +90,8 @@ 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): @@ -98,6 +102,8 @@ def run_test(self): 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() From 9d4478c28fc0fee068a9a91990edf29b1dab479a Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Sat, 29 Jan 2022 08:17:17 +0000 Subject: [PATCH 5/8] Comment out assert to see error from SignTransaction --- test/functional/feature_restore_utxo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index 4fe3b6edf2..3c3fde7dca 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -102,8 +102,8 @@ def run_test(self): 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) + # assert_equal(len(self.nodes[0].listunspent()), node0_utxos) # 4 != 5 + # assert_equal(len(self.nodes[1].listunspent()), node1_utxos) if __name__ == '__main__': TestRestoreUTXOs().main() From a973ab503e12d111e443f1bdf52e27e9010a9839 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Sat, 29 Jan 2022 10:59:21 +0000 Subject: [PATCH 6/8] Log UTXO set --- src/wallet/wallet.cpp | 8 ++++++++ test/functional/feature_restore_utxo.py | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d8cc45b392..824998bfae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2489,6 +2489,12 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const return balance; } +static void PrintUTXOSet(std::vector& vCoins) { + for (const auto& coin : vCoins) { + LogPrintf("XXX UTXOS hash %s vout %d\n", coin.tx->GetHash().ToString(), coin.i); + } +} + void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector& vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) const { AssertLockHeld(cs_wallet); @@ -2620,10 +2626,12 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< // Checks the maximum number of UTXO's. if (nMaximumCount > 0 && vCoins.size() >= nMaximumCount) { + PrintUTXOSet(vCoins); return; } } } + PrintUTXOSet(vCoins); } /// @todo tokens: used only in listCoins by qt, so, limit with token = 0???? diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index 3c3fde7dca..01abc1ef7d 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -90,8 +90,8 @@ 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()) + #node0_utxos = len(self.nodes[0].listunspent()) + #node1_utxos = len(self.nodes[1].listunspent()) # Test rollbacks for x in range(50): From ba1e281327ad3d7d0fe3b6f6531572289b3d7e3c Mon Sep 17 00:00:00 2001 From: Anthony Fieroni Date: Mon, 31 Jan 2022 08:48:40 +0200 Subject: [PATCH 7/8] Fix node message in functional tests Signed-off-by: Anthony Fieroni --- test/functional/test_framework/messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index f66bd2c9b5..07c49a76a2 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,7 +31,7 @@ from test_framework.util import hex_str_to_bytes, assert_equal MIN_VERSION_SUPPORTED = 60001 -MY_VERSION = 70017 # past bip-31 for ping/pong +MY_VERSION = 70024 # past bip-31 for ping/pong MY_SUBVERSION = b"/python-mininode-tester:0.0.3/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) From cd858b3e01bae4e4e17057077fdba83bf2253562 Mon Sep 17 00:00:00 2001 From: Peter Bushnell Date: Wed, 2 Feb 2022 09:45:06 +0000 Subject: [PATCH 8/8] Give time for CCoinsViewCache to update after rollback --- src/wallet/wallet.cpp | 8 ------- test/functional/feature_restore_utxo.py | 31 ++++++++++++++++++++----- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 824998bfae..d8cc45b392 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2489,12 +2489,6 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const return balance; } -static void PrintUTXOSet(std::vector& vCoins) { - for (const auto& coin : vCoins) { - LogPrintf("XXX UTXOS hash %s vout %d\n", coin.tx->GetHash().ToString(), coin.i); - } -} - void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector& vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) const { AssertLockHeld(cs_wallet); @@ -2626,12 +2620,10 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< // Checks the maximum number of UTXO's. if (nMaximumCount > 0 && vCoins.size() >= nMaximumCount) { - PrintUTXOSet(vCoins); return; } } } - PrintUTXOSet(vCoins); } /// @todo tokens: used only in listCoins by qt, so, limit with token = 0???? diff --git a/test/functional/feature_restore_utxo.py b/test/functional/feature_restore_utxo.py index 01abc1ef7d..43b00a2520 100755 --- a/test/functional/feature_restore_utxo.py +++ b/test/functional/feature_restore_utxo.py @@ -9,6 +9,7 @@ from test_framework.authproxy import JSONRPCException from test_framework.util import assert_equal +import time class TestRestoreUTXOs(DefiTestFramework): def set_test_params(self): @@ -24,6 +25,24 @@ def clearmempool(self, node: int): 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) @@ -90,20 +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()) + 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) # 4 != 5 - # assert_equal(len(self.nodes[1].listunspent()), node1_utxos) + assert_equal(len(self.nodes[0].listunspent()), node0_utxos) + assert_equal(len(self.nodes[1].listunspent()), node1_utxos) if __name__ == '__main__': TestRestoreUTXOs().main()