From c04f74b1d4d69773a6833973d956205630a65a0d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 12 Dec 2021 19:08:12 +0530 Subject: [PATCH] merge bitcoin#15680: Remove resendwallettransactions RPC method --- src/wallet/rpcwallet.cpp | 44 ------------------------- src/wallet/wallet.cpp | 57 ++++++++++++--------------------- src/wallet/wallet.h | 2 -- test/functional/wallet_basic.py | 16 +-------- 4 files changed, 22 insertions(+), 97 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 41fddaa13f..155f6a3842 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3013,49 +3013,6 @@ static UniValue unloadwallet(const JSONRPCRequest& request) return NullUniValue; } -static UniValue resendwallettransactions(const JSONRPCRequest& request) -{ - std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); - CWallet* const pwallet = wallet.get(); - - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { - return NullUniValue; - } - - if (request.fHelp || request.params.size() != 0) - throw std::runtime_error( - RPCHelpMan{"resendwallettransactions", - "Immediately re-broadcast unconfirmed wallet transactions to all peers.\n" - "Intended only for testing; the wallet code periodically re-broadcasts\n" - "automatically.\n", - {}, - RPCResult{ - "Returns an RPC error if -walletbroadcast is set to false.\n" - "Returns array of transaction ids that were re-broadcast.\n" - }, - RPCExamples{""}, - }.ToString() - ); - - if (!pwallet->chain().p2pEnabled()) - throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); - - auto locked_chain = pwallet->chain().lock(); - LOCK2(mempool.cs, pwallet->cs_wallet); - - if (!pwallet->GetBroadcastTransactions()) { - throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet transaction broadcasting is disabled with -walletbroadcast"); - } - - std::vector txids = pwallet->ResendWalletTransactionsBefore(*locked_chain, GetTime()); - UniValue result(UniValue::VARR); - for (const uint256& txid : txids) - { - result.push_back(txid.ToString()); - } - return result; -} - static UniValue listunspent(const JSONRPCRequest& request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); @@ -4205,7 +4162,6 @@ static const CRPCCommand commands[] = { "hidden", "generate", &generate, {"nblocks","maxtries"} }, // Hidden as it isn't functional, just an error to let people know if miner isn't compiled #endif //ENABLE_MINER { "hidden", "instantsendtoaddress", &instantsendtoaddress, {} }, - { "hidden", "resendwallettransactions", &resendwallettransactions, {} }, { "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options"} }, { "wallet", "abandontransaction", &abandontransaction, {"txid"} }, { "wallet", "abortrescan", &abortrescan, {} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f2276636d9..c024c04614 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2598,52 +2598,37 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const return CTransaction(tx1) == CTransaction(tx2); } -std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime) -{ - std::vector result; - - LOCK2(mempool.cs, cs_wallet); - - // Sort them in chronological order - std::multimap mapSorted; - for (std::pair& item : mapWallet) - { - CWalletTx& wtx = item.second; - // Don't rebroadcast if newer than nTime: - if (wtx.nTimeReceived > nTime) - continue; - mapSorted.insert(std::make_pair(wtx.nTimeReceived, &wtx)); - } - for (const std::pair& item : mapSorted) - { - CWalletTx& wtx = *item.second; - if (wtx.RelayWalletTransaction(locked_chain)) - result.push_back(wtx.GetHash()); - } - return result; -} - void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) { // Do this infrequently and randomly to avoid giving away // that these are our transactions. - if (GetTime() < nNextResend || !fBroadcastTransactions) - return; + if (GetTime() < nNextResend || !fBroadcastTransactions) return; bool fFirst = (nNextResend == 0); nNextResend = GetTime() + GetRand(30 * 60); - if (fFirst) - return; + if (fFirst) return; // Only do it if there's been a new block since last time - if (nBestBlockTime < nLastResend) - return; + if (nBestBlockTime < nLastResend) return; nLastResend = GetTime(); - // Rebroadcast unconfirmed txes older than 5 minutes before the last - // block was found: - std::vector relayed = ResendWalletTransactionsBefore(locked_chain, nBestBlockTime-5*60); - if (!relayed.empty()) - WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); + int relayed_tx_count = 0; + + { // mempool.cs and cs_wallet scope + LOCK2(mempool.cs, cs_wallet); + + // Relay transactions + for (std::pair& item : mapWallet) { + CWalletTx& wtx = item.second; + // only rebroadcast unconfirmed txes older than 5 minutes before the + // last block was found + if (wtx.nTimeReceived > nBestBlockTime - 5 * 60) continue; + relayed_tx_count += wtx.RelayWalletTransaction(*locked_chain) ? 1 : 0; + } + } // mempool.cs and cs_wallet + + if (relayed_tx_count > 0) { + WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed_tx_count); + } } /** @} */ // end of mapWallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ad5d88a82e..5011195914 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1004,8 +1004,6 @@ public: void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override; void ReacceptWalletTransactions(); void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) override; - // ResendWalletTransactionsBefore may only be called if fBroadcastTransactions! - std::vector ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime); struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index cd3eb2815a..7faeba932d 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -213,23 +213,9 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[2].getbalance(), node_2_bal) node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('100'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex'])) - # Test ResendWalletTransactions: - # Create a couple of transactions, then start up a fourth - # node (nodes[3]) and ask nodes[0] to rebroadcast. - # EXPECT: nodes[3] should have those transactions in its mempool. - txid1 = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1) - txid2 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - self.sync_mempools(self.nodes[0:2]) - self.start_node(3) connect_nodes_bi(self.nodes, 0, 3) - self.sync_blocks() - - relayed = self.nodes[0].resendwallettransactions() - assert_equal(set(relayed), {txid1, txid2}) - self.sync_mempools() - - assert txid1 in self.nodes[3].getrawmempool() + self.sync_all() # check if we can list zero value tx as available coins # 1. create raw_tx