diff --git a/src/Makefile.test.include b/src/Makefile.test.include index abcfeb73a6..5907bcccdc 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -164,6 +164,7 @@ BITCOIN_TESTS =\ test/validation_chainstate_tests.cpp \ test/validation_chainstatemanager_tests.cpp \ test/validation_flush_tests.cpp \ + test/validationinterface_tests.cpp \ test/versionbits_tests.cpp if ENABLE_WALLET diff --git a/src/net.h b/src/net.h index aee79057a6..c557d6042c 100644 --- a/src/net.h +++ b/src/net.h @@ -1129,8 +1129,8 @@ public: std::vector vAddrToSend; const std::unique_ptr m_addr_known; bool fGetAddr{false}; - int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing){0}; - int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing){0}; + std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; + std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; // Don't relay addr messages to peers that we connect to as block-relay-only // peers (to prevent adversaries from inferring these links from addr diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0effb61250..1a26996d90 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -138,9 +138,9 @@ static const int MAX_UNCONNECTING_HEADERS = 10; static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; /** Average delay between local address broadcasts in seconds. */ -static constexpr unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60; +static constexpr std::chrono::hours AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL{24}; /** Average delay between peer address broadcasts in seconds. */ -static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30; +static constexpr std::chrono::seconds AVG_ADDRESS_BROADCAST_INTERVAL{30}; /** Average delay between trickled inventory transmissions in seconds. * Blocks and peers with noban permission bypass this, regular outbound peers get half this delay, * Masternode outbound peers get quarter this delay. */ @@ -1889,13 +1889,13 @@ void RelayTransaction(const uint256& txid, const CConnman& connman) }); } -static void RelayAddress(const CAddress& addr, bool fReachable, CConnman& connman) +static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& connman) { // Relay to a limited number of other nodes // Use deterministic randomness to send to the same nodes for 24 hours // at a time so the m_addr_knowns of the chosen nodes prevent repeats uint64_t hashAddr = addr.GetHash(); - const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24*60*60)); + const CSipHasher hasher = connman.GetDeterministicRandomizer(RANDOMIZER_ID_ADDRESS_RELAY).Write(hashAddr << 32).Write((GetTime() + hashAddr) / (24 * 60 * 60)); FastRandomContext insecure_rand; // Relay reachable addresses to 2 peers. Unreachable addresses are relayed randomly to 1 or 2 peers. @@ -4678,16 +4678,16 @@ bool PeerManagerImpl::SendMessages(CNode* pto) int64_t nNow = GetTimeMicros(); auto current_time = GetTime(); - if (pto->IsAddrRelayPeer() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) { + if (pto->IsAddrRelayPeer() && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { AdvertiseLocal(pto); - pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); + pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } // // Message: addr // - if (pto->IsAddrRelayPeer() && pto->nNextAddrSend < nNow) { - pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL); + if (pto->IsAddrRelayPeer() && pto->m_next_addr_send < current_time) { + pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector vAddr; vAddr.reserve(pto->vAddrToSend.size()); diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index f424053c0a..d27fb9bc20 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -151,7 +151,6 @@ public: interfaces::Node& node() const { return m_node; } interfaces::Wallet& wallet() const { return *m_wallet; } void setClientModel(ClientModel* client_model); - ClientModel& clientModel() const { return *m_client_model; } interfaces::CoinJoin::Client& coinJoin() const { return m_wallet->coinJoin(); } QString getWalletName() const; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f24e968f24..473f58c217 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2946,6 +2946,8 @@ UniValue CreateUTXOSnapshot(NodeContext& node, CChainState& chainstate, CAutoFil return result; } +void RegisterBlockchainRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -2991,8 +2993,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterBlockchainRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index ef970c937a..f3e6d2993a 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -170,6 +170,8 @@ static UniValue getcoinjoininfo(const JSONRPCRequest& request) return obj; } +void RegisterCoinJoinRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -181,8 +183,6 @@ static const CRPCCommand commands[] = #endif // ENABLE_WALLET }; // clang-format on -void RegisterCoinJoinRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 536c2766ef..fec1e50cb0 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1755,6 +1755,8 @@ static UniValue _bls(const JSONRPCRequest& request) bls_help(); } } +void RegisterEvoRPCCommands(CRPCTable &tableRPC) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) @@ -1763,8 +1765,6 @@ static const CRPCCommand commands[] = { "evo", "protx", &protx, {} }, }; // clang-format on -void RegisterEvoRPCCommands(CRPCTable &tableRPC) -{ for (const auto& command : commands) { tableRPC.appendCommand(command.name, &command); } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index cab85368d2..9ffa8f0271 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -1191,6 +1191,8 @@ static UniValue getsuperblockbudget(const JSONRPCRequest& request) return ValueFromAmount(CSuperblock::GetPaymentsLimit(nBlockHeight)); } +void RegisterGovernanceRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -1203,8 +1205,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterGovernanceRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 63b8973942..0058e419ff 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -736,6 +736,9 @@ static UniValue masternodelist(const JSONRPCRequest& request, ChainstateManager& return obj; } + +void RegisterMasternodeRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -744,8 +747,6 @@ static const CRPCCommand commands[] = { "dash", "masternodelist", &masternode, {} }, }; // clang-format on -void RegisterMasternodeRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3ae7a716af..d0ae3585c7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1249,6 +1249,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) return result; } +void RegisterMiningRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -1276,8 +1278,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterMiningRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index aac2c3a28e..7f6f7f9978 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -1363,11 +1363,12 @@ static RPCHelpMan getindexinfo() }); return result; -}, +} }; } -// clang-format off +void RegisterMiscRPCCommands(CRPCTable &t) +{ static const CRPCCommand commands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- @@ -1404,8 +1405,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterMiscRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index e972f7a942..a63cffdb97 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -884,6 +884,8 @@ static UniValue addpeeraddress(const JSONRPCRequest& request) return obj; } +void RegisterNetRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -906,8 +908,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterNetRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 592d73d0da..fa620da92e 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -989,6 +989,8 @@ static UniValue verifyislock(const JSONRPCRequest& request) return llmq_ctx.sigman->VerifyRecoveredSig(llmqType, *llmq_ctx.qman, signHeight, id, txid, sig, 0) || llmq_ctx.sigman->VerifyRecoveredSig(llmqType, *llmq_ctx.qman, signHeight, id, txid, sig, signOffset); } +void RegisterQuorumsRPCCommands(CRPCTable &tableRPC) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) @@ -998,8 +1000,6 @@ static const CRPCCommand commands[] = { "evo", "verifyislock", &verifyislock, {"id", "txid", "signature", "maxHeight"} }, }; // clang-format on -void RegisterQuorumsRPCCommands(CRPCTable &tableRPC) -{ for (const auto& command : commands) { tableRPC.appendCommand(command.name, &command); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 07482192e2..6f5f172597 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1682,6 +1682,8 @@ UniValue analyzepsbt(const JSONRPCRequest& request) return result; } +void RegisterRawTransactionRPCCommands(CRPCTable &t) +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -1708,8 +1710,6 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterRawTransactionRPCCommands(CRPCTable &t) -{ for (const auto& command : commands) { t.appendCommand(command.name, &command); } diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp new file mode 100644 index 0000000000..208be92852 --- /dev/null +++ b/src/test/validationinterface_tests.cpp @@ -0,0 +1,60 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include + +BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup) + +class TestInterface : public CValidationInterface +{ +public: + TestInterface(std::function on_call = nullptr, std::function on_destroy = nullptr) + : m_on_call(std::move(on_call)), m_on_destroy(std::move(on_destroy)) + { + } + virtual ~TestInterface() + { + if (m_on_destroy) m_on_destroy(); + } + void BlockChecked(const CBlock& block, const BlockValidationState& state) override + { + if (m_on_call) m_on_call(); + } + static void Call() + { + CBlock block; + BlockValidationState state; + GetMainSignals().BlockChecked(block, state); + } + std::function m_on_call; + std::function m_on_destroy; +}; + +// Regression test to ensure UnregisterAllValidationInterfaces calls don't +// destroy a validation interface while it is being called. Bug: +// https://github.com/bitcoin/bitcoin/pull/18551 +BOOST_AUTO_TEST_CASE(unregister_all_during_call) +{ + bool destroyed = false; + RegisterSharedValidationInterface(std::make_shared( + [&] { + // First call should decrements reference count 2 -> 1 + UnregisterAllValidationInterfaces(); + BOOST_CHECK(!destroyed); + // Second call should not decrement reference count 1 -> 0 + UnregisterAllValidationInterfaces(); + BOOST_CHECK(!destroyed); + }, + [&] { destroyed = true; })); + TestInterface::Call(); + BOOST_CHECK(destroyed); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 60c3603fda..19085d8322 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -21,56 +21,75 @@ #include #include -#include - -struct ValidationInterfaceConnections { - boost::signals2::scoped_connection UpdatedBlockTip; - boost::signals2::scoped_connection SynchronousUpdatedBlockTip; - boost::signals2::scoped_connection TransactionAddedToMempool; - boost::signals2::scoped_connection BlockConnected; - boost::signals2::scoped_connection BlockDisconnected; - boost::signals2::scoped_connection TransactionRemovedFromMempool; - boost::signals2::scoped_connection ChainStateFlushed; - boost::signals2::scoped_connection BlockChecked; - boost::signals2::scoped_connection NewPoWValidBlock; - boost::signals2::scoped_connection AcceptedBlockHeader; - boost::signals2::scoped_connection NotifyHeaderTip; - boost::signals2::scoped_connection NotifyTransactionLock; - boost::signals2::scoped_connection NotifyChainLock; - boost::signals2::scoped_connection NotifyGovernanceVote; - boost::signals2::scoped_connection NotifyGovernanceObject; - boost::signals2::scoped_connection NotifyInstantSendDoubleSpendAttempt; - boost::signals2::scoped_connection NotifyMasternodeListChanged; - boost::signals2::scoped_connection NotifyRecoveredSig; - -}; - +//! The MainSignalsInstance manages a list of shared_ptr +//! callbacks. +//! +//! A std::unordered_map is used to track what callbacks are currently +//! registered, and a std::list is to used to store the callbacks that are +//! currently registered as well as any callbacks that are just unregistered +//! and about to be deleted when they are done executing. struct MainSignalsInstance { - boost::signals2::signal UpdatedBlockTip; - boost::signals2::signal SynchronousUpdatedBlockTip; - boost::signals2::signal TransactionAddedToMempool; - boost::signals2::signal &, const CBlockIndex *pindex)> BlockConnected; - boost::signals2::signal&, const CBlockIndex* pindex)> BlockDisconnected; - boost::signals2::signal TransactionRemovedFromMempool; - boost::signals2::signal ChainStateFlushed; - boost::signals2::signal BlockChecked; - boost::signals2::signal&)> NewPoWValidBlock; - boost::signals2::signalAcceptedBlockHeader; - boost::signals2::signalNotifyHeaderTip; - boost::signals2::signal& islock)>NotifyTransactionLock; - boost::signals2::signal& clsig)>NotifyChainLock; - boost::signals2::signal& vote)>NotifyGovernanceVote; - boost::signals2::signal& object)>NotifyGovernanceObject; - boost::signals2::signalNotifyInstantSendDoubleSpendAttempt; - boost::signals2::signalNotifyMasternodeListChanged; - boost::signals2::signal& sig)>NotifyRecoveredSig; +private: + Mutex m_mutex; + //! List entries consist of a callback pointer and reference count. The + //! count is equal to the number of current executions of that entry, plus 1 + //! if it's registered. It cannot be 0 because that would imply it is + //! unregistered and also not being executed (so shouldn't exist). + struct ListEntry { std::shared_ptr callbacks; int count = 1; }; + std::list m_list GUARDED_BY(m_mutex); + std::unordered_map::iterator> m_map GUARDED_BY(m_mutex); + +public: // We are not allowed to assume the scheduler only runs in one thread, // but must ensure all callbacks happen in-order, so we end up creating // our own queue here :( SingleThreadedSchedulerClient m_schedulerClient; - std::unordered_map m_connMainSignals; explicit MainSignalsInstance(CScheduler *pscheduler) : m_schedulerClient(pscheduler) {} + + void Register(std::shared_ptr callbacks) + { + LOCK(m_mutex); + auto inserted = m_map.emplace(callbacks.get(), m_list.end()); + if (inserted.second) inserted.first->second = m_list.emplace(m_list.end()); + inserted.first->second->callbacks = std::move(callbacks); + } + + void Unregister(CValidationInterface* callbacks) + { + LOCK(m_mutex); + auto it = m_map.find(callbacks); + if (it != m_map.end()) { + if (!--it->second->count) m_list.erase(it->second); + m_map.erase(it); + } + } + + //! Clear unregisters every previously registered callback, erasing every + //! map entry. After this call, the list may still contain callbacks that + //! are currently executing, but it will be cleared when they are done + //! executing. + void Clear() + { + LOCK(m_mutex); + for (const auto& entry : m_map) { + if (!--entry.second->count) m_list.erase(entry.second); + } + m_map.clear(); + } + + template void Iterate(F&& f) + { + WAIT_LOCK(m_mutex, lock); + for (auto it = m_list.begin(); it != m_list.end();) { + ++it->count; + { + REVERSE_LOCK(lock); + f(*it->callbacks); + } + it = --it->count ? std::next(it) : m_list.erase(it); + } + } }; static CMainSignals g_signals; @@ -103,25 +122,7 @@ CMainSignals& GetMainSignals() void RegisterSharedValidationInterface(std::shared_ptr pwalletIn) { // Each connection captures pwalletIn to ensure that each callback is // executed before pwalletIn is destroyed. For more details see #18338. - ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn.get()]; - conns.AcceptedBlockHeader = g_signals.m_internals->AcceptedBlockHeader.connect(std::bind(&CValidationInterface::AcceptedBlockHeader, pwalletIn, std::placeholders::_1)); - conns.NotifyHeaderTip = g_signals.m_internals->NotifyHeaderTip.connect(std::bind(&CValidationInterface::NotifyHeaderTip, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.SynchronousUpdatedBlockTip = g_signals.m_internals->SynchronousUpdatedBlockTip.connect(std::bind(&CValidationInterface::SynchronousUpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.NotifyTransactionLock = g_signals.m_internals->NotifyTransactionLock.connect(std::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.NotifyChainLock = g_signals.m_internals->NotifyChainLock.connect(std::bind(&CValidationInterface::NotifyChainLock, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.ChainStateFlushed = g_signals.m_internals->ChainStateFlushed.connect(std::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, std::placeholders::_1)); - conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.NewPoWValidBlock = g_signals.m_internals->NewPoWValidBlock.connect(std::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.NotifyGovernanceObject = g_signals.m_internals->NotifyGovernanceObject.connect(std::bind(&CValidationInterface::NotifyGovernanceObject, pwalletIn, std::placeholders::_1)); - conns.NotifyGovernanceVote = g_signals.m_internals->NotifyGovernanceVote.connect(std::bind(&CValidationInterface::NotifyGovernanceVote, pwalletIn, std::placeholders::_1)); - conns.NotifyInstantSendDoubleSpendAttempt = g_signals.m_internals->NotifyInstantSendDoubleSpendAttempt.connect(std::bind(&CValidationInterface::NotifyInstantSendDoubleSpendAttempt, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.NotifyRecoveredSig = g_signals.m_internals->NotifyRecoveredSig.connect(std::bind(&CValidationInterface::NotifyRecoveredSig, pwalletIn, std::placeholders::_1)); - conns.NotifyMasternodeListChanged = g_signals.m_internals->NotifyMasternodeListChanged.connect(std::bind(&CValidationInterface::NotifyMasternodeListChanged, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4)); + g_signals.m_internals->Register(std::move(pwalletIn)); } void RegisterValidationInterface(CValidationInterface* callbacks) @@ -138,7 +139,7 @@ void UnregisterSharedValidationInterface(std::shared_ptr c void UnregisterValidationInterface(CValidationInterface* pwalletIn) { if (g_signals.m_internals) { - g_signals.m_internals->m_connMainSignals.erase(pwalletIn); + g_signals.m_internals->Unregister(pwalletIn); } } @@ -146,7 +147,7 @@ void UnregisterAllValidationInterfaces() { if (!g_signals.m_internals) { return; } - g_signals.m_internals->m_connMainSignals.clear(); + g_signals.m_internals->Clear(); } void CallFunctionInValidationInterfaceQueue(std::function func) { @@ -186,7 +187,7 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd // in the same critical section where the chain is updated auto event = [pindexNew, pindexFork, fInitialDownload, this] { - m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: new block hash=%s fork block hash=%s (in IBD=%s)", __func__, pindexNew->GetBlockHash().ToString(), @@ -195,12 +196,12 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd } void CMainSignals::SynchronousUpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { - m_internals->SynchronousUpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.SynchronousUpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); }); } void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx, int64_t nAcceptTime) { auto event = [ptx, nAcceptTime, this] { - m_internals->TransactionAddedToMempool(ptx, nAcceptTime); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(ptx, nAcceptTime); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s", __func__, ptx->GetHash().ToString()); @@ -208,7 +209,7 @@ void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx, int64_t void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) { auto event = [ptx, reason, this] { - m_internals->TransactionRemovedFromMempool(ptx, reason); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx, reason); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s", __func__, ptx->GetHash().ToString()); @@ -216,7 +217,7 @@ void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx, Mem void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex) { auto event = [pblock, pindex, this] { - m_internals->BlockConnected(pblock, pindex); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockConnected(pblock, pindex); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__, pblock->GetHash().ToString(), @@ -225,7 +226,7 @@ void CMainSignals::BlockConnected(const std::shared_ptr &pblock, c void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock, const CBlockIndex* pindex) { auto event = [pblock, pindex, this] { - m_internals->BlockDisconnected(pblock, pindex); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockDisconnected(pblock, pindex); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s block height=%d", __func__, pblock->GetHash().ToString(), @@ -234,7 +235,7 @@ void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { auto event = [locator, this] { - m_internals->ChainStateFlushed(locator); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.ChainStateFlushed(locator); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: block hash=%s", __func__, locator.IsNull() ? "null" : locator.vHave.front().ToString()); @@ -243,27 +244,27 @@ void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { void CMainSignals::BlockChecked(const CBlock& block, const BlockValidationState& state) { LOG_EVENT("%s: block hash=%s state=%s", __func__, block.GetHash().ToString(), state.ToString()); - m_internals->BlockChecked(block, state); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockChecked(block, state); }); } void CMainSignals::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr &block) { LOG_EVENT("%s: block hash=%s", __func__, block->GetHash().ToString()); - m_internals->NewPoWValidBlock(pindex, block); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NewPoWValidBlock(pindex, block); }); } void CMainSignals::AcceptedBlockHeader(const CBlockIndex *pindexNew) { LOG_EVENT("%s: accepted block header hash=%s", __func__, pindexNew->GetBlockHash().ToString()); - m_internals->AcceptedBlockHeader(pindexNew); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.AcceptedBlockHeader(pindexNew); }); } void CMainSignals::NotifyHeaderTip(const CBlockIndex *pindexNew, bool fInitialDownload) { LOG_EVENT("%s: accepted block header hash=%s initial=%d", __func__, pindexNew->GetBlockHash().ToString(), fInitialDownload); - m_internals->NotifyHeaderTip(pindexNew, fInitialDownload); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyHeaderTip(pindexNew, fInitialDownload); }); } void CMainSignals::NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr& islock) { auto event = [tx, islock, this] { - m_internals->NotifyTransactionLock(tx, islock); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyTransactionLock(tx, islock); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: transaction lock txid=%s", __func__, tx->GetHash().ToString()); @@ -271,7 +272,7 @@ void CMainSignals::NotifyTransactionLock(const CTransactionRef &tx, const std::s void CMainSignals::NotifyChainLock(const CBlockIndex* pindex, const std::shared_ptr& clsig) { auto event = [pindex, clsig, this] { - m_internals->NotifyChainLock(pindex, clsig); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyChainLock(pindex, clsig); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: notify chainlock at block=%s cl=%s", __func__, pindex->GetBlockHash().ToString(), @@ -280,21 +281,21 @@ void CMainSignals::NotifyChainLock(const CBlockIndex* pindex, const std::shared_ void CMainSignals::NotifyGovernanceVote(const std::shared_ptr& vote) { auto event = [vote, this] { - m_internals->NotifyGovernanceVote(vote); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyGovernanceVote(vote); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: notify governance vote: %s", __func__, vote->GetHash().ToString()); } void CMainSignals::NotifyGovernanceObject(const std::shared_ptr& object) { auto event = [object, this] { - m_internals->NotifyGovernanceObject(object); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyGovernanceObject(object); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: notify governance object: %s", __func__, object->GetHash().ToString()); } void CMainSignals::NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx) { auto event = [currentTx, previousTx, this] { - m_internals->NotifyInstantSendDoubleSpendAttempt(currentTx, previousTx); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyInstantSendDoubleSpendAttempt(currentTx, previousTx); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: notify instant doublespendattempt currenttxid=%s previoustxid=%s", __func__, currentTx->GetHash().ToString(), @@ -303,7 +304,7 @@ void CMainSignals::NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& cu void CMainSignals::NotifyRecoveredSig(const std::shared_ptr& sig) { auto event = [sig, this] { - m_internals->NotifyRecoveredSig(sig); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyRecoveredSig(sig); }); }; ENQUEUE_AND_LOG_EVENT(event, "%s: notify recoveredsig=%s", __func__, sig->GetHash().ToString()); @@ -311,5 +312,5 @@ void CMainSignals::NotifyRecoveredSig(const std::shared_ptrNotifyMasternodeListChanged(undo, oldMNList, diff, connman); + m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyMasternodeListChanged(undo, oldMNList, diff, connman); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 6f9b8a539b..4a29c8c088 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -194,9 +194,7 @@ protected: * Notifies listeners that a block which builds directly on our current tip * has been received and connected to the headers tree, though not validated yet */ virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& block) {}; - friend void ::RegisterSharedValidationInterface(std::shared_ptr); - friend void ::UnregisterValidationInterface(CValidationInterface*); - friend void ::UnregisterAllValidationInterfaces(); + friend class CMainSignals; }; struct MainSignalsInstance; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 200bf3e831..d530fde5c9 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -245,7 +245,7 @@ public: return false; } if (name) { - *name = it->second.name; + *name = it->second.GetLabel(); } if (is_mine) { *is_mine = m_wallet->IsMine(dest); @@ -261,7 +261,7 @@ public: std::vector result; for (const auto& item : m_wallet->m_address_book) { if (item.second.IsChange()) continue; - result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.name, item.second.purpose); + result.emplace_back(item.first, m_wallet->IsMine(item.first), item.second.GetLabel(), item.second.purpose); } return result; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d6706fd4aa..f773eb2612 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1014,7 +1014,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) file << strprintf("%s %s ", EncodeSecret(key), strTime); const auto* address_book_entry = pwallet->FindAddressBookEntry(pkhash); if (address_book_entry) { - file << strprintf("label=%s", EncodeDumpString(address_book_entry->name)); + file << strprintf("label=%s", EncodeDumpString(address_book_entry->GetLabel())); } else if (mapKeyPool.count(keyid)) { file << "reserve=1"; } else { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a19490c298..8fb0f0fc4c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -532,7 +532,7 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) { const auto* address_book_entry = pwallet->FindAddressBookEntry(address); if (address_book_entry) { - addressInfo.push_back(address_book_entry->name); + addressInfo.push_back(address_book_entry->GetLabel()); } } jsonGrouping.push_back(addressInfo); @@ -1151,7 +1151,7 @@ static UniValue ListReceived(const CWallet * const pwallet, const UniValue& para { if (item_it->second.IsChange()) continue; const CTxDestination& address = item_it->first; - const std::string& label = item_it->second.name; + const std::string& label = item_it->second.GetLabel(); auto it = mapTally.find(address); if (it == mapTally.end() && !fIncludeEmpty) continue; @@ -1354,7 +1354,7 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, entry.pushKV("amount", ValueFromAmount(-s.amount)); const auto* address_book_entry = pwallet->FindAddressBookEntry(s.destination); if (address_book_entry) { - entry.pushKV("label", address_book_entry->name); + entry.pushKV("label", address_book_entry->GetLabel()); } entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); @@ -1373,7 +1373,7 @@ static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, std::string label; const auto* address_book_entry = pwallet->FindAddressBookEntry(r.destination); if (address_book_entry) { - label = address_book_entry->name; + label = address_book_entry->GetLabel(); } if (filter_label && label != *filter_label) { continue; @@ -1934,54 +1934,62 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) if (!wallet) return NullUniValue; CWallet* const pwallet = wallet.get(); - LOCK(pwallet->cs_wallet); + int64_t nSleepTime; + { + LOCK(pwallet->cs_wallet); + + if (!pwallet->IsCrypted()) { + throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); + } + + // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed + SecureString strWalletPass; + strWalletPass.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. + strWalletPass = request.params[0].get_str().c_str(); + + // Get the timeout + nSleepTime = request.params[1].get_int64(); + // Timeout cannot be negative, otherwise it will relock immediately + if (nSleepTime < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); + } + // Clamp timeout + constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? + if (nSleepTime > MAX_SLEEP_TIME) { + nSleepTime = MAX_SLEEP_TIME; + } + + bool fForMixingOnly = false; + if (!request.params[2].isNull()) + fForMixingOnly = request.params[2].get_bool(); + + if (fForMixingOnly && !pwallet->IsLocked()) { + // Downgrading from "fuly unlocked" mode to "mixing only" one is not supported. + // Updating unlock time when current unlock mode is not changed or when it is upgraded + // from "mixing only" to "fuly unlocked" is ok. + throw JSONRPCError(RPC_WALLET_ALREADY_UNLOCKED, "Error: Wallet is already fully unlocked."); + } + + if (strWalletPass.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); + } + + if (!pwallet->Unlock(strWalletPass, fForMixingOnly)) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } + + pwallet->TopUpKeyPool(); + + pwallet->nRelockTime = GetTime() + nSleepTime; - if (!pwallet->IsCrypted()) { - throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); } - - // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed - SecureString strWalletPass; - strWalletPass.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. - strWalletPass = request.params[0].get_str().c_str(); - - // Get the timeout - int64_t nSleepTime = request.params[1].get_int64(); - // Timeout cannot be negative, otherwise it will relock immediately - if (nSleepTime < 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); - } - // Clamp timeout - constexpr int64_t MAX_SLEEP_TIME = 100000000; // larger values trigger a macos/libevent bug? - if (nSleepTime > MAX_SLEEP_TIME) { - nSleepTime = MAX_SLEEP_TIME; - } - - bool fForMixingOnly = false; - if (!request.params[2].isNull()) - fForMixingOnly = request.params[2].get_bool(); - - if (fForMixingOnly && !pwallet->IsLocked()) { - // Downgrading from "fuly unlocked" mode to "mixing only" one is not supported. - // Updating unlock time when current unlock mode is not changed or when it is upgraded - // from "mixing only" to "fuly unlocked" is ok. - throw JSONRPCError(RPC_WALLET_ALREADY_UNLOCKED, "Error: Wallet is already fully unlocked."); - } - - if (strWalletPass.empty()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase can not be empty"); - } - - if (!pwallet->Unlock(strWalletPass, fForMixingOnly)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); - } - - pwallet->TopUpKeyPool(); - - pwallet->nRelockTime = GetTime() + nSleepTime; - + // rpcRunLater must be called without cs_wallet held otherwise a deadlock + // can occur. The deadlock would happen when RPCRunLater removes the + // previous timer (and waits for the callback to finish if already running) + // and the callback locks cs_wallet. + AssertLockNotHeld(wallet->cs_wallet); // Keep a weak pointer to the wallet so that it is possible to unload the // wallet before the following callback is called. If a valid shared pointer // is acquired in the callback then the wallet is still loaded. @@ -3186,7 +3194,7 @@ static UniValue listunspent(const JSONRPCRequest& request) const auto* address_book_entry = pwallet->FindAddressBookEntry(address); if (address_book_entry) { - entry.pushKV("label", address_book_entry->name); + entry.pushKV("label", address_book_entry->GetLabel()); } std::unique_ptr provider = pwallet->GetSolvingProvider(scriptPubKey); @@ -3706,7 +3714,7 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v { UniValue ret(UniValue::VOBJ); if (verbose) { - ret.pushKV("name", data.name); + ret.pushKV("name", data.GetLabel()); } ret.pushKV("purpose", data.purpose); return ret; @@ -3808,7 +3816,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) // value of the name key/value pair in the labels array below. const auto* address_book_entry = pwallet->FindAddressBookEntry(dest); if (pwallet->chain().rpcEnableDeprecated("label") && address_book_entry) { - ret.pushKV("label", address_book_entry->name); + ret.pushKV("label", address_book_entry->GetLabel()); } ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); @@ -3844,7 +3852,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) if (pwallet->chain().rpcEnableDeprecated("labelspurpose")) { labels.push_back(AddressBookDataToJSON(*address_book_entry, true)); } else { - labels.push_back(address_book_entry->name); + labels.push_back(address_book_entry->GetLabel()); } } ret.pushKV("labels", std::move(labels)); @@ -3887,7 +3895,7 @@ static UniValue getaddressesbylabel(const JSONRPCRequest& request) std::set addresses; for (const std::pair& item : pwallet->m_address_book) { if (item.second.IsChange()) continue; - if (item.second.name == label) { + if (item.second.GetLabel() == label) { std::string address = EncodeDestination(item.first); // CWallet::m_address_book is not expected to contain duplicate // address strings, but build a separate set as a precaution just in @@ -3950,7 +3958,7 @@ static UniValue listlabels(const JSONRPCRequest& request) for (const std::pair& entry : pwallet->m_address_book) { if (entry.second.IsChange()) continue; if (purpose.empty() || entry.second.purpose == purpose) { - label_set.insert(entry.second.name); + label_set.insert(entry.second.GetLabel()); } } @@ -4188,6 +4196,8 @@ static UniValue upgradewallet(const JSONRPCRequest& request) return error.original; } +Span GetWalletRPCCommands() +{ // clang-format off static const CRPCCommand commands[] = { // category name actor (function) argNames @@ -4258,7 +4268,5 @@ static const CRPCCommand commands[] = }; // clang-format on -Span GetWalletRPCCommands() -{ return MakeSpan(commands); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c82873ddc4..224ed4252f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3938,7 +3938,10 @@ bool CWallet::DelAddressBook(const CTxDestination& address) // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted) // NOTE: This isn't a problem for sending addresses because they never have any DestData yet! // When adding new DestData, it should be considered here whether to retain or delete it (or move it?). - assert(!IsMine(address)); + if (IsMine(address)) { + WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT); + return false; + } { LOCK(cs_wallet); @@ -4190,7 +4193,7 @@ std::set CWallet::GetLabelAddresses(const std::string& label) co { if (item.second.IsChange()) continue; const CTxDestination& address = item.first; - const std::string& strName = item.second.name; + const std::string& strName = item.second.GetLabel(); if (strName == label) result.insert(address); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 55713ead89..aad3641c58 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -191,15 +191,15 @@ private: bool m_change{true}; std::string m_label; public: - const std::string& name; std::string purpose; - CAddressBookData() : name(m_label), purpose("unknown") {} + CAddressBookData() : purpose("unknown") {} typedef std::map StringMap; StringMap destdata; bool IsChange() const { return m_change; } + const std::string& GetLabel() const { return m_label; } void SetLabel(const std::string& label) { m_change = false; m_label = label; diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py new file mode 100755 index 0000000000..1f0012d95f --- /dev/null +++ b/test/functional/p2p_addr_relay.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +# Copyright (c) 2020 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 addr relay +""" + +from test_framework.messages import ( + CAddress, + NODE_NETWORK, + msg_addr, +) +from test_framework.mininode import ( + P2PInterface, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) +import time + +ADDRS = [] +for i in range(10): + addr = CAddress() + addr.time = int(time.time()) + i + addr.nServices = NODE_NETWORK + addr.ip = "123.123.123.{}".format(i % 256) + addr.port = 8333 + i + ADDRS.append(addr) + + +class AddrReceiver(P2PInterface): + def on_addr(self, message): + for addr in message.addrs: + assert_equal(addr.nServices, 9) + assert addr.ip.startswith('123.123.123.') + assert (8333 <= addr.port < 8343) + + +class AddrTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def run_test(self): + self.log.info('Create connection that sends addr messages') + addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) + msg = msg_addr() + + self.log.info('Send too large addr message') + msg.addrs = ADDRS * 101 + with self.nodes[0].assert_debug_log(['addr message size = 1010']): + addr_source.send_and_ping(msg) + + self.log.info('Check that addr message content is relayed and added to addrman') + addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) + msg.addrs = ADDRS + with self.nodes[0].assert_debug_log([ + 'Added 10 addresses from 127.0.0.1: 0 tried', + 'received: addr (301 bytes) peer=0', + ]): + addr_source.send_and_ping(msg) + self.nodes[0].setmocktime(int(time.time()) + 30 * 60) + addr_receiver.sync_with_ping() + + +if __name__ == '__main__': + AddrTest().main() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 2a6ef84d2c..1f7123e446 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -272,6 +272,10 @@ class TestNode(): # -342 Service unavailable, RPC server started but is shutting down due to error if e.error['code'] != -28 and e.error['code'] != -342: raise # unknown JSON RPC exception + except ConnectionResetError: + # This might happen when the RPC server is in warmup, but shut down before the call to getblockcount + # succeeds. Try again to properly raise the FailedToStartError + pass except OSError as e: if e.errno == errno.ETIMEDOUT: pass # Treat identical to ConnectionResetError diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ecb7038feb..71d09fdb55 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -174,6 +174,7 @@ BASE_SCRIPTS = [ 'rpc_blockchain.py', 'rpc_deprecated.py', 'wallet_disable.py', + 'p2p_addr_relay.py', 'p2p_getaddr_caching.py', 'p2p_getdata.py', 'rpc_net.py', diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 2f98c89501..bff24f772d 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -88,6 +88,7 @@ class AvoidReuseTest(BitcoinTestFramework): self.nodes[0].generate(110) self.sync_all() + self.test_change_remains_change(self.nodes[1]) reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) self.test_sending_from_reused_address_without_avoid_reuse() reset_balance(self.nodes[1], self.nodes[0].getnewaddress()) @@ -141,6 +142,30 @@ class AvoidReuseTest(BitcoinTestFramework): # Unload temp wallet self.nodes[1].unloadwallet(tempwallet) + def test_change_remains_change(self, node): + self.log.info("Test that change doesn't turn into non-change when spent") + + reset_balance(node, node.getnewaddress()) + addr = node.getnewaddress() + txid = node.sendtoaddress(addr, 1) + out = node.listunspent(minconf=0, query_options={'minimumAmount': 2}) + assert_equal(len(out), 1) + assert_equal(out[0]['txid'], txid) + changeaddr = out[0]['address'] + + # Make sure it's starting out as change as expected + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + + # Spend it + reset_balance(node, node.getnewaddress()) + + # It should still be change + assert node.getaddressinfo(changeaddr)['ischange'] + for logical_tx in node.listtransactions(): + assert logical_tx.get('address') != changeaddr + def test_sending_from_reused_address_without_avoid_reuse(self): ''' Test the same as test_sending_from_reused_address_fails, except send the 10 BTC with