backport: bitcoin#18160, #18454, #18487, #18524, #18532, #18546, #18551, #18561, #18563, #18587

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

src/net.h

@@ -1129,8 +1129,8 @@ class CNode
     std::vector<CAddress> vAddrToSend;
     const std::unique_ptr<CRollingBloomFilter> 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

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<std::chrono::microseconds>();
 
-        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<CAddress> vAddr;
             vAddr.reserve(pto->vAddrToSend.size());
 

src/qt/walletmodel.h

@@ -151,7 +151,6 @@ class WalletModel : public QObject
     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;

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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);
     }

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 <boost/test/unit_test.hpp>
+#include <consensus/validation.h>
+#include <primitives/block.h>
+#include <scheduler.h>
+#include <test/util/setup_common.h>
+#include <util/check.h>
+#include <validationinterface.h>
+
+BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup)
+
+class TestInterface : public CValidationInterface
+{
+public:
+    TestInterface(std::function<void()> on_call = nullptr, std::function<void()> 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<void()> m_on_call;
+    std::function<void()> 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<TestInterface>(
+        [&] {
+            // 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()