From 4f5795938748eefd6ce6e587a6c1c16233aaea1b Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@protonmail.com>
Date: Sat, 14 Mar 2020 18:42:25 +0100
Subject: [PATCH 01/14] Merge #16902: O(1) OP_IF/NOTIF/ELSE/ENDIF script
 implementation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

e6e622e5a0e22c2ac1b50b96af818e412d67ac54 Implement O(1) OP_IF/NOTIF/ELSE/ENDIF logic (Pieter Wuille)
d0e8f4d5d8ddaccb37f98b7989fb944081e41ab8 [refactor] interpreter: define interface for vfExec (Anthony Towns)
89fb241c54fc85befacfa3703d8e21bf3b8a76eb Benchmark script verification with 100 nested IFs (Pieter Wuille)

Pull request description:

  While investigating what mechanisms are possible to maximize the per-opcode verification cost of scripts, I noticed that the logic for determining whether a particular opcode is to be executed is O(n) in the nesting depth. This issue was also pointed out by Sergio Demian Lerner in https://bitslog.wordpress.com/2017/04/17/new-quadratic-delays-in-bitcoin-scripts/, and this PR implements a variant of the O(1) algorithm suggested there.

  This is not a problem currently, because even with a nesting depth of 100 (the maximum possible right now due to the 201 ops limit), the slowdown caused by this on my machine is around 70 ns per opcode (or 0.25 s per block) at worst, far lower than what is possible with other opcodes.

  This PR mostly serves as a proof of concept that it's possible to avoid it, which may be relevant in discussions around increasing the opcode limits in future script versions. Without it, the execution time of scripts can grow quadratically with the nesting depth, which very quickly becomes unreasonable.

  This improves upon #14245 by completely removing the `vfExec` vector.

ACKs for top commit:
  jnewbery:
    Code review ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54
  MarcoFalke:
    ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54 🐴
  fjahr:
    ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54
  ajtowns:
    ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54
  laanwj:
    concept and code review ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54
  jonatack:
    ACK e6e622e5a0e22c2ac1b50b96af818e412d67ac54 code review, build, benches, fuzzing

Tree-SHA512: 1dcfac3411ff04773de461959298a177f951cb5f706caa2734073bcec62224d7cd103767cfeef85cd129813e70c14c74fa8f1e38e4da70ec38a0f615aab1f7f7
---
 src/Makefile.bench.include  |  3 +-
 src/bench/verify_script.cpp | 33 +++++++++++++++++
 src/script/interpreter.cpp  | 71 +++++++++++++++++++++++++++++++++++--
 3 files changed, 103 insertions(+), 4 deletions(-)
 create mode 100644 src/bench/verify_script.cpp

diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 06928d9021..0eb0ce8888 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -47,7 +47,8 @@ bench_bench_dash_SOURCES = \
   bench/lockedpool.cpp \
   bench/poly1305.cpp \
   bench/prevector.cpp \
-  bench/string_cast.cpp
+  bench/string_cast.cpp \
+  bench/verify_script.cpp
 
 nodist_bench_bench_dash_SOURCES = $(GENERATED_BENCH_FILES)
 
diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp
new file mode 100644
index 0000000000..9a8c0a4732
--- /dev/null
+++ b/src/bench/verify_script.cpp
@@ -0,0 +1,33 @@
+// Copyright (c) 2016-2019 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 <bench/bench.h>
+#include <script/script.h>
+#include <script/standard.h>
+
+#include <array>
+
+
+static void VerifyNestedIfScript(benchmark::Bench& bench) {
+    std::vector<std::vector<unsigned char>> stack;
+    CScript script;
+    for (int i = 0; i < 100; ++i) {
+        script << OP_1 << OP_IF;
+    }
+    for (int i = 0; i < 1000; ++i) {
+        script << OP_1;
+    }
+    for (int i = 0; i < 100; ++i) {
+        script << OP_ENDIF;
+    }
+    bench.run([&] {
+        auto stack_copy = stack;
+        ScriptError error;
+        bool ret = EvalScript(stack_copy, script, 0, BaseSignatureChecker(), SigVersion::BASE, &error);
+        assert(ret);
+    });
+}
+
+
+BENCHMARK(VerifyNestedIfScript);
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 9b9f9e76db..d48debdb33 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -274,6 +274,70 @@ int FindAndDelete(CScript& script, const CScript& b)
     return nFound;
 }
 
+namespace {
+/** A data type to abstract out the condition stack during script execution.
+ *
+ * Conceptually it acts like a vector of booleans, one for each level of nested
+ * IF/THEN/ELSE, indicating whether we're in the active or inactive branch of
+ * each.
+ *
+ * The elements on the stack cannot be observed individually; we only need to
+ * expose whether the stack is empty and whether or not any false values are
+ * present at all. To implement OP_ELSE, a toggle_top modifier is added, which
+ * flips the last value without returning it.
+ *
+ * This uses an optimized implementation that does not materialize the
+ * actual stack. Instead, it just stores the size of the would-be stack,
+ * and the position of the first false value in it.
+ */
+class ConditionStack {
+private:
+    //! A constant for m_first_false_pos to indicate there are no falses.
+    static constexpr uint32_t NO_FALSE = std::numeric_limits<uint32_t>::max();
+
+    //! The size of the implied stack.
+    uint32_t m_stack_size = 0;
+    //! The position of the first false value on the implied stack, or NO_FALSE if all true.
+    uint32_t m_first_false_pos = NO_FALSE;
+
+public:
+    bool empty() { return m_stack_size == 0; }
+    bool all_true() { return m_first_false_pos == NO_FALSE; }
+    void push_back(bool f)
+    {
+        if (m_first_false_pos == NO_FALSE && !f) {
+            // The stack consists of all true values, and a false is added.
+            // The first false value will appear at the current size.
+            m_first_false_pos = m_stack_size;
+        }
+        ++m_stack_size;
+    }
+    void pop_back()
+    {
+        assert(m_stack_size > 0);
+        --m_stack_size;
+        if (m_first_false_pos == m_stack_size) {
+            // When popping off the first false value, everything becomes true.
+            m_first_false_pos = NO_FALSE;
+        }
+    }
+    void toggle_top()
+    {
+        assert(m_stack_size > 0);
+        if (m_first_false_pos == NO_FALSE) {
+            // The current stack is all true values; the first false will be the top.
+            m_first_false_pos = m_stack_size - 1;
+        } else if (m_first_false_pos == m_stack_size - 1) {
+            // The top is the first false value; toggling it will make everything true.
+            m_first_false_pos = NO_FALSE;
+        } else {
+            // There is a false value, but not on top. No action is needed as toggling
+            // anything but the first false value is unobservable.
+        }
+    }
+};
+}
+
 /** Helper for OP_CHECKSIG and OP_CHECKSIGVERIFY
  *
  * A return value of false means the script fails entirely. When true is returned, the
@@ -303,6 +367,7 @@ static bool EvalChecksig(const valtype& vchSig, const valtype& vchPubKey, CScrip
     return true;
 }
 
+
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* serror)
 {
     static const CScriptNum bnZero(0);
@@ -318,7 +383,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
     CScript::const_iterator pbegincodehash = script.begin();
     opcodetype opcode;
     valtype vchPushValue;
-    std::vector<bool> vfExec;
+    ConditionStack vfExec;
     std::vector<valtype> altstack;
     set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR);
     if (script.size() > MAX_SCRIPT_SIZE)
@@ -330,7 +395,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
     {
         while (pc < pend)
         {
-            bool fExec = !count(vfExec.begin(), vfExec.end(), false);
+            bool fExec = vfExec.all_true();
 
             //
             // Read instruction
@@ -521,7 +586,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                 {
                     if (vfExec.empty())
                         return set_error(serror, SCRIPT_ERR_UNBALANCED_CONDITIONAL);
-                    vfExec.back() = !vfExec.back();
+                    vfExec.toggle_top();
                 }
                 break;
 

From 0a1803a13af59d642cf28bdcc522280fb7abd782 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@protonmail.com>
Date: Mon, 16 Mar 2020 11:26:26 +0100
Subject: [PATCH 02/14] Merge #18360: Bump transifex slug and update English
 translations for 0.20

2a1b85f3c54fdaa8d13aeaf7e1acab1afb98fd97 tx: Bump transifex slug to 020x (Wladimir J. van der Laan)
82dd8860bbe81567e4afb033d9026dc0e69b1574 qt: Periodical translations update (Wladimir J. van der Laan)

Pull request description:

  Need to merge this so that translations for 0.20 can start.

Top commit has no ACKs.

Tree-SHA512: c78307eea3130b9bbc301f23790340e4c86a3f3877ab308bf67d9e1c9e977f7dda4b2cd83b96d9398b01dce616a6c962b98115123628bba35170eb0f301c4dd8
---
 .tx/config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.tx/config b/.tx/config
index f20b64d17e..0d3706af28 100644
--- a/.tx/config
+++ b/.tx/config
@@ -1,7 +1,7 @@
 [main]
 host = https://www.transifex.com
 
-[dash.qt-translation-018x]
+[dash.qt-translation-020x]
 file_filter = src/qt/locale/dash_<lang>.ts
 source_file = src/qt/locale/dash_en.xlf
 source_lang = en

From fc930dc0f4b031a08d24295f96cefd81bb0b5c45 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Mon, 23 Mar 2020 16:37:44 -0400
Subject: [PATCH 03/14] Merge #18278: interfaces: Describe and follow some code
 conventions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

3dc27a15242a22b5301904375e5880372e9b7f4d doc: Add internal interface conventions to developer notes (Russell Yanofsky)
1dca9dc4c772fa0a4ec52c4d88b7cd3d243aea7b refactor: Change createWallet, fillPSBT argument order (Russell Yanofsky)
96dfe5ced64979e51649d20555aa182defc80119 refactor: Change Chain::broadcastTransaction param order (Russell Yanofsky)
6ceb21909ce66b7b4762a855889acd46bb6b77f3 refactor: Rename Chain::Notifications methods to be consistent with other interfaces methods (Russell Yanofsky)
1c2ab1a6d29f2c6c065dae4f4a4e2ad1286311b3 refactor: Rename Node::disconnect methods (Russell Yanofsky)
77e4b0657298c715c835d8d2eb11e173852e6815 refactor: Get rid of Wallet::IsWalletFlagSet method (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  This PR doesn't change behavior at all, it just cleans up code in [`src/interfaces`](https://github.com/bitcoin/bitcoin/tree/master/src/interfaces) to simplify #10102, and [documents](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-conv/doc/developer-notes.md#internal-interface-guidelines) coding conventions there better

ACKs for top commit:
  hebasto:
    re-ACK 3dc27a15242a22b5301904375e5880372e9b7f4d, the only change since the [previous](https://github.com/bitcoin/bitcoin/pull/18278#pullrequestreview-372582146) review is rebasing.
  MarcoFalke:
    ACK 3dc27a15242a22b5301904375e5880372e9b7f4d 🕍

Tree-SHA512: 62e6a0f2488e3924e559d2074ed460b92e7a0a5d98eab492221cb20d59d04bbe32aef2a8aeba5e4ea9168cfa91acd5bc973dce6677be0180bd7a919354df53ed
---
 doc/developer-notes.md           | 122 +++++++++++++++++++++++++++++++
 src/interfaces/chain.h           |  23 +++---
 src/interfaces/node.h            |   4 +-
 src/interfaces/wallet.h          |  16 ++--
 src/node/interfaces.cpp          |  24 +++---
 src/qt/overviewpage.cpp          |   8 +-
 src/qt/receivecoinsdialog.cpp    |   4 +-
 src/qt/rpcconsole.cpp            |   4 +-
 src/qt/sendcoinsdialog.cpp       |  20 ++---
 src/qt/walletmodel.cpp           |  14 +---
 src/qt/walletmodel.h             |   2 -
 src/wallet/interfaces.cpp        |  14 ++--
 src/wallet/test/wallet_tests.cpp |   4 +-
 src/wallet/wallet.cpp            |  24 +++---
 src/wallet/wallet.h              |  16 ++--
 15 files changed, 206 insertions(+), 93 deletions(-)

diff --git a/doc/developer-notes.md b/doc/developer-notes.md
index d5250a93d1..a409a82a75 100644
--- a/doc/developer-notes.md
+++ b/doc/developer-notes.md
@@ -44,6 +44,7 @@ Developer Notes
         - [Suggestions and examples](#suggestions-and-examples)
     - [Release notes](#release-notes)
     - [RPC interface guidelines](#rpc-interface-guidelines)
+    - [Internal interface guidelines](#internal-interface-guidelines)
 
 <!-- markdown-toc end -->
 
@@ -1189,3 +1190,124 @@ A few guidelines for introducing and reviewing new RPC interfaces:
   timestamps in the documentation.
 
   - *Rationale*: User-facing consistency.
+
+Internal interface guidelines
+-----------------------------
+
+Internal interfaces between parts of the codebase that are meant to be
+independent (node, wallet, GUI), are defined in
+[`src/interfaces/`](../src/interfaces/). The main interface classes defined
+there are [`interfaces::Chain`](../src/interfaces/chain.h), used by wallet to
+access the node's latest chain state,
+[`interfaces::Node`](../src/interfaces/node.h), used by the GUI to control the
+node, and [`interfaces::Wallet`](../src/interfaces/wallet.h), used by the GUI
+to control an individual wallet. There are also more specialized interface
+types like [`interfaces::Handler`](../src/interfaces/handler.h)
+[`interfaces::ChainClient`](../src/interfaces/chain.h) passed to and from
+various interface methods.
+
+Interface classes are written in a particular style so node, wallet, and GUI
+code doesn't need to run in the same process, and so the class declarations
+work more easily with tools and libraries supporting interprocess
+communication:
+
+- Interface classes should be abstract and have methods that are [pure
+  virtual](https://en.cppreference.com/w/cpp/language/abstract_class). This
+  allows multiple implementations to inherit from the same interface class,
+  particularly so one implementation can execute functionality in the local
+  process, and other implementations can forward calls to remote processes.
+
+- Interface method definitions should wrap existing functionality instead of
+  implementing new functionality. Any substantial new node or wallet
+  functionality should be implemented in [`src/node/`](../src/node/) or
+  [`src/wallet/`](../src/wallet/) and just exposed in
+  [`src/interfaces/`](../src/interfaces/) instead of being implemented there,
+  so it can be more modular and accessible to unit tests.
+
+- Interface method parameter and return types should either be serializable or
+  be other interface classes. Interface methods shouldn't pass references to
+  objects that can't be serialized or accessed from another process.
+
+  Examples:
+
+  ```c++
+  // Good: takes string argument and returns interface class pointer
+  virtual unique_ptr<interfaces::Wallet> loadWallet(std::string filename) = 0;
+
+  // Bad: returns CWallet reference that can't be used from another process
+  virtual CWallet& loadWallet(std::string filename) = 0;
+  ```
+
+  ```c++
+  // Good: accepts and returns primitive types
+  virtual bool findBlock(const uint256& hash, int& out_height, int64_t& out_time) = 0;
+
+  // Bad: returns pointer to internal node in a linked list inaccessible to
+  // other processes
+  virtual const CBlockIndex* findBlock(const uint256& hash) = 0;
+  ```
+
+  ```c++
+  // Good: takes plain callback type and returns interface pointer
+  using TipChangedFn = std::function<void(int block_height, int64_t block_time)>;
+  virtual std::unique_ptr<interfaces::Handler> handleTipChanged(TipChangedFn fn) = 0;
+
+  // Bad: returns boost connection specific to local process
+  using TipChangedFn = std::function<void(int block_height, int64_t block_time)>;
+  virtual boost::signals2::scoped_connection connectTipChanged(TipChangedFn fn) = 0;
+  ```
+
+- For consistency and friendliness to code generation tools, interface method
+  input and inout parameters should be ordered first and output parameters
+  should come last.
+
+  Example:
+
+  ```c++
+  // Good: error output param is last
+  virtual bool broadcastTransaction(const CTransactionRef& tx, CAmount max_fee, std::string& error) = 0;
+
+  // Bad: error output param is between input params
+  virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& error, CAmount max_fee) = 0;
+  ```
+
+- For friendliness to code generation tools, interface methods should not be
+  overloaded:
+
+  Example:
+
+  ```c++
+  // Good: method names are unique
+  virtual bool disconnectByAddress(const CNetAddr& net_addr) = 0;
+  virtual bool disconnectById(NodeId id) = 0;
+
+  // Bad: methods are overloaded by type
+  virtual bool disconnect(const CNetAddr& net_addr) = 0;
+  virtual bool disconnect(NodeId id) = 0;
+  ```
+
+- For consistency and friendliness to code generation tools, interface method
+  names should be `lowerCamelCase` and standalone function names should be
+  `UpperCamelCase`.
+
+  Examples:
+
+  ```c++
+  // Good: lowerCamelCase method name
+  virtual void blockConnected(const CBlock& block, int height) = 0;
+
+  // Bad: uppercase class method
+  virtual void BlockConnected(const CBlock& block, int height) = 0;
+  ```
+
+  ```c++
+  // Good: UpperCamelCase standalone function name
+  std::unique_ptr<Node> MakeNode(LocalInit& init);
+
+  // Bad: lowercase standalone function
+  std::unique_ptr<Node> makeNode(LocalInit& init);
+  ```
+
+  Note: This last convention isn't generally followed outside of
+  [`src/interfaces/`](../src/interfaces/), though it did come up for discussion
+  before in [#14635](https://github.com/bitcoin/bitcoin/pull/14635).
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 73929875e7..2afa6231ef 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -183,7 +183,10 @@ public:
     //! Transaction is added to memory pool, if the transaction fee is below the
     //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true.
     //! Return false if the transaction could not be added due to the fee or for another reason.
-    virtual bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) = 0;
+    virtual bool broadcastTransaction(const CTransactionRef& tx,
+        const CAmount& max_tx_fee,
+        bool relay,
+        std::string& err_string) = 0;
 
     //! Calculate mempool ancestor and descendant counts for the given transaction.
     virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0;
@@ -243,14 +246,14 @@ public:
     {
     public:
         virtual ~Notifications() {}
-        virtual void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {}
-        virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) {}
-        virtual void BlockConnected(const CBlock& block, int height) {}
-        virtual void BlockDisconnected(const CBlock& block, int height) {}
-        virtual void UpdatedBlockTip() {}
-        virtual void ChainStateFlushed(const CBlockLocator& locator) {}
-        virtual void NotifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig) {}
-        virtual void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock) {}
+        virtual void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {}
+        virtual void transactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) {}
+        virtual void blockConnected(const CBlock& block, int height) {}
+        virtual void blockDisconnected(const CBlock& block, int height) {}
+        virtual void updatedBlockTip() {}
+        virtual void chainStateFlushed(const CBlockLocator& locator) {}
+        virtual void notifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig) {}
+        virtual void notifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock) {}
     };
 
     //! Register handler for notifications.
@@ -276,7 +279,7 @@ public:
     //! Write a setting to <datadir>/settings.json.
     virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0;
 
-    //! Synchronously send TransactionAddedToMempool notifications about all
+    //! Synchronously send transactionAddedToMempool notifications about all
     //! current mempool transactions to the specified handler and return after
     //! the last one is sent. These notifications aren't coordinated with async
     //! notifications sent by handleNotifications, so out of date async
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 1b8f0e81c2..b361c16a36 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -210,10 +210,10 @@ public:
     virtual bool unban(const CSubNet& ip) = 0;
 
     //! Disconnect node by address.
-    virtual bool disconnect(const CNetAddr& net_addr) = 0;
+    virtual bool disconnectByAddress(const CNetAddr& net_addr) = 0;
 
     //! Disconnect node by id.
-    virtual bool disconnect(NodeId id) = 0;
+    virtual bool disconnectById(NodeId id) = 0;
 
     //! Get total bytes recv.
     virtual int64_t getTotalBytesRecv() = 0;
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index d3154753dd..eeef9a68de 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -220,11 +220,11 @@ public:
     virtual bool isFullyMixed(const COutPoint& outpoint) = 0;
 
     //! Fill PSBT.
-    virtual TransactionError fillPSBT(PartiallySignedTransaction& psbtx,
-        bool& complete,
-        int sighash_type = 1 /* SIGHASH_ALL */,
-        bool sign = true,
-        bool bip32derivs = false) const = 0;
+    virtual TransactionError fillPSBT(int sighash_type,
+        bool sign,
+        bool bip32derivs,
+        PartiallySignedTransaction& psbtx,
+        bool& complete) = 0;
 
     //! Get balances.
     virtual WalletBalances getBalances() = 0;
@@ -289,10 +289,10 @@ public:
     virtual bool hdEnabled() = 0;
 
     // Return whether the wallet is blank.
-    virtual bool canGetAddresses() const = 0;
+    virtual bool canGetAddresses() = 0;
 
-    // check if a certain wallet flag is set.
-    virtual bool IsWalletFlagSet(uint64_t flag) = 0;
+    // Return whether private keys enabled.
+    virtual bool privateKeysDisabled() = 0;
 
     virtual CoinJoin::Client& coinJoin() = 0;
 
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 963d08a50f..222d98466d 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -290,14 +290,14 @@ public:
         }
         return false;
     }
-    bool disconnect(const CNetAddr& net_addr) override
+    bool disconnectByAddress(const CNetAddr& net_addr) override
     {
         if (m_context->connman) {
             return m_context->connman->DisconnectNode(net_addr);
         }
         return false;
     }
-    bool disconnect(NodeId id) override
+    bool disconnectById(NodeId id) override
     {
         if (m_context->connman) {
             return m_context->connman->DisconnectNode(id);
@@ -498,32 +498,32 @@ public:
     virtual ~NotificationsProxy() = default;
     void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override
     {
-        m_notifications->TransactionAddedToMempool(tx, nAcceptTime);
+        m_notifications->transactionAddedToMempool(tx, nAcceptTime);
     }
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override
     {
-        m_notifications->TransactionRemovedFromMempool(tx, reason);
+        m_notifications->transactionRemovedFromMempool(tx, reason);
     }
     void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override
     {
-        m_notifications->BlockConnected(*block, index->nHeight);
+        m_notifications->blockConnected(*block, index->nHeight);
     }
     void BlockDisconnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override
     {
-        m_notifications->BlockDisconnected(*block, index->nHeight);
+        m_notifications->blockDisconnected(*block, index->nHeight);
     }
     void UpdatedBlockTip(const CBlockIndex* index, const CBlockIndex* fork_index, bool is_ibd) override
     {
-        m_notifications->UpdatedBlockTip();
+        m_notifications->updatedBlockTip();
     }
-    void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); }
+    void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->chainStateFlushed(locator); }
     void NotifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig) override
     {
-        m_notifications->NotifyChainLock(pindexChainLock, clsig);
+        m_notifications->notifyChainLock(pindexChainLock, clsig);
     }
     void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock) override
     {
-        m_notifications->NotifyTransactionLock(tx, islock);
+        m_notifications->notifyTransactionLock(tx, islock);
     }
     std::shared_ptr<Chain::Notifications> m_notifications;
 };
@@ -727,7 +727,7 @@ public:
         auto it = m_node.mempool->GetIter(txid);
         return it && (*it)->GetCountWithDescendants() > 1;
     }
-    bool broadcastTransaction(const CTransactionRef& tx, std::string& err_string, const CAmount& max_tx_fee, bool relay) override
+    bool broadcastTransaction(const CTransactionRef& tx, const CAmount& max_tx_fee, bool relay, std::string& err_string) override
     {
         const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback*/ false);
         // Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures.
@@ -843,7 +843,7 @@ public:
         if (!m_node.mempool) return;
         LOCK2(::cs_main, m_node.mempool->cs);
         for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) {
-            notifications.TransactionAddedToMempool(entry.GetSharedTx(), 0);
+            notifications.transactionAddedToMempool(entry.GetSharedTx(), 0);
         }
     }
     NodeContext& m_node;
diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp
index 6fc68a0be1..0a8966fa1c 100644
--- a/src/qt/overviewpage.cpp
+++ b/src/qt/overviewpage.cpp
@@ -189,7 +189,7 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances)
 {
     int unit = walletModel->getOptionsModel()->getDisplayUnit();
     m_balances = balances;
-    if (walletModel->privateKeysDisabled()) {
+    if (walletModel->wallet().privateKeysDisabled()) {
         ui->labelBalance->setText(BitcoinUnits::floorHtmlWithUnit(unit, balances.watch_only_balance, false, BitcoinUnits::separatorAlways));
         ui->labelUnconfirmed->setText(BitcoinUnits::floorHtmlWithUnit(unit, balances.unconfirmed_watch_only_balance, false, BitcoinUnits::separatorAlways));
         ui->labelImmature->setText(BitcoinUnits::floorHtmlWithUnit(unit, balances.immature_watch_only_balance, false, BitcoinUnits::separatorAlways));
@@ -214,7 +214,7 @@ void OverviewPage::setBalance(const interfaces::WalletBalances& balances)
     // for symmetry reasons also show immature label when the watch-only one is shown
     ui->labelImmature->setVisible(showImmature || showWatchOnlyImmature);
     ui->labelImmatureText->setVisible(showImmature || showWatchOnlyImmature);
-    ui->labelWatchImmature->setVisible(!walletModel->privateKeysDisabled() && showWatchOnlyImmature); // show watch-only immature balance
+    ui->labelWatchImmature->setVisible(!walletModel->wallet().privateKeysDisabled() && showWatchOnlyImmature); // show watch-only immature balance
 
     updateCoinJoinProgress();
 
@@ -273,9 +273,9 @@ void OverviewPage::setWalletModel(WalletModel *model)
 
         connect(model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &OverviewPage::updateDisplayUnit);
 
-        updateWatchOnlyLabels((wallet.haveWatchOnly() && !model->privateKeysDisabled()) || gArgs.GetBoolArg("-debug-ui", false));
+        updateWatchOnlyLabels((wallet.haveWatchOnly() && !model->wallet().privateKeysDisabled()) || gArgs.GetBoolArg("-debug-ui", false));
         connect(model, &WalletModel::notifyWatchonlyChanged, [this](bool showWatchOnly) {
-            updateWatchOnlyLabels(showWatchOnly && !walletModel->privateKeysDisabled());
+            updateWatchOnlyLabels(showWatchOnly && !walletModel->wallet().privateKeysDisabled());
         });
 
         // explicitly update PS frame and transaction list to reflect actual settings
diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp
index fd040ff0c2..1b525170d2 100644
--- a/src/qt/receivecoinsdialog.cpp
+++ b/src/qt/receivecoinsdialog.cpp
@@ -86,11 +86,11 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model)
         columnResizingFixer = new GUIUtil::TableViewLastColumnResizingFixer(tableView, AMOUNT_MINIMUM_COLUMN_WIDTH, DATE_COLUMN_WIDTH, this);
 
         // Set the button to be enabled or disabled based on whether the wallet can give out new addresses.
-        ui->receiveButton->setEnabled(model->canGetAddresses());
+        ui->receiveButton->setEnabled(model->wallet().canGetAddresses());
 
         // Enable/disable the receive button if the wallet is now able/unable to give out new addresses.
         connect(model, &WalletModel::canGetAddressesChanged, [this] {
-            ui->receiveButton->setEnabled(model->canGetAddresses());
+            ui->receiveButton->setEnabled(model->wallet().canGetAddresses());
         });
     }
 }
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 8118491014..9b730c5a9a 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1359,7 +1359,7 @@ void RPCConsole::disconnectSelectedNode()
         // Get currently selected peer address
         NodeId id = nodes.at(i).data().toLongLong();
         // Find the node, disconnect it and clear the selected node
-        if(m_node.disconnect(id))
+        if(m_node.disconnectById(id))
             clearSelectedNode();
     }
 }
@@ -1384,7 +1384,7 @@ void RPCConsole::banSelectedNode(int bantime)
         const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
         if (stats) {
             m_node.ban(stats->nodeStats.addr, bantime);
-            m_node.disconnect(stats->nodeStats.addr);
+            m_node.disconnectByAddress(stats->nodeStats.addr);
         }
     }
     clearSelectedNode();
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index 6b1c7a023a..c3c2cd6475 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -215,7 +215,7 @@ void SendCoinsDialog::setModel(WalletModel *_model)
         updateFeeSectionControls();
         updateSmartFeeLabel();
 
-        if (model->privateKeysDisabled()) {
+        if (model->wallet().privateKeysDisabled()) {
             ui->sendButton->setText(tr("Cr&eate Unsigned"));
             ui->sendButton->setToolTip(tr("Creates a Partially Signed Bitcoin Transaction (PSBT) for use with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
         }
@@ -356,12 +356,12 @@ void SendCoinsDialog::send(QList<SendCoinsRecipient> recipients)
     }
 
     QString questionString;
-    if (model->privateKeysDisabled()) {
+    if (model->wallet().privateKeysDisabled()) {
         questionString.append(tr("Do you want to draft this transaction?"));
     } else {
         questionString.append(tr("Are you sure you want to send?"));
     }
-    if (model->privateKeysDisabled()) {
+    if (model->wallet().privateKeysDisabled()) {
         questionString.append("<br /><span style='font-size:10pt;'>");
         questionString.append(tr("This will produce a Partially Signed Transaction (PSBT) which you can copy and then sign with e.g. an offline %1 wallet, or a PSBT-compatible hardware wallet.").arg(PACKAGE_NAME));
         questionString.append("</span>");
@@ -452,8 +452,8 @@ void SendCoinsDialog::send(QList<SendCoinsRecipient> recipients)
         informative_text = tr("To review recipient list click \"Show Details...\"");
         detailed_text = formatted.join("\n\n");
     }
-    const QString confirmation = model->privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins");
-    const QString confirmButtonText = model->privateKeysDisabled() ? tr("Copy PSBT to clipboard") : tr("Send");
+    const QString confirmation = model->wallet().privateKeysDisabled() ? tr("Confirm transaction proposal") : tr("Confirm send coins");
+    const QString confirmButtonText = model->wallet().privateKeysDisabled() ? tr("Copy PSBT to clipboard") : tr("Send");
     SendConfirmationDialog confirmationDialog(confirmation, questionString, informative_text, detailed_text, SEND_CONFIRM_DELAY, confirmButtonText, this);
     confirmationDialog.exec();
     QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result());
@@ -465,11 +465,11 @@ void SendCoinsDialog::send(QList<SendCoinsRecipient> recipients)
     }
 
     bool send_failure = false;
-    if (model->privateKeysDisabled()) {
+    if (model->wallet().privateKeysDisabled()) {
         CMutableTransaction mtx = CMutableTransaction{*(currentTransaction.getWtx())};
         PartiallySignedTransaction psbtx(mtx);
         bool complete = false;
-        const TransactionError err = model->wallet().fillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */);
+        const TransactionError err = model->wallet().fillPSBT(SIGHASH_ALL, false /* sign */, true /* bip32derivs */, psbtx, complete);
         assert(!complete);
         assert(err == TransactionError::OK);
         // Serialize the PSBT
@@ -642,7 +642,7 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances)
     if(model && model->getOptionsModel())
     {
         CAmount balance = 0;
-        if (model->privateKeysDisabled()) {
+        if (model->wallet().privateKeysDisabled()) {
             balance = balances.watch_only_balance;
             ui->labelBalanceName->setText(tr("Watch-only balance:"));
         } else if (m_coin_control->IsUsingCoinJoin()) {
@@ -732,7 +732,7 @@ void SendCoinsDialog::on_buttonMinimizeFee_clicked()
 void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry)
 {
     // Include watch-only for wallets without private key
-    m_coin_control->fAllowWatchOnly = model->privateKeysDisabled();
+    m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled();
 
     // Calculate available amount to send.
     CAmount amount = model->wallet().getAvailableBalance(*m_coin_control);
@@ -786,7 +786,7 @@ void SendCoinsDialog::updateCoinControlState(CCoinControl& ctrl)
     // Either custom fee will be used or if not selected, the confirmation target from dropdown box
     ctrl.m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex());
     // Include watch-only for wallets without private key
-    ctrl.fAllowWatchOnly = model->privateKeysDisabled();
+    ctrl.fAllowWatchOnly = model->wallet().privateKeysDisabled();
 }
 
 void SendCoinsDialog::updateSmartFeeLabel()
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 94f8632809..faadbdaaaf 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -25,7 +25,7 @@
 #include <util/system.h> // for GetBoolArg
 #include <util/translation.h>
 #include <wallet/coincontrol.h>
-#include <wallet/wallet.h>
+#include <wallet/wallet.h> // for CRecipient
 
 #include <spork.h>
 
@@ -240,7 +240,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
     int nChangePosRet = -1;
 
     auto& newTx = transaction.getWtx();
-    newTx = m_wallet->createTransaction(vecSend, coinControl, !privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
+    newTx = m_wallet->createTransaction(vecSend, coinControl, !wallet().privateKeysDisabled() /* sign */, nChangePosRet, nFeeRequired, error);
     transaction.setTransactionFee(nFeeRequired);
     if (fSubtractFeeFromAmount && newTx)
         transaction.reassignAmounts(nChangePosRet);
@@ -593,16 +593,6 @@ bool WalletModel::isWalletEnabled()
    return !gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET);
 }
 
-bool WalletModel::privateKeysDisabled() const
-{
-    return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
-}
-
-bool WalletModel::canGetAddresses() const
-{
-    return m_wallet->canGetAddresses();
-}
-
 QString WalletModel::getWalletName() const
 {
     return QString::fromStdString(m_wallet->getWalletName());
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index 220718f547..f424053c0a 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -142,8 +142,6 @@ public:
     bool saveReceiveRequest(const std::string &sAddress, const int64_t nId, const std::string &sRequest);
 
     static bool isWalletEnabled();
-    bool privateKeysDisabled() const;
-    bool canGetAddresses() const;
 
     int getNumISLocks() const;
 
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index a0d95bf567..d8afe15771 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -409,11 +409,11 @@ public:
     int getRealOutpointCoinJoinRounds(const COutPoint& outpoint) override { return m_wallet->GetRealOutpointCoinJoinRounds(outpoint); }
     bool isFullyMixed(const COutPoint& outpoint) override { return m_wallet->IsFullyMixed(outpoint); }
 
-    TransactionError fillPSBT(PartiallySignedTransaction& psbtx,
-        bool& complete,
-        int sighash_type = 1 /* SIGHASH_ALL */,
-        bool sign = true,
-        bool bip32derivs = false) const override
+    TransactionError fillPSBT(int sighash_type,
+        bool sign,
+        bool bip32derivs,
+        PartiallySignedTransaction& psbtx,
+        bool& complete) override
     {
         return m_wallet->FillPSBT(psbtx, complete, sighash_type, sign, bip32derivs);
     }
@@ -545,8 +545,8 @@ public:
     }
     unsigned int getConfirmTarget() override { return m_wallet->m_confirm_target; }
     bool hdEnabled() override { return m_wallet->IsHDEnabled(); }
-    bool canGetAddresses() const override { return m_wallet->CanGetAddresses(); }
-    bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); }
+    bool canGetAddresses() override { return m_wallet->CanGetAddresses(); }
+    bool privateKeysDisabled() override { return m_wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); }
     CoinJoin::Client& coinJoin() override { return m_coinjoin; }
     CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; }
     void remove() override
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index ddf52096ff..cc651562c8 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -1258,7 +1258,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
     auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
     m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
     auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
-    BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), error, DEFAULT_TRANSACTION_MAXFEE, false));
+    BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
 
 
     // Reload wallet and make sure new transactions are detected despite events
@@ -1295,7 +1295,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
             block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
             m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
             mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
-            BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), error, DEFAULT_TRANSACTION_MAXFEE, false));
+            BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
             LEAVE_CRITICAL_SECTION(cs_wallets);
             LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
             SyncWithValidationInterfaceQueue();
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 214a6050c5..d4b38cce82 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -436,7 +436,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
     return false;
 }
 
-void CWallet::ChainStateFlushed(const CBlockLocator& loc)
+void CWallet::chainStateFlushed(const CBlockLocator& loc)
 {
     WalletBatch batch(*database);
     batch.WriteBestBlock(loc);
@@ -1218,7 +1218,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
     fAnonymizableTallyCachedNonDenom = false;
 }
 
-void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcceptTime) {
+void CWallet::transactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcceptTime) {
     LOCK(cs_wallet);
     CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
     SyncTransaction(ptx, confirm);
@@ -1229,7 +1229,7 @@ void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcc
     }
 }
 
-void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
+void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
     if (reason != MemPoolRemovalReason::CONFLICT) {
         LOCK(cs_wallet);
         auto it = mapWallet.find(ptx->GetHash());
@@ -1239,7 +1239,7 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolR
     }
 }
 
-void CWallet::BlockConnected(const CBlock& block, int height)
+void CWallet::blockConnected(const CBlock& block, int height)
 {
     const uint256& block_hash = block.GetHash();
     LOCK(cs_wallet);
@@ -1249,7 +1249,7 @@ void CWallet::BlockConnected(const CBlock& block, int height)
     for (size_t index = 0; index < block.vtx.size(); index++) {
         CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
         SyncTransaction(block.vtx[index], confirm);
-        TransactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::MANUAL);
+        transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::MANUAL);
     }
 
     // reset cache to make sure no longer immature coins are included
@@ -1257,7 +1257,7 @@ void CWallet::BlockConnected(const CBlock& block, int height)
     fAnonymizableTallyCachedNonDenom = false;
 }
 
-void CWallet::BlockDisconnected(const CBlock& block, int height)
+void CWallet::blockDisconnected(const CBlock& block, int height)
 {
     LOCK(cs_wallet);
 
@@ -1277,7 +1277,7 @@ void CWallet::BlockDisconnected(const CBlock& block, int height)
     fAnonymizableTallyCachedNonDenom = false;
 }
 
-void CWallet::UpdatedBlockTip()
+void CWallet::updatedBlockTip()
 {
     m_best_block_time = GetTime();
 }
@@ -2067,7 +2067,7 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay)
     // Irrespective of the failure reason, un-marking fInMempool
     // out-of-order is incorrect - it should be unmarked when
     // TransactionRemovedFromMempool fires.
-    bool ret = pwallet->chain().broadcastTransaction(tx, err_string, pwallet->m_default_max_tx_fee, relay);
+    bool ret = pwallet->chain().broadcastTransaction(tx, pwallet->m_default_max_tx_fee, relay, err_string);
     fInMempool |= ret;
     return ret;
 }
@@ -4582,7 +4582,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st
             }
         }
 
-        walletInstance->ChainStateFlushed(chain.getTipLocator());
+        walletInstance->chainStateFlushed(chain.getTipLocator());
 
         // Try to create wallet backup right after new wallet was created
         bilingual_str strBackupError;
@@ -4782,7 +4782,7 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st
                 return nullptr;
             }
         }
-        walletInstance->ChainStateFlushed(chain.getTipLocator());
+        walletInstance->chainStateFlushed(chain.getTipLocator());
         walletInstance->database->IncrementUpdateCounter();
     }
 
@@ -5013,7 +5013,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error
 }
 #endif // USE_BDB
 
-void CWallet::NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock)
+void CWallet::notifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock)
 {
     LOCK(cs_wallet);
     // Only notify UI if this transaction is in this wallet
@@ -5034,7 +5034,7 @@ void CWallet::NotifyTransactionLock(const CTransactionRef &tx, const std::shared
     }
 }
 
-void CWallet::NotifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig)
+void CWallet::notifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig)
 {
     NotifyChainLockReceived(pindexChainLock->nHeight);
 }
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 3afed452e4..50dfb77d55 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -964,10 +964,10 @@ public:
     void MarkDirty();
     bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true);
     void LoadToWallet(CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override;
-    void BlockConnected(const CBlock& block, int height) override;
-    void BlockDisconnected(const CBlock& block, int height) override;
-    void UpdatedBlockTip() override;
+    void transactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override;
+    void blockConnected(const CBlock& block, int height) override;
+    void blockDisconnected(const CBlock& block, int height) override;
+    void updatedBlockTip() override;
     int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update);
 
     struct ScanResult {
@@ -986,7 +986,7 @@ public:
         uint256 last_failed_block;
     };
     ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate);
-    void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
+    void transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
     void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void ResendWalletTransactions();
     struct Balance {
@@ -1126,7 +1126,7 @@ public:
     bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const;
     CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const;
     CAmount GetChange(const CTransaction& tx) const;
-    void ChainStateFlushed(const CBlockLocator& loc) override;
+    void chainStateFlushed(const CBlockLocator& loc) override;
 
     DBErrors LoadWallet(bool& fFirstRunRet);
     void AutoLockMasternodeCollaterals();
@@ -1241,8 +1241,8 @@ public:
     /* Returns true if the wallet can give out new addresses. This means it has keys in the keypool or can generate new keys */
     bool CanGetAddresses(bool internal = false) const;
 
-    void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock) override;
-    void NotifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig) override;
+    void notifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock) override;
+    void notifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr<const llmq::CChainLockSig>& clsig) override;
 
     /** Load a CGovernanceObject into m_gobjects. */
     bool LoadGovernanceObject(const CGovernanceObject& obj);

From 66d5d5a79881137674bebab3d9d72ce0cb7e9adf Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 24 Mar 2020 11:12:30 -0400
Subject: [PATCH 04/14] Merge #17720: test: add unit test for non-standard
 "scriptsig-not-pushonly" txs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

5aab011805ceb12801644170700b1a62e0bf4a5d test: add unit test for non-standard "scriptsig-not-pushonly" txs (Sebastian Falbesoner)

Pull request description:

  Approaches another missing unit test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason "scriptsig-not-pushonly" if any one of the input's scriptSig consists of any other ops than just PUSHs.

ACKs for top commit:
  MarcoFalke:
    ACK 5aab011805ceb12801644170700b1a62e0bf4a5d 🍟
  practicalswift:
    ACK 5aab011805ceb12801644170700b1a62e0bf4a5d -- patch looks correct

Tree-SHA512: fbe25bcf57e5f0c8d2397eb67e61fe8d9145ba83032789adb2b67d6fcbcd87e6427e9d965e8cd7bbaaea482e39ec2f110f71ef2de079c7d1fba2712848caa9ba
---
 src/test/transaction_tests.cpp | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index d130f5cc37..42f678ed2d 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -426,6 +426,40 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
     BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
     BOOST_CHECK_EQUAL(reason, "scriptsig-size");
 
+    // Check scriptSig format (non-standard if there are any other ops than just PUSHs)
+    t.vin[0].scriptSig = CScript()
+        << OP_TRUE << OP_0 << OP_1NEGATE << OP_16 // OP_n (single byte pushes: n = 1, 0, -1, 16)
+        << std::vector<unsigned char>(75, 0)      // OP_PUSHx [...x bytes...]
+        << std::vector<unsigned char>(235, 0)     // OP_PUSHDATA1 x [...x bytes...]
+        << std::vector<unsigned char>(1234, 0)    // OP_PUSHDATA2 x [...x bytes...]
+        << OP_9;
+    BOOST_CHECK(IsStandardTx(CTransaction(t), reason));
+
+    const std::vector<unsigned char> non_push_ops = { // arbitrary set of non-push operations
+        OP_NOP, OP_VERIFY, OP_IF, OP_ROT, OP_3DUP, OP_SIZE, OP_EQUAL, OP_ADD, OP_SUB,
+        OP_HASH256, OP_CODESEPARATOR, OP_CHECKSIG, OP_CHECKLOCKTIMEVERIFY };
+
+    CScript::const_iterator pc = t.vin[0].scriptSig.begin();
+    while (pc < t.vin[0].scriptSig.end()) {
+        opcodetype opcode;
+        CScript::const_iterator prev_pc = pc;
+        t.vin[0].scriptSig.GetOp(pc, opcode); // advance to next op
+        // for the sake of simplicity, we only replace single-byte push operations
+        if (opcode >= 1 && opcode <= OP_PUSHDATA4)
+            continue;
+
+        int index = prev_pc - t.vin[0].scriptSig.begin();
+        unsigned char orig_op = *prev_pc; // save op
+        // replace current push-op with each non-push-op
+        for (auto op : non_push_ops) {
+            t.vin[0].scriptSig[index] = op;
+            BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
+            BOOST_CHECK_EQUAL(reason, "scriptsig-not-pushonly");
+        }
+        t.vin[0].scriptSig[index] = orig_op; // restore op
+        BOOST_CHECK(IsStandardTx(CTransaction(t), reason));
+    }
+
     // Check bare multisig (standard if policy flag fIsBareMultisigStd is set)
     fIsBareMultisigStd = true;
     t.vout[0].scriptPubKey = GetScriptForMultisig(1, {key.GetPubKey()}); // simple 1-of-1

From aebde26ca0d85614094cf75ccf4759f4cff90fcb Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@protonmail.com>
Date: Wed, 25 Mar 2020 15:10:50 +0100
Subject: [PATCH 05/14] Merge #18402: gui: display mapped AS in peers info
 window

76db4b260e4826a1e59a5e44c92e4c10ec986527 gui: avoid QT Designer/Form Editor re-formatting (Jon Atack)
aae26053f958ae9a96a25d32c6341b14daaa4f26 gui: display Mapped AS in peers info window (Jon Atack)

Pull request description:

  Continuing the asmap integration of #16702 which added `mapped_as` to the rpc getpeerinfo output, this adds the mapped AS to the Peers detail window in the GUI wallet.

  `$ src/qt/bitcoin-qt -asmap=<path-to-asmap-file>` (asmap on)

  ![Screenshot from 2020-03-22 12-29-56](https://user-images.githubusercontent.com/2415484/77248754-c0ae4600-6c33-11ea-9d27-a06560c180c0.jpg)

  `$ src/qt/bitcoin-qt` (asmap off)

  ![Screenshot from 2020-03-22 12-32-46](https://user-images.githubusercontent.com/2415484/77248749-bdb35580-6c33-11ea-925c-6e19ecc083ab.jpg)

  Added a tooltip and a couple of minor fixups.

ACKs for top commit:
  laanwj:
    ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527

Tree-SHA512: 5f44c05c247bfabc9c161884d3af47c50a571cd02777b320ce389e61efa47706adbf0ea5e6644ae40423cb579d8bd0bb3c84fc6b618293a7add8e4327f07f63f
---
 src/qt/forms/debugwindow.ui | 32 +++++++++++++++++++++++++++++---
 src/qt/rpcconsole.cpp       |  5 +++--
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui
index ecbec6794b..ad58b5fb1b 100644
--- a/src/qt/forms/debugwindow.ui
+++ b/src/qt/forms/debugwindow.ui
@@ -693,12 +693,12 @@
          </item>
          <item>
           <widget class="QLineEdit" name="lineEdit">
-           <property name="placeholderText">
-            <string/>
-           </property>
            <property name="enabled">
             <bool>false</bool>
            </property>
+           <property name="placeholderText">
+            <string/>
+           </property>
           </widget>
          </item>
         </layout>
@@ -1403,6 +1403,32 @@
                 </widget>
                </item>
                <item row="20" column="0">
+                <widget class="QLabel" name="peerMappedASLabel">
+                 <property name="toolTip">
+                  <string>The mapped Autonomous System used for diversifying peer selection.</string>
+                 </property>
+                 <property name="text">
+                  <string>Mapped AS</string>
+                 </property>
+                </widget>
+               </item>
+               <item row="20" column="2">
+                <widget class="QLabel" name="peerMappedAS">
+                 <property name="cursor">
+                  <cursorShape>IBeamCursor</cursorShape>
+                 </property>
+                 <property name="text">
+                  <string>N/A</string>
+                 </property>
+                 <property name="textFormat">
+                  <enum>Qt::PlainText</enum>
+                 </property>
+                 <property name="textInteractionFlags">
+                  <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set>
+                 </property>
+                </widget>
+               </item>
+               <item row="21" column="0">
                 <spacer name="verticalSpacer_3">
                  <property name="orientation">
                   <enum>Qt::Vertical</enum>
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 9b730c5a9a..b1afeacc13 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1238,15 +1238,16 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
     ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_wait_usec));
     ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_usec));
     ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset));
-    ui->peerVersion->setText(QString("%1").arg(QString::number(stats->nodeStats.nVersion)));
+    ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
     ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
     ui->peerDirection->setText(stats->nodeStats.fInbound
             ? tr("Inbound")
             : stats->nodeStats.fRelayTxes
                 ? tr("Outbound")
                 : tr("Outbound block-relay"));
-    ui->peerHeight->setText(QString("%1").arg(QString::number(stats->nodeStats.nStartingHeight)));
+    ui->peerHeight->setText(QString::number(stats->nodeStats.nStartingHeight));
     ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
+    ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : tr("N/A"));
     auto dmn = clientModel->getMasternodeList().GetMNByService(stats->nodeStats.addr);
     if (dmn == nullptr) {
         ui->peerNodeType->setText(tr("Regular"));

From 8ae94ada7cf714a453eb62bee26bb16be2206829 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Thu, 26 Mar 2020 16:05:35 +0800
Subject: [PATCH 06/14] Merge #18397: build: Fix libevent linking for
 bench_bitcoin binary

cd04286825c6512b46bf59ab7b3dfffb0e36d65b build: Fix typo in EVENT_CFLAGS variable (Hennadii Stepanov)
f709ad0c907d87d03002455967cc30ae7d704d80 build: Fix libevent linking for bench_bitcoin binary (Hennadii Stepanov)

Pull request description:

  This change fixes `libevent` linking error for the `bench_bitcoin` binary.

  This PR is an alternative to #18377.
  Fix #18373.

  Also fixed a typo in `EVENT_CFLAGS` variable noted by **brakmic**.

ACKs for top commit:
  fanquake:
    ACK cd04286825c6512b46bf59ab7b3dfffb0e36d65b

Tree-SHA512: a62f7457e86b11d3a55d603ea5d83f3a413792e2f28a0c72300e54d12591bd6f0acc1d76a4bd4b591e0223bc6d530e7a4b9a8b939fe2fdbf2dddfda5b1b537be
---
 configure.ac               | 4 ++--
 src/Makefile.bench.include | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 08c0452d23..ff5e5e910a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1473,7 +1473,7 @@ if test x$use_pkgconfig = xyes; then
       if test x$use_qr != xno; then
         BITCOIN_QT_CHECK([PKG_CHECK_MODULES([QR], [libqrencode], [have_qrencode=yes], [have_qrencode=no])])
       fi
-      if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests != xnononono; then
+      if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnononono; then
         PKG_CHECK_MODULES([EVENT], [libevent >= 2.0.21], [use_libevent=yes], [AC_MSG_ERROR(libevent version 2.0.21 or greater not found.)])
         if test x$TARGET_OS != xwindows; then
           PKG_CHECK_MODULES([EVENT_PTHREADS], [libevent_pthreads >= 2.0.21],, [AC_MSG_ERROR(libevent_pthreads version 2.0.21 or greater not found.)])
@@ -1493,7 +1493,7 @@ if test x$use_pkgconfig = xyes; then
   )
 else
 
-  if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests != xnononono; then
+  if test x$build_bitcoin_cli$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench != xnonononono; then
     AC_CHECK_HEADER([event2/event.h],, AC_MSG_ERROR(libevent headers missing),)
     AC_CHECK_LIB([event],[main],EVENT_LIBS=-levent,AC_MSG_ERROR(libevent missing))
     if test x$TARGET_OS != xwindows; then
diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include
index 0eb0ce8888..684cdced6f 100644
--- a/src/Makefile.bench.include
+++ b/src/Makefile.bench.include
@@ -52,7 +52,7 @@ bench_bench_dash_SOURCES = \
 
 nodist_bench_bench_dash_SOURCES = $(GENERATED_BENCH_FILES)
 
-bench_bench_dash_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/
+bench_bench_dash_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/
 bench_bench_dash_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 bench_bench_dash_LDADD = \
   $(LIBBITCOIN_SERVER) \

From e47989205088f4b83803e7a283458d3c9fe92e8e Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@protonmail.com>
Date: Fri, 27 Mar 2020 14:02:48 +0100
Subject: [PATCH 07/14] Merge #18107: build: Add cov_fuzz target

faf7d4fa86b700ec272806cd2bd8666a92405619 build: Add cov_fuzz target (MarcoFalke)
fac71e364e4bbaeffc35e45aff8c8c2c6f2b5c67 build: link fuzz/test_runner.py for out-of-tree builds (MarcoFalke)
faf2c5aca01643eb560287e08f9c0a7ca0ac9c88 build: Remove unused USE_COVERAGE (MarcoFalke)

Pull request description:

  Only libFuzzer is supported right now, so clang is required. Thus, this needs a workaround such as https://github.com/bitcoin/bitcoin/issues/12602#issuecomment-562788247

  Can be tested with:

  ```
  mkdir build && cd build
  ../configure --enable-fuzz --with-sanitizers=fuzzer --enable-lcov --enable-lcov-branch-coverage CC=clang CXX=clang++
  make $MAKEJOBS
  make cov_fuzz

ACKs for top commit:
  practicalswift:
    ACK faf7d4fa86b700ec272806cd2bd8666a92405619

Tree-SHA512: 6828f8f81d95f6781713d0b09d7eba2ffdb50217e09ca839db61791a4ed70024859c7a0cb01d9eede79166d574dd57ece01f9d9fe2610d4a72a4ca4a4ce0b838
---
 Makefile.am  | 22 ++++++++++++++++++++--
 configure.ac |  2 +-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index f947805431..bf14f8d497 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -73,7 +73,7 @@ OSX_PACKAGING = $(OSX_DEPLOY_SCRIPT) $(OSX_FANCY_PLIST) $(OSX_INSTALLER_ICONS) \
 COVERAGE_INFO = baseline.info \
   test_dash_filtered.info total_coverage.info \
   baseline_filtered.info functional_test.info functional_test_filtered.info \
-  test_dash_coverage.info test_dash.info
+  test_dash_coverage.info test_dash.info fuzz.info fuzz_coverage.info
 
 dist-hook:
 	-$(GIT) archive --format=tar HEAD -- src/clientversion.cpp | $(AMTAR) -C $(top_distdir) -xf -
@@ -211,6 +211,15 @@ baseline_filtered.info: baseline.info
 	$(abs_builddir)/contrib/filter-lcov.py $(LCOV_FILTER_PATTERN) $< $@
 	$(LCOV) -a $@ $(LCOV_OPTS) -o $@
 
+fuzz.info: baseline_filtered.info
+	@TIMEOUT=15 test/fuzz/test_runner.py qa-assets/fuzz_seed_corpus -l DEBUG
+	$(LCOV) -c $(LCOV_OPTS) -d $(abs_builddir)/src --t fuzz-tests -o $@
+	$(LCOV) -z $(LCOV_OPTS) -d $(abs_builddir)/src
+
+fuzz_filtered.info: fuzz.info
+	$(abs_builddir)/contrib/filter-lcov.py $(LCOV_FILTER_PATTERN) $< $@
+	$(LCOV) -a $@ $(LCOV_OPTS) -o $@
+
 test_dash.info: baseline_filtered.info
 	$(MAKE) -C src/ check
 	$(LCOV) -c $(LCOV_OPTS) -d $(abs_builddir)/src -t test_dash -o $@
@@ -229,12 +238,19 @@ functional_test_filtered.info: functional_test.info
 	$(abs_builddir)/contrib/filter-lcov.py $(LCOV_FILTER_PATTERN) $< $@
 	$(LCOV) -a $@ $(LCOV_OPTS) -o $@
 
+fuzz_coverage.info: fuzz_filtered.info
+	$(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a fuzz_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt
+
 test_dash_coverage.info: baseline_filtered.info test_dash_filtered.info
 	$(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a test_dash_filtered.info -o $@
 
 total_coverage.info: test_dash_filtered.info functional_test_filtered.info
 	$(LCOV) -a $(LCOV_OPTS) baseline_filtered.info -a test_dash_filtered.info -a functional_test_filtered.info -o $@ | $(GREP) "\%" | $(AWK) '{ print substr($$3,2,50) "/" $$5 }' > coverage_percent.txt
 
+fuzz.coverage/.dirstamp: fuzz_coverage.info
+	$(GENHTML) -s $(LCOV_OPTS) $< -o $(@D)
+	@touch $@
+
 test_dash.coverage/.dirstamp:  test_dash_coverage.info
 	$(GENHTML) -s $(LCOV_OPTS) $< -o $(@D)
 	@touch $@
@@ -243,6 +259,8 @@ total.coverage/.dirstamp: total_coverage.info
 	$(GENHTML) -s $(LCOV_OPTS) $< -o $(@D)
 	@touch $@
 
+cov_fuzz: fuzz.coverage/.dirstamp
+
 cov: test_dash.coverage/.dirstamp total.coverage/.dirstamp
 
 endif
@@ -318,7 +336,7 @@ clean-docs:
 	rm -rf doc/doxygen
 
 clean-local: clean-docs
-	rm -rf coverage_percent.txt test_dash.coverage/ total.coverage/ test/tmp/ cache/ $(OSX_APP)
+	rm -rf coverage_percent.txt test_dash.coverage/ total.coverage/ fuzz.coverage/ test/tmp/ cache/ $(OSX_APP)
 	rm -rf test/functional/__pycache__ test/functional/test_framework/__pycache__ test/cache share/rpcauth/__pycache__
 	rm -rf osx_volname dist/ dpi36.background.tiff dpi72.background.tiff
 
diff --git a/configure.ac b/configure.ac
index ff5e5e910a..5e46881183 100644
--- a/configure.ac
+++ b/configure.ac
@@ -829,7 +829,6 @@ if test x$use_lcov = xyes; then
     [AC_MSG_ERROR("lcov testing requested but --coverage linker flag does not work")])
   AX_CHECK_COMPILE_FLAG([--coverage],[CXXFLAGS="$CXXFLAGS --coverage"],
     [AC_MSG_ERROR("lcov testing requested but --coverage flag does not work")])
-  AC_DEFINE(USE_COVERAGE, 1, [Define this symbol if coverage is enabled])
   CXXFLAGS="$CXXFLAGS -Og"
 fi
 
@@ -1828,6 +1827,7 @@ AC_CONFIG_LINKS([contrib/devtools/symbol-check.py:contrib/devtools/symbol-check.
 AC_CONFIG_LINKS([contrib/devtools/test-symbol-check.py:contrib/devtools/test-symbol-check.py])
 AC_CONFIG_LINKS([contrib/filter-lcov.py:contrib/filter-lcov.py])
 AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
+AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
 AC_CONFIG_LINKS([test/util/bitcoin-util-test.py:test/util/bitcoin-util-test.py])
 AC_CONFIG_LINKS([test/util/rpcauth-test.py:test/util/rpcauth-test.py])
 

From 1fd6393d320775ce1848dbc36fa320e825a625bf Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 27 Mar 2020 09:58:08 -0400
Subject: [PATCH 08/14] Merge #18447: test: Add coverage for script parse error
 in ParseScript

dcda81c47101196e53e379d965a2692515ef8363 test: add coverage for script parse error in ParseScript (pierrenn)

Pull request description:

  Follow up on this suggestion :  https://github.com/bitcoin/bitcoin/pull/18416#issuecomment-603966799

  This adds a test case to raise the `script parse error` in `ParseScript`.

ACKs for top commit:
  instagibbs:
    utACK https://github.com/bitcoin/bitcoin/pull/18447/commits/dcda81c47101196e53e379d965a2692515ef8363

Tree-SHA512: ae0ef2c00f34cee818c83582f190d5f4043159e922862f2b442b7b895b8ff3ca421533699247c12c367be77813b5205830a771cd47a18e8932807ccace2d6a1c
---
 test/util/data/bitcoin-util-test.json | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json
index d2a3fea69c..a84b7c9ea1 100644
--- a/test/util/data/bitcoin-util-test.json
+++ b/test/util/data/bitcoin-util-test.json
@@ -199,6 +199,12 @@
     "output_cmp": "txcreate2.json",
     "description": "Creates a new transaction with a single empty output script (output in json)"
   },
+  { "exec": "./dash-tx",
+    "args": ["-create", "outscript=0:123badscript"],
+    "return_code": 1,
+    "error_txt": "error: script parse error",
+    "description": "Create a new transaction with an invalid output script"
+  },
   { "exec": "./dash-tx",
     "args": ["-create", "outscript=0:OP_DROP", "nversion=1"],
     "output_cmp": "txcreatescript1.hex",

From 67ac335e75f45c590aadd1104e45615499f758e9 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Fri, 3 Apr 2020 18:36:16 +0800
Subject: [PATCH 09/14] Merge #18503: init: Replace URL_WEBSITE with
 PACKAGE_URL

fad2f68353944c83cad7b77415645215ae3f1093 init: Replace URL_WEBSITE with PACKAGE_URL (MarcoFalke)

Pull request description:

  This is needed for rebranding efforts such as #18489

ACKs for top commit:
  hebasto:
    ACK fad2f68353944c83cad7b77415645215ae3f1093, tested on Linux Mint 19.3:
  fanquake:
    ACK fad2f68353944c83cad7b77415645215ae3f1093 - clicked a link.

Tree-SHA512: c26e18cd328d3dd3fd7e25413e1bab780026687a148f126b8673e5f6cc13249f6c16689e45eba9da1545915c6001f96cd33f4e656c08cda3eae1c3fd88da23ea
---
 src/init.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index 88e241ce1a..417934c26e 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -799,13 +799,12 @@ void SetupServerArgs(NodeContext& node)
 std::string LicenseInfo()
 {
     const std::string URL_SOURCE_CODE = "<https://github.com/dashpay/dash>";
-    const std::string URL_WEBSITE = "<https://dash.org>";
 
     return CopyrightHolders(_("Copyright (C)").translated, 2014, COPYRIGHT_YEAR) + "\n" +
            "\n" +
            strprintf(_("Please contribute if you find %s useful. "
                        "Visit %s for further information about the software.").translated,
-               PACKAGE_NAME, URL_WEBSITE) +
+               PACKAGE_NAME, "<" PACKAGE_URL ">") +
            "\n" +
            strprintf(_("The source code is available from %s.").translated,
                URL_SOURCE_CODE) +

From dabaaf59e442913f5b19be9f0ea5a1e48e9e1924 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Mon, 30 Mar 2020 19:06:40 -0400
Subject: [PATCH 10/14] Merge #18420: test: listsinceblock block height checks

83e1d92413e262e6a876336ec433a6fbc335223a test: listsinceblock block height checks (Jon Atack)

Pull request description:

  This is the second commit of #17535.

  This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in #17437. It also cleans up code in the test function without changing or removing existing checks.

ACKs for top commit:
  fjahr:
    tested ACK 83e1d92413e262e6a876336ec433a6fbc335223a
  ryanofsky:
    Code review ACK 83e1d92413e262e6a876336ec433a6fbc335223a. Nice test improvements!

Tree-SHA512: 92874b49a3bc0236500495f32dfcf683e1971ca3d4c51702c69ed4ce7dfce21273754f02f93d1243d73793701d9fdf49e14b149477cd249cbbd9e4e8d5bd49f8
---
 test/functional/wallet_listsinceblock.py | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py
index 8e0d1b62d4..0f8ebdf211 100755
--- a/test/functional/wallet_listsinceblock.py
+++ b/test/functional/wallet_listsinceblock.py
@@ -110,23 +110,21 @@ class ListSinceBlockTest(BitcoinTestFramework):
         senttx = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1)
 
         # generate on both sides
-        lastblockhash = self.nodes[1].generate(6)[5]
-        self.nodes[2].generate(7)
-        self.log.debug('lastblockhash={}'.format(lastblockhash))
+        nodes1_last_blockhash = self.nodes[1].generate(6)[-1]
+        nodes2_first_blockhash = self.nodes[2].generate(7)[0]
+        self.log.debug("nodes[1] last blockhash = {}".format(nodes1_last_blockhash))
+        self.log.debug("nodes[2] first blockhash = {}".format(nodes2_first_blockhash))
 
         self.sync_all(self.nodes[:2])
         self.sync_all(self.nodes[2:])
 
         self.join_network()
 
-        # listsinceblock(lastblockhash) should now include tx, as seen from nodes[0]
-        lsbres = self.nodes[0].listsinceblock(lastblockhash)
-        found = False
-        for tx in lsbres['transactions']:
-            if tx['txid'] == senttx:
-                found = True
-                break
-        assert found
+        # listsinceblock(nodes1_last_blockhash) should now include tx as seen from nodes[0]
+        # and return the block height which listsinceblock now exposes since a5e7795.
+        transactions = self.nodes[0].listsinceblock(nodes1_last_blockhash)['transactions']
+        found = next(tx for tx in transactions if tx['txid'] == senttx)
+        assert_equal(found['blockheight'], self.nodes[0].getblockheader(nodes2_first_blockhash)['height'])
 
     def test_double_spend(self):
         '''

From fa6aafa19b766aac6e0b4ab9049f198903e49546 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Tue, 31 Mar 2020 09:35:53 -0400
Subject: [PATCH 11/14] Merge #18481: test: add BIP37 'filterclear' test to
 p2p_filter.py

00559229588feb19de2a0cb7506f70c483a1f433 test: add BIP37 'filterclear' test to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  Integrates the message type `filterclear` to the test framework and adds a simple test to `p2p_filter.py`, checking that arbitrary txs get relayed again after deleting the filter.

ACKs for top commit:
  naumenkogs:
    utACK 00559229588feb19de2a0cb7506f70c483a1f433

Tree-SHA512: fe64e99a526865770707d8077b9968d3923f248045ec7fa56cd380dba85ac77a71a473d244ef3aede2fc0d287b8d7c6bc0156b6033b0c949c2058cc08e255697
---
 test/functional/p2p_filter.py              |  8 ++++++++
 test/functional/test_framework/messages.py | 17 +++++++++++++++++
 test/functional/test_framework/mininode.py |  3 +++
 3 files changed, 28 insertions(+)

diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py
index 99e6ed7c1d..15906fd1fb 100755
--- a/test/functional/p2p_filter.py
+++ b/test/functional/p2p_filter.py
@@ -11,6 +11,7 @@ from test_framework.messages import (
     MSG_FILTERED_BLOCK,
     msg_getdata,
     msg_filterload,
+    msg_filterclear,
 )
 from test_framework.mininode import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
@@ -91,6 +92,13 @@ class FilterTest(BitcoinTestFramework):
         filter_node.wait_for_tx(txid)
         assert not filter_node.merkleblock_received
 
+        self.log.info('Check that after deleting filter all txs get relayed again')
+        filter_node.send_message(msg_filterclear())
+        filter_node.sync_with_ping()
+        for _ in range(5):
+            txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 7)
+            filter_node.wait_for_tx(txid)
+
 
 if __name__ == '__main__':
     FilterTest().main()
diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index fbe32a8b66..1a3d1a6c45 100755
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -1880,6 +1880,23 @@ class msg_filterload:
             self.data, self.nHashFuncs, self.nTweak, self.nFlags)
 
 
+class msg_filterclear:
+    __slots__ = ()
+    command = b"filterclear"
+
+    def __init__(self):
+        pass
+
+    def deserialize(self, f):
+        pass
+
+    def serialize(self):
+        return b""
+
+    def __repr__(self):
+        return "msg_filterclear()"
+
+
 class msg_sendcmpct:
     __slots__ = ("announce", "version")
     command = b"sendcmpct"
diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
index fa028d3f4d..ad3e3f5e0e 100755
--- a/test/functional/test_framework/mininode.py
+++ b/test/functional/test_framework/mininode.py
@@ -38,6 +38,7 @@ from test_framework.messages import (
     msg_cfilter,
     msg_clsig,
     msg_cmpctblock,
+    msg_filterclear,
     msg_filterload,
     msg_getaddr,
     msg_getblocks,
@@ -86,6 +87,7 @@ MESSAGEMAP = {
     b"cfheaders": msg_cfheaders,
     b"cfilter": msg_cfilter,
     b"cmpctblock": msg_cmpctblock,
+    b"filterclear": msg_filterclear,
     b"filterload": msg_filterload,
     b"getaddr": msg_getaddr,
     b"getblocks": msg_getblocks,
@@ -392,6 +394,7 @@ class P2PInterface(P2PConnection):
     def on_cfilter(self, message): pass
     def on_cmpctblock(self, message): pass
     def on_feefilter(self, message): pass
+    def on_filterclear(self, message): pass
     def on_filterload(self, message): pass
     def on_getaddr(self, message): pass
     def on_getblocks(self, message): pass

From 0784b6a14896040c7740a5143469d7b9da2a8c8c Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 3 Apr 2020 02:12:39 +0800
Subject: [PATCH 12/14] Merge #18494: test: replace (send_message +
 sync_with_ping) with send_and_ping
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

6112a209828c43930f677c45461339cdf68a56e9 test: replace (send_message + sync_with_ping) with send_and_ping (Jon Atack)

Pull request description:

  This is a follow-up to faf1d047313e71658fb31f6b94fdd5d37705ab85 yesterday.

ACKs for top commit:
  vasild:
    utACK 6112a20
  MarcoFalke:
    ACK 6112a209828c43930f677c45461339cdf68a56e9 🎞

Tree-SHA512: 749644ac9a1ef0e1aa6c3ac5e899eb3fa7fb9c0909352f922a80412df2bc0e539692a7757af550eff4d4914cbe57b0c75ce3948f569acc7a52852e91a55ad457
---
 test/functional/feature_maxuploadtarget.py    | 11 +++---
 test/functional/p2p_blockfilters.py           |  6 ++--
 test/functional/p2p_filter.py                 |  6 ++--
 test/functional/p2p_leak_tx.py                |  3 +-
 test/functional/p2p_sendheaders.py            |  3 +-
 test/functional/p2p_sendheaders_compressed.py |  3 +-
 test/functional/p2p_tx_download.py            |  5 ++-
 test/functional/p2p_unrequested_blocks.py     | 35 ++++++-------------
 test/functional/rpc_blockchain.py             |  3 +-
 9 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py
index 067dd4db84..6641f2ced6 100755
--- a/test/functional/feature_maxuploadtarget.py
+++ b/test/functional/feature_maxuploadtarget.py
@@ -99,8 +99,7 @@ class MaxUploadTest(BitcoinTestFramework):
         # 144MB will be reserved for relaying new blocks, so expect this to
         # succeed for ~70 tries.
         for i in range(success_count):
-            p2p_conns[0].send_message(getdata_request)
-            p2p_conns[0].sync_with_ping()
+            p2p_conns[0].send_and_ping(getdata_request)
             assert_equal(p2p_conns[0].block_receive_map[big_old_block], i+1)
 
         assert_equal(len(self.nodes[0].getpeerinfo()), 3)
@@ -117,7 +116,7 @@ class MaxUploadTest(BitcoinTestFramework):
         # We'll try 200 times
         getdata_request.inv = [CInv(MSG_BLOCK, big_new_block)]
         for i in range(200):
-            p2p_conns[1].send_message(getdata_request)
+            p2p_conns[1].send_and_ping(getdata_request)
             p2p_conns[1].sync_with_ping()
             assert_equal(p2p_conns[1].block_receive_map[big_new_block], i+1)
 
@@ -137,8 +136,7 @@ class MaxUploadTest(BitcoinTestFramework):
         # and p2p_conns[2] should be able to retrieve the old block.
         self.nodes[0].setmocktime(current_mocktime)
         p2p_conns[2].sync_with_ping()
-        p2p_conns[2].send_message(getdata_request)
-        p2p_conns[2].sync_with_ping()
+        p2p_conns[2].send_and_ping(getdata_request)
         assert_equal(p2p_conns[2].block_receive_map[big_old_block], 1)
 
         self.log.info("Peer 2 able to download old block")
@@ -155,8 +153,7 @@ class MaxUploadTest(BitcoinTestFramework):
         #retrieve 20 blocks which should be enough to break the 1MB limit
         getdata_request.inv = [CInv(MSG_BLOCK, big_new_block)]
         for i in range(20):
-            self.nodes[0].p2p.send_message(getdata_request)
-            self.nodes[0].p2p.sync_with_ping()
+            self.nodes[0].p2p.send_and_ping(getdata_request)
             assert_equal(self.nodes[0].p2p.block_receive_map[big_new_block], i+1)
 
         getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)]
diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py
index 21e44bd4a2..303a473cd0 100755
--- a/test/functional/p2p_blockfilters.py
+++ b/test/functional/p2p_blockfilters.py
@@ -164,8 +164,7 @@ class CompactFiltersTest(BitcoinTestFramework):
             start_height=1,
             stop_hash=int(stop_hash, 16)
         )
-        node0.send_message(request)
-        node0.sync_with_ping()
+        node0.send_and_ping(request)
         response = node0.pop_cfilters()
         assert_equal(len(response), 10)
 
@@ -183,8 +182,7 @@ class CompactFiltersTest(BitcoinTestFramework):
             start_height=1000,
             stop_hash=int(stale_block_hash, 16)
         )
-        node0.send_message(request)
-        node0.sync_with_ping()
+        node0.send_and_ping(request)
         response = node0.pop_cfilters()
         assert_equal(len(response), 1)
 
diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py
index 15906fd1fb..56d555c3e4 100755
--- a/test/functional/p2p_filter.py
+++ b/test/functional/p2p_filter.py
@@ -61,8 +61,7 @@ class FilterTest(BitcoinTestFramework):
     def run_test(self):
         self.log.info('Add filtered P2P connection to the node')
         filter_node = self.nodes[0].add_p2p_connection(FilterNode())
-        filter_node.send_message(filter_node.watch_filter_init)
-        filter_node.sync_with_ping()
+        filter_node.send_and_ping(filter_node.watch_filter_init)
         filter_address = self.nodes[0].decodescript(filter_node.watch_script_pubkey)['addresses'][0]
 
         self.log.info('Check that we receive merkleblock and tx if the filter matches a tx in a block')
@@ -93,8 +92,7 @@ class FilterTest(BitcoinTestFramework):
         assert not filter_node.merkleblock_received
 
         self.log.info('Check that after deleting filter all txs get relayed again')
-        filter_node.send_message(msg_filterclear())
-        filter_node.sync_with_ping()
+        filter_node.send_and_ping(msg_filterclear())
         for _ in range(5):
             txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 7)
             filter_node.wait_for_tx(txid)
diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py
index 43f1701f9c..e5c7184bfe 100755
--- a/test/functional/p2p_leak_tx.py
+++ b/test/functional/p2p_leak_tx.py
@@ -39,8 +39,7 @@ class P2PLeakTxTest(BitcoinTestFramework):
             want_tx = msg_getdata()
             want_tx.inv.append(CInv(t=MSG_TX, h=int(txid, 16)))
             inbound_peer.last_message.pop('notfound', None)
-            inbound_peer.send_message(want_tx)
-            inbound_peer.sync_with_ping()
+            inbound_peer.send_and_ping(want_tx)
 
             if inbound_peer.last_message.get('notfound'):
                 self.log.debug('tx {} was not yet announced to us.'.format(txid))
diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py
index c9f709580a..5d99586a9b 100755
--- a/test/functional/p2p_sendheaders.py
+++ b/test/functional/p2p_sendheaders.py
@@ -300,8 +300,7 @@ class SendHeadersTest(BitcoinTestFramework):
                 new_block.solve()
                 test_node.send_header_for_blocks([new_block])
                 test_node.wait_for_getdata([new_block.sha256])
-                test_node.send_message(msg_block(new_block))
-                test_node.sync_with_ping()  # make sure this block is processed
+                test_node.send_and_ping(msg_block(new_block))  # make sure this block is processed
                 wait_until(lambda: inv_node.block_announced, timeout=60, lock=mininode_lock)
                 inv_node.clear_block_announcements()
                 test_node.clear_block_announcements()
diff --git a/test/functional/p2p_sendheaders_compressed.py b/test/functional/p2p_sendheaders_compressed.py
index d8a726cae0..a4cea902b4 100644
--- a/test/functional/p2p_sendheaders_compressed.py
+++ b/test/functional/p2p_sendheaders_compressed.py
@@ -303,8 +303,7 @@ class SendHeadersTest(BitcoinTestFramework):
                 new_block.solve()
                 test_node.send_header_for_blocks([new_block])
                 test_node.wait_for_getdata([new_block.sha256])
-                test_node.send_message(msg_block(new_block))
-                test_node.sync_with_ping()  # make sure this block is processed
+                test_node.send_and_ping(msg_block(new_block))  # make sure this block is processed
                 wait_until(lambda: inv_node.block_announced, timeout=60, lock=mininode_lock)
                 inv_node.clear_block_announcements()
                 test_node.clear_block_announcements()
diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py
index b0e1e799f6..4fc6509b5d 100755
--- a/test/functional/p2p_tx_download.py
+++ b/test/functional/p2p_tx_download.py
@@ -63,8 +63,7 @@ class TxDownloadTest(BitcoinTestFramework):
         self.log.info("Announce the txid from each incoming peer to node 0")
         msg = msg_inv([CInv(t=1, h=txid)])
         for p in self.nodes[0].p2ps:
-            p.send_message(msg)
-            p.sync_with_ping()
+            p.send_and_ping(msg)
 
         outstanding_peer_index = [i for i in range(len(self.nodes[0].p2ps))]
 
@@ -102,7 +101,7 @@ class TxDownloadTest(BitcoinTestFramework):
             "Announce the transaction to all nodes from all {} incoming peers, but never send it".format(NUM_INBOUND))
         msg = msg_inv([CInv(t=1, h=txid)])
         for p in self.peers:
-            p.send_message(msg)
+            p.send_and_ping(msg)
             p.sync_with_ping()
             self.bump_mocktime(1)
 
diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py
index 94e03f940f..87fcc79521 100755
--- a/test/functional/p2p_unrequested_blocks.py
+++ b/test/functional/p2p_unrequested_blocks.py
@@ -84,11 +84,9 @@ class AcceptBlockTest(BitcoinTestFramework):
             blocks_h2.append(create_block(tips[i], create_coinbase(2), block_time + 1))
             blocks_h2[i].solve()
             block_time += 1
-        test_node.send_message(msg_block(blocks_h2[0]))
-        min_work_node.send_message(msg_block(blocks_h2[1]))
+        test_node.send_and_ping(msg_block(blocks_h2[0]))
+        min_work_node.send_and_ping(msg_block(blocks_h2[1]))
 
-        for x in [test_node, min_work_node]:
-            x.sync_with_ping()
         assert_equal(self.nodes[0].getblockcount(), 2)
         assert_equal(self.nodes[1].getblockcount(), 1)
         self.log.info("First height 2 block accepted by node0; correctly rejected by node1")
@@ -97,9 +95,8 @@ class AcceptBlockTest(BitcoinTestFramework):
         block_h1f = create_block(int("0x" + self.nodes[0].getblockhash(0), 0), create_coinbase(1), block_time)
         block_time += 1
         block_h1f.solve()
-        test_node.send_message(msg_block(block_h1f))
+        test_node.send_and_ping(msg_block(block_h1f))
 
-        test_node.sync_with_ping()
         tip_entry_found = False
         for x in self.nodes[0].getchaintips():
             if x['hash'] == block_h1f.hash:
@@ -112,9 +109,8 @@ class AcceptBlockTest(BitcoinTestFramework):
         block_h2f = create_block(block_h1f.sha256, create_coinbase(2), block_time)
         block_time += 1
         block_h2f.solve()
-        test_node.send_message(msg_block(block_h2f))
+        test_node.send_and_ping(msg_block(block_h2f))
 
-        test_node.sync_with_ping()
         # Since the earlier block was not processed by node, the new block
         # can't be fully validated.
         tip_entry_found = False
@@ -131,9 +127,8 @@ class AcceptBlockTest(BitcoinTestFramework):
         # 4b. Now send another block that builds on the forking chain.
         block_h3 = create_block(block_h2f.sha256, create_coinbase(3), block_h2f.nTime+1)
         block_h3.solve()
-        test_node.send_message(msg_block(block_h3))
+        test_node.send_and_ping(msg_block(block_h3))
 
-        test_node.sync_with_ping()
         # Since the earlier block was not processed by node, the new block
         # can't be fully validated.
         tip_entry_found = False
@@ -159,8 +154,7 @@ class AcceptBlockTest(BitcoinTestFramework):
             tip = next_block
 
         # Now send the block at height 5 and check that it wasn't accepted (missing header)
-        test_node.send_message(msg_block(all_blocks[1]))
-        test_node.sync_with_ping()
+        test_node.send_and_ping(msg_block(all_blocks[1]))
         assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, all_blocks[1].hash)
         assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblockheader, all_blocks[1].hash)
 
@@ -168,8 +162,7 @@ class AcceptBlockTest(BitcoinTestFramework):
         headers_message = msg_headers()
         headers_message.headers.append(CBlockHeader(all_blocks[0]))
         test_node.send_message(headers_message)
-        test_node.send_message(msg_block(all_blocks[1]))
-        test_node.sync_with_ping()
+        test_node.send_and_ping(msg_block(all_blocks[1]))
         self.nodes[0].getblock(all_blocks[1].hash)
 
         # Now send the blocks in all_blocks
@@ -194,9 +187,7 @@ class AcceptBlockTest(BitcoinTestFramework):
 
         test_node = self.nodes[0].add_p2p_connection(P2PInterface())
 
-        test_node.send_message(msg_block(block_h1f))
-
-        test_node.sync_with_ping()
+        test_node.send_and_ping(msg_block(block_h1f))
         assert_equal(self.nodes[0].getblockcount(), 2)
         self.log.info("Unrequested block that would complete more-work chain was ignored")
 
@@ -217,9 +208,7 @@ class AcceptBlockTest(BitcoinTestFramework):
         self.log.info("Inv at tip triggered getdata for unprocessed block")
 
         # 7. Send the missing block for the third time (now it is requested)
-        test_node.send_message(msg_block(block_h1f))
-
-        test_node.sync_with_ping()
+        test_node.send_and_ping(msg_block(block_h1f))
         assert_equal(self.nodes[0].getblockcount(), 290)
         self.nodes[0].getblock(all_blocks[286].hash)
         assert_equal(self.nodes[0].getbestblockhash(), all_blocks[286].hash)
@@ -246,9 +235,8 @@ class AcceptBlockTest(BitcoinTestFramework):
         headers_message.headers.append(CBlockHeader(block_290f))
         headers_message.headers.append(CBlockHeader(block_291))
         headers_message.headers.append(CBlockHeader(block_292))
-        test_node.send_message(headers_message)
+        test_node.send_and_ping(headers_message)
 
-        test_node.sync_with_ping()
         tip_entry_found = False
         for x in self.nodes[0].getchaintips():
             if x['hash'] == block_292.hash:
@@ -258,9 +246,8 @@ class AcceptBlockTest(BitcoinTestFramework):
         assert_raises_rpc_error(-1, "Block not found on disk", self.nodes[0].getblock, block_292.hash)
 
         test_node.send_message(msg_block(block_289f))
-        test_node.send_message(msg_block(block_290f))
+        test_node.send_and_ping(msg_block(block_290f))
 
-        test_node.sync_with_ping()
         self.nodes[0].getblock(block_289f.hash)
         self.nodes[0].getblock(block_290f.hash)
 
diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
index 653e7fb11e..847ffbb402 100755
--- a/test/functional/rpc_blockchain.py
+++ b/test/functional/rpc_blockchain.py
@@ -390,8 +390,7 @@ class BlockchainTest(BitcoinTestFramework):
         def solve_and_send_block(prevhash, height, time):
             b = create_block(prevhash, create_coinbase(height), time)
             b.solve()
-            node.p2p.send_message(msg_block(b))
-            node.p2p.sync_with_ping()
+            node.p2p.send_and_ping(msg_block(b))
             return b
 
         b21f = solve_and_send_block(int(b20hash, 16), 21, b20['time'] + 1)

From ff89f705c0aeb3a9c1194bbc6636c95828d64f81 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Sun, 5 Apr 2020 04:51:42 +0800
Subject: [PATCH 13/14] Merge #18496: test: remove redundant sync_with_ping
 after add_p2p_connection

4670006762ffce654bb12edb5a7e64ad004122a7 test: remove redundant sync_with_ping after add_p2p_connection (Jon Atack)

Pull request description:

  Now that #18247 is merged, these calls are redundant.

ACKs for top commit:
  vasild:
    utACK 4670006

Tree-SHA512: bdbfe8bcf9dbdde0a8115e3a62bfe359910798d7a3010d920ffca07049cb5f97bf8fb9b6f70079b0607105192b61a6d665774e59a2b678597b47ad6237595ad5
---
 test/functional/p2p_sendheaders.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py
index 5d99586a9b..cfa9f6f342 100755
--- a/test/functional/p2p_sendheaders.py
+++ b/test/functional/p2p_sendheaders.py
@@ -236,10 +236,6 @@ class SendHeadersTest(BitcoinTestFramework):
         # will occur outside of direct fetching
         test_node = self.nodes[0].add_p2p_connection(BaseNode(), services=NODE_HEADERS_COMPRESSED)
 
-        # Ensure verack's have been processed by our peer
-        inv_node.sync_with_ping()
-        test_node.sync_with_ping()
-
         self.test_null_locators(test_node, inv_node)
         self.test_nonnull_locators(test_node, inv_node)
 

From 11eefa21d4eb44b1c349ea88749dad4c884eb814 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Sun, 5 Apr 2020 21:17:50 +0800
Subject: [PATCH 14/14] Merge #18515: test: add BIP37 remote crash bug
 [CVE-2013-5700] test to p2p_filter.py

0ed2d8e07d3806d78d03a77d2153f22f9d733a07 test: add BIP37 remote crash bug [CVE-2013-5700] test to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  Integrates the missing message type `filteradd` to the test framework and checks that the BIP37 implementation is not vulnerable to the "remote crash bug" [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700) anymore. Prior to v.0.8.4, it was possible to trigger a division-by-zero error on the following line in the function `CBloomFilter::Hash()`:
  https://github.com/bitcoin/bitcoin/blob/f0d6487e290761a4fb03798240a351b5fddfdb38/src/bloom.cpp#L45
  By setting a zero-length filter via `filterload`, `vData.size()` is 0, so the modulo operation above, called on any .insert() or .contains() operation then crashed the node. The test uses the approach of just sending an arbitrary `filteradd` message after, which calls `CBloomFilter::insert()` (and in turn `CBloomFilter::Hash()`) on the node. The vulnerability was fixed by commit https://github.com/bitcoin/bitcoin/commit/37c6389c5a0ca63ae3573440ecdfe95d28ad8f07 (an intentional covert fix, [according to gmaxwell](https://github.com/bitcoin/bitcoin/issues/18483#issuecomment-608224095)), which introduced flags `isEmpty`/`isFull` that wouldn't call the `Hash()` member function if `isFull` is true (set to true by default constructor).

  To validate that the test fails if the implementation is vulnerable, one can simply set the flags to false in the member function `UpdateEmptyFull()` (that is called after a filter received via `filterload` is constructed), which activates the vulnerable code path calling `Hash` in any case on adding or testing for data in the filter:
  ```diff
  diff --git a/src/bloom.cpp b/src/bloom.cpp
  index bd6069b..ef294a3 100644
  --- a/src/bloom.cpp
  +++ b/src/bloom.cpp
  @@ -199,8 +199,8 @@ void CBloomFilter::UpdateEmptyFull()
           full &= vData[i] == 0xff;
           empty &= vData[i] == 0;
       }
  -    isFull = full;
  -    isEmpty = empty;
  +    isFull = false;
  +    isEmpty = false;
   }
  ```
  Resulting in:
  ```
  $ ./p2p_filter.py
  [...]
  2020-04-03T14:38:59.593000Z TestFramework (INFO): Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed
  2020-04-03T14:38:59.695000Z TestFramework (ERROR): Assertion failed
  [...]
  [... some exceptions following ...]
  ```

ACKs for top commit:
  naumenkogs:
    utACK 0ed2d8e07d3806d78d03a77d2153f22f9d733a07

Tree-SHA512: 02d0253d13eab70c4bd007b0750c56a5a92d05d419d53033523eeb3ed80318bc95196ab90f7745ea3ac9ebae7caee3adbf2a055a40a4124e0915226e49018fe8
---
 test/functional/p2p_filter.py              |  5 +++++
 test/functional/test_framework/messages.py | 19 +++++++++++++++++++
 test/functional/test_framework/mininode.py |  3 +++
 3 files changed, 27 insertions(+)

diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py
index 56d555c3e4..b73d5784aa 100755
--- a/test/functional/p2p_filter.py
+++ b/test/functional/p2p_filter.py
@@ -11,6 +11,7 @@ from test_framework.messages import (
     MSG_FILTERED_BLOCK,
     msg_getdata,
     msg_filterload,
+    msg_filteradd,
     msg_filterclear,
 )
 from test_framework.mininode import P2PInterface
@@ -97,6 +98,10 @@ class FilterTest(BitcoinTestFramework):
             txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 7)
             filter_node.wait_for_tx(txid)
 
+        self.log.info("Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed")
+        filter_node.send_and_ping(msg_filterload(data=b'', nHashFuncs=1))
+        filter_node.send_and_ping(msg_filteradd(data=b'letstrytocrashthisnode'))
+
 
 if __name__ == '__main__':
     FilterTest().main()
diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index 1a3d1a6c45..236f6af63e 100755
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -1880,6 +1880,25 @@ class msg_filterload:
             self.data, self.nHashFuncs, self.nTweak, self.nFlags)
 
 
+class msg_filteradd:
+    __slots__ = ("data")
+    command = b"filteradd"
+
+    def __init__(self, data):
+        self.data = data
+
+    def deserialize(self, f):
+        self.data = deser_string(f)
+
+    def serialize(self):
+        r = b""
+        r += ser_string(self.data)
+        return r
+
+    def __repr__(self):
+        return "msg_filteradd(data={})".format(self.data)
+
+
 class msg_filterclear:
     __slots__ = ()
     command = b"filterclear"
diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py
index ad3e3f5e0e..e300db9032 100755
--- a/test/functional/test_framework/mininode.py
+++ b/test/functional/test_framework/mininode.py
@@ -38,6 +38,7 @@ from test_framework.messages import (
     msg_cfilter,
     msg_clsig,
     msg_cmpctblock,
+    msg_filteradd,
     msg_filterclear,
     msg_filterload,
     msg_getaddr,
@@ -87,6 +88,7 @@ MESSAGEMAP = {
     b"cfheaders": msg_cfheaders,
     b"cfilter": msg_cfilter,
     b"cmpctblock": msg_cmpctblock,
+    b"filteradd": msg_filteradd,
     b"filterclear": msg_filterclear,
     b"filterload": msg_filterload,
     b"getaddr": msg_getaddr,
@@ -394,6 +396,7 @@ class P2PInterface(P2PConnection):
     def on_cfilter(self, message): pass
     def on_cmpctblock(self, message): pass
     def on_feefilter(self, message): pass
+    def on_filteradd(self, message): pass
     def on_filterclear(self, message): pass
     def on_filterload(self, message): pass
     def on_getaddr(self, message): pass