From 1baf2d59339607e16f2b03e094d638d5fb7579e0 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 19 Jun 2023 22:04:48 +0300 Subject: [PATCH 01/18] feat: Allow mining blocks of a specific version on non-mainnet networks (#5433) ## Issue being fixed or feature implemented Mining blocks with a specific version can be useful on testnet and devnets too ## What was done? lift restrictions for `-blockversion` ## How Has This Been Tested? it should just work :) ## Breaking Changes n//a ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/miner.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 277f7155779f1..7247e2c011568 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -135,9 +135,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc bool fDIP0008Active_context = nHeight >= chainparams.GetConsensus().DIP0008Height; pblock->nVersion = ComputeBlockVersion(pindexPrev, chainparams.GetConsensus(), chainparams.BIP9CheckMasternodesUpgraded()); - // -regtest only: allow overriding block.nVersion with + // Non-mainnet only: allow overriding block.nVersion with // -blockversion=N to test forking scenarios - if (chainparams.MineBlocksOnDemand()) + if (Params().NetworkIDString() != CBaseChainParams::MAIN) pblock->nVersion = gArgs.GetArg("-blockversion", pblock->nVersion); pblock->nTime = GetAdjustedTime(); From 382400f0b51f2cd44b2cfc780614f9c8f2945779 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Fri, 7 Jul 2023 16:03:55 -0500 Subject: [PATCH 02/18] perf: avoid rehashing block; use stored hash (#5435) before 12% image after 10% image Redundant rehash Avoid redundant rehash Reindexed 0-500000 on testnet None _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/evo/deterministicmns.cpp | 6 +++--- src/evo/deterministicmns.h | 2 +- src/evo/specialtxman.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 22ec8526381bc..fdce158ced358 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -638,7 +638,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde newList.SetHeight(nHeight); } - newList.SetBlockHash(block.GetHash()); + newList.SetBlockHash(pindex->GetBlockHash()); oldList = GetListForBlock(pindex->pprev); diff = oldList.BuildDiff(newList); @@ -677,10 +677,10 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde return true; } -bool CDeterministicMNManager::UndoBlock(const CBlock& block, const CBlockIndex* pindex) +bool CDeterministicMNManager::UndoBlock(const CBlockIndex* pindex) { int nHeight = pindex->nHeight; - uint256 blockHash = block.GetHash(); + uint256 blockHash = pindex->GetBlockHash(); CDeterministicMNList curList; CDeterministicMNList prevList; diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index d02fdd51c065a..aaa9e5a8177d5 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -595,7 +595,7 @@ class CDeterministicMNManager bool ProcessBlock(const CBlock& block, const CBlockIndex* pindex, CValidationState& state, const CCoinsViewCache& view, bool fJustCheck) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool UndoBlock(const CBlock& block, const CBlockIndex* pindex); + bool UndoBlock(const CBlockIndex* pindex); void UpdatedBlockTip(const CBlockIndex* pindex); diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 18aac604e324a..c39400fd77d96 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -189,7 +189,7 @@ bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq: } } - if (!deterministicMNManager->UndoBlock(block, pindex)) { + if (!deterministicMNManager->UndoBlock(pindex)) { return false; } From 950fdde4e8ac576abaae71cd555883eb174879b5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 19 Jun 2023 22:37:33 +0300 Subject: [PATCH 03/18] docs: add 2 more contributors to 19.2.0 release notes (#5446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue being fixed or feature implemented @ogabrielides @kittywhiskers I somehow failed to add you guys to the list of v19.2 contributors 🙈 sorry! ## What was done? ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- doc/release-notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 02523da7eabd6..34f1b2fb2a40b 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -115,8 +115,10 @@ See detailed [set of changes](https://github.com/dashpay/dash/compare/v19.1.0... Thanks to everyone who directly contributed to this release: +- Kittywhiskers Van Gogh (kittywhiskers) - Konstantin Akimov (knst) - Nathan Marley (nmarley) +- Odysseas Gabrielides (ogabrielides) - PastaPastaPasta - thephez - UdjinM6 From 49e024338a071ebecc5efb2d83c6aa522a731ada Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Thu, 22 Jun 2023 07:27:27 +0300 Subject: [PATCH 04/18] feat(rpc): Ability to filter HPMNs in masternodelist and protx list rpcs (#5447) ## Issue being fixed or feature implemented Added the filter `hpmn` for both `masternodelist` and `protx list` rpcs. ## What was done? ## How Has This Been Tested? Calling this RPC on Testnet. ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --------- Co-authored-by: UdjinM6 --- doc/release-notes-5447.md | 6 ++++++ src/rpc/evo.cpp | 5 ++++- src/rpc/masternode.cpp | 9 +++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 doc/release-notes-5447.md diff --git a/doc/release-notes-5447.md b/doc/release-notes-5447.md new file mode 100644 index 0000000000000..ac9e37c390881 --- /dev/null +++ b/doc/release-notes-5447.md @@ -0,0 +1,6 @@ +Updated RPCs +-------- + +- `masternodelist`: New mode `hpmn` filters only HPMNs. +- `protx list`: New type `hpmn` filters only HPMNs. + diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index ba85e19bf18ff..753d441d44bf5 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1204,6 +1204,7 @@ static void protx_list_help(const JSONRPCRequest& request) " registered - List all ProTx which are registered at the given chain height.\n" " This will also include ProTx which failed PoSe verification.\n" " valid - List only ProTx which are active/valid at the given chain height.\n" + " hpmn - List only ProTx corresponding to HPMNs at the given chain height.\n" #ifdef ENABLE_WALLET " wallet - List only ProTx which are found in your wallet at the given chain height.\n" " This will also include ProTx which failed PoSe verification.\n" @@ -1349,7 +1350,7 @@ static UniValue protx_list(const JSONRPCRequest& request) } }); #endif - } else if (type == "valid" || type == "registered") { + } else if (type == "valid" || type == "registered" || type == "hpmn") { if (request.params.size() > 3) { protx_list_help(request); } @@ -1365,7 +1366,9 @@ static UniValue protx_list(const JSONRPCRequest& request) CDeterministicMNList mnList = deterministicMNManager->GetListForBlock(::ChainActive()[height]); bool onlyValid = type == "valid"; + bool onlyHPMN = type == "hpmn"; mnList.ForEachMN(onlyValid, [&](const auto& dmn) { + if (onlyHPMN && dmn.nType != MnType::HighPerformance) return; ret.push_back(BuildDMNListEntry(wallet.get(), dmn, detailed)); }); } else { diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 1e3d388bca7fa..7f7a62699fd00 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -37,6 +37,7 @@ static void masternode_list_help(const JSONRPCRequest& request) "Available modes:\n" " addr - Print ip address associated with a masternode (can be additionally filtered, partial match)\n" " recent - Print info in JSON format for active and recently banned masternodes (can be additionally filtered, partial match)\n" + " hpmn - Print info in JSON format for HPMNs only\n" " full - Print info in format 'status payee lastpaidtime lastpaidblock IP'\n" " (can be additionally filtered, partial match)\n" " info - Print info in format 'status payee IP'\n" @@ -581,7 +582,7 @@ static UniValue masternodelist(const JSONRPCRequest& request) strMode != "owneraddress" && strMode != "votingaddress" && strMode != "lastpaidtime" && strMode != "lastpaidblock" && strMode != "payee" && strMode != "pubkeyoperator" && - strMode != "status" && strMode != "recent")) + strMode != "status" && strMode != "recent" && strMode != "hpmn")) { masternode_list_help(request); } @@ -609,6 +610,7 @@ static UniValue masternodelist(const JSONRPCRequest& request) }; bool showRecentMnsOnly = strMode == "recent"; + bool showHPMNsOnly = strMode == "hpmn"; int tipHeight = WITH_LOCK(cs_main, return ::ChainActive().Tip()->nHeight); mnList.ForEachMN(false, [&](auto& dmn) { if (showRecentMnsOnly && mnList.IsMNPoSeBanned(dmn)) { @@ -616,6 +618,9 @@ static UniValue masternodelist(const JSONRPCRequest& request) return; } } + if (showHPMNsOnly && dmn.nType != MnType::HighPerformance) { + return; + } std::string strOutpoint = dmn.collateralOutpoint.ToStringShort(); Coin coin; @@ -663,7 +668,7 @@ static UniValue masternodelist(const JSONRPCRequest& request) if (strFilter !="" && strInfo.find(strFilter) == std::string::npos && strOutpoint.find(strFilter) == std::string::npos) return; obj.pushKV(strOutpoint, strInfo); - } else if (strMode == "json" || strMode == "recent") { + } else if (strMode == "json" || strMode == "recent" || strMode == "hpmn") { std::ostringstream streamInfo; streamInfo << dmn.proTxHash.ToString() << " " << dmn.pdmnState->addr.ToString() << " " << From ff60d10934f2cf43ea57bc7d3460f0ae0f7e0070 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 27 Jun 2023 21:51:40 +0300 Subject: [PATCH 05/18] feat: introduce `wipewallettxes` RPC and `wipetxes` command for `dash-wallet` tool (#5451) ## Issue being fixed or feature implemented Given the hard fork that happened on testnet, there is now lots of the transactions that were made on the fork that is no longer valid. Some transactions could be relayed and mined again but some like coinjoin mixing won't be relayed because of 0 fee and transactions spending coinbases from the forked branch are no longer valid at all. ## What was done? Introduce `wipewallettxes` RPC and `wipetxes` command for `dash-wallet` tool to be able to get rid of some/all txes in the wallet. ## How Has This Been Tested? run tests, use rpc/command on testnet wallet ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/bitcoin-wallet.cpp | 1 + src/rpc/client.cpp | 1 + src/wallet/rpcwallet.cpp | 69 +++++++++++++++++++++++++++ src/wallet/wallet.cpp | 6 ++- src/wallet/wallet.h | 3 +- src/wallet/wallettool.cpp | 28 ++++++++++- test/functional/rpc_wipewallettxes.py | 41 ++++++++++++++++ test/functional/test_runner.py | 1 + test/functional/tool_wallet.py | 26 ++++++++++ 9 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 test/functional/rpc_wipewallettxes.py diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index fcdf22d329721..c891a3cbe5bd9 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -31,6 +31,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) // Hidden argsman.AddArg("salvage", "Attempt to recover private keys from a corrupt wallet", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); + argsman.AddArg("wipetxes", "Wipe all transactions from a wallet", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); } static bool WalletAppInit(int argc, char* argv[]) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 4fb93be805114..1bbff08c871ee 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -196,6 +196,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "echojson", 9, "arg9" }, { "rescanblockchain", 0, "start_height"}, { "rescanblockchain", 1, "stop_height"}, + { "wipewallettxes", 0, "keep_confirmed"}, { "createwallet", 1, "disable_private_keys"}, { "createwallet", 2, "blank"}, { "createwallet", 4, "avoid_reuse"}, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 992963f47c7e2..9bcb468b073e1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3564,6 +3564,74 @@ static UniValue rescanblockchain(const JSONRPCRequest& request) return response; } +static UniValue wipewallettxes(const JSONRPCRequest& request) +{ + RPCHelpMan{"wipewallettxes", + "\nWipe wallet transactions.\n" + "Note: Use \"rescanblockchain\" to initiate the scanning progress and recover wallet transactions.\n", + { + {"keep_confirmed", RPCArg::Type::BOOL, /* default */ "false", "Do not wipe confirmed transactions"}, + }, + RPCResult{RPCResult::Type::NONE, "", ""}, + RPCExamples{ + HelpExampleCli("wipewallettxes", "") + + HelpExampleRpc("wipewallettxes", "") + }, + }.Check(request); + + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + CWallet* const pwallet = wallet.get(); + + WalletRescanReserver reserver(pwallet); + if (!reserver.reserve()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort rescan or wait."); + } + + LOCK(pwallet->cs_wallet); + + bool keep_confirmed{false}; + if (!request.params[0].isNull()) { + keep_confirmed = request.params[0].get_bool(); + } + + const size_t WALLET_SIZE{pwallet->mapWallet.size()}; + const size_t STEPS{20}; + const size_t BATCH_SIZE = std::max(WALLET_SIZE / STEPS, size_t(1000)); + + pwallet->ShowProgress(strprintf("%s " + _("Wiping wallet transactions...").translated, pwallet->GetDisplayName()), 0); + + for (size_t progress = 0; progress < STEPS; ++progress) { + std::vector vHashIn; + std::vector vHashOut; + size_t count{0}; + + for (auto& [txid, wtx] : pwallet->mapWallet) { + if (progress < STEPS - 1 && ++count > BATCH_SIZE) break; + if (keep_confirmed && wtx.m_confirm.status == CWalletTx::CONFIRMED) continue; + vHashIn.push_back(txid); + } + + if (vHashIn.size() > 0 && pwallet->ZapSelectTx(vHashIn, vHashOut) != DBErrors::LOAD_OK) { + pwallet->ShowProgress(strprintf("%s " + _("Wiping wallet transactions...").translated, pwallet->GetDisplayName()), 100); + throw JSONRPCError(RPC_WALLET_ERROR, "Could not properly delete transactions."); + } + + CHECK_NONFATAL(vHashOut.size() == vHashIn.size()); + + if (pwallet->IsAbortingRescan() || pwallet->chain().shutdownRequested()) { + pwallet->ShowProgress(strprintf("%s " + _("Wiping wallet transactions...").translated, pwallet->GetDisplayName()), 100); + throw JSONRPCError(RPC_MISC_ERROR, "Wiping was aborted by user."); + } + + pwallet->ShowProgress(strprintf("%s " + _("Wiping wallet transactions...").translated, pwallet->GetDisplayName()), std::max(1, std::min(99, int(progress * 100 / STEPS)))); + } + + pwallet->ShowProgress(strprintf("%s " + _("Wiping wallet transactions...").translated, pwallet->GetDisplayName()), 100); + + return NullUniValue; +} + class DescribeWalletAddressVisitor { public: @@ -4166,6 +4234,7 @@ static const CRPCCommand commands[] = { "wallet", "walletpassphrase", &walletpassphrase, {"passphrase","timeout","mixingonly"} }, { "wallet", "walletprocesspsbt", &walletprocesspsbt, {"psbt","sign","sighashtype","bip32derivs"} }, { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, + { "wallet", "wipewallettxes", &wipewallettxes, {"keep_confirmed"} }, }; // clang-format on diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d4cb379eda175..0d15f78d5b7f3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1884,7 +1884,6 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc WalletLogPrintf("Rescan started from block %s...\n", start_block.ToString()); - fAbortRescan = false; ShowProgress(strprintf("%s " + _("Rescanning...").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if -rescan on startup uint256 tip_hash; // The way the 'block_height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0. @@ -3667,6 +3666,9 @@ void CWallet::AutoLockMasternodeCollaterals() DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) { AssertLockHeld(cs_wallet); + + WalletLogPrintf("ZapSelectTx started for %d transactions...\n", vHashIn.size()); + DBErrors nZapSelectTxRet = WalletBatch(*database).ZapSelectTx(vHashIn, vHashOut); for (uint256 hash : vHashOut) { const auto& it = mapWallet.find(hash); @@ -3690,6 +3692,8 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector fAbortRescan{false}; + std::atomic fAbortRescan{false}; // reset by WalletRescanReserver::reserve() std::atomic fScanningWallet{false}; // controlled by WalletRescanReserver std::atomic m_scanning_start{0}; std::atomic m_scanning_progress{0}; @@ -1306,6 +1306,7 @@ class WalletRescanReserver } m_wallet->m_scanning_start = GetTimeMillis(); m_wallet->m_scanning_progress = 0; + m_wallet->fAbortRescan = false; m_could_reserve = true; return true; } diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 8044aad7e056c..c51a217a2cd12 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -112,7 +112,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); } - } else if (command == "info" || command == "salvage") { + } else if (command == "info" || command == "salvage" || command == "wipetxes") { if (command == "info") { std::shared_ptr wallet_instance = MakeWallet(name, path, /* create= */ false); if (!wallet_instance) return false; @@ -135,6 +135,32 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name) #else tfm::format(std::cerr, "Salvage command is not available as BDB support is not compiled"); return false; +#endif + } else if (command == "wipetxes") { +#ifdef USE_BDB + std::shared_ptr wallet_instance = MakeWallet(name, path, /* create= */ false); + if (wallet_instance == nullptr) return false; + + std::vector vHash; + std::vector vHashOut; + + LOCK(wallet_instance->cs_wallet); + + for (auto& [txid, _] : wallet_instance->mapWallet) { + vHash.push_back(txid); + } + + if (wallet_instance->ZapSelectTx(vHash, vHashOut) != DBErrors::LOAD_OK) { + tfm::format(std::cerr, "Could not properly delete transactions"); + wallet_instance->Close(); + return false; + } + + wallet_instance->Close(); + return vHashOut.size() == vHash.size(); +#else + tfm::format(std::cerr, "Wipetxes command is not available as BDB support is not compiled"); + return false; #endif } } else { diff --git a/test/functional/rpc_wipewallettxes.py b/test/functional/rpc_wipewallettxes.py new file mode 100644 index 0000000000000..ff1d252d43710 --- /dev/null +++ b/test/functional/rpc_wipewallettxes.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test transaction wiping using the wipewallettxes RPC.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_raises_rpc_error + + +class WipeWalletTxesTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + self.log.info("Test that wipewallettxes removes txes and rescanblockchain is able to recover them") + self.nodes[0].generate(101) + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + self.nodes[0].generate(1) + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 103) + self.nodes[0].wipewallettxes() + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 0) + self.nodes[0].rescanblockchain() + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 103) + + self.log.info("Test that wipewallettxes removes txes but keeps confirmed ones when asked to") + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 104) + self.nodes[0].wipewallettxes(True) + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 103) + self.nodes[0].rescanblockchain() + assert_equal(self.nodes[0].getwalletinfo()["txcount"], 103) + assert_raises_rpc_error(-5, "Invalid or non-wallet transaction id", self.nodes[0].gettransaction, txid) + + +if __name__ == '__main__': + WipeWalletTxesTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index f2fac09566b33..485fb03cbfc71 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -239,6 +239,7 @@ 'wallet_create_tx.py', 'p2p_fingerprint.py', 'rpc_platform_filter.py', + 'rpc_wipewallettxes.py', 'feature_dip0020_activation.py', 'feature_uacomment.py', 'wallet_coinbase_category.py', diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 86ec0305b0d89..2f4ef5e9f5365 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -228,6 +228,31 @@ def test_salvage(self): self.assert_tool_output('', '-wallet=salvage', 'salvage') + def test_wipe(self): + out = textwrap.dedent('''\ + Wallet info + =========== + Encrypted: no + HD (hd seed available): yes + Keypool Size: 2 + Transactions: 1 + Address Book: 1 + ''') + self.assert_tool_output(out, '-wallet=' + self.default_wallet_name, 'info') + + self.assert_tool_output('', '-wallet=' + self.default_wallet_name, 'wipetxes') + + out = textwrap.dedent('''\ + Wallet info + =========== + Encrypted: no + HD (hd seed available): yes + Keypool Size: 2 + Transactions: 0 + Address Book: 1 + ''') + self.assert_tool_output(out, '-wallet=' + self.default_wallet_name, 'info') + def run_test(self): self.wallet_path = os.path.join(self.nodes[0].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename) self.test_invalid_tool_commands_and_args() @@ -238,6 +263,7 @@ def run_test(self): self.test_getwalletinfo_on_different_wallet() if self.is_bdb_compiled(): self.test_salvage() + self.test_wipe() if __name__ == '__main__': ToolWalletTest().main() From f3dc889e93f34d54416cc44e1d2f65aab69c986c Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 28 Jun 2023 19:00:18 +0300 Subject: [PATCH 06/18] feat(qt): refresh the whole wallet instead of processing individual updates for huge notification queues (#5453) ## Issue being fixed or feature implemented It's super slow for wallets with 100.000s of txes to process lots of notifications produced by rescan. Skip them all and simply refresh the whole wallet instead. In my case (500k+ txes testnet wallet) gui update after `rescanblockchain` time is down from _forever_ to ~30 seconds. Same for `wipewallettxes true` (#5451 ). Gui update after `wipewallettxes`/`wipewallettxes false` is instant (cause there are no txes anymore) vs _forever_ before the patch. ## What was done? refresh the whole wallet when notification queue is above 10K operations actual changes (ignoring whitespaces): https://github.com/dashpay/dash/commit/d013cb4f5cf7627f75241a4a6bbfe345dd8a2dd1?w=1 ## How Has This Been Tested? running on top of #5451 and #5452 , wiping and rescanning w/ and w/out this patch. ## Breaking Changes should be none ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/qt/transactiontablemodel.cpp | 38 +++++++++++++++++++++++--------- src/qt/transactiontablemodel.h | 2 ++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 31cdd450a3550..9e7f3d7c44b6a 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -22,6 +22,7 @@ #include #include #include +#include // Amount column is right-aligned it contains numbers @@ -73,14 +74,18 @@ class TransactionTablePriv void refreshWallet(interfaces::Wallet& wallet) { qDebug() << "TransactionTablePriv::refreshWallet"; - cachedWallet.clear(); - { + parent->beginResetModel(); + try { + cachedWallet.clear(); for (const auto& wtx : wallet.getWalletTxs()) { if (TransactionRecord::showTransaction()) { cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, wtx)); } } + } catch(const std::exception& e) { + QMessageBox::critical(nullptr, PACKAGE_NAME, QString("Failed to refresh wallet table: ") + QString::fromStdString(e.what())); } + parent->endResetModel(); } /* Update our model of the wallet incrementally, to synchronize our model of the wallet @@ -242,6 +247,11 @@ TransactionTableModel::~TransactionTableModel() delete priv; } +void TransactionTableModel::refreshWallet() +{ + priv->refreshWallet(walletModel->wallet()); +} + /** Updates the column title to "Amount (DisplayUnit)" and emits headerDataChanged() signal for table headers to react. */ void TransactionTableModel::updateAmountColumnTitle() { @@ -802,18 +812,24 @@ static void ShowProgress(TransactionTableModel *ttm, const std::string &title, i if (nProgress == 100) { fQueueNotifications = false; - if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons - bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); - assert(invoked); - } - for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) - { - if (vQueueNotifications.size() - i <= 10) { - bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + if (vQueueNotifications.size() < 10000) { + if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons + bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); assert(invoked); } + for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) + { + if (vQueueNotifications.size() - i <= 10) { + bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + assert(invoked); + } - vQueueNotifications[i].invoke(ttm); + vQueueNotifications[i].invoke(ttm); + } + } else { + // it's much faster to just refresh the whole thing instead + bool invoked = QMetaObject::invokeMethod(ttm, "refreshWallet", Qt::QueuedConnection); + assert(invoked); } std::vector().swap(vQueueNotifications); // clear } diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index 5d1f6aadc1e74..16e85b935ea06 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -113,6 +113,8 @@ class TransactionTableModel : public QAbstractTableModel QVariant txAddressDecoration(const TransactionRecord *wtx) const; public Q_SLOTS: + /* Refresh the whole wallet, helpful for huge notification queues */ + void refreshWallet(); /* New transaction, or transaction changed status */ void updateTransaction(const QString &hash, int status, bool showTransaction); void updateAddressBook(const QString &address, const QString &label, From 27e68397d0c9566578ac10fe4d1b3863daf22b86 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 28 Jun 2023 19:00:38 +0300 Subject: [PATCH 07/18] fix: Allow tx index to catch up with the block index in TestChainSetup dtor (#5454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue being fixed or feature implemented TL;DR: Should hopefully fix crashes like https://gitlab.com/dashpay/dash/-/jobs/4522256293 In dashd we flush all callbacks first and then destroy `g_txindex`. In tests we had to move `g_txindex` to `TestChainSetup` and its dtor is executed first, so the order is broken. It also explains why this crash happens so rare. In most cases tx index is up to date and you need some kind of a hiccup for scheduler to lag behind a bit. Basically, between `g_txindex.reset()` and `FlushBackgroundCallbacks` `BaseIndex::BlockConnected` finally arrives. But it’s processed on a (now) null instance hence a crash. If it’s earlier - it’s processed normally, if it’s later - it’s flushed without execution, so there is a tiny window to catch this crash. ## What was done? Give tx index a bit of time to process everything ## How Has This Been Tested? run tests (but this crash is rare 🤷‍♂️ ) ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/test/util/setup_common.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 220b4a1cf6124..3edb9e385ee7b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -365,6 +365,15 @@ CBlock TestChainSetup::CreateBlock(const std::vector& txns, TestChainSetup::~TestChainSetup() { + // Allow tx index to catch up with the block index cause otherwise + // we might be destroying it while scheduler still has some work for it + // e.g. via BlockConnected signal + int64_t time_start = GetTimeMillis(); + while (!g_txindex->BlockUntilSyncedToCurrentChain()) { + static constexpr int64_t timeout_ms = 10 * 1000; + assert(time_start + timeout_ms > GetTimeMillis()); + UninterruptibleSleep(std::chrono::milliseconds{100}); + } g_txindex->Interrupt(); g_txindex->Stop(); g_txindex.reset(); From 8ef7002679d465f9088fdb8302d5bd1e1f393549 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 1 Jul 2023 03:23:49 +0300 Subject: [PATCH 08/18] fix(rpc): Improve `upgradetohd` (#5455) Allow `upgradetohd` in IBD, better errors, no GUI lock-up Pls see individual commits. Most of it is changes in whitespaces, might want to use ?w=1 to review i.e. https://github.com/dashpay/dash/pull/5455/files?w=1 run tests, try `upgradetohd` on testnet n/a - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/rpc/client.cpp | 1 - src/wallet/rpcwallet.cpp | 110 +++++++++++++++++++++------------------ 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 1bbff08c871ee..2bc1cf720fd56 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -200,7 +200,6 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createwallet", 1, "disable_private_keys"}, { "createwallet", 2, "blank"}, { "createwallet", 4, "avoid_reuse"}, - { "upgradetohd", 3, "rescan"}, { "createwallet", 5, "load_on_startup"}, { "loadwallet", 1, "load_on_startup"}, { "unloadwallet", 1, "load_on_startup"}, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9bcb468b073e1..a6c0d0eba5c61 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2665,67 +2665,66 @@ static UniValue upgradetohd(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); } - LOCK(pwallet->cs_wallet); - - // Do not do anything to HD wallets - if (pwallet->IsHDEnabled()) { - throw JSONRPCError(RPC_WALLET_ERROR, "Cannot upgrade a wallet to HD if it is already upgraded to HD."); - } - - if (!pwallet->SetMaxVersion(FEATURE_HD)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Cannot downgrade wallet"); - } + bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty(); - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); - } + { + LOCK(pwallet->cs_wallet); - bool prev_encrypted = pwallet->IsCrypted(); + // Do not do anything to HD wallets + if (pwallet->IsHDEnabled()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot upgrade a wallet to HD if it is already upgraded to HD."); + } - SecureString secureWalletPassphrase; - secureWalletPassphrase.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - if (request.params[2].isNull()) { - if (prev_encrypted) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Cannot upgrade encrypted wallet to HD without the wallet passphrase"); + if (!pwallet->SetMaxVersion(FEATURE_HD)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot downgrade wallet"); } - } else { - secureWalletPassphrase = request.params[2].get_str().c_str(); - if (!pwallet->Unlock(secureWalletPassphrase)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "The wallet passphrase entered was incorrect"); + + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); } - } - bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty(); + bool prev_encrypted = pwallet->IsCrypted(); - SecureString secureMnemonic; - secureMnemonic.reserve(256); - if (!generate_mnemonic) { - if (pwallet->chain().isInitialBlockDownload()) { - throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set mnemonic while still in Initial Block Download"); + SecureString secureWalletPassphrase; + secureWalletPassphrase.reserve(100); + // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) + // Alternately, find a way to make request.params[0] mlock()'d to begin with. + if (request.params[2].isNull()) { + if (prev_encrypted) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Cannot upgrade encrypted wallet to HD without the wallet passphrase"); + } + } else { + secureWalletPassphrase = request.params[2].get_str().c_str(); + if (!pwallet->Unlock(secureWalletPassphrase)) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "The wallet passphrase entered was incorrect"); + } } - secureMnemonic = request.params[0].get_str().c_str(); - } - SecureString secureMnemonicPassphrase; - secureMnemonicPassphrase.reserve(256); - if (!request.params[1].isNull()) { - secureMnemonicPassphrase = request.params[1].get_str().c_str(); - } - - pwallet->WalletLogPrintf("Upgrading wallet to HD\n"); - pwallet->SetMinVersion(FEATURE_HD); + SecureString secureMnemonic; + secureMnemonic.reserve(256); + if (!generate_mnemonic) { + secureMnemonic = request.params[0].get_str().c_str(); + } - if (prev_encrypted) { - if (!spk_man->GenerateNewHDChainEncrypted(secureMnemonic, secureMnemonicPassphrase, secureWalletPassphrase)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Failed to generate encrypted HD wallet"); + SecureString secureMnemonicPassphrase; + secureMnemonicPassphrase.reserve(256); + if (!request.params[1].isNull()) { + secureMnemonicPassphrase = request.params[1].get_str().c_str(); } - } else { - spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase); - if (!secureWalletPassphrase.empty()) { - if (!pwallet->EncryptWallet(secureWalletPassphrase)) { - throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Failed to encrypt HD wallet"); + + pwallet->WalletLogPrintf("Upgrading wallet to HD\n"); + pwallet->SetMinVersion(FEATURE_HD); + + if (prev_encrypted) { + if (!spk_man->GenerateNewHDChainEncrypted(secureMnemonic, secureMnemonicPassphrase, secureWalletPassphrase)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Failed to generate encrypted HD wallet"); + } + } else { + spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase); + if (!secureWalletPassphrase.empty()) { + if (!pwallet->EncryptWallet(secureWalletPassphrase)) { + throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Failed to encrypt HD wallet"); + } } } } @@ -2737,7 +2736,16 @@ static UniValue upgradetohd(const JSONRPCRequest& request) if (!reserver.reserve()) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - pwallet->ScanForWalletTransactions(pwallet->chain().getBlockHash(0), {}, reserver, true); + CWallet::ScanResult result = pwallet->ScanForWalletTransactions(pwallet->chain().getBlockHash(0), {}, reserver, true); + switch (result.status) { + case CWallet::ScanResult::SUCCESS: + break; + case CWallet::ScanResult::FAILURE: + throw JSONRPCError(RPC_MISC_ERROR, "Rescan failed. Potentially corrupted data files."); + case CWallet::ScanResult::USER_ABORT: + throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted."); + // no default case, so the compiler can warn about missing cases + } } return true; From 2c2f81675e9ec5a736ceed0e8faa6ca62ead69de Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 28 Jun 2023 19:01:00 +0300 Subject: [PATCH 09/18] feat(wallet): TopUpKeyPool improvements (#5456) - make progress calculations sane - show progress in GUI but only when you need 100+ new keys - make it stop on shutdown request - spam less in debug.log run tests, run `keypoolrefill` with `1100` (add 100 keys, no gui popup) and `10000` (100+ keys, progress bar) on testnet wallet, check logs, verify it can be interrupted on shutdown n/a - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --- src/wallet/scriptpubkeyman.cpp | 45 ++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 0af873ca12565..d866009ba4313 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -7,6 +7,7 @@ #include #include #include