From 3340dbadd38f5624642cf0e14dddbe6f83a3863b Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 5 Aug 2020 16:37:55 -0400 Subject: [PATCH] Remove -zapwallettxes -zapwallettxes is made a hidden option to inform users that it is removed and they should be using abandontransaction to do the stuck transaction thing. --- doc/release-notes-19671.md | 6 ++ src/dummywallet.cpp | 1 - src/wallet/init.cpp | 22 ++----- src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 57 ------------------ src/wallet/wallet.h | 1 - src/wallet/walletdb.cpp | 17 ------ src/wallet/walletdb.h | 1 - test/functional/test_runner.py | 1 - test/functional/wallet_basic.py | 2 - test/functional/wallet_multiwallet.py | 5 -- test/functional/wallet_zapwallettxes.py | 79 ------------------------- test/lint/check-doc.py | 2 +- 13 files changed, 12 insertions(+), 184 deletions(-) create mode 100644 doc/release-notes-19671.md delete mode 100755 test/functional/wallet_zapwallettxes.py diff --git a/doc/release-notes-19671.md b/doc/release-notes-19671.md new file mode 100644 index 000000000..fb2d56d9a --- /dev/null +++ b/doc/release-notes-19671.md @@ -0,0 +1,6 @@ +Wallet +------ + +* The `-zapwallettxes` startup option has been removed and its functionality removed from the wallet. + This option was originally intended to allow for the fee bumping of transactions that did not + signal RBF. This functionality has been superseded with the abandon transaction feature. diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 380d4eb8a..031b58373 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -49,7 +49,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const "-walletdir=", "-walletnotify=", "-walletrbf", - "-zapwallettxes=", "-dblogsize=", "-flushwallet", "-privdb", diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index bf05ef844..aaa3d957f 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -66,13 +66,13 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - argsman.AddArg("-zapwallettxes=", "Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup" - " (1 = keep tx meta data e.g. payment request information, 2 = drop tx meta data)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-dblogsize=", strprintf("Flush wallet database activity from memory to disk log every megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST); + + argsman.AddHiddenArgs({"-zapwallettxes"}); } bool WalletInit::ParameterInteraction() const @@ -85,26 +85,12 @@ bool WalletInit::ParameterInteraction() const return true; } - const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; - if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__); } - bool zapwallettxes = gArgs.GetBoolArg("-zapwallettxes", false); - // -zapwallettxes implies dropping the mempool on startup - if (zapwallettxes && gArgs.SoftSetBoolArg("-persistmempool", false)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -persistmempool=0\n", __func__); - } - - // -zapwallettxes implies a rescan - if (zapwallettxes) { - if (is_multiwallet) { - return InitError(strprintf(Untranslated("%s is only allowed with a single wallet file"), "-zapwallettxes")); - } - if (gArgs.SoftSetBoolArg("-rescan", true)) { - LogPrintf("%s: parameter interaction: -zapwallettxes enabled -> setting -rescan=1\n", __func__); - } + if (gArgs.IsArgSet("-zapwallettxes")) { + return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); } if (gArgs.GetBoolArg("-sysperms", false)) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7a03c4f54..d8b75f3fc 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2494,7 +2494,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) RPCHelpMan{"loadwallet", "\nLoads a wallet from a wallet file or directory." "\nNote that all wallet command-line options used when starting bitcoind will be" - "\napplied to the new wallet (eg -zapwallettxes, rescan, etc).\n", + "\napplied to the new wallet (eg -rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fa00d1255..100ee4a8a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3178,25 +3178,6 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vWtx) -{ - DBErrors nZapWalletTxRet = WalletBatch(*database,"cr+").ZapWalletTx(vWtx); - if (nZapWalletTxRet == DBErrors::NEED_REWRITE) - { - if (database->Rewrite("\x04pool")) - { - for (const auto& spk_man_pair : m_spk_managers) { - spk_man_pair.second->RewriteDB(); - } - } - } - - if (nZapWalletTxRet != DBErrors::LOAD_OK) - return nZapWalletTxRet; - - return DBErrors::LOAD_OK; -} - bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose) { bool fUpdated = false; @@ -3778,20 +3759,6 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, { const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); - // needed to restore wallet transaction meta data after -zapwallettxes - std::list vWtx; - - if (gArgs.GetBoolArg("-zapwallettxes", false)) { - chain.initMessage(_("Zapping all transactions from wallet...").translated); - - std::unique_ptr tempWallet = MakeUnique(&chain, location, CreateWalletDatabase(location.GetPath())); - DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); - if (nZapWalletRet != DBErrors::LOAD_OK) { - error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); - return nullptr; - } - } - chain.initMessage(_("Loading wallet...").translated); int64_t nStart = GetTimeMillis(); @@ -4068,30 +4035,6 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->database->IncrementUpdateCounter(); - - // Restore wallet transaction metadata after -zapwallettxes=1 - if (gArgs.GetBoolArg("-zapwallettxes", false) && gArgs.GetArg("-zapwallettxes", "1") != "2") - { - WalletBatch batch(*walletInstance->database); - - for (const CWalletTx& wtxOld : vWtx) - { - uint256 hash = wtxOld.GetHash(); - std::map::iterator mi = walletInstance->mapWallet.find(hash); - if (mi != walletInstance->mapWallet.end()) - { - const CWalletTx* copyFrom = &wtxOld; - CWalletTx* copyTo = &mi->second; - copyTo->mapValue = copyFrom->mapValue; - copyTo->vOrderForm = copyFrom->vOrderForm; - copyTo->nTimeReceived = copyFrom->nTimeReceived; - copyTo->nTimeSmart = copyFrom->nTimeSmart; - copyTo->fFromMe = copyFrom->fFromMe; - copyTo->nOrderPos = copyFrom->nOrderPos; - batch.WriteTx(*copyTo); - } - } - } } { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f421de0cf..e9c510657 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1075,7 +1075,6 @@ public: void chainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); - DBErrors ZapWalletTx(std::list& vWtx); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& purpose); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index fa6814d0d..f25acee1e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -926,23 +926,6 @@ DBErrors WalletBatch::ZapSelectTx(std::vector& vTxHashIn, std::vector& vWtx) -{ - // build list of wallet TXs - std::vector vTxHash; - DBErrors err = FindWalletTx(vTxHash, vWtx); - if (err != DBErrors::LOAD_OK) - return err; - - // erase each wallet TX - for (const uint256& hash : vTxHash) { - if (!EraseTx(hash)) - return DBErrors::CORRUPT; - } - - return DBErrors::LOAD_OK; -} - void MaybeCompactWalletDB() { static std::atomic fOneThread(false); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 64d60b1f4..c13de0131 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -257,7 +257,6 @@ public: DBErrors LoadWallet(CWallet* pwallet); DBErrors FindWalletTx(std::vector& vTxHash, std::list& vWtx); - DBErrors ZapWalletTx(std::list& vWtx); DBErrors ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut); /* Function to determine if a certain KV/key-type is a key (cryptographical key) type */ static bool IsKeyType(const std::string& strType); diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 01232bda3..28fdf354d 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -105,7 +105,6 @@ BASE_SCRIPTS = [ 'wallet_listtransactions.py', # vv Tests less than 60s vv 'p2p_sendheaders.py', - 'wallet_zapwallettxes.py', 'wallet_importmulti.py', 'mempool_limit.py', 'rpc_txoutproof.py', diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 71a1a3f4f..c52c974e0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -526,8 +526,6 @@ class WalletTest(BitcoinTestFramework): maintenance = [ '-rescan', '-reindex', - '-zapwallettxes=1', - '-zapwallettxes=2', ] chainlimit = 6 for m in maintenance: diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 1872545cd..5c9d7ff62 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -134,11 +134,6 @@ class MultiWalletTest(BitcoinTestFramework): open(not_a_dir, 'a', encoding="utf8").close() self.nodes[0].assert_start_raises_init_error(['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory') - self.log.info("Do not allow -zapwallettxes with multiwallet") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") - # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) diff --git a/test/functional/wallet_zapwallettxes.py b/test/functional/wallet_zapwallettxes.py deleted file mode 100755 index 1287092ca..000000000 --- a/test/functional/wallet_zapwallettxes.py +++ /dev/null @@ -1,79 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2014-2018 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Test the zapwallettxes functionality. - -- start two bitcoind nodes -- create two transactions on node 0 - one is confirmed and one is unconfirmed. -- restart node 0 and verify that both the confirmed and the unconfirmed - transactions are still available. -- restart node 0 with zapwallettxes and persistmempool, and verify that both - the confirmed and the unconfirmed transactions are still available. -- restart node 0 with just zapwallettxes and verify that the confirmed - transactions are still available, but that the unconfirmed transaction has - been zapped. -""" -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import ( - assert_equal, - assert_raises_rpc_error, -) - - -class ZapWalletTXesTest (BitcoinTestFramework): - def set_test_params(self): - self.setup_clean_chain = True - self.num_nodes = 2 - - def skip_test_if_missing_module(self): - self.skip_if_no_wallet() - - def run_test(self): - self.log.info("Mining blocks...") - self.nodes[0].generate(1) - self.sync_all() - self.nodes[1].generate(100) - self.sync_all() - - # This transaction will be confirmed - txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 10) - - self.nodes[0].generate(1) - self.sync_all() - - # This transaction will not be confirmed - txid2 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 20) - - # Confirmed and unconfirmed transactions are now in the wallet. - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0. Both confirmed and unconfirmed transactions remain in the wallet. - self.restart_node(0) - - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0 with zapwallettxes and persistmempool. The unconfirmed - # transaction is zapped from the wallet, but is re-added when the mempool is reloaded. - self.restart_node(0, ["-persistmempool=1", "-zapwallettxes=2"]) - - self.wait_until(lambda: self.nodes[0].getmempoolinfo()['size'] == 1, timeout=3) - self.nodes[0].syncwithvalidationinterfacequeue() # Flush mempool to wallet - - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - assert_equal(self.nodes[0].gettransaction(txid2)['txid'], txid2) - - # Restart node0 with zapwallettxes, but not persistmempool. - # The unconfirmed transaction is zapped and is no longer in the wallet. - self.restart_node(0, ["-zapwallettxes=2"]) - - # tx1 is still be available because it was confirmed - assert_equal(self.nodes[0].gettransaction(txid1)['txid'], txid1) - - # This will raise an exception because the unconfirmed transaction has been zapped - assert_raises_rpc_error(-5, 'Invalid or non-wallet transaction id', self.nodes[0].gettransaction, txid2) - -if __name__ == '__main__': - ZapWalletTXesTest().main() diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py index bd947d194..f77242d33 100755 --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -23,7 +23,7 @@ CMD_GREP_WALLET_ARGS = r"git grep --function-context 'void WalletInit::AddWallet CMD_GREP_WALLET_HIDDEN_ARGS = r"git grep --function-context 'void DummyWalletInit::AddWalletOptions' -- {}".format(CMD_ROOT_DIR) CMD_GREP_DOCS = r"git grep --perl-regexp '{}' {}".format(REGEX_DOC, CMD_ROOT_DIR) # list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb']) +SET_DOC_OPTIONAL = set(['-h', '-help', '-dbcrashratio', '-forcecompactdb', '-zapwallettxes']) def lint_missing_argument_documentation():