From 0f5954c434fdd04b9abca6ddc6f1bbf895b6c6be Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Mon, 23 Mar 2015 13:47:18 -0400 Subject: [PATCH] Regression test for ResendWalletTransactions Adds a regression test for the wallet's ResendWalletTransactions function, which uses a new, hidden RPC command "resendwallettransactions." I refactored main's Broadcast signal so it is passed the best-block time, which let me remove a global variable shared between main.cpp and the wallet (nTimeBestReceived). I also manually tested the "rebroadcast unconfirmed every half hour or so" functionality by: 1. Running bitcoind -connect=0.0.0.0:8333 2. Creating a couple of send-to-self transactions 3. Connect to a peer using -addnode 4. Waited a while, monitoring debug.log, until I see: ```2015-03-23 18:48:10 ResendWalletTransactions: rebroadcast 2 unconfirmed transactions``` One last change: don't bother putting ResendWalletTransactions messages in debug.log unless unconfirmed transactions were actually rebroadcast. --- qa/rpc-tests/wallet.py | 21 +++++++++++++- src/main.cpp | 2 +- src/main.h | 1 - src/rpcserver.cpp | 3 ++ src/rpcserver.h | 1 + src/validationinterface.cpp | 4 +-- src/validationinterface.h | 4 +-- src/wallet/rpcwallet.cpp | 22 ++++++++++++++ src/wallet/wallet.cpp | 57 ++++++++++++++++++++++--------------- src/wallet/wallet.h | 5 ++-- 10 files changed, 88 insertions(+), 32 deletions(-) diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index dc4e0f77b..01e9fa57b 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -16,6 +16,7 @@ # h) node0 should now have 2 unspent outputs; send these to node2 via raw tx broadcast by node1 # i) have node1 mine a block # j) check balances - node0 should have 0, node2 should have 100 +# k) test ResendWalletTransactions - create transactions, startup fourth node, make sure it syncs # from test_framework import BitcoinTestFramework @@ -26,7 +27,7 @@ class WalletTest (BitcoinTestFramework): def setup_chain(self): print("Initializing test directory "+self.options.tmpdir) - initialize_chain_clean(self.options.tmpdir, 3) + initialize_chain_clean(self.options.tmpdir, 4) def setup_network(self, split=False): self.nodes = start_nodes(3, self.options.tmpdir) @@ -132,5 +133,23 @@ class WalletTest (BitcoinTestFramework): assert_equal(self.nodes[2].getbalance(), Decimal('59.99800000')) assert_equal(self.nodes[0].getbalance(), Decimal('39.99800000')) + # 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) + sync_mempools(self.nodes) + + self.nodes.append(start_node(3, self.options.tmpdir)) + connect_nodes_bi(self.nodes, 0, 3) + sync_blocks(self.nodes) + + relayed = self.nodes[0].resendwallettransactions() + assert_equal(set(relayed), set([txid1, txid2])) + sync_mempools(self.nodes) + + assert(txid1 in self.nodes[3].getrawmempool()) + if __name__ == '__main__': WalletTest ().main () diff --git a/src/main.cpp b/src/main.cpp index 0ffacc338..8f50e7fc1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4475,7 +4475,7 @@ bool SendMessages(CNode* pto, bool fSendTrickle) // transactions become unconfirmed and spams other nodes. if (!fReindex && !fImporting && !IsInitialBlockDownload()) { - GetMainSignals().Broadcast(); + GetMainSignals().Broadcast(nTimeBestReceived); } // diff --git a/src/main.h b/src/main.h index b0bab6f7b..31de3f6ac 100644 --- a/src/main.h +++ b/src/main.h @@ -116,7 +116,6 @@ extern BlockMap mapBlockIndex; extern uint64_t nLastBlockTx; extern uint64_t nLastBlockSize; extern const std::string strMessageMagic; -extern int64_t nTimeBestReceived; extern CWaitableCriticalSection csBestBlock; extern CConditionVariable cvBlockChange; extern bool fImporting; diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index ba7172522..d30fa32eb 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -333,6 +333,9 @@ static const CRPCCommand vRPCCommands[] = { "hidden", "invalidateblock", &invalidateblock, true, false }, { "hidden", "reconsiderblock", &reconsiderblock, true, false }, { "hidden", "setmocktime", &setmocktime, true, false }, +#ifdef ENABLE_WALLET + { "hidden", "resendwallettransactions", &resendwallettransactions, true, true }, +#endif #ifdef ENABLE_WALLET /* Wallet */ diff --git a/src/rpcserver.h b/src/rpcserver.h index f63438ecb..7011d41fc 100644 --- a/src/rpcserver.h +++ b/src/rpcserver.h @@ -207,6 +207,7 @@ extern json_spirit::Value getwalletinfo(const json_spirit::Array& params, bool f extern json_spirit::Value getblockchaininfo(const json_spirit::Array& params, bool fHelp); extern json_spirit::Value getnetworkinfo(const json_spirit::Array& params, bool fHelp); extern json_spirit::Value setmocktime(const json_spirit::Array& params, bool fHelp); +extern json_spirit::Value resendwallettransactions(const json_spirit::Array& params, bool fHelp); extern json_spirit::Value getrawtransaction(const json_spirit::Array& params, bool fHelp); // in rcprawtransaction.cpp extern json_spirit::Value listunspent(const json_spirit::Array& params, bool fHelp); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index ae4cd3c59..aa9aefb0d 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -18,13 +18,13 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); - g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn)); + g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); g_signals.BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); } void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); - g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn)); + g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); diff --git a/src/validationinterface.h b/src/validationinterface.h index b21b6e578..cb261f6aa 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -33,7 +33,7 @@ protected: virtual void SetBestChain(const CBlockLocator &locator) {}; virtual void UpdatedTransaction(const uint256 &hash) {}; virtual void Inventory(const uint256 &hash) {}; - virtual void ResendWalletTransactions() {}; + virtual void ResendWalletTransactions(int64_t nBestBlockTime) {}; virtual void BlockChecked(const CBlock&, const CValidationState&) {}; friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); @@ -52,7 +52,7 @@ struct CMainSignals { /** Notifies listeners about an inventory item being seen on the network. */ boost::signals2::signal Inventory; /** Tells listeners to broadcast their data. */ - boost::signals2::signal Broadcast; + boost::signals2::signal Broadcast; /** Notifies listeners of a block validation result */ boost::signals2::signal BlockChecked; }; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9318c1b2b..29f3eda15 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2096,3 +2096,25 @@ Value getwalletinfo(const Array& params, bool fHelp) obj.push_back(Pair("unlocked_until", nWalletUnlockTime)); return obj; } + +Value resendwallettransactions(const Array& params, bool fHelp) +{ + if (fHelp || params.size() != 0) + throw runtime_error( + "resendwallettransactions\n" + "Immediately re-broadcast unconfirmed wallet transactions to all peers.\n" + "Intended only for testing; the wallet code periodically re-broadcasts\n" + "automatically.\n" + "Returns array of transaction ids that were re-broadcast.\n" + ); + + LOCK2(cs_main, pwalletMain->cs_wallet); + + std::vector txids = pwalletMain->ResendWalletTransactionsBefore(GetTime()); + Array result; + BOOST_FOREACH(const uint256& txid, txids) + { + result.push_back(txid.ToString()); + } + return result; +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 09bcda577..a10123002 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1114,15 +1114,17 @@ void CWallet::ReacceptWalletTransactions() } } -void CWalletTx::RelayWalletTransaction() +bool CWalletTx::RelayWalletTransaction() { if (!IsCoinBase()) { if (GetDepthInMainChain() == 0) { LogPrintf("Relaying wtx %s\n", GetHash().ToString()); RelayTransaction((CTransaction)*this); + return true; } } + return false; } set CWalletTx::GetConflicts() const @@ -1324,7 +1326,31 @@ bool CWalletTx::IsTrusted() const return true; } -void CWallet::ResendWalletTransactions() +std::vector CWallet::ResendWalletTransactionsBefore(int64_t nTime) +{ + std::vector result; + + LOCK(cs_wallet); + // Sort them in chronological order + multimap mapSorted; + BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) + { + CWalletTx& wtx = item.second; + // Don't rebroadcast if newer than nTime: + if (wtx.nTimeReceived > nTime) + continue; + mapSorted.insert(make_pair(wtx.nTimeReceived, &wtx)); + } + BOOST_FOREACH(PAIRTYPE(const unsigned int, CWalletTx*)& item, mapSorted) + { + CWalletTx& wtx = *item.second; + if (wtx.RelayWalletTransaction()) + result.push_back(wtx.GetHash()); + } + return result; +} + +void CWallet::ResendWalletTransactions(int64_t nBestBlockTime) { // Do this infrequently and randomly to avoid giving away // that these are our transactions. @@ -1336,30 +1362,15 @@ void CWallet::ResendWalletTransactions() return; // Only do it if there's been a new block since last time - if (nTimeBestReceived < nLastResend) + if (nBestBlockTime < nLastResend) return; nLastResend = GetTime(); - // Rebroadcast any of our txes that aren't in a block yet - LogPrintf("ResendWalletTransactions()\n"); - { - LOCK(cs_wallet); - // Sort them in chronological order - multimap mapSorted; - BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) - { - CWalletTx& wtx = item.second; - // Don't rebroadcast until it's had plenty of time that - // it should have gotten in already by now. - if (nTimeBestReceived - (int64_t)wtx.nTimeReceived > 5 * 60) - mapSorted.insert(make_pair(wtx.nTimeReceived, &wtx)); - } - BOOST_FOREACH(PAIRTYPE(const unsigned int, CWalletTx*)& item, mapSorted) - { - CWalletTx& wtx = *item.second; - wtx.RelayWalletTransaction(); - } - } + // Rebroadcast unconfirmed txes older than 5 minutes before the last + // block was found: + std::vector relayed = ResendWalletTransactionsBefore(nBestBlockTime-5*60); + if (!relayed.empty()) + LogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); } /** @} */ // end of mapWallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4a13f0219..6ae1c87b1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -381,7 +381,7 @@ public: int64_t GetTxTime() const; int GetRequestCount() const; - void RelayWalletTransaction(); + bool RelayWalletTransaction(); std::set GetConflicts() const; }; @@ -614,7 +614,8 @@ public: void EraseFromWallet(const uint256 &hash); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); - void ResendWalletTransactions(); + void ResendWalletTransactions(int64_t nBestBlockTime); + std::vector ResendWalletTransactionsBefore(int64_t nTime); CAmount GetBalance() const; CAmount GetUnconfirmedBalance() const; CAmount GetImmatureBalance() const;