From cc38451955023e29412c9be5b07c1ae401af1112 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Thu, 17 Jun 2021 20:05:11 +0300
Subject: [PATCH 01/33] Merge #12755: [tests] Better stderr testing

beee49b [tests] Allow stderr to be tested against specified string (John Newbery)
e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery)
c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery)

Pull request description:

  **Due to a merge conflict, this is now based on #10267. Please review that PR first!**

  Subset of #12379 now that parts of that PR have been merged.

  #12362 was only observed when running the functional tests locally because:

  - by defatul libc logs to `/dev/tty` instead of stderr
  - the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail.

  This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr:

  - commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure.
  - commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal.
  commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown.

Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048
---
 test/functional/feature_includeconf.py        | 17 ++++-----
 test/functional/feature_pruning.py            |  3 +-
 test/functional/p2p_node_network_limited.py   |  3 --
 test/functional/rpc_blockchain.py             |  2 -
 test/functional/rpc_fundrawtransaction_hd.py  |  3 --
 .../test_framework/test_framework.py          | 29 +++++++-------
 test/functional/test_framework/test_node.py   | 38 +++++++++++++++----
 test/functional/test_framework/util.py        |  2 +
 test/functional/wallet_basic.py               |  3 +-
 test/functional/wallet_dump.py                |  5 +--
 test/functional/wallet_hd.py                  |  3 +-
 test/functional/wallet_import_rescan.py       |  3 +-
 test/functional/wallet_keypool_hd.py          |  3 --
 test/functional/wallet_keypool_topup.py       |  2 -
 test/functional/wallet_upgradetohd.py         |  3 +-
 15 files changed, 59 insertions(+), 60 deletions(-)

diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
index 0e318eb8c114..50bc5dc4278e 100755
--- a/test/functional/feature_includeconf.py
+++ b/test/functional/feature_includeconf.py
@@ -15,9 +15,8 @@
    file.
 """
 import os
-import tempfile
 
-from test_framework.test_framework import BitcoinTestFramework, assert_equal
+from test_framework.test_framework import BitcoinTestFramework
 
 class IncludeConfTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -44,20 +43,18 @@ def run_test(self):
 
         self.log.info("-includeconf cannot be used as command-line arg. subversion should still end with 'main; relative)/'")
         self.stop_node(0)
-        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
-            self.start_node(0, extra_args=["-includeconf=relative2.conf"], stderr=log_stderr)
 
-            subversion = self.nodes[0].getnetworkinfo()["subversion"]
-            assert subversion.endswith("main; relative)/")
-            log_stderr.seek(0)
-            stderr = log_stderr.read().decode('utf-8').strip()
-            assert_equal(stderr, 'warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf')
+        self.start_node(0, extra_args=["-includeconf=relative2.conf"])
+
+        subversion = self.nodes[0].getnetworkinfo()["subversion"]
+        assert subversion.endswith("main; relative)/")
+        self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf")
 
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
 
-        self.restart_node(0)
+        self.start_node(0)
 
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
         assert subversion.endswith("main; relative)/")
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 8805e4fa4a9c..4aed419d0a09 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -12,7 +12,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 import os
-import sys
 
 MIN_BLOCKS_TO_KEEP = 288
 
@@ -55,7 +54,7 @@ def setup_network(self):
         self.sync_blocks(self.nodes[0:5])
 
     def setup_nodes(self):
-        self.add_nodes(self.num_nodes, self.extra_args, timewait=900, stderr=sys.stdout)
+        self.add_nodes(self.num_nodes, self.extra_args, timewait=900)
         self.start_nodes()
 
     def create_big_chain(self):
diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py
index 94f7722bf90d..99e6d8254fdc 100755
--- a/test/functional/p2p_node_network_limited.py
+++ b/test/functional/p2p_node_network_limited.py
@@ -13,8 +13,6 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal, disconnect_nodes, connect_nodes_bi, sync_blocks
 
-import sys
-
 class P2PIgnoreInv(P2PInterface):
     firstAddrnServices = 0
     def on_inv(self, message):
@@ -34,7 +32,6 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 3
-        self.stderr = sys.stdout
         self.extra_args = [['-prune=550', '-txindex=0', '-addrmantest'], [], []]
 
     def disconnect_all(self):
diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
index 3257713d523d..ea2afeafbabc 100755
--- a/test/functional/rpc_blockchain.py
+++ b/test/functional/rpc_blockchain.py
@@ -21,7 +21,6 @@
 from decimal import Decimal
 import http.client
 import subprocess
-import sys
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -50,7 +49,6 @@ class BlockchainTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
         self.setup_clean_chain = True
-        self.stderr = sys.stdout
         self.extra_args = [['-stopatheight=207', '-prune=1', '-txindex=0']]
 
     def run_test(self):
diff --git a/test/functional/rpc_fundrawtransaction_hd.py b/test/functional/rpc_fundrawtransaction_hd.py
index 1b8601bd2345..b4545ad19c57 100755
--- a/test/functional/rpc_fundrawtransaction_hd.py
+++ b/test/functional/rpc_fundrawtransaction_hd.py
@@ -3,8 +3,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-import sys
-
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
@@ -15,7 +13,6 @@ def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 4
         self.extra_args = [['-usehd=1']] * self.num_nodes
-        self.stderr = sys.stdout
 
     def setup_network(self):
         super().setup_network()
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 8e9936648552..e751f1a6806a 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -270,12 +270,9 @@ def setup_network(self):
     def setup_nodes(self):
         """Override this method to customize test node setup"""
         extra_args = None
-        stderr = None
         if hasattr(self, "extra_args"):
             extra_args = self.extra_args
-        if hasattr(self, "stderr"):
-            stderr = self.stderr
-        self.add_nodes(self.num_nodes, extra_args, stderr=stderr)
+        self.add_nodes(self.num_nodes, extra_args)
         self.start_nodes()
 
     def run_test(self):
@@ -284,7 +281,7 @@ def run_test(self):
 
     # Public helper methods. These can be accessed by the subclass test scripts.
 
-    def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None, stderr=None):
+    def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None):
         """Instantiate TestNode objects"""
         if self.bind_to_localhost_only:
             extra_confs = [["bind=127.0.0.1"]] * num_nodes
@@ -299,7 +296,7 @@ def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, bin
         assert_equal(len(binary), num_nodes)
         old_num_nodes = len(self.nodes)
         for i in range(num_nodes):
-            self.nodes.append(TestNode(old_num_nodes + i, get_datadir_path(self.options.tmpdir, old_num_nodes + i), self.extra_args_from_options, chain=self.chain, rpchost=rpchost, timewait=timewait, bitcoind=binary[i], bitcoin_cli=self.options.bitcoincli, stderr=stderr, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli))
+            self.nodes.append(TestNode(old_num_nodes + i, get_datadir_path(self.options.tmpdir, old_num_nodes + i), self.extra_args_from_options, chain=self.chain, rpchost=rpchost, timewait=timewait, bitcoind=binary[i], bitcoin_cli=self.options.bitcoincli, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli))
 
     def start_node(self, i, *args, **kwargs):
         """Start a dashd"""
@@ -312,7 +309,7 @@ def start_node(self, i, *args, **kwargs):
         if self.options.coveragedir is not None:
             coverage.write_all_rpc_commands(self.options.coveragedir, node.rpc)
 
-    def start_nodes(self, extra_args=None, stderr=None, *args, **kwargs):
+    def start_nodes(self, extra_args=None, *args, **kwargs):
         """Start multiple dashds"""
 
         if extra_args is None:
@@ -320,7 +317,7 @@ def start_nodes(self, extra_args=None, stderr=None, *args, **kwargs):
         assert_equal(len(extra_args), self.num_nodes)
         try:
             for i, node in enumerate(self.nodes):
-                node.start(extra_args[i], stderr, *args, **kwargs)
+                node.start(extra_args[i], *args, **kwargs)
             for node in self.nodes:
                 node.wait_for_rpc_connection()
         except:
@@ -332,24 +329,24 @@ def start_nodes(self, extra_args=None, stderr=None, *args, **kwargs):
             for node in self.nodes:
                 coverage.write_all_rpc_commands(self.options.coveragedir, node.rpc)
 
-    def stop_node(self, i, wait=0):
+    def stop_node(self, i, expected_stderr='', wait=0):
         """Stop a dashd test node"""
-        self.nodes[i].stop_node(wait=wait)
+        self.nodes[i].stop_node(expected_stderr=expected_stderr, wait=wait)
         self.nodes[i].wait_until_stopped()
 
-    def stop_nodes(self, wait=0):
+    def stop_nodes(self, expected_stderr='', wait=0):
         """Stop multiple dashd test nodes"""
         for node in self.nodes:
             # Issue RPC to stop nodes
-            node.stop_node(wait=wait)
+            node.stop_node(expected_stderr=expected_stderr, wait=wait)
 
         for node in self.nodes:
             # Wait for nodes to stop
             node.wait_until_stopped()
 
-    def restart_node(self, i, extra_args=None):
+    def restart_node(self, i, extra_args=None, expected_stderr=''):
         """Stop and start a test node"""
-        self.stop_node(i)
+        self.stop_node(i, expected_stderr)
         self.start_node(i, extra_args)
 
     def wait_for_node_exit(self, i, timeout):
@@ -440,7 +437,7 @@ def _start_logging(self):
             rpc_handler.setLevel(logging.DEBUG)
             rpc_logger.addHandler(rpc_handler)
 
-    def _initialize_chain(self, extra_args=None, stderr=None):
+    def _initialize_chain(self, extra_args=None):
         """Initialize a pre-mined blockchain for use by the test.
 
         Create a cache of a 200-block-long chain (with wallet) for MAX_NODES
@@ -470,7 +467,7 @@ def _initialize_chain(self, extra_args=None, stderr=None):
                     args.append("-connect=127.0.0.1:" + str(p2p_port(0)))
                 if extra_args is not None:
                     args.extend(extra_args)
-                self.nodes.append(TestNode(i, get_datadir_path(self.options.cachedir, i), chain=self.chain, extra_conf=["bind=127.0.0.1"], extra_args=[],extra_args_from_options=self.extra_args_from_options, rpchost=None, timewait=None, bitcoind=self.options.bitcoind, bitcoin_cli=self.options.bitcoincli, stderr=stderr, mocktime=self.mocktime, coverage_dir=None))
+                self.nodes.append(TestNode(i, get_datadir_path(self.options.cachedir, i), chain=self.chain, extra_conf=["bind=127.0.0.1"], extra_args=[], extra_args_from_options=self.extra_args_from_options, rpchost=None, timewait=None, bitcoind=self.options.bitcoind, bitcoin_cli=self.options.bitcoincli, mocktime=self.mocktime, coverage_dir=None))
                 self.nodes[i].args = args
                 self.start_node(i)
 
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 3ffc61555f45..f08482af3acf 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -60,10 +60,12 @@ class TestNode():
     To make things easier for the test writer, any unrecognised messages will
     be dispatched to the RPC connection."""
 
-    def __init__(self, i, datadir, extra_args_from_options, chain, rpchost, timewait, bitcoind, bitcoin_cli, stderr, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False):
+    def __init__(self, i, datadir, extra_args_from_options, chain, rpchost, timewait, bitcoind, bitcoin_cli, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False):
         self.index = i
         self.datadir = datadir
         self.chain = chain
+        self.stdout_dir = os.path.join(self.datadir, "stdout")
+        self.stderr_dir = os.path.join(self.datadir, "stderr")
         self.rpchost = rpchost
         if timewait:
             self.rpc_timeout = timewait
@@ -72,7 +74,6 @@ def __init__(self, i, datadir, extra_args_from_options, chain, rpchost, timewait
             self.rpc_timeout = 60
         self.rpc_timeout *= Options.timeout_scale
         self.binary = bitcoind
-        self.stderr = stderr
         self.coverage_dir = coverage_dir
         self.mocktime = mocktime
         if extra_conf != None:
@@ -135,20 +136,33 @@ def __getattr__(self, name):
             assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
             return getattr(self.rpc, name)
 
-    def start(self, extra_args=None, stderr=None, *args, **kwargs):
+    def start(self, extra_args=None, stdout=None, stderr=None, *args, **kwargs):
         """Start the node."""
         if extra_args is None:
             extra_args = self.extra_args
+
+        # Add a new stdout and stderr file each time dashd is started
         if stderr is None:
-            stderr = self.stderr
+            stderr = tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False)
+        if stdout is None:
+            stdout = tempfile.NamedTemporaryFile(dir=self.stdout_dir, delete=False)
+        self.stderr = stderr
+        self.stdout = stdout
+
         all_args = self.args + self.extra_args_from_options + extra_args
         if self.mocktime != 0:
             all_args = all_args + ["-mocktime=%d" % self.mocktime]
+
         # Delete any existing cookie file -- if such a file exists (eg due to
         # unclean shutdown), it will get overwritten anyway by dashd, and
         # potentially interfere with our attempt to authenticate
         delete_cookie_file(self.datadir, self.chain)
-        self.process = subprocess.Popen(all_args, stderr=stderr, *args, **kwargs)
+
+        # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal
+        subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1")
+
+        self.process = subprocess.Popen(all_args, env=subp_env, stdout=stdout, stderr=stderr, *args, **kwargs)
+
         self.running = True
         self.log.debug("dashd started, waiting for RPC to come up")
 
@@ -190,7 +204,7 @@ def get_wallet_rpc(self, wallet_name):
             wallet_path = "wallet/{}".format(urllib.parse.quote(wallet_name))
             return self.rpc / wallet_path
 
-    def stop_node(self, wait=0):
+    def stop_node(self, expected_stderr='', wait=0):
         """Stop the node."""
         if not self.running:
             return
@@ -199,6 +213,13 @@ def stop_node(self, wait=0):
             self.stop(wait=wait)
         except http.client.CannotSendRequest:
             self.log.exception("Unable to stop node.")
+
+        # Check that stderr is as expected
+        self.stderr.seek(0)
+        stderr = self.stderr.read().decode('utf-8').strip()
+        if stderr != expected_stderr:
+            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
+
         del self.p2ps[:]
 
     def is_node_stopped(self):
@@ -251,9 +272,10 @@ def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, mat
 
         Will throw if dashd starts without an error.
         Will throw if an expected_msg is provided and it does not match dashd's stdout."""
-        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
+        with tempfile.NamedTemporaryFile(dir=self.stderr_dir, delete=False) as log_stderr, \
+             tempfile.NamedTemporaryFile(dir=self.stdout_dir, delete=False) as log_stdout:
             try:
-                self.start(extra_args, stderr=log_stderr, *args, **kwargs)
+                self.start(extra_args, stdout=log_stdout, stderr=log_stderr, *args, **kwargs)
                 self.wait_for_rpc_connection()
                 self.stop_node()
                 self.wait_until_stopped()
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index ac8622e0a9ea..abc0d71ba6b5 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -334,6 +334,8 @@ def initialize_datadir(dirname, n, chain):
         f.write("discover=0\n")
         f.write("listenonion=0\n")
         f.write("printtoconsole=0\n")
+        os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
+        os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
     return datadir
 
 def get_datadir_path(dirname, n):
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
index 2d097ac58455..0b4ef954dc38 100755
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the wallet."""
 from decimal import Decimal
-import sys
 import time
 
 from test_framework.test_framework import BitcoinTestFramework
@@ -25,7 +24,7 @@ def set_test_params(self):
         self.extra_args = [['-usehd={:d}'.format(i%2==0)] for i in range(4)]
 
     def setup_network(self):
-        self.add_nodes(4, self.extra_args, stderr=sys.stdout)
+        self.add_nodes(4, self.extra_args)
         self.start_node(0)
         self.start_node(1)
         self.start_node(2)
diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py
index b71d5cfa69f5..7a64ce93e43a 100755
--- a/test/functional/wallet_dump.py
+++ b/test/functional/wallet_dump.py
@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the dumpwallet RPC."""
 import os
-import sys
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -80,13 +79,13 @@ def setup_network(self):
         # use our own cache and -usehd=1 as extra arg as the default cache is run with -usehd=0
         self.options.tmpdir = os.path.join(self.options.tmpdir, 'hd')
         self.options.cachedir = os.path.join(self.options.cachedir, 'hd')
-        self._initialize_chain(extra_args=self.extra_args[0], stderr=sys.stdout)
+        self._initialize_chain(extra_args=self.extra_args[0])
         self.set_cache_mocktime()
         # Use 1 minute timeout because the initial getnewaddress RPC can take
         # longer than the default 30 seconds due to an expensive
         # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in
         # the test often takes even longer.
-        self.add_nodes(self.num_nodes, extra_args=self.extra_args, timewait=60, stderr=sys.stdout)
+        self.add_nodes(self.num_nodes, extra_args=self.extra_args, timewait=60)
         self.start_nodes()
 
     def run_test (self):
diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py
index 38c107d7e43e..3b285d3fed1c 100755
--- a/test/functional/wallet_hd.py
+++ b/test/functional/wallet_hd.py
@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test Hierarchical Deterministic wallet function."""
 
-import sys
 import shutil
 import os
 
@@ -21,7 +20,7 @@ def set_test_params(self):
         self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=0']]
 
     def setup_network(self):
-        self.add_nodes(self.num_nodes, self.extra_args, stderr=sys.stdout)
+        self.add_nodes(self.num_nodes, self.extra_args)
         self.start_nodes()
 
     def run_test(self):
diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index 7f9b93bd80a2..9709561bf99d 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -25,7 +25,6 @@
 import collections
 import enum
 import itertools
-import sys
 
 Call = enum.Enum("Call", "single multi")
 Data = enum.Enum("Data", "address pub priv")
@@ -131,7 +130,7 @@ def setup_network(self):
                 # txindex is enabled by default in Dash and needs to be disabled for import-rescan.py
                 extra_args[i] += ["-prune=1", "-txindex=0", "-reindex"]
 
-        self.add_nodes(self.num_nodes, extra_args=extra_args, stderr=sys.stdout)
+        self.add_nodes(self.num_nodes, extra_args=extra_args)
         self.start_nodes()
         for i in range(1, self.num_nodes):
             connect_nodes(self.nodes[i], 0)
diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py
index b7e838c42c94..fc78e3f89047 100755
--- a/test/functional/wallet_keypool_hd.py
+++ b/test/functional/wallet_keypool_hd.py
@@ -7,8 +7,6 @@
 
 # Add python-bitcoinrpc to module search path:
 
-import sys
-
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
@@ -18,7 +16,6 @@ def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
         self.extra_args = [['-usehd=1']]
-        self.stderr = sys.stdout
 
     def run_test(self):
         nodes = self.nodes
diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py
index 88e6e38173cf..a5bce6599327 100755
--- a/test/functional/wallet_keypool_topup.py
+++ b/test/functional/wallet_keypool_topup.py
@@ -12,7 +12,6 @@
 - connect node1 to node0. Verify that they sync and node1 receives its funds."""
 import os
 import shutil
-import sys
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -26,7 +25,6 @@ def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolmin=20']]
-        self.stderr = sys.stdout
 
     def run_test(self):
         wallet_path = os.path.join(self.nodes[1].datadir, self.chain, "wallets", "wallet.dat")
diff --git a/test/functional/wallet_upgradetohd.py b/test/functional/wallet_upgradetohd.py
index 2ad81805423e..de4c249157a0 100755
--- a/test/functional/wallet_upgradetohd.py
+++ b/test/functional/wallet_upgradetohd.py
@@ -8,7 +8,6 @@
 Test upgrade to a Hierarchical Deterministic wallet via upgradetohd rpc
 """
 
-import sys
 import shutil
 import os
 
@@ -24,7 +23,7 @@ def set_test_params(self):
         self.num_nodes = 1
 
     def setup_network(self):
-        self.add_nodes(self.num_nodes, stderr=sys.stdout)
+        self.add_nodes(self.num_nodes)
         self.start_nodes()
 
     def recover_non_hd(self):

From 13300e413a87a210c00dc3454f3d44cf3542bd63 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Thu, 17 Jun 2021 21:26:27 +0300
Subject: [PATCH 02/33] More of 12755 ("no governance validation" warnings in
 tests)

TODO: Maybe stop warning about no governance validation on pruned nodes?
---
 test/functional/p2p_node_network_limited.py | 2 ++
 test/functional/rpc_blockchain.py           | 2 +-
 test/functional/wallet_import_rescan.py     | 4 ++++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py
index 99e6d8254fdc..581e4dd3c38a 100755
--- a/test/functional/p2p_node_network_limited.py
+++ b/test/functional/p2p_node_network_limited.py
@@ -114,6 +114,8 @@ def run_test(self):
 
         # sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED)
         sync_blocks([self.nodes[0], self.nodes[1]])
+        self.stop_node(0, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
+
 
 if __name__ == '__main__':
     NodeNetworkLimitedTest().main()
diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
index ea2afeafbabc..93af518c2eb3 100755
--- a/test/functional/rpc_blockchain.py
+++ b/test/functional/rpc_blockchain.py
@@ -103,7 +103,7 @@ def _test_getblockchaininfo(self):
         assert res['pruned']
         assert not res['automatic_pruning']
 
-        self.restart_node(0, ['-stopatheight=207', '-txindex=0'])
+        self.restart_node(0, ['-stopatheight=207', '-txindex=0'], expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
         res = self.nodes[0].getblockchaininfo()
         # should have exact keys
         assert_equal(sorted(res.keys()), keys)
diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index 9709561bf99d..eba242753403 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -188,6 +188,10 @@ def run_test(self):
                 variant.check(variant.sent_txid, variant.sent_amount, 1)
             else:
                 variant.check()
+        for i, import_node in enumerate(IMPORT_NODES, 2):
+            if import_node.prune:
+                self.stop_node(i, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
+
 
 if __name__ == "__main__":
     ImportRescanTest().main()

From ec9f526cec964845fdb4c99bcb95d95a25a636fd Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Mon, 14 May 2018 16:38:18 +0200
Subject: [PATCH 03/33] Merge #13197: util: warn about ignored recursive
 -includeconf calls

2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm)
c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm)

Pull request description:

  This is a follow-up PR to #10267, and addresses https://github.com/bitcoin/bitcoin/pull/10267#issuecomment-387546144.

  ~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~

Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
---
 src/util/system.cpp                    | 18 ++++++++++++++++++
 test/functional/feature_includeconf.py |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/util/system.cpp b/src/util/system.cpp
index 75c90f9e97a4..7db50f96943b 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -824,6 +824,14 @@ void ArgsManager::ReadConfigFiles()
                 includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
             }
 
+            // Remove -includeconf from configuration, so we can warn about recursion
+            // later
+            {
+                LOCK(cs_args);
+                m_config_args.erase("-includeconf");
+                m_config_args.erase(std::string("-") + GetChainName() + ".includeconf");
+            }
+
             for (const std::string& to_include : includeconf) {
                 fsbridge::ifstream include_config(GetConfigFile(to_include));
                 if (include_config.good()) {
@@ -833,6 +841,16 @@ void ArgsManager::ReadConfigFiles()
                     fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
                 }
             }
+
+            // Warn about recursive -includeconf
+            includeconf = GetArgs("-includeconf");
+            {
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+            }
+            for (const std::string& to_include : includeconf) {
+                fprintf(stderr, "warning: -includeconf cannot be used from included files; ignoring -includeconf=%s\n", to_include.c_str());
+            }
         }
     } else {
         // Create an empty dash.conf if it does not exist
diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
index 50bc5dc4278e..eb6d847c5c19 100755
--- a/test/functional/feature_includeconf.py
+++ b/test/functional/feature_includeconf.py
@@ -53,11 +53,11 @@ def run_test(self):
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
-
         self.start_node(0)
 
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
         assert subversion.endswith("main; relative)/")
+        self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from included files; ignoring -includeconf=relative2.conf")
 
         self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
@@ -66,7 +66,7 @@ def run_test(self):
         with open(os.path.join(self.options.tmpdir, "node0", "dash.conf"), "a", encoding='utf8') as f:
             f.write("includeconf=relative2.conf\n")
 
-        self.restart_node(0)
+        self.start_node(0)
 
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
         assert subversion.endswith("main; relative; relative2)/")

From 21bb444ad3d9d89232e4c8a56991e38e2966867a Mon Sep 17 00:00:00 2001
From: Jonas Schnelli <dev@jonasschnelli.ch>
Date: Mon, 21 May 2018 09:41:44 +0200
Subject: [PATCH 04/33] Merge #12924: Fix hdmaster-key / seed-key confusion
 (scripted diff)

6249021d1 [docs] Add release notes for HD master key -> HD seed rename (John Newbery)
79053a5f2 [rpc] [wallet] Add 'hdmasterkeyid' alias return values. (John Newbery)
c75c35141 [refactor] manually change remaining instances of master key to seed. (John Newbery)
131d4450b scripted-diff: Rename master key to seed (John Newbery)

Pull request description:

  Addresses #12084 and #8684

  This renames a couple of functions and members (no functional changes, expect log prints):

  - Rename CKey::SetMaster to CKey::SetSeed
  - Rename CHDChain::masterKeyId to CHDChain::seedID
  - Rename CHDChain::hdMasterKeyID to CHDChain::hdSeedID
  - Rename CWallet::GenerateNewHDMasterKey to CWallet::GenerateNewHDSeed
  - Rename CWallet::SetHDMasterKey to CWallet::SetHDSeed

  As well it introduces a tiny API change:

  - RPC API change: Rename "hdmasterkeyid" to "hdseedid", rename "hdmaster" in wallet-dump output to "hdseed"

  Fixes also a bug:

  - Bugfix: use "s" instead of the incorrect "m" for the seed-key hd-keypath key metadata

Tree-SHA512: c913252636f213135a3b64df5de5d21844fb9c2d646567c1aad0ec65745188587de26119de99492c67e559bd49fdd9606b54276f00dddb84301785beba58f281
---
 src/hdchain.cpp                | 4 ++--
 src/key.cpp                    | 2 +-
 src/key.h                      | 2 +-
 src/test/bip32_tests.cpp       | 2 +-
 src/test/bip39_tests.cpp       | 2 +-
 src/wallet/rpcdump.cpp         | 2 +-
 test/functional/wallet_dump.py | 4 ++--
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/hdchain.cpp b/src/hdchain.cpp
index 192c8dc2e1e5..78a6631698f9 100644
--- a/src/hdchain.cpp
+++ b/src/hdchain.cpp
@@ -55,7 +55,7 @@ void CHDChain::Debug(const std::string& strName) const
             std::cout << "seed: " << HexStr(vchSeed).c_str() << std::endl;
 
             CExtKey extkey;
-            extkey.SetMaster(vchSeed.data(), vchSeed.size());
+            extkey.SetSeed(vchSeed.data(), vchSeed.size());
 
             std::cout << "extended private masterkey: " << EncodeExtKey(extkey).c_str() << std::endl;
 
@@ -167,7 +167,7 @@ void CHDChain::DeriveChildExtKey(uint32_t nAccountIndex, bool fInternal, uint32_
     CExtKey changeKey;              //key at m/purpose'/coin_type'/account'/change
     CExtKey childKey;               //key at m/purpose'/coin_type'/account'/change/address_index
 
-    masterKey.SetMaster(vchSeed.data(), vchSeed.size());
+    masterKey.SetSeed(vchSeed.data(), vchSeed.size());
 
     // Use hardened derivation for purpose, coin_type and account
     // (keys >= 0x80000000 are hardened after bip32)
diff --git a/src/key.cpp b/src/key.cpp
index a75d56618d84..860f1ded8d76 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -273,7 +273,7 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const {
     return key.Derive(out.key, out.chaincode, _nChild, chaincode);
 }
 
-void CExtKey::SetMaster(const unsigned char *seed, unsigned int nSeedLen) {
+void CExtKey::SetSeed(const unsigned char *seed, unsigned int nSeedLen) {
     static const unsigned char hashkey[] = {'B','i','t','c','o','i','n',' ','s','e','e','d'};
     std::vector<unsigned char, secure_allocator<unsigned char>> vout(64);
     CHMAC_SHA512(hashkey, sizeof(hashkey)).Write(seed, nSeedLen).Finalize(vout.data());
diff --git a/src/key.h b/src/key.h
index 585363107bd4..92e31652da03 100644
--- a/src/key.h
+++ b/src/key.h
@@ -158,7 +158,7 @@ struct CExtKey {
     void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]);
     bool Derive(CExtKey& out, unsigned int nChild) const;
     CExtPubKey Neuter() const;
-    void SetMaster(const unsigned char* seed, unsigned int nSeedLen);
+    void SetSeed(const unsigned char* seed, unsigned int nSeedLen);
     template <typename Stream>
     void Serialize(Stream& s) const
     {
diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp
index bd15854d12f6..633cbef1e596 100644
--- a/src/test/bip32_tests.cpp
+++ b/src/test/bip32_tests.cpp
@@ -91,7 +91,7 @@ static void RunTest(const TestVector &test) {
     std::vector<unsigned char> seed = ParseHex(test.strHexMaster);
     CExtKey key;
     CExtPubKey pubkey;
-    key.SetMaster(seed.data(), seed.size());
+    key.SetSeed(seed.data(), seed.size());
     pubkey = key.Neuter();
     for (const TestDerivation &derive : test.vDerive) {
         unsigned char data[74];
diff --git a/src/test/bip39_tests.cpp b/src/test/bip39_tests.cpp
index e5ffc6242fe4..cefccc67798f 100644
--- a/src/test/bip39_tests.cpp
+++ b/src/test/bip39_tests.cpp
@@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(bip39_vectors)
         CExtKey key;
         CExtPubKey pubkey;
 
-        key.SetMaster(seed.data(), 64);
+        key.SetSeed(seed.data(), 64);
         pubkey = key.Neuter();
 
         // printf("CBitcoinExtKey: %s\n", EncodeExtKey(key).c_str());
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index ec793aff9ce8..7f9d4ee7f065 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -939,7 +939,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)
         file << "# HD seed: " << HexStr(vchSeed) << "\n\n";
 
         CExtKey masterKey;
-        masterKey.SetMaster(&vchSeed[0], vchSeed.size());
+        masterKey.SetSeed(&vchSeed[0], vchSeed.size());
 
         file << "# extended private masterkey: " << EncodeExtKey(masterKey) << "\n";
 
diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py
index 7a64ce93e43a..6ab1efde736f 100755
--- a/test/functional/wallet_dump.py
+++ b/test/functional/wallet_dump.py
@@ -34,10 +34,10 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old):
                     addr_keypath = comment.split(" addr=")[1]
                     addr = addr_keypath.split(" ")[0]
                     keypath = None
-                    if keytype == "inactivehdmaster=1":
+                    if keytype == "inactivehdseed=1":
                         # ensure the old master is still available
                         assert(hd_master_addr_old == addr)
-                    elif keytype == "hdmaster=1":
+                    elif keytype == "hdseed=1":
                         # ensure we have generated a new hd master key
                         assert(hd_master_addr_old != addr)
                         hd_master_addr_ret = addr

From c8ee5e16654d09019e4c086848cc22ff06d48182 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 23 May 2018 19:04:25 +0200
Subject: [PATCH 05/33] Merge #13011: Cache witness hash in CTransaction

fac1223a568fa1ad6dd602350598eed278d115e8 Cache witness hash in CTransaction (MarcoFalke)
faab55fbb17f2ea5080bf02bc59eeef5ca746f07 Make CMutableTransaction constructor explicit (MarcoFalke)

Pull request description:

  This speeds up:
  * compactblocks (v2)
  * ATMP
  * validation and miner (via `BlockWitnessMerkleRoot`)
  * sigcache (see also unrelated #13204)
  * rpc and rest (nice, but irrelevant)

  This presumably slows down rescan, which uses a `CTransaction` and its `GetHash`, but never uses the `GetWitnessHash`. The slow down is proportional to the number of witness transactions in the rescan window. I.e. early in the chain there should be no measurable slow down. Later in the chain, there should be a slow down, but acceptable given the speedups in the modules mentioned above.

Tree-SHA512: 443e86acfcceb5af2163e68840c581d44159af3fd1fce266cab3504b29fcd74c50812b69a00d41582e7e1c5ea292f420ce5e892cdfab691da9c24ed1c44536c7
---
 src/coinjoin/coinjoin-client.cpp     |  2 +-
 src/dash-tx.cpp                      |  2 +-
 src/primitives/transaction.cpp       |  6 +++---
 src/primitives/transaction.h         |  6 ++----
 src/test/coins_tests.cpp             |  4 ++--
 src/test/mempool_tests.cpp           | 12 ++++++------
 src/test/script_tests.cpp            |  8 ++++----
 src/test/test_dash.cpp               |  2 +-
 src/test/txvalidationcache_tests.cpp |  2 +-
 src/wallet/wallet.cpp                |  4 ++--
 10 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/src/coinjoin/coinjoin-client.cpp b/src/coinjoin/coinjoin-client.cpp
index e03fdd349d09..bc3e913952c8 100644
--- a/src/coinjoin/coinjoin-client.cpp
+++ b/src/coinjoin/coinjoin-client.cpp
@@ -557,7 +557,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
     if (fMasternodeMode || pnode == nullptr) return false;
     if (!mixingMasternode) return false;
 
-    finalMutableTransaction = finalTransactionNew;
+    finalMutableTransaction = CMutableTransaction{finalTransactionNew};
     LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */
 
     // STEP 1: check final transaction general rules
diff --git a/src/dash-tx.cpp b/src/dash-tx.cpp
index fd4152b1d7e3..d44c6310d91e 100644
--- a/src/dash-tx.cpp
+++ b/src/dash-tx.cpp
@@ -501,7 +501,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
     // mergedTx will end up with all the signatures; it
     // starts as a clone of the raw tx:
     CMutableTransaction mergedTx{tx};
-    const CTransaction txv{tx};
+    const CMutableTransaction txv{tx};
     CCoinsView viewDummy;
     CCoinsViewCache view(&viewDummy);
 
diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp
index a2c8fcd92721..03221d7d8822 100644
--- a/src/primitives/transaction.cpp
+++ b/src/primitives/transaction.cpp
@@ -91,9 +91,9 @@ uint256 CTransaction::ComputeHash() const
 }
 
 /* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */
-CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nType(TRANSACTION_NORMAL), nLockTime(0), hash() {}
-CTransaction::CTransaction(const CMutableTransaction &tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash(ComputeHash()) {}
-CTransaction::CTransaction(CMutableTransaction &&tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash(ComputeHash()) {}
+CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nType(TRANSACTION_NORMAL), nLockTime(0), hash{} {}
+CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {}
+CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {}
 
 CAmount CTransaction::GetValueOut() const
 {
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
index 06744ddb7867..eecb844006ab 100644
--- a/src/primitives/transaction.h
+++ b/src/primitives/transaction.h
@@ -232,9 +232,7 @@ class CTransaction
         return vin.empty() && vout.empty();
     }
 
-    const uint256& GetHash() const {
-        return hash;
-    }
+    const uint256& GetHash() const { return hash; }
 
     // Return sum of txouts.
     CAmount GetValueOut() const;
@@ -277,7 +275,7 @@ struct CMutableTransaction
     std::vector<uint8_t> vExtraPayload; // only available for special transaction types
 
     CMutableTransaction();
-    CMutableTransaction(const CTransaction& tx);
+    explicit CMutableTransaction(const CTransaction& tx);
 
     SERIALIZE_METHODS(CMutableTransaction, obj)
     {
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 7ded9d4fabf7..2b23a62b96e5 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
                 if (InsecureRandRange(10) == 0 && coinbase_coins.size()) {
                     auto utxod = FindRandomFrom(coinbase_coins);
                     // Reuse the exact same coinbase
-                    tx = std::get<0>(utxod->second);
+                    tx = CMutableTransaction{std::get<0>(utxod->second)};
                     // shouldn't be available for reconnection if it's been duplicated
                     disconnected_coins.erase(utxod->first);
 
@@ -331,7 +331,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
                 // 1/20 times reconnect a previously disconnected tx
                 if (randiter % 20 == 2 && disconnected_coins.size()) {
                     auto utxod = FindRandomFrom(disconnected_coins);
-                    tx = std::get<0>(utxod->second);
+                    tx = CMutableTransaction{std::get<0>(utxod->second)};
                     prevout = tx.vin[0].prevout;
                     if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevout)) {
                         disconnected_coins.erase(utxod->first);
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index e927de17ae8d..3c7288e1b92f 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -605,7 +605,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     // [tx1]
     //
     CTransactionRef tx1 = make_tx(MK_OUTPUTS(10 * COIN));
-    pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(*tx1));
+    pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(tx1));
 
     // Ancestors / descendants should be 1 / 1 (itself / itself)
     pool.GetTransactionAncestry(tx1->GetHash(), ancestors, descendants);
@@ -617,7 +617,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     // [tx1].0 <- [tx2]
     //
     CTransactionRef tx2 = make_tx(MK_OUTPUTS(495 * CENT, 5 * COIN), MK_INPUTS(tx1));
-    pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(*tx2));
+    pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(tx2));
 
     // Ancestors / descendants should be:
     // transaction  ancestors   descendants
@@ -636,7 +636,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     // [tx1].0 <- [tx2].0 <- [tx3]
     //
     CTransactionRef tx3 = make_tx(MK_OUTPUTS(290 * CENT, 200 * CENT), MK_INPUTS(tx2));
-    pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(*tx3));
+    pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(tx3));
 
     // Ancestors / descendants should be:
     // transaction  ancestors   descendants
@@ -661,7 +661,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     //              \---1 <- [tx4]
     //
     CTransactionRef tx4 = make_tx(MK_OUTPUTS(290 * CENT, 250 * CENT), MK_INPUTS(tx2), MK_INPUT_IDX(1));
-    pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(*tx4));
+    pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(tx4));
 
     // Ancestors / descendants should be:
     // transaction  ancestors   descendants
@@ -698,13 +698,13 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
         CTransactionRef& tyi = *ty[i];
         tyi = make_tx(MK_OUTPUTS(v), i > 0 ? MK_INPUTS(*ty[i-1]) : std::vector<CTransactionRef>());
         v -= 50 * CENT;
-        pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(*tyi));
+        pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(tyi));
         pool.GetTransactionAncestry(tyi->GetHash(), ancestors, descendants);
         BOOST_CHECK_EQUAL(ancestors, i+1);
         BOOST_CHECK_EQUAL(descendants, i+1);
     }
     CTransactionRef ty6 = make_tx(MK_OUTPUTS(5 * COIN), MK_INPUTS(tx3, ty5));
-    pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(*ty6));
+    pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(ty6));
 
     // Ancestors / descendants should be:
     // transaction  ancestors           descendants
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 508dbf3770b1..0c14229cd864 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -133,7 +133,7 @@ CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey)
     return txCredit;
 }
 
-CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit)
+CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CTransaction& txCredit)
 {
     CMutableTransaction txSpend;
     txSpend.nVersion = 1;
@@ -155,7 +155,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, int flags, co
     bool expect = (scriptError == SCRIPT_ERR_OK);
     bool fEnableDIP0020Opcodes = (SCRIPT_ENABLE_DIP0020_OPCODES & flags) != 0;
     ScriptError err;
-    CMutableTransaction txCredit = BuildCreditingTransaction(scriptPubKey);
+    const CTransaction txCredit{BuildCreditingTransaction(scriptPubKey)};
     CMutableTransaction tx = BuildSpendingTransaction(scriptSig, txCredit);
     CMutableTransaction tx2 = tx;
     BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message);
@@ -1018,7 +1018,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12)
     CScript scriptPubKey12;
     scriptPubKey12 << OP_1 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << OP_2 << OP_CHECKMULTISIG;
 
-    CMutableTransaction txFrom12 = BuildCreditingTransaction(scriptPubKey12);
+    const CTransaction txFrom12{BuildCreditingTransaction(scriptPubKey12)};
     CMutableTransaction txTo12 = BuildSpendingTransaction(CScript(), txFrom12);
 
     CScript goodsig1 = sign_multisig(scriptPubKey12, key1, txTo12);
@@ -1049,7 +1049,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23)
     CScript scriptPubKey23;
     scriptPubKey23 << OP_2 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << ToByteVector(key3.GetPubKey()) << OP_3 << OP_CHECKMULTISIG;
 
-    CMutableTransaction txFrom23 = BuildCreditingTransaction(scriptPubKey23);
+    const CTransaction txFrom23{BuildCreditingTransaction(scriptPubKey23)};
     CMutableTransaction txTo23 = BuildSpendingTransaction(CScript(), txFrom23);
 
     std::vector<CKey> keys;
diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp
index 9c170ce7d2f0..786dc00ca894 100644
--- a/src/test/test_dash.cpp
+++ b/src/test/test_dash.cpp
@@ -232,7 +232,7 @@ CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns,
         if (!CalcCbTxMerkleRootQuorums(block, chainActive.Tip(), cbTx.merkleRootQuorums, state)) {
             BOOST_ASSERT(false);
         }
-        CMutableTransaction tmpTx = *block.vtx[0];
+        CMutableTransaction tmpTx{*block.vtx[0]};
         SetTxPayload(tmpTx, cbTx);
         block.vtx[0] = MakeTransactionRef(tmpTx);
     }
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 6b1f115787ca..66c8c848a22b 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -24,7 +24,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
 BOOST_AUTO_TEST_SUITE(tx_validationcache_tests)
 
 static bool
-ToMemPool(CMutableTransaction& tx)
+ToMemPool(const CMutableTransaction& tx)
 {
     LOCK(cs_main);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 2ef5acb12b3d..ca54483b00ee 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2584,8 +2584,8 @@ bool CWalletTx::IsTrusted() const
 
 bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
 {
-        CMutableTransaction tx1 = *this->tx;
-        CMutableTransaction tx2 = *_tx.tx;
+        CMutableTransaction tx1 {*this->tx};
+        CMutableTransaction tx2 {*_tx.tx};
         for (auto& txin : tx1.vin) txin.scriptSig = CScript();
         for (auto& txin : tx2.vin) txin.scriptSig = CScript();
         return CTransaction(tx1) == CTransaction(tx2);

From 01fccc4694b11a210cef0dfba26178cda640efae Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter.wuille@gmail.com>
Date: Thu, 30 Nov 2017 16:48:31 -0800
Subject: [PATCH 06/33] partial merge bitcoin#11403: Abstract out IsSolvable
 from Witnessifier

---
 src/policy/policy.h | 32 ++++++++++++++++----------------
 src/script/sign.cpp | 15 +++++++++++++++
 src/script/sign.h   |  6 ++++++
 3 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/src/policy/policy.h b/src/policy/policy.h
index 3a8764d4085a..012013934f4e 100644
--- a/src/policy/policy.h
+++ b/src/policy/policy.h
@@ -43,26 +43,26 @@ static const unsigned int DUST_RELAY_TX_FEE = 3000;
  * with. However scripts violating these flags may still be present in valid
  * blocks and we must accept those blocks.
  */
-static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS |
-                                                         SCRIPT_VERIFY_DERSIG |
-                                                         SCRIPT_VERIFY_STRICTENC |
-                                                         SCRIPT_VERIFY_MINIMALDATA |
-                                                         SCRIPT_VERIFY_NULLDUMMY |
-                                                         SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS |
-                                                         SCRIPT_VERIFY_CLEANSTACK |
-                                                         SCRIPT_VERIFY_NULLFAIL |
-                                                         SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY |
-                                                         SCRIPT_VERIFY_CHECKSEQUENCEVERIFY |
-                                                         SCRIPT_VERIFY_LOW_S |
-                                                         SCRIPT_ENABLE_DIP0020_OPCODES |
-                                                         SCRIPT_VERIFY_CONST_SCRIPTCODE;
+static constexpr unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS |
+                                                             SCRIPT_VERIFY_DERSIG |
+                                                             SCRIPT_VERIFY_STRICTENC |
+                                                             SCRIPT_VERIFY_MINIMALDATA |
+                                                             SCRIPT_VERIFY_NULLDUMMY |
+                                                             SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS |
+                                                             SCRIPT_VERIFY_CLEANSTACK |
+                                                             SCRIPT_VERIFY_NULLFAIL |
+                                                             SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY |
+                                                             SCRIPT_VERIFY_CHECKSEQUENCEVERIFY |
+                                                             SCRIPT_VERIFY_LOW_S |
+                                                             SCRIPT_ENABLE_DIP0020_OPCODES |
+                                                             SCRIPT_VERIFY_CONST_SCRIPTCODE;
 
 /** For convenience, standard but not mandatory verify flags. */
-static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS;
+static constexpr unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS;
 
 /** Used as the flags parameter to sequence and nLocktime checks in non-consensus code. */
-static const unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE |
-                                                           LOCKTIME_MEDIAN_TIME_PAST;
+static constexpr unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS = LOCKTIME_VERIFY_SEQUENCE |
+                                                               LOCKTIME_MEDIAN_TIME_PAST;
 
 CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee);
 
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index a071171a0a82..ecd4012b7e05 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -352,3 +352,18 @@ class DummySignatureCreator final : public BaseSignatureCreator {
 }
 
 const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator();
+
+bool IsSolvable(const CKeyStore& store, const CScript& script)
+{
+    // This check is to make sure that the script we created can actually be solved for and signed by us
+    // if we were to have the private keys. This is just to make sure that the script is valid and that,
+    // if found in a transaction, we would still accept and relay that transaction.
+    DummySignatureCreator creator(&store);
+    SignatureData sigs;
+    if (ProduceSignature(creator, script, sigs)) {
+        // VerifyScript check is just defensive, and should never fail.
+        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()));
+        return true;
+    }
+    return false;
+}
diff --git a/src/script/sign.h b/src/script/sign.h
index 53c25e336dca..7178c1f9f097 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -82,4 +82,10 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
 void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data);
 void UpdateInput(CTxIn& input, const SignatureData& data);
 
+/* Check whether we know how to sign for an output like this, assuming we
+ * have all private keys. While this function does not need private keys, the passed
+ * keystore is used to look up public keys and redeemscripts by hash.
+ * Solvability is unrelated to whether we consider this output to be ours. */
+bool IsSolvable(const CKeyStore& store, const CScript& script);
+
 #endif // BITCOIN_SCRIPT_SIGN_H

From 05f4da48fcbc683daa5ef0f46ac8a3f1f3c92418 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Fri, 18 Jun 2021 13:55:36 +0300
Subject: [PATCH 07/33] leftovers of bitcoin#12714

---
 src/script/sign.cpp | 4 ++--
 src/script/sign.h   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index ecd4012b7e05..71ac54cb1b8e 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -353,12 +353,12 @@ class DummySignatureCreator final : public BaseSignatureCreator {
 
 const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator();
 
-bool IsSolvable(const CKeyStore& store, const CScript& script)
+bool IsSolvable(const SigningProvider& provider, const CScript& script)
 {
     // This check is to make sure that the script we created can actually be solved for and signed by us
     // if we were to have the private keys. This is just to make sure that the script is valid and that,
     // if found in a transaction, we would still accept and relay that transaction.
-    DummySignatureCreator creator(&store);
+    DummySignatureCreator creator(&provider);
     SignatureData sigs;
     if (ProduceSignature(creator, script, sigs)) {
         // VerifyScript check is just defensive, and should never fail.
diff --git a/src/script/sign.h b/src/script/sign.h
index 7178c1f9f097..f8b5e98d2c7b 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -84,8 +84,8 @@ void UpdateInput(CTxIn& input, const SignatureData& data);
 
 /* Check whether we know how to sign for an output like this, assuming we
  * have all private keys. While this function does not need private keys, the passed
- * keystore is used to look up public keys and redeemscripts by hash.
+ * provider is used to look up public keys and redeemscripts by hash.
  * Solvability is unrelated to whether we consider this output to be ours. */
-bool IsSolvable(const CKeyStore& store, const CScript& script);
+bool IsSolvable(const SigningProvider& provider, const CScript& script);
 
 #endif // BITCOIN_SCRIPT_SIGN_H

From 99608c8b991c525e9bbb4e56712041c85a5634fb Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Fri, 18 Jun 2021 13:56:49 +0300
Subject: [PATCH 08/33] leftovers of bitcoin#12803

---
 src/script/sign.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 71ac54cb1b8e..3125c67d3889 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -358,11 +358,10 @@ bool IsSolvable(const SigningProvider& provider, const CScript& script)
     // This check is to make sure that the script we created can actually be solved for and signed by us
     // if we were to have the private keys. This is just to make sure that the script is valid and that,
     // if found in a transaction, we would still accept and relay that transaction.
-    DummySignatureCreator creator(&provider);
     SignatureData sigs;
-    if (ProduceSignature(creator, script, sigs)) {
+    if (ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, script, sigs)) {
         // VerifyScript check is just defensive, and should never fail.
-        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()));
+        assert(VerifyScript(sigs.scriptSig, script, STANDARD_SCRIPT_VERIFY_FLAGS, DUMMY_CHECKER));
         return true;
     }
     return false;

From 6f694a65eaf1033037e4209d4abf83c9cbc416a7 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 29 May 2018 14:59:25 +0200
Subject: [PATCH 09/33] Merge #13142: Separate IsMine from solvability

c004ffc9b42a738043e19e4c812fc7e0566119c5 Make handling of invalid in IsMine more uniform (Pieter Wuille)
a53f0feff8d42b7a40d417f77dc8de682dd88fd9 Add some checks for invalid recursion in IsMine (Pieter Wuille)
b5802a9f5f69815d3290361fd8c96d76a037832f Simplify IsMine logic (Pieter Wuille)
4e91820531889e309dc4335fe0de8229c6426040 Make IsMine stop distinguishing solvable/unsolvable (Pieter Wuille)
6d714c3419b368671bd071a8992950c3dc00e613 Make coincontrol use IsSolvable to determine solvability (Pieter Wuille)

Pull request description:

  Our current `IsMine` logic does several things with outputs:
  * Determine "spendability" (roughly corresponding to "could we sign for this")
  * Determine "watching" (is this an output directly or indirectly a watched script)
  * Determine invalidity (is this output definitely not legally spendable, detecting accidental uncompressed pubkeys in witnesses)
  * Determine "solvability" (would we be able to sign for this ignoring the fact that we may be missing some private keys).

  The last item (solvability) is mostly unrelated and only rarely needed (there is just one instance, inside the wallet's coin control logic). This PR changes that instance to use the separate `IsSolvable` function, and stop `IsMine` from distinguishing between solvable and unsolvable.

  As an extra, this also simplifies the `IsMine` logic and adds some extra checks (which wouldn't be hit unless someone adds already invalid scripts to their wallet).

Tree-SHA512: 95a6ef75fbf2eedc5ed938c48a8e5d77dcf09c933372acdd0333129fb7301994a78498f9aacce2c8db74275e19260549dd67a83738e187d40b5090cc04f33adf
---
 src/script/ismine.cpp    | 80 ++++++++++++++++++++++++++--------------
 src/script/ismine.h      |  8 +---
 src/wallet/coincontrol.h |  2 +-
 src/wallet/wallet.cpp    |  6 +--
 4 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp
index 90ede11a7c7b..cb1ba6fc5bdc 100644
--- a/src/script/ismine.cpp
+++ b/src/script/ismine.cpp
@@ -27,6 +27,19 @@ enum class IsMineSigVersion
     P2SH = 1,       //! P2SH redeemScript
 };
 
+/**
+ * This is an internal representation of isminetype + invalidity.
+ * Its order is significant, as we return the max of all explored
+ * possibilities.
+ */
+enum class IsMineResult
+{
+    NO = 0,          //! Not ours
+    WATCH_ONLY = 1,  //! Included in watch-only balance
+    SPENDABLE = 2,   //! Included in all balances
+    INVALID = 3,     //! Not spendable by anyone
+};
+
 bool PermitsUncompressed(IsMineSigVersion sigversion)
 {
     return sigversion == IsMineSigVersion::TOP || sigversion == IsMineSigVersion::P2SH;
@@ -41,15 +54,13 @@ bool HaveKeys(const std::vector<valtype>& pubkeys, const CKeyStore& keystore)
     return true;
 }
 
-isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, IsMineSigVersion sigversion)
+IsMineResult IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion)
 {
+    IsMineResult ret = IsMineResult::NO;
+
     std::vector<valtype> vSolutions;
     txnouttype whichType;
-    if (!Solver(scriptPubKey, whichType, vSolutions)) {
-        if (keystore.HaveWatchOnly(scriptPubKey))
-            return ISMINE_WATCH_UNSOLVABLE;
-        return ISMINE_NO;
-    }
+    Solver(scriptPubKey, whichType, vSolutions);
 
     CKeyID keyID;
     switch (whichType)
@@ -60,39 +71,43 @@ isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, b
     case TX_PUBKEY:
         keyID = CPubKey(vSolutions[0]).GetID();
         if (!PermitsUncompressed(sigversion) && vSolutions[0].size() != 33) {
-            isInvalid = true;
-            return ISMINE_NO;
+            return IsMineResult::INVALID;
+        }
+        if (keystore.HaveKey(keyID)) {
+            ret = std::max(ret, IsMineResult::SPENDABLE);
         }
-        if (keystore.HaveKey(keyID))
-            return ISMINE_SPENDABLE;
         break;
     case TX_PUBKEYHASH:
         keyID = CKeyID(uint160(vSolutions[0]));
         if (!PermitsUncompressed(sigversion)) {
             CPubKey pubkey;
             if (keystore.GetPubKey(keyID, pubkey) && !pubkey.IsCompressed()) {
-                isInvalid = true;
-                return ISMINE_NO;
+                return IsMineResult::INVALID;
             }
         }
-        if (keystore.HaveKey(keyID))
-            return ISMINE_SPENDABLE;
+        if (keystore.HaveKey(keyID)) {
+            ret = std::max(ret, IsMineResult::SPENDABLE);
+        }
         break;
     case TX_SCRIPTHASH:
     {
+        if (sigversion != IsMineSigVersion::TOP) {
+            // P2SH inside P2SH is invalid.
+            return IsMineResult::INVALID;
+        }
         CScriptID scriptID = CScriptID(uint160(vSolutions[0]));
         CScript subscript;
         if (keystore.GetCScript(scriptID, subscript)) {
-            isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::P2SH);
-            if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid))
-                return ret;
+            ret = std::max(ret, IsMineInner(keystore, subscript, IsMineSigVersion::P2SH));
         }
         break;
     }
     case TX_MULTISIG:
     {
         // Never treat bare multisig outputs as ours (they can still be made watchonly-though)
-        if (sigversion == IsMineSigVersion::TOP) break;
+        if (sigversion == IsMineSigVersion::TOP) {
+            break;
+        }
 
         // Only consider transactions "mine" if we own ALL the
         // keys involved. Multi-signature transactions that are
@@ -103,30 +118,39 @@ isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, b
         if (!PermitsUncompressed(sigversion)) {
             for (size_t i = 0; i < keys.size(); i++) {
                 if (keys[i].size() != 33) {
-                    isInvalid = true;
-                    return ISMINE_NO;
+                    return IsMineResult::INVALID;
                 }
             }
         }
-        if (HaveKeys(keys, keystore))
-            return ISMINE_SPENDABLE;
+        if (HaveKeys(keys, keystore)) {
+            ret = std::max(ret, IsMineResult::SPENDABLE);
+        }
         break;
     }
     }
 
-    if (keystore.HaveWatchOnly(scriptPubKey)) {
-        // TODO: This could be optimized some by doing some work after the above solver
-        SignatureData sigs;
-        return ProduceSignature(keystore, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigs) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE;
+    if (ret == IsMineResult::NO && keystore.HaveWatchOnly(scriptPubKey)) {
+        ret = std::max(ret, IsMineResult::WATCH_ONLY);
     }
-    return ISMINE_NO;
+    return ret;
 }
 
 } // namespace
 
 isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid)
 {
-    return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::TOP);
+    isInvalid = false;
+    switch (IsMineInner(keystore, scriptPubKey, IsMineSigVersion::TOP)) {
+    case IsMineResult::INVALID:
+        isInvalid = true;
+    case IsMineResult::NO:
+        return ISMINE_NO;
+    case IsMineResult::WATCH_ONLY:
+        return ISMINE_WATCH_ONLY;
+    case IsMineResult::SPENDABLE:
+        return ISMINE_SPENDABLE;
+    }
+    assert(false);
 }
 
 isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
diff --git a/src/script/ismine.h b/src/script/ismine.h
index 37f4acae0318..44415a6ce826 100644
--- a/src/script/ismine.h
+++ b/src/script/ismine.h
@@ -17,12 +17,8 @@ class CScript;
 enum isminetype
 {
     ISMINE_NO = 0,
-    //! Indicates that we don't know how to create a scriptSig that would solve this if we were given the appropriate private keys
-    ISMINE_WATCH_UNSOLVABLE = 1,
-    //! Indicates that we know how to create a scriptSig that would solve this if we were given the appropriate private keys
-    ISMINE_WATCH_SOLVABLE = 2,
-    ISMINE_WATCH_ONLY = ISMINE_WATCH_SOLVABLE | ISMINE_WATCH_UNSOLVABLE,
-    ISMINE_SPENDABLE = 4,
+    ISMINE_WATCH_ONLY = 1,
+    ISMINE_SPENDABLE = 2,
     ISMINE_ALL = ISMINE_WATCH_ONLY | ISMINE_SPENDABLE
 };
 /** used for bitflags of isminetype */
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index d82d350c84c9..4f7669ae980f 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -35,7 +35,7 @@ class CCoinControl
     bool fAllowOtherInputs;
     //! If false, only include as many inputs as necessary to fulfill a coin selection request. Only usable together with fAllowOtherInputs
     bool fRequireAllInputs;
-    //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria
+    //! Includes watch only addresses which are solvable
     bool fAllowWatchOnly;
     //! Override automatic min/max checks on fee, m_feerate must be set if true
     bool fOverrideFeeRate;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index ca54483b00ee..19576116124a 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2965,10 +2965,10 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
                 continue;
             }
 
-            bool fSpendableIn = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (coinControl && coinControl->fAllowWatchOnly && (mine & ISMINE_WATCH_SOLVABLE) != ISMINE_NO);
-            bool fSolvableIn = (mine & (ISMINE_SPENDABLE | ISMINE_WATCH_SOLVABLE)) != ISMINE_NO;
+            bool solvable = IsSolvable(*this, pcoin->tx->vout[i].scriptPubKey);
+            bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
 
-            vCoins.push_back(COutput(pcoin, i, nDepth, fSpendableIn, fSolvableIn, safeTx));
+            vCoins.push_back(COutput(pcoin, i, nDepth, spendable, solvable, safeTx));
 
             // Checks the sum amount of all UTXO's.
             if (nMinimumSumAmount != MAX_MONEY) {

From e8da18d43b04545e817925a84026140d17ab3eea Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 30 May 2018 16:13:02 +0200
Subject: [PATCH 10/33] Merge #13194: Remove template matching and pseudo
 opcodes

c814e2e7e81fd01fcb07f4a28435741bdc463801 Remove template matching and pseudo opcodes (Pieter Wuille)

Pull request description:

  The current code contains a rather complex script template matching engine, which is only used for 3 particular script types (P2PK, P2PKH, multisig). The first two of these are trivial to match for otherwise, and a specialized matcher for multisig is both more compact and more efficient than a generic one.

  The goal is being more flexible, so that for example larger standard multisigs inside SegWit outputs are easier to implement.

  As a side-effect, it also gets rid of the pseudo opcodes hack.

Tree-SHA512: 643b409c5c36821519f613a43efd399af0ec99b6131f35cd4024decfb2d483d719e0e921cd088bc9832a7ac797cb4a6b1158b8574c82f7fbebb75f1b31b359df
---
 src/pubkey.h            |   8 +--
 src/script/script.cpp   |   5 --
 src/script/script.h     |   6 --
 src/script/standard.cpp | 149 +++++++++++++++++-----------------------
 4 files changed, 66 insertions(+), 102 deletions(-)

diff --git a/src/pubkey.h b/src/pubkey.h
index 875ab76bf6de..cf61c4aabed7 100644
--- a/src/pubkey.h
+++ b/src/pubkey.h
@@ -33,10 +33,10 @@ class CPubKey
     /**
      * secp256k1:
      */
-    static const unsigned int PUBLIC_KEY_SIZE             = 65;
-    static const unsigned int COMPRESSED_PUBLIC_KEY_SIZE  = 33;
-    static const unsigned int SIGNATURE_SIZE              = 72;
-    static const unsigned int COMPACT_SIGNATURE_SIZE      = 65;
+    static constexpr unsigned int PUBLIC_KEY_SIZE             = 65;
+    static constexpr unsigned int COMPRESSED_PUBLIC_KEY_SIZE  = 33;
+    static constexpr unsigned int SIGNATURE_SIZE              = 72;
+    static constexpr unsigned int COMPACT_SIGNATURE_SIZE      = 65;
     /**
      * see www.keylength.com
      * script supports up to 75 for single byte push
diff --git a/src/script/script.cpp b/src/script/script.cpp
index f83a5c9e27a2..78bc07a6f332 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cpp
@@ -145,11 +145,6 @@ const char* GetOpName(opcodetype opcode)
 
     case OP_INVALIDOPCODE          : return "OP_INVALIDOPCODE";
 
-    // Note:
-    //  The template matching params OP_SMALLINTEGER/etc are defined in opcodetype enum
-    //  as kind of implementation hack, they are *NOT* real opcodes.  If found in real
-    //  Script, just let the default: case deal with them.
-
     default:
         return "OP_UNKNOWN";
     }
diff --git a/src/script/script.h b/src/script/script.h
index 4a2a506f56e1..05456538afb0 100644
--- a/src/script/script.h
+++ b/src/script/script.h
@@ -187,12 +187,6 @@ enum opcodetype
     OP_CHECKDATASIG = 0xba,
     OP_CHECKDATASIGVERIFY = 0xbb,
 
-    // template matching params
-    OP_SMALLINTEGER = 0xfa,
-    OP_PUBKEYS = 0xfb,
-    OP_PUBKEYHASH = 0xfd,
-    OP_PUBKEY = 0xfe,
-
     OP_INVALIDOPCODE = 0xff,
 };
 
diff --git a/src/script/standard.cpp b/src/script/standard.cpp
index 07c1a10bb6ae..6a3202d37606 100644
--- a/src/script/standard.cpp
+++ b/src/script/standard.cpp
@@ -32,22 +32,54 @@ const char* GetTxnOutputType(txnouttype t)
     return nullptr;
 }
 
-bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::vector<unsigned char> >& vSolutionsRet)
+static bool MatchPayToPubkey(const CScript& script, valtype& pubkey)
 {
-    // Templates
-    static std::multimap<txnouttype, CScript> mTemplates;
-    if (mTemplates.empty())
-    {
-        // Standard tx, sender provides pubkey, receiver adds signature
-        mTemplates.insert(std::make_pair(TX_PUBKEY, CScript() << OP_PUBKEY << OP_CHECKSIG));
+    if (script.size() == CPubKey::PUBLIC_KEY_SIZE + 2 && script[0] == CPubKey::PUBLIC_KEY_SIZE && script.back() == OP_CHECKSIG) {
+        pubkey = valtype(script.begin() + 1, script.begin() + CPubKey::PUBLIC_KEY_SIZE + 1);
+        return CPubKey::ValidSize(pubkey);
+    }
+    if (script.size() == CPubKey::COMPRESSED_PUBLIC_KEY_SIZE + 2 && script[0] == CPubKey::COMPRESSED_PUBLIC_KEY_SIZE && script.back() == OP_CHECKSIG) {
+        pubkey = valtype(script.begin() + 1, script.begin() + CPubKey::COMPRESSED_PUBLIC_KEY_SIZE + 1);
+        return CPubKey::ValidSize(pubkey);
+    }
+    return false;
+}
 
-        // Bitcoin address tx, sender provides hash of pubkey, receiver provides signature and pubkey
-        mTemplates.insert(std::make_pair(TX_PUBKEYHASH, CScript() << OP_DUP << OP_HASH160 << OP_PUBKEYHASH << OP_EQUALVERIFY << OP_CHECKSIG));
+static bool MatchPayToPubkeyHash(const CScript& script, valtype& pubkeyhash)
+{
+    if (script.size() == 25 && script[0] == OP_DUP && script[1] == OP_HASH160 && script[2] == 20 && script[23] == OP_EQUALVERIFY && script[24] == OP_CHECKSIG) {
+        pubkeyhash = valtype(script.begin () + 3, script.begin() + 23);
+        return true;
+    }
+    return false;
+}
+
+/** Test for "small positive integer" script opcodes - OP_1 through OP_16. */
+static constexpr bool IsSmallInteger(opcodetype opcode)
+{
+    return opcode >= OP_1 && opcode <= OP_16;
+}
 
-        // Sender provides N pubkeys, receivers provides M signatures
-        mTemplates.insert(std::make_pair(TX_MULTISIG, CScript() << OP_SMALLINTEGER << OP_PUBKEYS << OP_SMALLINTEGER << OP_CHECKMULTISIG));
+static bool MatchMultisig(const CScript& script, unsigned int& required, std::vector<valtype>& pubkeys)
+{
+    opcodetype opcode;
+    valtype data;
+    CScript::const_iterator it = script.begin();
+    if (script.size() < 1 || script.back() != OP_CHECKMULTISIG) return false;
+
+    if (!script.GetOp(it, opcode, data) || !IsSmallInteger(opcode)) return false;
+    required = CScript::DecodeOP_N(opcode);
+    while (script.GetOp(it, opcode, data) && CPubKey::ValidSize(data)) {
+        pubkeys.emplace_back(std::move(data));
     }
+    if (!IsSmallInteger(opcode)) return false;
+    unsigned int keys = CScript::DecodeOP_N(opcode);
+    if (pubkeys.size() != keys || keys < required) return false;
+    return (it + 1 == script.end());
+}
 
+bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::vector<unsigned char> >& vSolutionsRet)
+{
     vSolutionsRet.clear();
 
     // Shortcut for pay-to-script-hash, which are more constrained than the other types:
@@ -70,84 +102,27 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::v
         return true;
     }
 
-    // Scan templates
-    const CScript& script1 = scriptPubKey;
-    for (const std::pair<const txnouttype, CScript>& tplate : mTemplates)
-    {
-        const CScript& script2 = tplate.second;
-        vSolutionsRet.clear();
+    std::vector<unsigned char> data;
+    if (MatchPayToPubkey(scriptPubKey, data)) {
+        typeRet = TX_PUBKEY;
+        vSolutionsRet.push_back(std::move(data));
+        return true;
+    }
 
-        opcodetype opcode1, opcode2;
-        std::vector<unsigned char> vch1, vch2;
+    if (MatchPayToPubkeyHash(scriptPubKey, data)) {
+        typeRet = TX_PUBKEYHASH;
+        vSolutionsRet.push_back(std::move(data));
+        return true;
+    }
 
-        // Compare
-        CScript::const_iterator pc1 = script1.begin();
-        CScript::const_iterator pc2 = script2.begin();
-        while (true)
-        {
-            if (pc1 == script1.end() && pc2 == script2.end())
-            {
-                // Found a match
-                typeRet = tplate.first;
-                if (typeRet == TX_MULTISIG)
-                {
-                    // Additional checks for TX_MULTISIG:
-                    unsigned char m = vSolutionsRet.front()[0];
-                    unsigned char n = vSolutionsRet.back()[0];
-                    if (m < 1 || n < 1 || m > n || vSolutionsRet.size()-2 != n)
-                        return false;
-                }
-                return true;
-            }
-            if (!script1.GetOp(pc1, opcode1, vch1))
-                break;
-            if (!script2.GetOp(pc2, opcode2, vch2))
-                break;
-
-            // Template matching opcodes:
-            if (opcode2 == OP_PUBKEYS)
-            {
-                while (CPubKey::ValidSize(vch1))
-                {
-                    vSolutionsRet.push_back(vch1);
-                    if (!script1.GetOp(pc1, opcode1, vch1))
-                        break;
-                }
-                if (!script2.GetOp(pc2, opcode2, vch2))
-                    break;
-                // Normal situation is to fall through
-                // to other if/else statements
-            }
-
-            if (opcode2 == OP_PUBKEY)
-            {
-                if (!CPubKey::ValidSize(vch1))
-                    break;
-                vSolutionsRet.push_back(vch1);
-            }
-            else if (opcode2 == OP_PUBKEYHASH)
-            {
-                if (vch1.size() != sizeof(uint160))
-                    break;
-                vSolutionsRet.push_back(vch1);
-            }
-            else if (opcode2 == OP_SMALLINTEGER)
-            {   // Single-byte small integer pushed onto vSolutions
-                if (opcode1 == OP_0 ||
-                    (opcode1 >= OP_1 && opcode1 <= OP_16))
-                {
-                    char n = (char)CScript::DecodeOP_N(opcode1);
-                    vSolutionsRet.push_back(valtype(1, n));
-                }
-                else
-                    break;
-            }
-            else if (opcode1 != opcode2 || vch1 != vch2)
-            {
-                // Others must match exactly
-                break;
-            }
-        }
+    unsigned int required;
+    std::vector<std::vector<unsigned char>> keys;
+    if (MatchMultisig(scriptPubKey, required, keys)) {
+        typeRet = TX_MULTISIG;
+        vSolutionsRet.push_back({static_cast<unsigned char>(required)}); // safe as required is in range 1..16
+        vSolutionsRet.insert(vSolutionsRet.end(), keys.begin(), keys.end());
+        vSolutionsRet.push_back({static_cast<unsigned char>(keys.size())}); // safe as size is in range 1..16
+        return true;
     }
 
     vSolutionsRet.clear();

From 58793161ac6eb2ed53e7d42a5112657284ff4efe Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 30 May 2018 19:38:48 +0200
Subject: [PATCH 11/33] Merge #13252: Wallet: Refactor ReserveKeyFromKeyPool
 for safety

4b62bdf5136c174621509bf7866fbd89b61cc66a Wallet: Refactor ReserveKeyFromKeyPool for safety (Ben Woosley)

Pull request description:

  ReserveKeyFromKeyPool's previous behaviour is to set nIndex to -1 if the keypool is
  empty, OR throw an exception for technical failures. Instead, we now return false
  if the keypool is empty, true if the operation succeeded.

  This is to make failure more easily detectable by calling code.

Tree-SHA512: 753f057ad13bd4c28d121f426bf0967ed72b827d97fb24582f9326ec60072abc5482e3db69ccada7c5fc66de9957fc59098432dd223fc4116991cab44c6d7aef
---
 src/wallet/wallet.cpp | 31 +++++++++++++++----------------
 src/wallet/wallet.h   | 17 ++++++++++++++++-
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 19576116124a..2c7f078ea0f2 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4325,7 +4325,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
     return true;
 }
 
-void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fInternal)
+bool CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal)
 {
     nIndex = -1;
     keypool.vchPubKey = CPubKey();
@@ -4335,12 +4335,13 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fIn
         if (!IsLocked(true))
             TopUpKeyPool();
 
-        fInternal = fInternal && IsHDEnabled();
-        std::set<int64_t>& setKeyPool = fInternal ? setInternalKeyPool : setExternalKeyPool;
+        bool fReturningInternal = IsHDEnabled() && fRequestedInternal;
+        std::set<int64_t>& setKeyPool = fReturningInternal ? setInternalKeyPool : setExternalKeyPool;
 
         // Get the oldest key
-        if(setKeyPool.empty())
-            return;
+        if (setKeyPool.empty()) {
+            return false;
+        }
 
         WalletBatch batch(*database);
 
@@ -4352,14 +4353,17 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fIn
         if (!HaveKey(keypool.vchPubKey.GetID())) {
             throw std::runtime_error(std::string(__func__) + ": unknown key in key pool");
         }
-        if (keypool.fInternal != fInternal) {
+        if (keypool.fInternal != fReturningInternal) {
             throw std::runtime_error(std::string(__func__) + ": keypool entry misclassified");
         }
+        if (!keypool.vchPubKey.IsValid()) {
+            throw std::runtime_error(std::string(__func__) + ": keypool entry invalid");
+        }
 
-        assert(keypool.vchPubKey.IsValid());
         m_pool_key_to_index.erase(keypool.vchPubKey.GetID());
         LogPrintf("keypool reserve %d\n", nIndex);
     }
+    return true;
 }
 
 void CWallet::KeepKey(int64_t nIndex)
@@ -4393,10 +4397,8 @@ bool CWallet::GetKeyFromPool(CPubKey& result, bool internal)
     CKeyPool keypool;
     {
         LOCK(cs_wallet);
-        int64_t nIndex = 0;
-        ReserveKeyFromKeyPool(nIndex, keypool, internal);
-        if (nIndex == -1)
-        {
+        int64_t nIndex;
+        if (!ReserveKeyFromKeyPool(nIndex, keypool, internal)) {
             if (IsLocked(true)) return false;
             // TODO: implement keypool for all accouts?
 
@@ -4599,13 +4601,10 @@ bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool fInternalIn)
     if (nIndex == -1)
     {
         CKeyPool keypool;
-        pwallet->ReserveKeyFromKeyPool(nIndex, keypool, fInternalIn);
-        if (nIndex != -1) {
-            vchPubKey = keypool.vchPubKey;
-        }
-        else {
+        if (!pwallet->ReserveKeyFromKeyPool(nIndex, keypool, fInternalIn)) {
             return false;
         }
+        vchPubKey = keypool.vchPubKey;
         fInternal = keypool.fInternal;
     }
     assert(vchPubKey.IsValid());
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index c835b6c26718..10c9c211ea99 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1075,7 +1075,22 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
     size_t KeypoolCountExternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     size_t KeypoolCountInternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     bool TopUpKeyPool(unsigned int kpSize = 0);
-    void ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fInternal);
+
+    /**
+     * Reserves a key from the keypool and sets nIndex to its index
+     *
+     * @param[out] nIndex the index of the key in keypool
+     * @param[out] keypool the keypool the key was drawn from, which could be the
+     *     the pre-split pool if present, or the internal or external pool
+     * @param fRequestedInternal true if the caller would like the key drawn
+     *     from the internal keypool, false if external is preferred
+     *
+     * @return true if succeeded, false if failed due to empty keypool
+     * @throws std::runtime_error if keypool read failed, key was invalid,
+     *     was not found in the wallet, or was misclassified in the internal
+     *     or external keypool
+     */
+    bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal);
     void KeepKey(int64_t nIndex);
     void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey);
     bool GetKeyFromPool(CPubKey &key, bool fInternal /*= false*/);

From 0fcb967e4dc0e9724e1008461d3c6764a7e39eff Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Wed, 30 May 2018 13:42:58 -0400
Subject: [PATCH 12/33] Merge #13112: Throw an error for unknown args

903055730b Test gArgs erroring on unknown args (Andrew Chow)
4f8704d57f Give an error and exit if there are unknown parameters (Andrew Chow)
174f7c8080 Use a struct for arguments and nested map for categories (Andrew Chow)

Pull request description:

  Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist.

  Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior.

  Closes #1044

Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7
---
 src/bench/bench_dash.cpp        |  10 ++-
 src/dash-cli.cpp                |  17 ++--
 src/dash-tx.cpp                 |  10 ++-
 src/dashd.cpp                   |  13 +--
 src/init.cpp                    |  13 +++
 src/interfaces/node.cpp         |   6 +-
 src/interfaces/node.h           |   4 +-
 src/qt/dash.cpp                 |  25 +++---
 src/test/getarg_tests.cpp       |  16 +++-
 src/test/util_tests.cpp         |  67 +++++++++-----
 src/util/system.cpp             | 154 ++++++++++++++++++++++++--------
 src/util/system.h               |  34 +++++--
 test/functional/feature_help.py |  11 +++
 13 files changed, 282 insertions(+), 98 deletions(-)

diff --git a/src/bench/bench_dash.cpp b/src/bench/bench_dash.cpp
index fcbc5f50ac10..4a0ea5c5ca03 100644
--- a/src/bench/bench_dash.cpp
+++ b/src/bench/bench_dash.cpp
@@ -48,12 +48,20 @@ static void SetupBenchArgs()
     gArgs.AddArg("-plot-plotlyurl=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), false, OptionsCategory::OPTIONS);
+
+    // Hidden
+    gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
 }
 
 int main(int argc, char** argv)
 {
     SetupBenchArgs();
-    gArgs.ParseParameters(argc, argv);
+    std::string error;
+    if (!gArgs.ParseParameters(argc, argv, error)) {
+        fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
+        return false;
+    }
 
     if (HelpRequested(gArgs)) {
         std::cout << gArgs.GetHelpMessage();
diff --git a/src/dash-cli.cpp b/src/dash-cli.cpp
index 7ea565af51c7..c497f6049009 100644
--- a/src/dash-cli.cpp
+++ b/src/dash-cli.cpp
@@ -43,6 +43,7 @@ static void SetupCliArgs()
     gArgs.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-rpcclienttimeout=<n>", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), false, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-rpccookiefile=<loc>", _("Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)"), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u or testnet: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort()), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::OPTIONS);
@@ -52,6 +53,10 @@ static void SetupCliArgs()
     gArgs.AddArg("-stdinrpcpass", strprintf("Read RPC password from standard input as a single line.  When combined with -stdin, the first line from standard input is used for the RPC password."), false, OptionsCategory::OPTIONS);
 
     SetupChainParamsBaseOptions();
+
+    // Hidden
+    gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -83,7 +88,11 @@ static int AppInitRPC(int argc, char* argv[])
     // Parameters
     //
     SetupCliArgs();
-    gArgs.ParseParameters(argc, argv);
+    std::string error;
+    if (!gArgs.ParseParameters(argc, argv, error)) {
+        fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
+        return EXIT_FAILURE;
+    }
 
     if (gArgs.IsArgSet("-printcrashinfo")) {
         std::cout << GetCrashInfoStrFromSerializedStr(gArgs.GetArg("-printcrashinfo", "")) << std::endl;
@@ -114,10 +123,8 @@ static int AppInitRPC(int argc, char* argv[])
         fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
         return EXIT_FAILURE;
     }
-    try {
-        gArgs.ReadConfigFiles();
-    } catch (const std::exception& e) {
-        fprintf(stderr,"Error reading configuration file: %s\n", e.what());
+    if (!gArgs.ReadConfigFiles(error, true)) {
+        fprintf(stderr, "Error reading configuration file: %s\n", error.c_str());
         return EXIT_FAILURE;
     }
     if (!datadirFromCmdLine && !fs::is_directory(GetDataDir(false))) {
diff --git a/src/dash-tx.cpp b/src/dash-tx.cpp
index d44c6310d91e..191581b615e1 100644
--- a/src/dash-tx.cpp
+++ b/src/dash-tx.cpp
@@ -61,6 +61,10 @@ static void SetupBitcoinTxArgs()
 
     gArgs.AddArg("load=NAME:FILENAME", "Load JSON file FILENAME into register NAME", false, OptionsCategory::REGISTER_COMMANDS);
     gArgs.AddArg("set=NAME:JSON-STRING", "Set register NAME to given JSON-STRING", false, OptionsCategory::REGISTER_COMMANDS);
+
+    // Hidden
+    gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
 }
 
 //
@@ -73,7 +77,11 @@ static int AppInitRawTx(int argc, char* argv[])
     // Parameters
     //
     SetupBitcoinTxArgs();
-    gArgs.ParseParameters(argc, argv);
+    std::string error;
+    if (!gArgs.ParseParameters(argc, argv, error)) {
+        fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
+        return EXIT_FAILURE;
+    }
 
     if (gArgs.IsArgSet("-printcrashinfo")) {
         std::cout << GetCrashInfoStrFromSerializedStr(gArgs.GetArg("-printcrashinfo", "")) << std::endl;
diff --git a/src/dashd.cpp b/src/dashd.cpp
index a5e90b1b3d8b..6c78848db24e 100644
--- a/src/dashd.cpp
+++ b/src/dashd.cpp
@@ -68,7 +68,11 @@ static bool AppInit(int argc, char* argv[])
 #if HAVE_DECL_DAEMON
     gArgs.AddArg("-daemon", "Run in the background as a daemon and accept commands", false, OptionsCategory::OPTIONS);
 #endif
-    gArgs.ParseParameters(argc, argv);
+    std::string error;
+    if (!gArgs.ParseParameters(argc, argv, error)) {
+        fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
+        return false;
+    }
 
     if (gArgs.IsArgSet("-printcrashinfo")) {
         std::cout << GetCrashInfoStrFromSerializedStr(gArgs.GetArg("-printcrashinfo", "")) << std::endl;
@@ -103,11 +107,8 @@ static bool AppInit(int argc, char* argv[])
             fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
             return false;
         }
-        try
-        {
-            gArgs.ReadConfigFiles();
-        } catch (const std::exception& e) {
-            fprintf(stderr,"Error reading configuration file: %s\n", e.what());
+        if (!gArgs.ReadConfigFiles(error)) {
+            fprintf(stderr, "Error reading configuration file: %s\n", error.c_str());
             return false;
         }
         if (!datadirFromCmdLine && !fs::is_directory(GetDataDir(false)))
diff --git a/src/init.cpp b/src/init.cpp
index d1b32f4f6053..ebe6b51c58fe 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -668,6 +668,19 @@ void SetupServerArgs()
     gArgs.AddArg("-statsport=<port>", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), false, OptionsCategory::STATSD);
     gArgs.AddArg("-statsns=<ns>", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_NAMESPACE), false, OptionsCategory::STATSD);
     gArgs.AddArg("-statsperiod=<seconds>", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), false, OptionsCategory::STATSD);
+
+    // Hidden options
+    gArgs.AddArg("-benchmark", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-blockminsize", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-dbcrashratio", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-debugnet", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-forcecompactdb", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-rpcssl", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-socks", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-tor", "", false, OptionsCategory::HIDDEN);
+    gArgs.AddArg("-whitelistalwaysrelay", "", false, OptionsCategory::HIDDEN);
 }
 
 std::string LicenseInfo()
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 99b68d6786af..7279a9a7b02f 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -160,11 +160,11 @@ class NodeImpl : public Node
     CoinJoinOptionsImpl m_coinjoin;
 #endif
 
-    void parseParameters(int argc, const char* const argv[]) override
+    bool parseParameters(int argc, const char* const argv[], std::string& error) override
     {
-        gArgs.ParseParameters(argc, argv);
+        return gArgs.ParseParameters(argc, argv, error);
     }
-    void readConfigFiles() override { gArgs.ReadConfigFiles(); }
+    bool readConfigFiles(std::string& error) override { return gArgs.ReadConfigFiles(error); }
     bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
     bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
     void selectParams(const std::string& network) override { SelectParams(network); }
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 58223ca66cb1..f2a918be279d 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -93,7 +93,7 @@ class Node
     virtual ~Node() {}
 
     //! Set command line arguments.
-    virtual void parseParameters(int argc, const char* const argv[]) = 0;
+    virtual bool parseParameters(int argc, const char* const argv[], std::string& error) = 0;
 
     //! Set a command line argument if it doesn't already have a value
     virtual bool softSetArg(const std::string& arg, const std::string& value) = 0;
@@ -102,7 +102,7 @@ class Node
     virtual bool softSetBoolArg(const std::string& arg, bool value) = 0;
 
     //! Load settings from configuration file.
-    virtual void readConfigFiles() = 0;
+    virtual bool readConfigFiles(std::string& error) = 0;
 
     //! Choose network parameters.
     virtual void selectParams(const std::string& network) = 0;
diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp
index 21efd607afb0..b1fc4c735df1 100644
--- a/src/qt/dash.cpp
+++ b/src/qt/dash.cpp
@@ -580,15 +580,9 @@ int main(int argc, char *argv[])
 
     std::unique_ptr<interfaces::Node> node = interfaces::MakeNode();
 
-    /// 1. Parse command-line options. These take precedence over anything else.
-    // Command-line options take precedence:
-    node->setupServerArgs();
-    SetupUIArgs();
-    node->parseParameters(argc, argv);
-
     // Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory
 
-    /// 2. Basic Qt initialization (not dependent on parameters or configuration)
+    /// 1. Basic Qt initialization (not dependent on parameters or configuration)
     Q_INIT_RESOURCE(dash);
     Q_INIT_RESOURCE(dash_locale);
 
@@ -613,6 +607,17 @@ int main(int argc, char *argv[])
     qRegisterMetaType<WalletModel*>("WalletModel*");
 #endif
 
+    /// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
+    // Command-line options take precedence:
+    node->setupServerArgs();
+    SetupUIArgs();
+    std::string error;
+    if (!node->parseParameters(argc, argv, error)) {
+        QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
+            QObject::tr("Error parsing command line arguments: %1.").arg(QString::fromStdString(error)));
+        return EXIT_FAILURE;
+    }
+
     /// 3. Application identification
     // must be set before OptionsModel is initialized or translations are loaded,
     // as it is used to locate QSettings
@@ -654,11 +659,9 @@ int main(int argc, char *argv[])
                               QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));
         return EXIT_FAILURE;
     }
-    try {
-        node->readConfigFiles();
-    } catch (const std::exception& e) {
+    if (!node->readConfigFiles(error)) {
         QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
-                              QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what()));
+            QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error)));
         return EXIT_FAILURE;
     }
 
diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
index eec164c50083..ce76a35b624e 100644
--- a/src/test/getarg_tests.cpp
+++ b/src/test/getarg_tests.cpp
@@ -27,11 +27,21 @@ static void ResetArgs(const std::string& strArg)
     for (std::string& s : vecArg)
         vecChar.push_back(s.c_str());
 
-    gArgs.ParseParameters(vecChar.size(), vecChar.data());
+    std::string error;
+    gArgs.ParseParameters(vecChar.size(), vecChar.data(), error);
+}
+
+static void SetupArgs(const std::vector<std::string>& args)
+{
+    gArgs.ClearArgs();
+    for (const std::string& arg : args) {
+        gArgs.AddArg(arg, "", false, OptionsCategory::OPTIONS);
+    }
 }
 
 BOOST_AUTO_TEST_CASE(boolarg)
 {
+    SetupArgs({"-foo"});
     ResetArgs("-foo");
     BOOST_CHECK(gArgs.GetBoolArg("-foo", false));
     BOOST_CHECK(gArgs.GetBoolArg("-foo", true));
@@ -84,6 +94,7 @@ BOOST_AUTO_TEST_CASE(boolarg)
 
 BOOST_AUTO_TEST_CASE(stringarg)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven");
@@ -108,6 +119,7 @@ BOOST_AUTO_TEST_CASE(stringarg)
 
 BOOST_AUTO_TEST_CASE(intarg)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("");
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11);
     BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 0);
@@ -127,6 +139,7 @@ BOOST_AUTO_TEST_CASE(intarg)
 
 BOOST_AUTO_TEST_CASE(doubledash)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("--foo");
     BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true);
 
@@ -137,6 +150,7 @@ BOOST_AUTO_TEST_CASE(doubledash)
 
 BOOST_AUTO_TEST_CASE(boolargno)
 {
+    SetupArgs({"-foo", "-bar"});
     ResetArgs("-nofoo");
     BOOST_CHECK(!gArgs.GetBoolArg("-foo", true));
     BOOST_CHECK(!gArgs.GetBoolArg("-foo", false));
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 65eb7f82b355..2b564d851209 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -132,27 +132,37 @@ struct TestArgsManager : public ArgsManager
             LOCK(cs_args);
             m_config_args.clear();
         }
-        ReadConfigStream(streamConfig);
+        std::string error;
+        ReadConfigStream(streamConfig, error);
     }
     void SetNetworkOnlyArg(const std::string arg)
     {
         LOCK(cs_args);
         m_network_only_args.insert(arg);
     }
+    void SetupArgs(int argv, const char* args[])
+    {
+        for (int i = 0; i < argv; ++i) {
+            AddArg(args[i], "", false, OptionsCategory::OPTIONS);
+        }
+    }
 };
 
 BOOST_AUTO_TEST_CASE(util_ParseParameters)
 {
     TestArgsManager testArgs;
+    const char* avail_args[] = {"-a", "-b", "-ccc", "-d"};
     const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"};
 
-    testArgs.ParseParameters(0, (char**)argv_test);
+    std::string error;
+    testArgs.SetupArgs(4, avail_args);
+    testArgs.ParseParameters(0, (char**)argv_test, error);
     BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
 
-    testArgs.ParseParameters(1, (char**)argv_test);
+    testArgs.ParseParameters(1, (char**)argv_test, error);
     BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty());
 
-    testArgs.ParseParameters(7, (char**)argv_test);
+    testArgs.ParseParameters(7, (char**)argv_test, error);
     // expectation: -ignored is ignored (program name argument),
     // -a, -b and -ccc end up in map, -d ignored because it is after
     // a non-option argument (non-GNU option parsing)
@@ -173,9 +183,12 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
 BOOST_AUTO_TEST_CASE(util_GetBoolArg)
 {
     TestArgsManager testArgs;
+    const char* avail_args[] = {"-a", "-b", "-c", "-d", "-e", "-f"};
     const char *argv_test[] = {
         "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
-    testArgs.ParseParameters(7, (char**)argv_test);
+    std::string error;
+    testArgs.SetupArgs(6, avail_args);
+    testArgs.ParseParameters(7, (char**)argv_test, error);
 
     // Each letter should be set.
     for (char opt : "abcdef")
@@ -207,8 +220,11 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
     TestArgsManager testArgs;
 
     // Params test
+    const char* avail_args[] = {"-foo", "-bar"};
     const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"};
-    testArgs.ParseParameters(4, (char**)argv_test);
+    testArgs.SetupArgs(2, avail_args);
+    std::string error;
+    testArgs.ParseParameters(4, (char**)argv_test, error);
 
     // This was passed twice, second one overrides the negative setting.
     BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
@@ -220,7 +236,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
 
     // Config test
     const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n";
-    testArgs.ParseParameters(1, (char**)argv_test);
+    testArgs.ParseParameters(1, (char**)argv_test, error);
     testArgs.ReadConfigString(conf_test);
 
     // This was passed twice, second one overrides the negative setting,
@@ -235,7 +251,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
     // Combined test
     const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"};
     const char *combo_test_conf = "foo=1\nnobar=1\n";
-    testArgs.ParseParameters(3, (char**)combo_test_args);
+    testArgs.ParseParameters(3, (char**)combo_test_args, error);
     testArgs.ReadConfigString(combo_test_conf);
 
     // Command line overrides, but doesn't erase old setting
@@ -275,6 +291,8 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
        "iii=2\n";
 
     TestArgsManager test_args;
+    const char* avail_args[] = {"-a", "-b", "-ccc", "-d", "-e", "-fff", "-ggg", "-h", "-i", "-iii"};
+    test_args.SetupArgs(10, avail_args);
 
     test_args.ReadConfigString(str_config);
     // expectation: a, b, ccc, d, fff, ggg, h, i end up in map
@@ -472,6 +490,8 @@ BOOST_AUTO_TEST_CASE(util_GetArg)
 BOOST_AUTO_TEST_CASE(util_GetChainName)
 {
     TestArgsManager test_args;
+    const char* avail_args[] = {"-testnet", "-regtest"};
+    test_args.SetupArgs(2, avail_args);
 
     const char* argv_testnet[] = {"cmd", "-testnet"};
     const char* argv_regtest[] = {"cmd", "-regtest"};
@@ -481,39 +501,40 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
     // equivalent to "-testnet"
     // regtest in testnet section is ignored
     const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1";
+    std::string error;
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "main");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest");
 
-    test_args.ParseParameters(3, (char**)argv_test_no_reg);
+    test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(3, (char**)argv_test_no_reg);
+    test_args.ParseParameters(3, (char**)argv_test_no_reg, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
@@ -521,23 +542,23 @@ BOOST_AUTO_TEST_CASE(util_GetChainName)
     // [test] regtest=1 potentially relevant) doesn't break things
     test_args.SelectConfigNetwork("test");
 
-    test_args.ParseParameters(0, (char**)argv_testnet);
+    test_args.ParseParameters(0, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_testnet);
+    test_args.ParseParameters(2, (char**)argv_testnet, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(2, (char**)argv_regtest);
+    test_args.ParseParameters(2, (char**)argv_regtest, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 
-    test_args.ParseParameters(2, (char**)argv_test_no_reg);
+    test_args.ParseParameters(2, (char**)argv_test_no_reg, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_EQUAL(test_args.GetChainName(), "test");
 
-    test_args.ParseParameters(3, (char**)argv_both);
+    test_args.ParseParameters(3, (char**)argv_both, error);
     test_args.ReadConfigString(testnetconf);
     BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error);
 }
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 7db50f96943b..4f421f8c810f 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -420,7 +420,7 @@ void ArgsManager::SelectConfigNetwork(const std::string& network)
     m_network = network;
 }
 
-void ArgsManager::ParseParameters(int argc, const char* const argv[])
+bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::string& error)
 {
     LOCK(cs_args);
     m_override_args.clear();
@@ -452,6 +452,14 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
         } else {
             m_override_args[key].push_back(val);
         }
+
+        // Check that the arg is known
+        if (!(IsSwitchChar(key[0]) && key.size() == 1)) {
+            if (!IsArgKnown(key, error)) {
+                error = strprintf("Invalid parameter %s", key.c_str());
+                return false;
+            }
+        }
     }
 
     // we do not allow -includeconf from command line, so we clear it here
@@ -464,6 +472,23 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
             m_override_args.erase(it);
         }
     }
+    return true;
+}
+
+bool ArgsManager::IsArgKnown(const std::string& key, std::string& error)
+{
+    size_t option_index = key.find('.');
+    std::string arg_no_net;
+    if (option_index == std::string::npos) {
+        arg_no_net = key;
+    } else {
+        arg_no_net = std::string("-") + key.substr(option_index + 1, std::string::npos);
+    }
+
+    for (const auto& arg_map : m_available_args) {
+        if (arg_map.second.count(arg_no_net)) return true;
+    }
+    return false;
 }
 
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
@@ -557,62 +582,101 @@ void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strV
 
 void ArgsManager::AddArg(const std::string& name, const std::string& help, const bool debug_only, const OptionsCategory& cat)
 {
-    std::pair<OptionsCategory, std::string> key(cat, name);
-    assert(m_available_args.count(key) == 0);
-    m_available_args.emplace(key, std::pair<std::string, bool>(help, debug_only));
+    // Split arg name from its help param
+    size_t eq_index = name.find('=');
+    if (eq_index == std::string::npos) {
+        eq_index = name.size();
+    }
+
+    std::map<std::string, Arg>& arg_map = m_available_args[cat];
+    auto ret = arg_map.emplace(name.substr(0, eq_index), Arg(name.substr(eq_index, name.size() - eq_index), help, debug_only));
+    assert(ret.second); // Make sure an insertion actually happened
 }
 
 std::string ArgsManager::GetHelpMessage()
 {
     const bool show_debug = gArgs.GetBoolArg("-help-debug", false);
 
-    std::string usage = HelpMessageGroup("Options:");
-
-    OptionsCategory last_cat = OptionsCategory::OPTIONS;
-    for (auto& arg : m_available_args) {
-        if (arg.first.first != last_cat) {
-            last_cat = arg.first.first;
-            if (last_cat == OptionsCategory::CONNECTION)
+    std::string usage = "";
+    for (const auto& arg_map : m_available_args) {
+        switch(arg_map.first) {
+            case OptionsCategory::OPTIONS:
+                usage += HelpMessageGroup("Options:");
+                break;
+            case OptionsCategory::CONNECTION:
                 usage += HelpMessageGroup("Connection options:");
-            else if (last_cat == OptionsCategory::INDEXING)
+                break;
+            case OptionsCategory::INDEXING:
                 usage += HelpMessageGroup("Indexing options:");
-            else if (last_cat == OptionsCategory::MASTERNODE)
+                break;
+            case OptionsCategory::MASTERNODE:
                 usage += HelpMessageGroup("Masternode options:");
-            else if (last_cat == OptionsCategory::STATSD)
+                break;
+            case OptionsCategory::STATSD:
                 usage += HelpMessageGroup("Statsd options:");
-            else if (last_cat == OptionsCategory::ZMQ)
+                break;
+            case OptionsCategory::ZMQ:
                 usage += HelpMessageGroup("ZeroMQ notification options:");
-            else if (last_cat == OptionsCategory::DEBUG_TEST)
+                break;
+            case OptionsCategory::DEBUG_TEST:
                 usage += HelpMessageGroup("Debugging/Testing options:");
-            else if (last_cat == OptionsCategory::NODE_RELAY)
+                break;
+            case OptionsCategory::NODE_RELAY:
                 usage += HelpMessageGroup("Node relay options:");
-            else if (last_cat == OptionsCategory::BLOCK_CREATION)
+                break;
+            case OptionsCategory::BLOCK_CREATION:
                 usage += HelpMessageGroup("Block creation options:");
-            else if (last_cat == OptionsCategory::RPC)
+                break;
+            case OptionsCategory::RPC:
                 usage += HelpMessageGroup("RPC server options:");
-            else if (last_cat == OptionsCategory::WALLET)
+                break;
+            case OptionsCategory::WALLET:
                 usage += HelpMessageGroup("Wallet options:");
-            else if (last_cat == OptionsCategory::WALLET)
+                break;
+            case OptionsCategory::WALLET_FEE:
                 usage += HelpMessageGroup("Wallet fee options:");
-            else if (last_cat == OptionsCategory::WALLET)
+                break;
+            case OptionsCategory::WALLET_HD:
                 usage += HelpMessageGroup("HD wallet options:");
-            else if (last_cat == OptionsCategory::WALLET)
+                break;
+            case OptionsCategory::WALLET_KEEPASS:
                 usage += HelpMessageGroup("KeePass options:");
-            else if (last_cat == OptionsCategory::WALLET)
+                break;
+            case OptionsCategory::WALLET_COINJOIN:
                 usage += HelpMessageGroup("CoinJoin options:");
-            else if (last_cat == OptionsCategory::WALLET_DEBUG_TEST && show_debug)
-                usage += HelpMessageGroup("Wallet debugging/testing options:");
-            else if (last_cat == OptionsCategory::CHAINPARAMS)
+                break;
+            case OptionsCategory::WALLET_DEBUG_TEST:
+                if (show_debug) usage += HelpMessageGroup("Wallet debugging/testing options:");
+                break;
+            case OptionsCategory::CHAINPARAMS:
                 usage += HelpMessageGroup("Chain selection options:");
-            else if (last_cat == OptionsCategory::GUI)
+                break;
+            case OptionsCategory::GUI:
                 usage += HelpMessageGroup("UI Options:");
-            else if (last_cat == OptionsCategory::COMMANDS)
+                break;
+            case OptionsCategory::COMMANDS:
                 usage += HelpMessageGroup("Commands:");
-            else if (last_cat == OptionsCategory::REGISTER_COMMANDS)
+                break;
+            case OptionsCategory::REGISTER_COMMANDS:
                 usage += HelpMessageGroup("Register Commands:");
+                break;
+            default:
+                break;
         }
-        if (show_debug || !arg.second.second) {
-            usage += HelpMessageOpt(arg.first.second, arg.second.first);
+
+        // When we get to the hidden options, stop
+        if (arg_map.first == OptionsCategory::HIDDEN) break;
+
+        for (const auto& arg : arg_map.second) {
+            if (show_debug || !arg.second.m_debug_only) {
+                std::string name;
+                if (arg.second.m_help_param.empty()) {
+                    name = arg.first;
+                } else {
+                    name = arg.first + arg.second.m_help_param;
+                }
+                usage += HelpMessageOpt(name, arg.second.m_help_text);
+            }
         }
     }
     return usage;
@@ -782,7 +846,7 @@ fs::path GetConfigFile(const std::string& confPath)
     return AbsPathForConfigVal(fs::path(confPath), false);
 }
 
-void ArgsManager::ReadConfigStream(std::istream& stream)
+bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
 {
     LOCK(cs_args);
 
@@ -793,15 +857,23 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
     {
         std::string strKey = std::string("-") + it->string_key;
         std::string strValue = it->value[0];
+
         if (InterpretNegatedOption(strKey, strValue)) {
             m_config_args[strKey].clear();
         } else {
             m_config_args[strKey].push_back(strValue);
         }
+
+        // Check that the arg is known
+        if (!IsArgKnown(strKey, error) && !ignore_invalid_keys) {
+            error = strprintf("Invalid configuration value %s", it->string_key.c_str());
+            return false;
+        }
     }
+    return true;
 }
 
-void ArgsManager::ReadConfigFiles()
+bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
 {
     {
         LOCK(cs_args);
@@ -812,7 +884,9 @@ void ArgsManager::ReadConfigFiles()
     fsbridge::ifstream stream(GetConfigFile(confPath));
 
     if (stream.good()) {
-        ReadConfigStream(stream);
+        if (!ReadConfigStream(stream, error, ignore_invalid_keys)) {
+            return false;
+        }
         // if there is an -includeconf in the override args, but it is empty, that means the user
         // passed '-noincludeconf' on the command line, in which case we should not include anything
         if (m_override_args.count("-includeconf") == 0) {
@@ -835,7 +909,9 @@ void ArgsManager::ReadConfigFiles()
             for (const std::string& to_include : includeconf) {
                 fsbridge::ifstream include_config(GetConfigFile(to_include));
                 if (include_config.good()) {
-                    ReadConfigStream(include_config);
+                    if (!ReadConfigStream(include_config, error, ignore_invalid_keys)) {
+                        return false;
+                    }
                     LogPrintf("Included configuration file %s\n", to_include.c_str());
                 } else {
                     fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
@@ -857,14 +933,16 @@ void ArgsManager::ReadConfigFiles()
         FILE* configFile = fopen(GetConfigFile(confPath).string().c_str(), "a");
         if (configFile != nullptr)
             fclose(configFile);
-        return; // Nothing to read, so just return
+        return true; // Nothing to read, so just return
     }
 
     // If datadir is changed in .conf file:
     ClearDatadirCache();
     if (!fs::is_directory(GetDataDir(false))) {
-        throw std::runtime_error(strprintf("specified data directory \"%s\" does not exist.", gArgs.GetArg("-datadir", "").c_str()));
+        error = strprintf("specified data directory \"%s\" does not exist.", gArgs.GetArg("-datadir", "").c_str());
+        return false;
     }
+    return true;
 }
 
 std::string ArgsManager::GetChainName() const
diff --git a/src/util/system.h b/src/util/system.h
index c864c350dad8..9c05874551bc 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -141,8 +141,7 @@ inline bool IsSwitchChar(char c)
 #endif
 }
 
-enum class OptionsCategory
-{
+enum class OptionsCategory {
     OPTIONS,
     CONNECTION,
     INDEXING,
@@ -162,7 +161,9 @@ enum class OptionsCategory
     RPC,
     GUI,
     COMMANDS,
-    REGISTER_COMMANDS
+    REGISTER_COMMANDS,
+
+    HIDDEN // Always the last option to avoid printing these in the help
 };
 
 class ArgsManager
@@ -170,14 +171,23 @@ class ArgsManager
 protected:
     friend class ArgsManagerHelper;
 
+    struct Arg
+    {
+        std::string m_help_param;
+        std::string m_help_text;
+        bool m_debug_only;
+
+        Arg(const std::string& help_param, const std::string& help_text, bool debug_only) : m_help_param(help_param), m_help_text(help_text), m_debug_only(debug_only) {};
+    };
+
     mutable CCriticalSection cs_args;
     std::map<std::string, std::vector<std::string>> m_override_args;
     std::map<std::string, std::vector<std::string>> m_config_args;
     std::string m_network;
     std::set<std::string> m_network_only_args;
-    std::map<std::pair<OptionsCategory, std::string>, std::pair<std::string, bool>> m_available_args;
+    std::map<OptionsCategory, std::map<std::string, Arg>> m_available_args;
 
-    void ReadConfigStream(std::istream& stream);
+    bool ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys = false);
 
 public:
     ArgsManager();
@@ -187,8 +197,8 @@ class ArgsManager
      */
     void SelectConfigNetwork(const std::string& network);
 
-    void ParseParameters(int argc, const char*const argv[]);
-    void ReadConfigFiles();
+    bool ParseParameters(int argc, const char* const argv[], std::string& error);
+    bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false);
 
     /**
      * Log warnings for options in m_section_only_args when
@@ -291,10 +301,20 @@ class ArgsManager
      */
     void AddArg(const std::string& name, const std::string& help, const bool debug_only, const OptionsCategory& cat);
 
+    /**
+     * Clear available arguments
+     */
+    void ClearArgs() { m_available_args.clear(); }
+
     /**
      * Get the help string
      */
     std::string GetHelpMessage();
+
+    /**
+     * Check whether we know of this arg
+     */
+    bool IsArgKnown(const std::string& key, std::string& error);
 };
 
 extern ArgsManager gArgs;
diff --git a/test/functional/feature_help.py b/test/functional/feature_help.py
index a7d236b41006..8add06cfa729 100755
--- a/test/functional/feature_help.py
+++ b/test/functional/feature_help.py
@@ -36,6 +36,17 @@ def run_test(self):
         output = self.nodes[0].process.stdout.read()
         assert b'version' in output
         self.log.info("Version text received: {} (...)".format(output[0:60]))
+
+        # Test that arguments not in the help results in an error
+        self.log.info("Start dashdd with -fakearg to make sure it does not start")
+        self.nodes[0].start(extra_args=['-fakearg'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        # Node should exit immediately and output an error to stderr
+        ret_code = self.nodes[0].process.wait(timeout=1)
+        assert_equal(ret_code, 1)
+        output = self.nodes[0].process.stderr.read()
+        assert b'Error parsing command line arguments' in output
+        self.log.info("Error message received: {} (...)".format(output[0:60]))
+
         # Clean up TestNode state
         self.nodes[0].running = False
         self.nodes[0].process = None

From a9dc0f3cdad1f8cc9464256dc2d3b1238ba498b9 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 31 May 2018 05:14:07 -0400
Subject: [PATCH 13/33] Merge #13349: bench: Don't return a bool from main

493a166948 bench: Don't return a bool from main (Wladimir J. van der Laan)

Pull request description:

  Return `1` from `main()` on error, not the bool `false` (introduced in #13112). This is the correct value to return on error, and also shuts up a clang warning.

Tree-SHA512: 52a0f1b2f6ae2697555f71ee2019ce657046f7f379f1f4faf3cce9d5f3fb21fcdc43a4c84895a2a8b6929997ba70bbe87c231f2f9553215b84c22333810d58d9
---
 src/bench/bench_dash.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/bench/bench_dash.cpp b/src/bench/bench_dash.cpp
index 4a0ea5c5ca03..ce1b228fae56 100644
--- a/src/bench/bench_dash.cpp
+++ b/src/bench/bench_dash.cpp
@@ -60,13 +60,13 @@ int main(int argc, char** argv)
     std::string error;
     if (!gArgs.ParseParameters(argc, argv, error)) {
         fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
-        return false;
+        return EXIT_FAILURE;
     }
 
     if (HelpRequested(gArgs)) {
         std::cout << gArgs.GetHelpMessage();
 
-        return 0;
+        return EXIT_SUCCESS;
     }
 
     // Set the datadir after parsing the bench options
@@ -111,4 +111,6 @@ int main(int argc, char** argv)
     CleanupBLSTests();
 
     ECC_Stop();
+
+    return EXIT_SUCCESS;
 }

From ff9623ec1c8b360399e399d63b5c15eb1e59e67a Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Sat, 19 Jun 2021 17:34:39 +0300
Subject: [PATCH 14/33] Merge #13441: Prevent shared conf files from failing
 with different available options in different binaries

c2dfbb4a97 Add unavailable options to hidden options category (Andrew Chow)

Pull request description:

  From IRC:

  ```
  <ossifrage> FYI, bitcoin-qt from the head I built today won't start if you have "daemon=0" in the config file, so you can't use the same config for either bitcoind or bitcoin-qt
  <ossifrage> Seems like bitcoin-qt should ignore this option?
  <provoostenator> ossifrage: probably caused by 13112. Another problem is disablewallet=1 will prevent a launch if you compile bitcoind without wallet. It probably needs to be relaxed slightly.
  ```

  Adds all of the options that are unavailable due to compiling options to the hidden category so that shared config files do not break with the alternative binaries.

Tree-SHA512: 1ef43f5f7ad46ecc2865d22ee683ef22831e8f131ec99b732bb36d90381f7964bf64829595e993c2d435823fe4425a20323c8e65307cf2463a9e40b8049ab559
---
 src/dashd.cpp                           |  3 -
 src/init.cpp                            | 77 ++++++++++++++++++++-----
 src/util/system.cpp                     |  7 +++
 src/util/system.h                       |  5 ++
 test/functional/feature_addressindex.py |  4 +-
 test/functional/wallet_keypool_topup.py |  2 +-
 6 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/src/dashd.cpp b/src/dashd.cpp
index 6c78848db24e..3dd8012093c8 100644
--- a/src/dashd.cpp
+++ b/src/dashd.cpp
@@ -65,9 +65,6 @@ static bool AppInit(int argc, char* argv[])
     //
     // If Qt is used, parameters/dash.conf are parsed in qt/dash.cpp's main()
     SetupServerArgs();
-#if HAVE_DECL_DAEMON
-    gArgs.AddArg("-daemon", "Run in the background as a daemon and accept commands", false, OptionsCategory::OPTIONS);
-#endif
     std::string error;
     if (!gArgs.ParseParameters(argc, argv, error)) {
         fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
diff --git a/src/init.cpp b/src/init.cpp
index ebe6b51c58fe..800307cf1dac 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -110,7 +110,7 @@ std::unique_ptr<PeerLogicValidation> peerLogic;
 class DummyWalletInit : public WalletInitInterface {
 public:
 
-    void AddWalletOptions() const override {}
+    void AddWalletOptions() const override;
     bool ParameterInteraction() const override {return true;}
     void RegisterRPC(CRPCTable &) const override {}
     bool Verify() const override {return true;}
@@ -127,6 +127,21 @@ class DummyWalletInit : public WalletInitInterface {
     bool InitAutoBackup() const override {return true;}
 };
 
+void DummyWalletInit::AddWalletOptions() const
+{
+    std::vector<std::string> opts = {"-createwalletbackups=<n>", "-disablewallet", "-instantsendnotify=<cmd>",
+        "-keypool=<n>", "-rescan=<mode>", "-salvagewallet", "-spendzeroconfchange", "-upgradewallet",
+        "-wallet=<path>", "-walletbackupsdir=<dir>", "-walletbroadcast", "-walletdir=<dir>",
+        "-walletnotify=<cmd>", "-zapwallettxes=<mode>", "-discardfee=<amt>", "-fallbackfee=<amt>",
+        "-mintxfee=<amt>", "-paytxfee=<amt>", "-txconfirmtarget=<n>", "-hdseed=<hex>", "-mnemonic=<text>",
+        "-mnemonicpassphrase=<text>", "-usehd", "-keepass", "-keepassid=<id>", "-keepasskey=<key>",
+        "-keepassname=<name>", "-keepassport=<port>", "-enablecoinjoin", "-coinjoinamount=<n>",
+        "-coinjoinautostart", "-coinjoindenomsgoal=<n>", "-coinjoindenomshardcap=<n>", "-coinjoinmultisession",
+        "-coinjoinrounds=<n>", "-coinjoinsessions=<n>", "-dblogsize=<n>", "-flushwallet", "-privdb",
+        "-walletrejectlongchains"};
+    gArgs.AddHiddenArgs(opts);
+}
+
 const WalletInitInterface& g_wallet_init_interface = DummyWalletInit();
 #endif
 
@@ -476,6 +491,12 @@ void SetupServerArgs()
 
     const auto regtestLLMQ = CreateChainParams(CBaseChainParams::REGTEST)->GetConsensus().llmqs.at(Consensus::LLMQ_TEST);
 
+    // Hidden Options
+    std::vector<std::string> hidden_args = {"-rpcssl", "-benchmark", "-h", "-help", "-socks", "-tor", "-debugnet", "-whitelistalwaysrelay",
+        "-blockminsize", "-dbcrashratio", "-forcecompactdb",
+        // GUI args. These will be overwritten by SetupUIArgs for the GUI
+        "-allowselfsignedrootcertificates", "-choosedatadir", "-lang=<lang>", "-min", "-resetguisettings", "-rootcertificates=<file>", "-splash", "-uiplatform"};
+
 
     // Set all of the args and their help
     // When adding new options to the categories, please keep and ensure alphabetical ordering.
@@ -503,6 +524,8 @@ void SetupServerArgs()
     gArgs.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), false, OptionsCategory::OPTIONS);
 #ifndef WIN32
     gArgs.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), false, OptionsCategory::OPTIONS);
+#else
+    hidden_args.emplace_back("-pid");
 #endif
     gArgs.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -rescan and -disablegovernance=false. "
             "Warning: Reverting this setting requires re-downloading the entire blockchain. "
@@ -510,6 +533,8 @@ void SetupServerArgs()
     gArgs.AddArg("-syncmempool", strprintf("Sync mempool from other nodes on start (default: %u)", DEFAULT_SYNC_MEMPOOL), false, OptionsCategory::OPTIONS);
 #ifndef WIN32
     gArgs.AddArg("-sysperms", "Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)", false, OptionsCategory::OPTIONS);
+#else
+    hidden_args.emplace_back("-sysperms");
 #endif
     gArgs.AddArg("-version", "Print version and exit", false, OptionsCategory::OPTIONS);
 
@@ -559,6 +584,8 @@ void SetupServerArgs()
 #else
     gArgs.AddArg("-upnp", strprintf("Use UPnP to map the listening port (default: %u)", 0), false, OptionsCategory::CONNECTION);
 #endif
+#else
+    hidden_args.emplace_back("-upnp");
 #endif
     gArgs.AddArg("-whitebind=<addr>", "Bind to given address and whitelist peers connecting to it. Use [host]:port notation for IPv6", false, OptionsCategory::CONNECTION);
     gArgs.AddArg("-whitelist=<IP address or network>", "Whitelist peers connecting from the given IP address (e.g. 1.2.3.4) or CIDR notated network (e.g. 1.2.3.0/24). Can be specified multiple times."
@@ -568,6 +595,7 @@ void SetupServerArgs()
 
 #if ENABLE_ZMQ
     gArgs.AddArg("-zmqpubhashblock=<address>", "Enable publish hash block in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubhashchainlock=<address>", "Enable publish hash block (locked via ChainLocks) in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubhashgovernanceobject=<address>", "Enable publish hash of governance objects (like proposals) in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubhashgovernancevote=<address>", "Enable publish hash of governance votes in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubhashinstantsenddoublespend=<address>", "Enable publish transaction hashes of attempted InstantSend double spend in <address>", false, OptionsCategory::ZMQ);
@@ -575,10 +603,34 @@ void SetupServerArgs()
     gArgs.AddArg("-zmqpubhashtx=<address>", "Enable publish hash transaction in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubhashtxlock=<address>", "Enable publish hash transaction (locked via InstantSend) in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubrawblock=<address>", "Enable publish raw block in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubrawchainlock=<address>", "Enable publish raw block (locked via ChainLocks) in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubrawchainlocksig=<address>", "Enable publish raw block (locked via ChainLocks) and CLSIG message in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubrawgovernancevote=<address>", "Enable publish raw governance objects (like proposals) in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubrawgovernanceobject=<address>", "Enable publish raw governance votes in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubrawinstantsenddoublespend=<address>", "Enable publish raw transactions of attempted InstantSend double spend in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubrawrecoveredsig=<address>", "Enable publish raw recovered signatures (recovered by LLMQs) in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubrawtx=<address>", "Enable publish raw transaction in <address>", false, OptionsCategory::ZMQ);
     gArgs.AddArg("-zmqpubrawtxlock=<address>", "Enable publish raw transaction (locked via InstantSend) in <address>", false, OptionsCategory::ZMQ);
+    gArgs.AddArg("-zmqpubrawtxlocksig=<address>", "Enable publish raw transaction (locked via InstantSend) and ISLOCK in <address>", false, OptionsCategory::ZMQ);
+#else
+    hidden_args.emplace_back("-zmqpubhashblock=<address>");
+    hidden_args.emplace_back("-zmqpubhashchainlock=<address>");
+    hidden_args.emplace_back("-zmqpubhashgovernanceobject=<address>");
+    hidden_args.emplace_back("-zmqpubhashgovernancevote=<address>");
+    hidden_args.emplace_back("-zmqpubhashinstantsenddoublespend=<address>");
+    hidden_args.emplace_back("-zmqpubhashrecoveredsig=<address>");
+    hidden_args.emplace_back("-zmqpubhashtx=<address>");
+    hidden_args.emplace_back("-zmqpubhashtxlock=<address>");
+    hidden_args.emplace_back("-zmqpubrawblock=<address>");
+    hidden_args.emplace_back("-zmqpubrawchainlock=<address>");
+    hidden_args.emplace_back("-zmqpubrawchainlocksig=<address>");
+    hidden_args.emplace_back("-zmqpubrawgovernancevote=<address>");
+    hidden_args.emplace_back("-zmqpubrawgovernanceobject=<address>");
+    hidden_args.emplace_back("-zmqpubrawinstantsenddoublespend=<address>");
+    hidden_args.emplace_back("-zmqpubrawrecoveredsig=<address>");
+    hidden_args.emplace_back("-zmqpubrawtx=<address>");
+    hidden_args.emplace_back("-zmqpubrawtxlock=<address>");
+    hidden_args.emplace_back("-zmqpubrawtxlocksig=<address>");
 #endif
 
     gArgs.AddArg("-checkblockindex", strprintf("Do a full consistency check for mapBlockIndex, setBlockIndexCandidates, chainActive and mapBlocksUnlinked occasionally. (default: %u)", defaultChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST);
@@ -598,9 +650,12 @@ void SetupServerArgs()
     gArgs.AddArg("-watchquorums=<n>", strprintf("Watch and validate quorum communication (default: %u)", llmq::DEFAULT_WATCH_QUORUMS), true, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-addrmantest", "Allows to test address relay on localhost", true, OptionsCategory::DEBUG_TEST);
 
+    gArgs.AddArg("-budgetparams=<masternode>:<budget>:<superblock>", "Override masternode, budget and superblock start heights", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-debug=<category>", strprintf("Output debugging information (default: %u, supplying <category> is optional)", 0) + ". " +
         "If <category> is not supplied or if <category> = 1, output all debugging information. <category> can be: " + ListLogCategories() + ".", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-debugexclude=<category>", strprintf("Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except one or more specified categories."), false, OptionsCategory::DEBUG_TEST);
+    gArgs.AddArg("-dip3params=<activation>:<enforcement>", "Override DIP3 activation and enforcement heights", false, OptionsCategory::DEBUG_TEST);
+    gArgs.AddArg("-dip8params=<activation>", "Override DIP8 activation height", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-disablegovernance", strprintf("Disable governance validation (0-1, default: %u)", 0), false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-help-debug", "Show all debugging options (usage: --help -help-debug)", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-highsubsidyblocks=<n>", strprintf("The number of blocks with a higher than normal subsidy to mine at the start of a devnet (default: %u)", devnetConsensus.nHighSubsidyBlocks), false, OptionsCategory::DEBUG_TEST);
@@ -622,6 +677,7 @@ void SetupServerArgs()
     gArgs.AddArg("-minsporkkeys=<n>", "Overrides minimum spork signers to change spork value. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you.", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-printtoconsole", "Send trace/debug info to console instead of debug.log file", false, OptionsCategory::DEBUG_TEST);
+    gArgs.AddArg("-pushversion", "Protocol version to report to other nodes", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-sporkaddr=<dashaddress>", "Override spork address. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you.", false, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-sporkkey=<privatekey>", "Set the private key to be used for signing spork messages.", false, OptionsCategory::DEBUG_TEST);
@@ -668,19 +724,14 @@ void SetupServerArgs()
     gArgs.AddArg("-statsport=<port>", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), false, OptionsCategory::STATSD);
     gArgs.AddArg("-statsns=<ns>", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_NAMESPACE), false, OptionsCategory::STATSD);
     gArgs.AddArg("-statsperiod=<seconds>", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), false, OptionsCategory::STATSD);
+#if HAVE_DECL_DAEMON
+    gArgs.AddArg("-daemon", "Run in the background as a daemon and accept commands", false, OptionsCategory::OPTIONS);
+#else
+    hidden_args.emplace_back("-daemon");
+#endif
 
-    // Hidden options
-    gArgs.AddArg("-benchmark", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-blockminsize", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-dbcrashratio", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-debugnet", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-forcecompactdb", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-rpcssl", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-socks", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-tor", "", false, OptionsCategory::HIDDEN);
-    gArgs.AddArg("-whitelistalwaysrelay", "", false, OptionsCategory::HIDDEN);
+    // Add the hidden options
+    gArgs.AddHiddenArgs(hidden_args);
 }
 
 std::string LicenseInfo()
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 4f421f8c810f..e2a2e9227596 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -593,6 +593,13 @@ void ArgsManager::AddArg(const std::string& name, const std::string& help, const
     assert(ret.second); // Make sure an insertion actually happened
 }
 
+void ArgsManager::AddHiddenArgs(const std::vector<std::string>& names)
+{
+    for (const std::string& name : names) {
+        AddArg(name, "", false, OptionsCategory::HIDDEN);
+    }
+}
+
 std::string ArgsManager::GetHelpMessage()
 {
     const bool show_debug = gArgs.GetBoolArg("-help-debug", false);
diff --git a/src/util/system.h b/src/util/system.h
index 9c05874551bc..f26b70bed16c 100644
--- a/src/util/system.h
+++ b/src/util/system.h
@@ -301,6 +301,11 @@ class ArgsManager
      */
     void AddArg(const std::string& name, const std::string& help, const bool debug_only, const OptionsCategory& cat);
 
+    /**
+     * Add many hidden arguments
+     */
+    void AddHiddenArgs(const std::vector<std::string>& args);
+
     /**
      * Clear available arguments
      */
diff --git a/test/functional/feature_addressindex.py b/test/functional/feature_addressindex.py
index 7767d860bba5..3775b13995ac 100755
--- a/test/functional/feature_addressindex.py
+++ b/test/functional/feature_addressindex.py
@@ -23,10 +23,10 @@ def set_test_params(self):
     def setup_network(self):
         self.add_nodes(self.num_nodes)
         # Nodes 0/1 are "wallet" nodes
-        self.start_node(0, ["-relaypriority=0"])
+        self.start_node(0, [])
         self.start_node(1, ["-addressindex"])
         # Nodes 2/3 are used for testing
-        self.start_node(2, ["-addressindex", "-relaypriority=0"])
+        self.start_node(2, ["-addressindex"])
         self.start_node(3, ["-addressindex"])
         connect_nodes(self.nodes[0], 1)
         connect_nodes(self.nodes[0], 2)
diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py
index a5bce6599327..ca410909c4ff 100755
--- a/test/functional/wallet_keypool_topup.py
+++ b/test/functional/wallet_keypool_topup.py
@@ -24,7 +24,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
-        self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolmin=20']]
+        self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100']]
 
     def run_test(self):
         wallet_path = os.path.join(self.nodes[1].datadir, self.chain, "wallets", "wallet.dat")

From 3d4017d30da5719fac1bbca9a103b568552dcd54 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Thu, 31 May 2018 10:39:09 +0200
Subject: [PATCH 15/33] Merge #13309: Directly operate with CMutableTransaction
 in SignSignature

6b8b63af1461dc11ffd813401e2c36fa44656715 Generic TransactionSignatureCreator works with both CTransaction and CMutableTransaction (Martin Ankerl)

Pull request description:

  Refactored `TransactionSignatureCreator` into a templated `GenericTransactionSignatureCreator` that works with both `CMutableTransaction` and `CTransaction`.

  The advantage is that now in `SignSignature`, the `MutableTransactionSignatureCreator` can now operate directly with the `CMutableTransaction` without the need to copy the data into a `CTransaction`.

  Running all unit tests brings a very noticable speedup on my machine:

      48.4 sec before this change
      36.4 sec with this change
      --------
      12.0 seconds saved

  running only `--run_test=transaction_tests/test_big_witness_transaction`:

      16.7 sec before this change
       5.9 sec with this change
      --------
      10.8 seconds saved

  This relates to my first attempt with the const_cast hack #13202, and to the slow unit test issue #10026.

  Also see #13050 which modifies the tests but not the production code (like this PR) to get a speedup.

Tree-SHA512: 2cff0e9699f484f26120a40e431a24c8bc8f9e780fd89cb0ecf20c5be3eab6c43f9c359cde244abd9f3620d06c7c354e3b9dd3da41fa2ca1ac1e09386fea25fb
---
 src/script/interpreter.cpp | 50 +++++++++++++++++++++++++++-----------
 src/script/interpreter.h   | 38 +++++++++++++----------------
 src/script/sign.cpp        |  7 +++---
 src/script/sign.h          | 15 +++---------
 src/wallet/wallet.cpp      |  3 +--
 5 files changed, 61 insertions(+), 52 deletions(-)

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 0f7b9e33034d..5e8a5ca9f862 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1032,7 +1032,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                         CSHA256()
                             .Write(vchMessage.data(), vchMessage.size())
                             .Finalize(vchHash.data());
-                        fSuccess = checker.VerifySignature(vchSig, CPubKey(vchPubKey), uint256(vchHash));
+                        fSuccess = CPubKey(vchPubKey).Verify(uint256(vchHash), vchSig);
                     }
 
                     if (!fSuccess && (flags & SCRIPT_VERIFY_NULLFAIL) && vchSig.size()) {
@@ -1308,9 +1308,11 @@ namespace {
  * Wrapper that serializes like CTransaction, but with the modifications
  *  required for the signature hash done in-place
  */
-class CTransactionSignatureSerializer {
+template <class T>
+class CTransactionSignatureSerializer
+{
 private:
-    const CTransaction& txTo;  //!< reference to the spending transaction (the one being serialized)
+    const T& txTo;             //!< reference to the spending transaction (the one being serialized)
     const CScript& scriptCode; //!< output script being consumed
     const unsigned int nIn;    //!< input index of txTo being signed
     const bool fAnyoneCanPay;  //!< whether the hashtype has the SIGHASH_ANYONECANPAY flag set
@@ -1318,7 +1320,7 @@ class CTransactionSignatureSerializer {
     const bool fHashNone;      //!< whether the hashtype is SIGHASH_NONE
 
 public:
-    CTransactionSignatureSerializer(const CTransaction &txToIn, const CScript &scriptCodeIn, unsigned int nInIn, int nHashTypeIn) :
+    CTransactionSignatureSerializer(const T& txToIn, const CScript& scriptCodeIn, unsigned int nInIn, int nHashTypeIn) :
         txTo(txToIn), scriptCode(scriptCodeIn), nIn(nInIn),
         fAnyoneCanPay(!!(nHashTypeIn & SIGHASH_ANYONECANPAY)),
         fHashSingle((nHashTypeIn & 0x1f) == SIGHASH_SINGLE),
@@ -1402,7 +1404,9 @@ class CTransactionSignatureSerializer {
     }
 };
 
-uint256 GetPrevoutHash(const CTransaction& txTo) {
+template <class T>
+uint256 GetPrevoutHash(const T& txTo)
+{
     CHashWriter ss(SER_GETHASH, 0);
     for (const auto& txin : txTo.vin) {
         ss << txin.prevout;
@@ -1410,7 +1414,9 @@ uint256 GetPrevoutHash(const CTransaction& txTo) {
     return ss.GetHash();
 }
 
-uint256 GetSequenceHash(const CTransaction& txTo) {
+template <class T>
+uint256 GetSequenceHash(const T& txTo)
+{
     CHashWriter ss(SER_GETHASH, 0);
     for (const auto& txin : txTo.vin) {
         ss << txin.nSequence;
@@ -1418,7 +1424,9 @@ uint256 GetSequenceHash(const CTransaction& txTo) {
     return ss.GetHash();
 }
 
-uint256 GetOutputsHash(const CTransaction& txTo) {
+template <class T>
+uint256 GetOutputsHash(const T& txTo)
+{
     CHashWriter ss(SER_GETHASH, 0);
     for (const auto& txout : txTo.vout) {
         ss << txout;
@@ -1428,14 +1436,20 @@ uint256 GetOutputsHash(const CTransaction& txTo) {
 
 } // namespace
 
-PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo)
+template <class T>
+PrecomputedTransactionData::PrecomputedTransactionData(const T& txTo)
 {
     hashPrevouts = GetPrevoutHash(txTo);
     hashSequence = GetSequenceHash(txTo);
     hashOutputs = GetOutputsHash(txTo);
 }
 
-uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache)
+// explicit instantiation
+template PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo);
+template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo);
+
+template <class T>
+uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache)
 {
     assert(nIn < txTo.vin.size());
 
@@ -1450,7 +1464,7 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig
     }
 
     // Wrapper to serialize only the necessary parts of the transaction being signed
-    CTransactionSignatureSerializer txTmp(txTo, scriptCode, nIn, nHashType);
+    CTransactionSignatureSerializer<T> txTmp(txTo, scriptCode, nIn, nHashType);
 
     // Serialize and hash
     CHashWriter ss(SER_GETHASH, 0);
@@ -1458,12 +1472,14 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig
     return ss.GetHash();
 }
 
-bool BaseSignatureChecker::VerifySignature(const std::vector<uint8_t>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
+template <class T>
+bool GenericTransactionSignatureChecker<T>::VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& pubkey, const uint256& sighash) const
 {
     return pubkey.Verify(sighash, vchSig);
 }
 
-bool TransactionSignatureChecker::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
+template <class T>
+bool GenericTransactionSignatureChecker<T>::CheckSig(const std::vector<unsigned char>& vchSigIn, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
 {
     CPubKey pubkey(vchPubKey);
     if (!pubkey.IsValid())
@@ -1484,7 +1500,8 @@ bool TransactionSignatureChecker::CheckSig(const std::vector<unsigned char>& vch
     return true;
 }
 
-bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) const
+template <class T>
+bool GenericTransactionSignatureChecker<T>::CheckLockTime(const CScriptNum& nLockTime) const
 {
     // There are two kinds of nLockTime: lock-by-blockheight
     // and lock-by-blocktime, distinguished by whether
@@ -1520,7 +1537,8 @@ bool TransactionSignatureChecker::CheckLockTime(const CScriptNum& nLockTime) con
     return true;
 }
 
-bool TransactionSignatureChecker::CheckSequence(const CScriptNum& nSequence) const
+template <class T>
+bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSequence) const
 {
     // Relative lock times are supported by comparing the passed
     // in operand to the sequence number of the input.
@@ -1566,6 +1584,10 @@ bool TransactionSignatureChecker::CheckSequence(const CScriptNum& nSequence) con
     return true;
 }
 
+// explicit instantiation
+template class GenericTransactionSignatureChecker<CTransaction>;
+template class GenericTransactionSignatureChecker<CMutableTransaction>;
+
 bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror)
 {
     set_error(serror, SCRIPT_ERR_UNKNOWN_ERROR);
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 631ffc834085..dcd98b9a569e 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -110,7 +110,8 @@ struct PrecomputedTransactionData
 {
     uint256 hashPrevouts, hashSequence, hashOutputs;
 
-    explicit PrecomputedTransactionData(const CTransaction& tx);
+    template <class T>
+    explicit PrecomputedTransactionData(const T& tx);
 };
 
 enum class SigVersion
@@ -118,13 +119,12 @@ enum class SigVersion
     BASE = 0,
 };
 
-uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr);
+template <class T>
+uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr);
 
 class BaseSignatureChecker
 {
 public:
-    virtual bool VerifySignature(const std::vector<uint8_t>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
-
     virtual bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
     {
         return false;
@@ -143,33 +143,29 @@ class BaseSignatureChecker
     virtual ~BaseSignatureChecker() {}
 };
 
-class TransactionSignatureChecker : public BaseSignatureChecker
+template <class T>
+class GenericTransactionSignatureChecker : public BaseSignatureChecker
 {
 private:
-    const CTransaction* txTo;
+    const T* txTo;
     unsigned int nIn;
     const CAmount amount;
     const PrecomputedTransactionData* txdata;
 
-public:
-    TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
-	TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
-
-    // The overriden functions are now final.
-    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const final;
-    bool CheckLockTime(const CScriptNum& nLockTime) const final;
-    bool CheckSequence(const CScriptNum& nSequence) const final;
-};
-
-class MutableTransactionSignatureChecker : public TransactionSignatureChecker
-{
-private:
-    const CTransaction txTo;
+protected:
+    virtual bool VerifySignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const;
 
 public:
-    MutableTransactionSignatureChecker(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : TransactionSignatureChecker(&txTo, nInIn, amount), txTo(*txToIn) {}
+    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
+    GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
+    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
+    bool CheckLockTime(const CScriptNum& nLockTime) const override;
+    bool CheckSequence(const CScriptNum& nSequence) const override;
 };
 
+using TransactionSignatureChecker = GenericTransactionSignatureChecker<CTransaction>;
+using MutableTransactionSignatureChecker = GenericTransactionSignatureChecker<CMutableTransaction>;
+
 bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = nullptr);
 bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = nullptr);
 
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 3125c67d3889..3098013535ac 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -14,9 +14,9 @@
 
 typedef std::vector<unsigned char> valtype;
 
-TransactionSignatureCreator::TransactionSignatureCreator(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
+MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
 
-bool TransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
+bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
 {
     CKey key;
     if (!provider.GetKey(address, key))
@@ -170,8 +170,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
 {
     assert(nIn < txTo.vin.size());
 
-    CTransaction txToConst(txTo);
-    TransactionSignatureCreator creator(&txToConst, nIn, amount, nHashType);
+    MutableTransactionSignatureCreator creator(&txTo, nIn, amount, nHashType);
 
     SignatureData sigdata;
     bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
diff --git a/src/script/sign.h b/src/script/sign.h
index f8b5e98d2c7b..3e016a33de0e 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -37,26 +37,19 @@ class BaseSignatureCreator {
 };
 
 /** A signature creator for transactions. */
-class TransactionSignatureCreator : public BaseSignatureCreator {
-    const CTransaction* txTo;
+class MutableTransactionSignatureCreator : public BaseSignatureCreator {
+    const CMutableTransaction* txTo;
     unsigned int nIn;
     int nHashType;
     CAmount amount;
-    const TransactionSignatureChecker checker;
+    const MutableTransactionSignatureChecker checker;
 
 public:
-    TransactionSignatureCreator(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL);
+    MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn = SIGHASH_ALL);
     const BaseSignatureChecker& Checker() const  override{ return checker; }
     bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
 };
 
-class MutableTransactionSignatureCreator : public TransactionSignatureCreator {
-    CTransaction tx;
-
-public:
-    MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount, int nHashTypeIn) : TransactionSignatureCreator(&tx, nInIn, amount, nHashTypeIn), tx(*txToIn) {}
-};
-
 /** A signature creator that just produces 72-byte empty signatures. */
 extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR;
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 2c7f078ea0f2..12a4df97edc1 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3891,14 +3891,13 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
 
         if (sign)
         {
-            CTransaction txNewConst(txNew);
             int nIn = 0;
             for(const auto& coin : vecCoins)
             {
                 const CScript& scriptPubKey = coin.txout.scriptPubKey;
                 SignatureData sigdata;
 
-                if (!ProduceSignature(*this, TransactionSignatureCreator(&txNewConst, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
+                if (!ProduceSignature(*this, MutableTransactionSignatureCreator(&txNew, nIn, coin.txout.nValue, SIGHASH_ALL), scriptPubKey, sigdata))
                 {
                     strFailReason = _("Signing transaction failed");
                     return false;

From dbb0c39f93ebbf25e87e3ae63ebdeb818aac56f0 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Fri, 1 Jun 2018 10:07:26 +0200
Subject: [PATCH 16/33] Merge #13355: Fix "gmake check" under OpenBSD 6.3
 (probably *BSD): Avoid using GNU grep specific regexp handling

db56755ca4a0e53785b5bf322d3b65ffe328b60a Fix "gmake check" under OpenBSD 6.3 (probably *BSD): Avoid using GNU grep specific regexp handling (practicalswift)

Pull request description:

  Fixes #13337 (!)

  GNU grep and BSD grep differs in the way they handle regexps when extended regular expressions are not enabled via the `-E` flag:

  ```
  $ grep --version | head -1
  grep (GNU grep) 3.1
  $ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE("
  BOOST_FIXTURE_TEST_SUITE(foo)
  $
  ```

  ```
  $ grep --version | head -1
  grep version 0.9
  $ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE("
  $
  ```

  The portable way to do it is:

  ```
  $ echo "BOOST_FIXTURE_TEST_SUITE(foo)" | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()"
  BOOST_FIXTURE_TEST_SUITE(foo)
  $
  ```

Tree-SHA512: d83c78f34421504dd8efc3921c98527f499045b702bd34715a5bc78e04ef2a5f49f601a55ad08632e870f137b1edada94a3f530291bc9107d8d6b16fe11e640b
---
 src/Makefile.test.include | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
index e1dbff91ec01..9d5053c2175b 100644
--- a/src/Makefile.test.include
+++ b/src/Makefile.test.include
@@ -195,8 +195,8 @@ if EMBEDDED_UNIVALUE
 endif
 
 %.cpp.test: %.cpp
-	@echo Running tests: `cat $< | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE(" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1` from $<
-	$(AM_V_at)$(TEST_BINARY) -l test_suite -t "`cat $< | grep "BOOST_FIXTURE_TEST_SUITE(\|BOOST_AUTO_TEST_SUITE(" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" > $<.log 2>&1 || (cat $<.log && false)
+	@echo Running tests: `cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1` from $<
+	$(AM_V_at)$(TEST_BINARY) -l test_suite -t "`cat $< | grep -E "(BOOST_FIXTURE_TEST_SUITE\\(|BOOST_AUTO_TEST_SUITE\\()" | cut -d '(' -f 2 | cut -d ',' -f 1 | cut -d ')' -f 1`" > $<.log 2>&1 || (cat $<.log && false)
 
 test/data/%.json.h: test/data/%.json
 	@$(MKDIR_P) $(@D)

From b12936ba71b29fdc21a9c79a9cda7dd1d61af72a Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Fri, 1 Jun 2018 10:56:53 +0200
Subject: [PATCH 17/33] Merge #13353: qa: Fixup setting of PATH env var

fa26cf015658ac2aa52b5e5656e38af9a12160cc qa: Fixup setting of PATH env var (MarcoFalke)

Pull request description:

  This was an oversight of mine in #13188

  Can be trivially tested with `BITCOIND=bitcoin-qt ./test/functional/wallet_disable.py` before and after this fix.

Tree-SHA512: 06c7b2f12158855eb2b6392861943821bd7ad3152cf0dd49ac4abd878e5b937ebee55e256ce5bdc1c2a9c775a452112c34533366c934ff5f0f412b3a7e1c8118
---
 test/functional/test_framework/test_framework.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index e751f1a6806a..4383903ff83a 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -163,9 +163,11 @@ def main(self):
 
         self.extra_args_from_options = self.options.dashd_extra_args
 
-        os.environ['PATH'] = config['environment']['BUILDDIR'] + os.pathsep + \
-                             config['environment']['BUILDDIR'] + os.path.sep + "qt" + os.pathsep + \
-                             os.environ['PATH']
+        os.environ['PATH'] = os.pathsep.join([
+            os.path.join(config['environment']['BUILDDIR'], 'src'),
+            os.path.join(config['environment']['BUILDDIR'], 'src', 'qt'),
+            os.environ['PATH']
+        ])
 
         # Set up temp directory and start logging
         if self.options.tmpdir:

From 7d9504a228d840ca3b76d46cc548b9b6ca87e79e Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Mon, 4 Jun 2018 08:44:07 +0200
Subject: [PATCH 18/33] Merge #13383: bench: Use non-throwing ParseDouble(...)
 instead of throwing boost::lexical_cast<double>(...)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

f41d339b781f41f05946e965da3e1bf5d0a9e50b bench: Use non-throwing ParseDouble(...) instead of throwing boost::lexical_cast<double>(...) (practicalswift)

Pull request description:

  * Non-Boost is better than Boost.
  * Non-throwing is better than throwing.
  * Explicit error handling is better than implicit error handling.
  * `ParseDouble(…)` deserves to be used outside of its unit tests :-)

Tree-SHA512: a8cf04a5f8363cb7ced0bcaf1fed00e1e5dd6a63a6c11e5f0ba4e5c845b0df7c2b050d887075f158cd62dc7e02843ecaafc15e42e383c066461c6d7399e06b49
---
 src/bench/bench_dash.cpp | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/bench/bench_dash.cpp b/src/bench/bench_dash.cpp
index ce1b228fae56..749085d3fd9f 100644
--- a/src/bench/bench_dash.cpp
+++ b/src/bench/bench_dash.cpp
@@ -6,12 +6,11 @@
 
 #include <crypto/sha256.h>
 #include <key.h>
+#include <random.h>
 #include <stacktraces.h>
-#include <validation.h>
+#include <util/strencodings.h>
 #include <util/system.h>
-#include <random.h>
-
-#include <boost/lexical_cast.hpp>
+#include <validation.h>
 
 #include <memory>
 
@@ -90,8 +89,11 @@ int main(int argc, char** argv)
     std::string scaling_str = gArgs.GetArg("-scaling", DEFAULT_BENCH_SCALING);
     bool is_list_only = gArgs.GetBoolArg("-list", false);
 
-    double scaling_factor = boost::lexical_cast<double>(scaling_str);
-
+    double scaling_factor;
+    if (!ParseDouble(scaling_str, &scaling_factor)) {
+        fprintf(stderr, "Error parsing scaling factor as double: %s\n", scaling_str.c_str());
+        return EXIT_FAILURE;
+    }
 
     std::unique_ptr<benchmark::Printer> printer(new benchmark::ConsolePrinter());
     std::string printer_arg = gArgs.GetArg("-printer", DEFAULT_BENCH_PRINTER);

From 8e23a74c365d548b7483032a9b0cd8eefb3aea58 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 5 Jun 2018 15:47:25 +0200
Subject: [PATCH 19/33] =?UTF-8?q?Merge=20#13366:=20Docs:=20Rename=20?=
 =?UTF-8?q?=E2=80=9COS=20X=E2=80=9D=20to=20the=20newer=20=E2=80=9CmacOS?=
 =?UTF-8?q?=E2=80=9D=20convention?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

989c8990bb765eef45c8ee471f084ca81a0bead4 Rename “OS X” to the newer “macOS” convention (Giulio Lombardo)

Pull request description:

  Since 2016, with [macOS 10.12 Sierra](https://en.wikipedia.org/wiki/MacOS_Sierra), Mac OS X has been renamed in macOS. It would be a nice if Bitcoin's macOS build instructions follow this naming convention to avoid misunderstandings.

Tree-SHA512: 51b7d54bfc39a1a9d0773c64780817c7beca7094aded80481086287474dfa272bf0a1dfa6ef6e3cae91548aa127f65fa730003dddcb97147cdc8c249146aea22
---
 README.md              |  2 +-
 contrib/init/README.md |  2 +-
 depends/README.md      |  6 +++---
 depends/description.md |  2 +-
 doc/README.md          |  4 ++--
 doc/README_osx.md      | 10 +++++-----
 doc/build-osx.md       |  4 ++--
 doc/gitian-building.md |  2 +-
 doc/init.md            |  6 +++---
 doc/release-process.md | 22 +++++++++++-----------
 src/qt/README.md       |  8 ++++----
 11 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/README.md b/README.md
index 238f16e94abc..c224df3b397c 100644
--- a/README.md
+++ b/README.md
@@ -55,7 +55,7 @@ There are also [regression and integration tests](/test), written
 in Python, that are run automatically on the build server.
 These tests can be run (if the [test dependencies](/test) are installed) with: `test/functional/test_runner.py`
 
-The Travis CI system makes sure that every pull request is built for Windows, Linux, and OS X, and that unit/sanity tests are run automatically.
+The Travis CI system makes sure that every pull request is built for Windows, Linux, and macOS, and that unit/sanity tests are run automatically.
 
 ### Manual Quality Assurance (QA) Testing
 
diff --git a/contrib/init/README.md b/contrib/init/README.md
index e0adc72f3df4..1dc3a1455479 100644
--- a/contrib/init/README.md
+++ b/contrib/init/README.md
@@ -5,7 +5,7 @@ Upstart: dashd.conf
 OpenRC:  dashd.openrc
          dashd.openrcconf
 CentOS:  dashd.init
-OS X:    org.dash.dashd.plist
+macOS:   org.dash.dashd.plist
 ```
 have been made available to assist packagers in creating node packages here.
 
diff --git a/depends/README.md b/depends/README.md
index ba79881e62ce..6a9a87be1616 100644
--- a/depends/README.md
+++ b/depends/README.md
@@ -22,7 +22,7 @@ Common `host-platform-triplets` for cross compilation are:
 
 - `i686-w64-mingw32` for Win32
 - `x86_64-w64-mingw32` for Win64
-- `x86_64-apple-darwin14` for MacOSX
+- `x86_64-apple-darwin14` for macOS
 - `arm-linux-gnueabihf` for Linux ARM 32 bit
 - `aarch64-linux-gnu` for Linux ARM 64 bit
 - `riscv32-linux-gnu` for Linux RISC-V 32 bit
@@ -57,7 +57,7 @@ The following can be set when running make: make FOO=bar
 
     SOURCES_PATH: downloaded sources will be placed here
     BASE_CACHE: built packages will be placed here
-    SDK_PATH: Path where sdk's can be found (used by OSX)
+    SDK_PATH: Path where sdk's can be found (used by macOS)
     FALLBACK_DOWNLOAD_PATH: If a source file can't be fetched, try here before giving up
     NO_QT: Don't download/build/cache qt and its dependencies
     NO_WALLET: Don't download/build/cache libs needed to enable the wallet
@@ -72,7 +72,7 @@ options will be passed to Dash Core's configure. In this case, `--disable-wallet
 Additional targets:
 
     download: run 'make download' to fetch all sources without building them
-    download-osx: run 'make download-osx' to fetch all sources needed for osx builds
+    download-osx: run 'make download-osx' to fetch all sources needed for macOS builds
     download-win: run 'make download-win' to fetch all sources needed for win builds
     download-linux: run 'make download-linux' to fetch all sources needed for linux builds
 
diff --git a/depends/description.md b/depends/description.md
index 74f9ef3f205e..9fc7093be4f3 100644
--- a/depends/description.md
+++ b/depends/description.md
@@ -7,7 +7,7 @@ In theory, binaries for any target OS/architecture can be created, from a
 builder running any OS/architecture. In practice, build-side tools must be
 specified when the defaults don't fit, and packages must be amended to work
 on new hosts. For now, a build architecture of x86_64 is assumed, either on
-Linux or OSX.
+Linux or macOS.
 
 ### No reliance on timestamps
 
diff --git a/doc/README.md b/doc/README.md
index b9109feeff1f..ec8298b7bbcd 100644
--- a/doc/README.md
+++ b/doc/README.md
@@ -18,7 +18,7 @@ Unpack the files into a directory and run:
 
 Unpack the files into a directory, and then run dash-qt.exe.
 
-### OS X
+### macOS
 
 Drag Dash-Qt to your applications folder, and then run Dash-Qt.
 
@@ -35,7 +35,7 @@ Building
 ---------------------
 The following are developer notes on how to build Dash Core on your native platform. They are not complete guides, but include notes on the necessary libraries, compile flags, etc.
 
-- [OS X Build Notes](build-osx.md)
+- [macOS Build Notes](build-osx.md)
 - [Unix Build Notes](build-unix.md)
 - [Windows Build Notes](build-windows.md)
 - [OpenBSD Build Notes](build-openbsd.md)
diff --git a/doc/README_osx.md b/doc/README_osx.md
index 975be4be9ed6..739e22d6341c 100644
--- a/doc/README_osx.md
+++ b/doc/README_osx.md
@@ -1,12 +1,12 @@
-Deterministic OS X DMG Notes.
+Deterministic macOS DMG Notes.
 
-Working OS X DMGs are created in Linux by combining a recent clang,
+Working macOS DMGs are created in Linux by combining a recent clang,
 the Apple binutils (ld, ar, etc) and DMG authoring tools.
 
 Apple uses clang extensively for development and has upstreamed the necessary
 functionality so that a vanilla clang can take advantage. It supports the use
 of -F, -target, -mmacosx-version-min, and --sysroot, which are all necessary
-when building for OS X.
+when building for macOS.
 
 Apple's version of binutils (called cctools) contains lots of functionality
 missing in the FSF's binutils. In addition to extra linker options for
@@ -38,7 +38,7 @@ Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.1
 Unfortunately, the usual linux tools (7zip, hpmount, loopback mount) are incapable of opening this file.
 To create a tarball suitable for Gitian input, there are two options:
 
-Using Mac OS X, you can mount the dmg, and then create it with:
+Using macOS, you can mount the dmg, and then create it with:
 ```
   $ hdiutil attach Xcode_7.3.1.dmg
   $ tar -C /Volumes/Xcode/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/ -czf MacOSX10.11.sdk.tar.gz MacOSX10.11.sdk
@@ -81,7 +81,7 @@ Background images and other features can be added to DMG files by inserting a
 .DS_Store before creation. This is generated by the script
 contrib/macdeploy/custom_dsstore.py.
 
-As of OS X Mavericks (10.9), using an Apple-blessed key to sign binaries is a
+As of OS X 10.9 Mavericks, using an Apple-blessed key to sign binaries is a
 requirement in order to satisfy the new Gatekeeper requirements. Because this
 private key cannot be shared, we'll have to be a bit creative in order for the
 build process to remain somewhat deterministic. Here's how it works:
diff --git a/doc/build-osx.md b/doc/build-osx.md
index 35a912488697..fe90bfe81587 100644
--- a/doc/build-osx.md
+++ b/doc/build-osx.md
@@ -1,11 +1,11 @@
-Mac OS X Build Instructions and Notes
+macOS Build Instructions and Notes
 ====================================
 The commands in this guide should be executed in a Terminal application.
 The built-in one is located in `/Applications/Utilities/Terminal.app`.
 
 Preparation
 -----------
-Install the OS X command line tools:
+Install the macOS command line tools:
 
 `xcode-select --install`
 
diff --git a/doc/gitian-building.md b/doc/gitian-building.md
index c32955dfdfe2..c418e59b974f 100644
--- a/doc/gitian-building.md
+++ b/doc/gitian-building.md
@@ -360,7 +360,7 @@ offline.
 Building Dash Core
 ----------------
 
-To build Dash Core (for Linux, OS X and Windows) just follow the steps under 'perform
+To build Dash Core (for Linux, macOS and Windows) just follow the steps under 'perform
 Gitian builds' in [doc/release-process.md](release-process.md#setup-and-perform-gitian-builds) in the Dash Core repository.
 
 This may take some time as it will build all the dependencies needed for each descriptor.
diff --git a/doc/init.md b/doc/init.md
index e20d3501e8ca..071435fea0b5 100644
--- a/doc/init.md
+++ b/doc/init.md
@@ -15,7 +15,7 @@ Service User
 
 All three Linux startup configurations assume the existence of a "dashcore" user
 and group.  They must be created before attempting to use these scripts.
-The OS X configuration assumes dashd will be set up for the current user.
+The macOS configuration assumes dashd will be set up for the current user.
 
 Configuration
 ---------------------------------
@@ -65,7 +65,7 @@ reasons to make the configuration file and data directory only readable by the
 dashcore user and group.  Access to dash-cli and other dashd rpc clients
 can then be controlled by group membership.
 
-### Mac OS X
+### macOS
 
 Binary:              `/usr/local/bin/dashd`  
 Configuration file:  `~/Library/Application Support/DashCore/dash.conf`  
@@ -111,7 +111,7 @@ Using this script, you can adjust the path and flags to the dashd program by
 setting the DASHD and FLAGS environment variables in the file
 /etc/sysconfig/dashd. You can also use the DAEMONOPTS environment variable here.
 
-### Mac OS X
+### macOS
 
 Copy org.dash.dashd.plist into ~/Library/LaunchAgents. Load the launch agent by
 running `launchctl load ~/Library/LaunchAgents/org.dash.dashd.plist`.
diff --git a/doc/release-process.md b/doc/release-process.md
index 425ecba97e0d..5ea90f7fe711 100644
--- a/doc/release-process.md
+++ b/doc/release-process.md
@@ -85,7 +85,7 @@ Ensure gitian-builder is up-to-date:
     echo '5a60e0a4b3e0b4d655317b2f12a810211c50242138322b16e7e01c6fbb89d92f inputs/osslsigncode-2.0.tar.gz' | sha256sum -c
     popd
 
-Create the OS X SDK tarball, see the [OS X readme](README_osx.md) for details, and copy it into the inputs directory.
+Create the macOS SDK tarball, see the [macOS readme](README_osx.md) for details, and copy it into the inputs directory.
 
 ### Optional: Seed the Gitian sources cache and offline git repositories
 
@@ -107,7 +107,7 @@ NOTE: Offline builds must use the --url flag to ensure Gitian fetches only from
 
 The gbuild invocations below <b>DO NOT DO THIS</b> by default.
 
-### Build and sign Dash Core for Linux, Windows, and OS X:
+### Build and sign Dash Core for Linux, Windows, and macOS:
 
     pushd ./gitian-builder
     ./bin/gbuild --num-make 2 --memory 3000 --commit dash=v${VERSION} ../dash/contrib/gitian-descriptors/gitian-linux.yml
@@ -130,7 +130,7 @@ Build output expected:
   1. source tarball (`dash-${VERSION}.tar.gz`)
   2. linux 32-bit and 64-bit dist tarballs (`dash-${VERSION}-linux[32|64].tar.gz`)
   3. windows 32-bit and 64-bit unsigned installers and dist zips (`dash-${VERSION}-win[32|64]-setup-unsigned.exe`, `dash-${VERSION}-win[32|64].zip`)
-  4. OS X unsigned installer and dist tarball (`dash-${VERSION}-osx-unsigned.dmg`, `dash-${VERSION}-osx64.tar.gz`)
+  4. macOS unsigned installer and dist tarball (`dash-${VERSION}-osx-unsigned.dmg`, `dash-${VERSION}-osx64.tar.gz`)
   5. Gitian signatures (in `gitian.sigs/${VERSION}-<linux|{win,osx}-unsigned>/(your Gitian key)/`)
 
 ### Verify other gitian builders signatures to your own. (Optional)
@@ -160,13 +160,13 @@ Commit your signature to gitian.sigs:
     git push  # Assuming you can push to the gitian.sigs tree
     popd
 
-Codesigner only: Create Windows/OS X detached signatures:
+Codesigner only: Create Windows/macOS detached signatures:
 - Only one person handles codesigning. Everyone else should skip to the next step.
-- Only once the Windows/OS X builds each have 3 matching signatures may they be signed with their respective release keys.
+- Only once the Windows/macOS builds each have 3 matching signatures may they be signed with their respective release keys.
 
-Codesigner only: Sign the osx binary:
+Codesigner only: Sign the macOS binary:
 
-    transfer dashcore-osx-unsigned.tar.gz to osx for signing
+    transfer dashcore-osx-unsigned.tar.gz to macOS for signing
     tar xf dashcore-osx-unsigned.tar.gz
     ./detached-sig-create.sh -s "Key ID" -o runtime
     Enter the keychain password and authorize the signature
@@ -191,12 +191,12 @@ Codesigner only: Commit the detached codesign payloads:
     git tag -s v${VERSION} HEAD
     git push the current branch and new tag
 
-Non-codesigners: wait for Windows/OS X detached signatures:
+Non-codesigners: wait for Windows/macOS detached signatures:
 
-- Once the Windows/OS X builds each have 3 matching signatures, they will be signed with their respective release keys.
+- Once the Windows/macOS builds each have 3 matching signatures, they will be signed with their respective release keys.
 - Detached signatures will then be committed to the [dash-detached-sigs](https://github.com/dashpay/dash-detached-sigs) repository, which can be combined with the unsigned apps to create signed binaries.
 
-Create (and optionally verify) the signed OS X binary:
+Create (and optionally verify) the signed macOS binary:
 
     pushd ./gitian-builder
     ./bin/gbuild -i --commit signature=v${VERSION} ../dash/contrib/gitian-descriptors/gitian-osx-signer.yml
@@ -215,7 +215,7 @@ Create (and optionally verify) the signed Windows binaries:
     mv build/out/dash-*win32-setup.exe ../dash-${VERSION}-win32-setup.exe
     popd
 
-Commit your signature for the signed OS X/Windows binaries:
+Commit your signature for the signed macOS/Windows binaries:
 
     pushd gitian.sigs
     git add ${VERSION}-osx-signed/"${SIGNER}"
diff --git a/src/qt/README.md b/src/qt/README.md
index cba965b7a750..255d8400b1a9 100644
--- a/src/qt/README.md
+++ b/src/qt/README.md
@@ -4,7 +4,7 @@ The current precise version for Qt 5 is specified in [qt.mk](/depends/packages/q
 
 ## Compile and run
 
-See build instructions ([OSX](/doc/build-osx.md), [Windows](/doc/build-windows.md), [Unix](/doc/build-unix.md), etc).
+See build instructions ([macOS](/doc/build-osx.md), [Windows](/doc/build-windows.md), [Unix](/doc/build-unix.md), etc).
 
 To run:
 
@@ -65,7 +65,7 @@ Represents the view to a single wallet.
 * `guiconstants.h`: UI colors, app name, etc
 * `guiutil.h`: several helper functions
 * `macdockiconhandler.(h/cpp)`
-* `macdockiconhandler.(h/cpp)`: display notifications in OSX
+* `macdockiconhandler.(h/cpp)`: display notifications in macOS
 
 ## Contribute
 
@@ -81,9 +81,9 @@ the UI layout.
 Download and install the community edition of [Qt Creator](https://www.qt.io/download/).
 Uncheck everything except Qt Creator during the installation process.
 
-Instructions for OSX:
+Instructions for macOS:
 
-1. Make sure you installed everything through Homebrew mentioned in the [OSX build instructions](/doc/build-osx.md)
+1. Make sure you installed everything through Homebrew mentioned in the [macOS build instructions](/doc/build-osx.md)
 2. Use `./configure` with the `--enable-debug` flag
 3. In Qt Creator do "New Project" -> Import Project -> Import Existing Project
 4. Enter "dash-qt" as project name, enter src/qt as location

From f679d8c788bda15d9289e4af60febed1554b3fc4 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 5 Jun 2018 18:46:12 +0200
Subject: [PATCH 20/33] Merge #13269: refactoring: Drop UpdateTransaction in
 favor of UpdateInput

6aa33feadbe11bfa505a80a691d84db966aca134 Drop UpdateTransaction in favor of UpdateInput (Ben Woosley)

Pull request description:

  Updating the input explicitly requires the caller to present a mutable
  input, which more clearly communicates the effects and intent of the call
  (and, often, the enclosing loop).

  In most cases, this input is already immediately available and need not be
  looked up.

Tree-SHA512: 8c7914a8b7ae975d8ad0e9d760e3c5da65776a5f79d060b8ffb6b3ff7a32235f71ad705f2185b368d9263742d7796bb562395d22b806d90e8502d8c496011e57
---
 src/coinjoin/coinjoin-util.cpp | 2 +-
 src/dash-tx.cpp                | 4 ++--
 src/rpc/rawtransaction.cpp     | 4 ++--
 src/script/sign.cpp            | 8 +-------
 src/script/sign.h              | 1 -
 src/wallet/wallet.cpp          | 4 ++--
 6 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/src/coinjoin/coinjoin-util.cpp b/src/coinjoin/coinjoin-util.cpp
index ff1f4b8a2d5f..12101b00ff1a 100644
--- a/src/coinjoin/coinjoin-util.cpp
+++ b/src/coinjoin/coinjoin-util.cpp
@@ -142,7 +142,7 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, con
             SignatureData sigdata;
             bool res = ProduceSignature(*pwallet, DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata);
             assert(res);
-            UpdateTransaction(dummyTx, nIn, sigdata);
+            UpdateInput(dummyTx.vin[nIn], sigdata);
             nIn++;
         }
     }
diff --git a/src/dash-tx.cpp b/src/dash-tx.cpp
index 191581b615e1..43471825940a 100644
--- a/src/dash-tx.cpp
+++ b/src/dash-tx.cpp
@@ -592,7 +592,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
 
     // Sign what we can:
     for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
-        const CTxIn& txin = mergedTx.vin[i];
+        CTxIn& txin = mergedTx.vin[i];
         const Coin& coin = view.AccessCoin(txin.prevout);
         if (coin.IsSpent()) {
             continue;
@@ -607,7 +607,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
 
         // ... and merge in other signatures:
         sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
-        UpdateTransaction(mergedTx, i, sigdata);
+        UpdateInput(txin, sigdata);
     }
 
     tx = mergedTx;
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 05f1477754c4..043a955dfef3 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -746,7 +746,7 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
             }
         }
 
-        UpdateTransaction(mergedTx, i, sigdata);
+        UpdateInput(txin, sigdata);
     }
 
     return EncodeHexTx(mergedTx);
@@ -878,7 +878,7 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         }
         sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));
 
-        UpdateTransaction(mtx, i, sigdata);
+        UpdateInput(txin, sigdata);
 
         ScriptError serror = SCRIPT_ERR_OK;
         if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 3098013535ac..6ea649e4670a 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -160,12 +160,6 @@ void UpdateInput(CTxIn& input, const SignatureData& data)
     input.scriptSig = data.scriptSig;
 }
 
-void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data)
-{
-    assert(tx.vin.size() > nIn);
-    UpdateInput(tx.vin[nIn], data);
-}
-
 bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
 {
     assert(nIn < txTo.vin.size());
@@ -174,7 +168,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C
 
     SignatureData sigdata;
     bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
-    UpdateTransaction(txTo, nIn, sigdata);
+    UpdateInput(txTo.vin.at(nIn), sigdata);
     return ret;
 }
 
diff --git a/src/script/sign.h b/src/script/sign.h
index 3e016a33de0e..97cb9a7d4c77 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -72,7 +72,6 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature
 
 /** Extract signature data from a transaction, and insert it. */
 SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn);
-void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data);
 void UpdateInput(CTxIn& input, const SignatureData& data);
 
 /* Check whether we know how to sign for an output like this, assuming we
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 12a4df97edc1..e0573ea9a6fa 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -3713,7 +3713,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
                             strFailReason = _("Signing transaction failed");
                             return false;
                         } else {
-                            UpdateTransaction(txNew, nIn, sigdata);
+                            UpdateInput(txNew.vin[nIn], sigdata);
                         }
 
                         nIn++;
@@ -3902,7 +3902,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
                     strFailReason = _("Signing transaction failed");
                     return false;
                 } else {
-                    UpdateTransaction(txNew, nIn, sigdata);
+                    UpdateInput(txNew.vin.at(nIn), sigdata);
                 }
 
                 nIn++;

From 2c7c4bcc08c39bdce6cffb659132ea59f4714f81 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Tue, 5 Jun 2018 19:39:12 +0200
Subject: [PATCH 21/33] Merge #13367: qa: Increase includeconf test coverage

fa4760fbb3f1099dcd3c43ebc53c2a761a2170e8 qa: Increase includeconf test coverage (MarcoFalke)

Pull request description:

  This adds some missing `return false` for error conditions and adds test coverage [1] for those.

  Also, extend recursion warning when the chain was set in one of the includeconfs.

  [1] See the red lines in https://marcofalke.github.io/btc_cov/total.coverage/src/util.cpp.gcov.html for missing coverage.

Tree-SHA512: d32563c9bb277879895a173e699034db5ecdb4061a1ec8890c566d61e36a09efa5eda19a029baf952ff6d568f8b9684a13a0bb90827850075470975e2088fee4
---
 src/util/system.cpp                    | 20 ++++++++++++++------
 test/functional/feature_includeconf.py | 19 ++++++++++++-------
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/util/system.cpp b/src/util/system.cpp
index e2a2e9227596..850b26717ae1 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -467,9 +467,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
     if (it != m_override_args.end()) {
         if (it->second.size() > 0) {
             for (const auto& ic : it->second) {
-                fprintf(stderr, "warning: -includeconf cannot be used from commandline; ignoring -includeconf=%s\n", ic.c_str());
+                error += "-includeconf cannot be used from commandline; -includeconf=" + ic + "\n";
             }
-            m_override_args.erase(it);
+            return false;
         }
     }
     return true;
@@ -897,11 +897,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
         // if there is an -includeconf in the override args, but it is empty, that means the user
         // passed '-noincludeconf' on the command line, in which case we should not include anything
         if (m_override_args.count("-includeconf") == 0) {
+            std::string chain_id = GetChainName();
             std::vector<std::string> includeconf(GetArgs("-includeconf"));
             {
                 // We haven't set m_network yet (that happens in SelectParams()), so manually check
                 // for network.includeconf args.
-                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));
                 includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
             }
 
@@ -910,7 +911,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
             {
                 LOCK(cs_args);
                 m_config_args.erase("-includeconf");
-                m_config_args.erase(std::string("-") + GetChainName() + ".includeconf");
+                m_config_args.erase(std::string("-") + chain_id + ".includeconf");
             }
 
             for (const std::string& to_include : includeconf) {
@@ -921,15 +922,22 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
                     }
                     LogPrintf("Included configuration file %s\n", to_include.c_str());
                 } else {
-                    fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
+                    error = "Failed to include configuration file " + to_include;
+                    return false;
                 }
             }
 
             // Warn about recursive -includeconf
             includeconf = GetArgs("-includeconf");
             {
-                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf"));
                 includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+                std::string chain_id_final = GetChainName();
+                if (chain_id_final != chain_id) {
+                    // Also warn about recursive includeconf for the chain that was specified in one of the includeconfs
+                    includeconf_net = GetArgs(std::string("-") + chain_id_final + ".includeconf");
+                    includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+                }
             }
             for (const std::string& to_include : includeconf) {
                 fprintf(stderr, "warning: -includeconf cannot be used from included files; ignoring -includeconf=%s\n", to_include.c_str());
diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
index eb6d847c5c19..a450aad17cda 100755
--- a/test/functional/feature_includeconf.py
+++ b/test/functional/feature_includeconf.py
@@ -41,14 +41,9 @@ def run_test(self):
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
         assert subversion.endswith("main; relative)/")
 
-        self.log.info("-includeconf cannot be used as command-line arg. subversion should still end with 'main; relative)/'")
+        self.log.info("-includeconf cannot be used as command-line arg")
         self.stop_node(0)
-
-        self.start_node(0, extra_args=["-includeconf=relative2.conf"])
-
-        subversion = self.nodes[0].getnetworkinfo()["subversion"]
-        assert subversion.endswith("main; relative)/")
-        self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf")
+        self.nodes[0].assert_start_raises_init_error(extra_args=["-includeconf=relative2.conf"], expected_msg="Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=relative2.conf")
 
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
@@ -59,8 +54,18 @@ def run_test(self):
         assert subversion.endswith("main; relative)/")
         self.stop_node(0, expected_stderr="warning: -includeconf cannot be used from included files; ignoring -includeconf=relative2.conf")
 
+        self.log.info("-includeconf cannot contain invalid arg")
+        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            f.write("foo=bar\n")
+        self.nodes[0].assert_start_raises_init_error(expected_msg="Error reading configuration file: Invalid configuration value foo")
+
+        self.log.info("-includeconf cannot be invalid path")
+        os.remove(os.path.join(self.options.tmpdir, "node0", "relative.conf"))
+        self.nodes[0].assert_start_raises_init_error(expected_msg="Error reading configuration file: Failed to include configuration file relative.conf")
+
         self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
         with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            # Restore initial file contents
             f.write("uacomment=relative\n")
 
         with open(os.path.join(self.options.tmpdir, "node0", "dash.conf"), "a", encoding='utf8') as f:

From 62c1477638ccfc40c0630cf11c73f7bf580569b0 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Thu, 7 Jun 2018 08:58:06 +0200
Subject: [PATCH 22/33] Merge #13394: cli: Ignore libevent warnings

0231ef6c6d4f45edffda4ac3bce2048f9c8a8c00 cli: Ignore libevent warnings (Cory Fields)

Pull request description:

  Should fix rpc tests that fail due to an unclean stderr.

  Untested as I'm not seeing these warnings. @promag mind seeing if this fixes your problem?

Tree-SHA512: fba5ae3f239b515e93e19f9c3eca659eb7fb21f1b1fec25b68285695bfd1ecbdcd9b2235543689aaf97bff85cbb762840f65365a67e791314e9a6b8db2c9e246
---
 src/dash-cli.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/dash-cli.cpp b/src/dash-cli.cpp
index c497f6049009..cf97031aa0b4 100644
--- a/src/dash-cli.cpp
+++ b/src/dash-cli.cpp
@@ -59,6 +59,18 @@ static void SetupCliArgs()
     gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
 }
 
+/** libevent event log callback */
+static void libevent_log_cb(int severity, const char *msg)
+{
+#ifndef EVENT_LOG_ERR // EVENT_LOG_ERR was added in 2.0.19; but before then _EVENT_LOG_ERR existed.
+# define EVENT_LOG_ERR _EVENT_LOG_ERR
+#endif
+    // Ignore everything other than errors
+    if (severity >= EVENT_LOG_ERR) {
+        throw std::runtime_error(strprintf("libevent error: %s", msg));
+    }
+}
+
 //////////////////////////////////////////////////////////////////////////////
 //
 // Start
@@ -524,6 +536,7 @@ int main(int argc, char* argv[])
         fprintf(stderr, "Error: Initializing networking failed\n");
         return EXIT_FAILURE;
     }
+    event_set_log_callback(&libevent_log_cb);
 
     try {
         int ret = AppInitRPC(argc, argv);

From e52bf1371bc5b3b49d713aa29c24e25b060baa91 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Thu, 7 Jun 2018 10:20:00 -0400
Subject: [PATCH 23/33] Merge #13404: [tests] speed up of
 tx_validationcache_tests by reusing of CTransaction.

ebebedce20 speed up of tx_validationcache_tests by reusing of CTransaction. (lucash.dev@gmail.com)

Pull request description:

  The code was converting CMutableTransaction to CTransaction multiple times, which implies recalculating the hash multiple times. This commit fixes this by reusing a single CTransaction.

  Run-time results:
  ```
  Before:  6.7s
  After: 5.5s
  --------------
  Saved: 1.2s
  ```
  This PR was split from #13050. Also, see #10026.

Tree-SHA512: 61fb81972a08299085a7d3d0060485b265aefc7a4f82ab548e5f94371c8643cfb97bf0ef34f4e1211bf853d0217fa1c3338e4117f36fda1b37d203f690e86d60
---
 src/test/txvalidationcache_tests.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index 66c8c848a22b..f6a435cee176 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -114,7 +114,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup)
 // should fail.
 // Capture this interaction with the upgraded_nop argument: set it when evaluating
 // any script flag that is implemented as an upgraded NOP code.
-static void ValidateCheckInputsForAllFlags(CMutableTransaction &tx, uint32_t failing_flags, bool add_to_cache)
+static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache)
 {
     PrecomputedTransactionData txdata(tx);
     // If we add many more flags, this loop can get too expensive, but we can

From 26a56f2b03251408f8b3da09840ab53b17608126 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Thu, 7 Jun 2018 19:21:06 +0200
Subject: [PATCH 24/33] Merge #13396: Drop unused arith_uint256 ! operator

2acd1d6716959b99e751cf85a7c47aaa383e937f Drop uint 256 not operator (Ben Woosley)

Pull request description:

  All the other operators are integer or bitwise operations, and this is unused
  apart from tests.

  Note attempting to call `!` on `arith_uint256` results in a build error after this change:
  ```
  test/arith_uint256_tests.cpp:201:17: error: invalid argument type 'const arith_uint256' to unary expression
      BOOST_CHECK(!ZeroL);
  ```

Tree-SHA512: 5791b643f426dac9829e9499d678786f1ad294edb2d840879252a1b642bda55941632114f64048660a5991a984aeba49eeb5dfe64ba0a6275cbe7b1c049d7095
---
 src/arith_uint256.h              | 8 --------
 src/test/arith_uint256_tests.cpp | 7 -------
 2 files changed, 15 deletions(-)

diff --git a/src/arith_uint256.h b/src/arith_uint256.h
index d4b0ec6152de..311ac9a40438 100644
--- a/src/arith_uint256.h
+++ b/src/arith_uint256.h
@@ -64,14 +64,6 @@ class base_uint
 
     explicit base_uint(const std::string& str);
 
-    bool operator!() const
-    {
-        for (int i = 0; i < WIDTH; i++)
-            if (pn[i] != 0)
-                return false;
-        return true;
-    }
-
     const base_uint operator~() const
     {
         base_uint ret;
diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp
index 302181fae08a..621e782fdf5e 100644
--- a/src/test/arith_uint256_tests.cpp
+++ b/src/test/arith_uint256_tests.cpp
@@ -198,13 +198,6 @@ BOOST_AUTO_TEST_CASE( shifts ) { // "<<"  ">>"  "<<="  ">>="
 
 BOOST_AUTO_TEST_CASE( unaryOperators ) // !    ~    -
 {
-    BOOST_CHECK(!ZeroL);
-    BOOST_CHECK(!(!OneL));
-    for (unsigned int i = 0; i < 256; ++i)
-        BOOST_CHECK(!(!(OneL<<i)));
-    BOOST_CHECK(!(!R1L));
-    BOOST_CHECK(!(!MaxL));
-
     BOOST_CHECK(~ZeroL == MaxL);
 
     unsigned char TmpArray[32];

From 1f537f32f7475162cd343cd739e48b00728ad52d Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Sun, 20 Jun 2021 02:08:33 +0300
Subject: [PATCH 25/33] Merge #13374: utils and libraries: checking for bitcoin
 address in translations

85f0135eaefe3d9f696689a7e83606c579da40a8 utils: checking for bitcoin addresses in translations (Max Kaplan)

Pull request description:

  Closes #13363

Tree-SHA512: 8509b4ab004139942c847b93d7b44096a13df8e429dd05459b430a1cf7eaef16c4906ab9dc854f4e635312e1ebb064cfab1bad97fec914c7e926c83ad45cc99b
---
 contrib/devtools/update-translations.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/contrib/devtools/update-translations.py b/contrib/devtools/update-translations.py
index f4e77b343c2c..3779c3f48bb9 100755
--- a/contrib/devtools/update-translations.py
+++ b/contrib/devtools/update-translations.py
@@ -30,6 +30,10 @@
 LOCALE_DIR = 'src/qt/locale'
 # Minimum number of messages for translation to be considered at all
 MIN_NUM_MESSAGES = 10
+# Regexp to check for Bitcoin addresses
+ADDRESS_REGEXP = re.compile('([13]|bc1)[a-zA-Z0-9]{30,}')
+# Regexp to check for Dash addresses
+ADDRESS_REGEXP_DASH = re.compile('[X7][a-zA-Z0-9]{30,}')
 
 def check_at_repository_root():
     if not os.path.exists('.git'):
@@ -125,6 +129,18 @@ def escape_cdata(text):
     text = text.replace('"', '&quot;')
     return text
 
+def contains_bitcoin_addr(text, errors):
+    if text != None and ADDRESS_REGEXP.search(text) != None:
+        errors.append('Translation "%s" contains a bitcoin address. This will be removed.' % (text))
+        return True
+    return False
+
+def contains_dash_addr(text, errors):
+    if text != None and ADDRESS_REGEXP_DASH.search(text) != None:
+        errors.append('Translation "%s" contains a Dash address. This will be removed.' % (text))
+        return True
+    return False
+
 def postprocess_translations(reduce_diff_hacks=False):
     print('Checking and postprocessing...')
 
@@ -163,7 +179,8 @@ def postprocess_translations(reduce_diff_hacks=False):
                     if translation is None:
                         continue
                     errors = []
-                    valid = check_format_specifiers(source, translation, errors, numerus)
+                    valid = check_format_specifiers(source, translation, errors, numerus) and not contains_bitcoin_addr(translation, errors)
+                    valid = valid and not contains_dash_addr(translation, errors)
 
                     for error in errors:
                         print('%s: %s' % (filename, error))

From 05dd5c7079ad3c460963ad850d97faeb3a9fdcb7 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Mon, 11 Jun 2018 14:33:34 +0200
Subject: [PATCH 26/33] Merge #13421: qa: Remove portseed_offset from test
 runner

fa6edfef358518022ee86c0abc77c1c068f106a3 qa: Remove portseed_offset from test runner (MarcoFalke)

Pull request description:

  The portseed_offset is no longer needed in the test runner, since we already kill leftover processes (see #12904). This "fixes" #10869 because we deterministically pick ports starting at 11000

Tree-SHA512: 1ee22e19e02acd3afadc7c6a2b391fd3b5cfcec22c0fe194f3207251e7b1264a04e47d90a3ff8be4aca7d0ec33219a2f5855076acb3565291767939bc2f2fa17
---
 test/functional/test_runner.py | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 796b1bc74aa0..d9571494744f 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -457,10 +457,6 @@ def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, t
         self.test_list = test_list
         self.flags = flags
         self.num_running = 0
-        # In case there is a graveyard of zombie dashds, we can apply a
-        # pseudorandom offset to hopefully jump over them.
-        # (625 is PORT_RANGE/MAX_NODES)
-        self.portseed_offset = int(time.time() * 1000) % 625
         self.jobs = []
 
     def get_next(self):
@@ -468,7 +464,7 @@ def get_next(self):
             # Add tests
             self.num_running += 1
             test = self.test_list.pop(0)
-            portseed = len(self.test_list) + self.portseed_offset
+            portseed = len(self.test_list)
             portseed_arg = ["--portseed={}".format(portseed)]
             log_stdout = tempfile.SpooledTemporaryFile(max_size=2**16)
             log_stderr = tempfile.SpooledTemporaryFile(max_size=2**16)

From e47f2e91726cc166f175a43a50a5ff2f79507595 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Mon, 11 Jun 2018 15:05:49 +0200
Subject: [PATCH 27/33] Merge #13294: Fix compiler warnings emitted when
 compiling under stock OpenBSD 6.3

a426098572884349a3d9081187eaeb999f6e2c5a Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (practicalswift)

Pull request description:

  Fix compiler warnings emitted when compiling under stock OpenBSD 6.3 (OpenBSD clang version 5.0.1, based on LLVM 5.0.1):

  ```
  random.cpp:182:13: warning: unused function 'GetDevURandom' [-Wunused-function]
  static void GetDevURandom(unsigned char *ent32)
              ^

  txmempool.cpp:707:45: warning: comparison of integers of different signs: 'uint64_t' (aka 'unsigned long long') and 'long long' [-Wsign-compare]
          assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ```

Tree-SHA512: da2ae86218054b10659ea694179433700ac91de8022e06007348168ed5adc3d8c4ad3b32a3fc5783a2cdf1ca7425aff586b839200dd3b226ebff72a7df15f120
---
 src/txmempool.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index c683a5135ab3..4b5ef21abb98 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -1108,18 +1108,18 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
         // Check children against mapNextTx
         CTxMemPool::setEntries setChildrenCheck;
         auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0));
-        int64_t childSizes = 0;
+        uint64_t child_sizes = 0;
         for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) {
             txiter childit = mapTx.find(iter->second->GetHash());
             assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
             if (setChildrenCheck.insert(childit).second) {
-                childSizes += childit->GetTxSize();
+                child_sizes += childit->GetTxSize();
             }
         }
         assert(setChildrenCheck == GetMemPoolChildren(it));
         // Also check to make sure size is greater than sum with immediate children.
         // just a sanity check, not definitive that this calc is correct...
-        assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize());
+        assert(it->GetSizeWithDescendants() >= child_sizes + it->GetTxSize());
 
         if (fDependsWait)
             waitingOnDependants.push_back(&(*it));

From 2615ebca43af2c147522a31c50287d81c4b44008 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 13 Jun 2018 15:46:46 +0200
Subject: [PATCH 28/33] Merge #13445: build: Reset default -g -O2 flags when
 enable debug

9882d1f044133832b3c0809676d5f26a861b9f44 Reset default -g -O2 flags when enable debug (Chun Kuan Lee)

Pull request description:

  The default CXXFLAGS is -g -O2, this should not appear when enable debug.
  fixes #13432

Tree-SHA512: 79447f3e1fab9e6cd12f5ca49b3d42187e856e0c159ed01140ea93d6ef1fbb1af3d65b338308566330491052c0177d12abe26796513502ddde31692665a0dbb4
---
 configure.ac | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index c727c6756331..c11362776296 100644
--- a/configure.ac
+++ b/configure.ac
@@ -281,6 +281,10 @@ AC_LANG_PUSH([C++])
 AX_CHECK_COMPILE_FLAG([-Werror],[CXXFLAG_WERROR="-Werror"],[CXXFLAG_WERROR=""])
 
 if test "x$enable_debug" = xyes; then
+  # Clear default -g -O2 flags
+  if test "x$CXXFLAGS_overridden" = xno; then
+	CXXFLAGS=""
+  fi
   # Prefer -Og, fall back to -O0 if that is unavailable.
   AX_CHECK_COMPILE_FLAG(
     [-Og],

From c1a84f6d59081a2c51259dffabc51bd33b6cc582 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 13 Jun 2018 19:24:58 +0200
Subject: [PATCH 29/33] Merge #13457: tests: Drop variadic macro

faf52f953b47aac6a39892b037eaa3f08d46b655 tests: Drop variadic macro (MarcoFalke)

Pull request description:

  The C++11 constructor of `std::vector` that takes an initializer list, is not `explicit`. Thus, the macro is not required and can be dropped.

  Hopefully fixes #13456

Tree-SHA512: 4095ed205f88138a7cd5b14790cc426899966f622a924a9b3f7de646a0d801a48ffb8921da760f1f93d5481298477c8a64dbec291381bb9aa77b075bdd2659f2
---
 src/test/mempool_tests.cpp | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 3c7288e1b92f..95ef91dfa455 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -588,9 +588,6 @@ inline CTransactionRef make_tx(std::vector<CAmount>&& output_values, std::vector
     return MakeTransactionRef(tx);
 }
 
-#define MK_OUTPUTS(amounts...) std::vector<CAmount>{amounts}
-#define MK_INPUTS(txs...) std::vector<CTransactionRef>{txs}
-#define MK_INPUT_IDX(idxes...) std::vector<uint32_t>{idxes}
 
 BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
 {
@@ -604,7 +601,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     //
     // [tx1]
     //
-    CTransactionRef tx1 = make_tx(MK_OUTPUTS(10 * COIN));
+    CTransactionRef tx1 = make_tx(/* output_values */ {10 * COIN});
     pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(tx1));
 
     // Ancestors / descendants should be 1 / 1 (itself / itself)
@@ -616,7 +613,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     //
     // [tx1].0 <- [tx2]
     //
-    CTransactionRef tx2 = make_tx(MK_OUTPUTS(495 * CENT, 5 * COIN), MK_INPUTS(tx1));
+    CTransactionRef tx2 = make_tx(/* output_values */ {495 * CENT, 5 * COIN}, /* inputs */ {tx1});
     pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(tx2));
 
     // Ancestors / descendants should be:
@@ -635,7 +632,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     //
     // [tx1].0 <- [tx2].0 <- [tx3]
     //
-    CTransactionRef tx3 = make_tx(MK_OUTPUTS(290 * CENT, 200 * CENT), MK_INPUTS(tx2));
+    CTransactionRef tx3 = make_tx(/* output_values */ {290 * CENT, 200 * CENT}, /* inputs */ {tx2});
     pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(tx3));
 
     // Ancestors / descendants should be:
@@ -660,7 +657,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     //              |
     //              \---1 <- [tx4]
     //
-    CTransactionRef tx4 = make_tx(MK_OUTPUTS(290 * CENT, 250 * CENT), MK_INPUTS(tx2), MK_INPUT_IDX(1));
+    CTransactionRef tx4 = make_tx(/* output_values */ {290 * CENT, 250 * CENT}, /* inputs */ {tx2}, /* input_indices */ {1});
     pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(tx4));
 
     // Ancestors / descendants should be:
@@ -696,14 +693,14 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
     CAmount v = 5 * COIN;
     for (uint64_t i = 0; i < 5; i++) {
         CTransactionRef& tyi = *ty[i];
-        tyi = make_tx(MK_OUTPUTS(v), i > 0 ? MK_INPUTS(*ty[i-1]) : std::vector<CTransactionRef>());
+        tyi = make_tx(/* output_values */ {v}, /* inputs */ i > 0 ? std::vector<CTransactionRef>{*ty[i - 1]} : std::vector<CTransactionRef>{});
         v -= 50 * CENT;
         pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(tyi));
         pool.GetTransactionAncestry(tyi->GetHash(), ancestors, descendants);
         BOOST_CHECK_EQUAL(ancestors, i+1);
         BOOST_CHECK_EQUAL(descendants, i+1);
     }
-    CTransactionRef ty6 = make_tx(MK_OUTPUTS(5 * COIN), MK_INPUTS(tx3, ty5));
+    CTransactionRef ty6 = make_tx(/* output_values */ {5 * COIN}, /* inputs */ {tx3, ty5});
     pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(ty6));
 
     // Ancestors / descendants should be:

From 5c913efd1d16bccdf4ce977ff3ac92dccb056bf0 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Mon, 18 Jun 2018 15:19:23 +0200
Subject: [PATCH 30/33] Merge #13443: Removed unused == operator from
 CMutableTransaction.

55771b7c6a8d4a204c63e70db73a0071c41a0dc4 Removed unused == operator from CMutableTransaction. (lucash.dev@gmail.com)

Pull request description:

  This removes the unused == operator from `CMutableTransaction`.

  The motivation is that unused code has a cost but offers no benefit (in general), while also adding the risk of introducing silent bugs. On top of that this particular code is quite inefficient, unnecessarily calculating the hash (it could, say, compare serializations). So if anyone ever needs to use a == comparison on `CMutableTransaction`, they'd be better of having to reimplement it (and add tests) than relying on code that's not being maintained.

  Note: after this, trying to use the == operator on CMutableTransactions results in a compilation error:
  ```
  ./primitives/transaction.h:405:15: error: invalid operands to binary expression ('CMutableTransaction' and
        'CMutableTransaction')
  ```

Tree-SHA512: a565af563e09d99347b6fe419f6d48c750b1377295af293a3e0c3c0d815e58aede8d7058987a68d66cfa7ed023e5d3285b12afabd17d0ff9cf11322ba3ce20fe
---
 src/coinjoin/coinjoin-client.cpp |  4 ++--
 src/coinjoin/coinjoin.h          |  2 +-
 src/primitives/transaction.h     | 11 -----------
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/src/coinjoin/coinjoin-client.cpp b/src/coinjoin/coinjoin-client.cpp
index bc3e913952c8..0dfe53edeb07 100644
--- a/src/coinjoin/coinjoin-client.cpp
+++ b/src/coinjoin/coinjoin-client.cpp
@@ -449,7 +449,7 @@ bool CCoinJoinClientSession::SendDenominate(const std::vector<std::pair<CTxDSIn,
         return false;
     }
 
-    if (txMyCollateral == CMutableTransaction()) {
+    if (CTransaction(txMyCollateral).IsNull()) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinClient:SendDenominate -- CoinJoin collateral not set\n");
         return false;
     }
@@ -913,7 +913,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(CConnman& connman, bool fDr
 
         //check our collateral and create new if needed
         std::string strReason;
-        if (txMyCollateral == CMutableTransaction()) {
+        if (CTransaction(txMyCollateral).IsNull()) {
             if (!CreateCollateralTransaction(txMyCollateral, strReason)) {
                 LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::DoAutomaticDenominating -- create collateral error:%s\n", strReason);
                 return false;
diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h
index 6912110b6f6a..e5b926934943 100644
--- a/src/coinjoin/coinjoin.h
+++ b/src/coinjoin/coinjoin.h
@@ -154,7 +154,7 @@ class CCoinJoinAccept
 
     friend bool operator==(const CCoinJoinAccept& a, const CCoinJoinAccept& b)
     {
-        return a.nDenom == b.nDenom && a.txCollateral == b.txCollateral;
+        return a.nDenom == b.nDenom && CTransaction(a.txCollateral) == CTransaction(b.txCollateral);
     }
 };
 
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
index eecb844006ab..091c498a8e85 100644
--- a/src/primitives/transaction.h
+++ b/src/primitives/transaction.h
@@ -301,17 +301,6 @@ struct CMutableTransaction
     uint256 GetHash() const;
 
     std::string ToString() const;
-
-    friend bool operator==(const CMutableTransaction& a, const CMutableTransaction& b)
-    {
-        return a.GetHash() == b.GetHash();
-    }
-
-    friend bool operator!=(const CMutableTransaction& a, const CMutableTransaction& b)
-    {
-        return !(a == b);
-    }
-
 };
 
 typedef std::shared_ptr<const CTransaction> CTransactionRef;

From 68e4e5b5fa9406c0b87090005a7f07c6b9b9c0b9 Mon Sep 17 00:00:00 2001
From: MarcoFalke <falke.marco@gmail.com>
Date: Fri, 29 Jun 2018 10:08:52 +1400
Subject: [PATCH 31/33] Merge #13563: bench: Simplify CoinSelection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

c2e4fc84ec bench: Simplify CoinSelection (João Barbosa)

Pull request description:

  Closes #13549.

  As pointed by @MarcoFalke:
   - `SelectCoinsMinConf` should always succeed as there are enough coins in the wallet.
   - Removed creating the coins in the wallet.

Tree-SHA512: 965c363bcaf0ca7a1dec35b5cf4866abcf190c53eb7012dc4aeb4d29830f13a7465644bfb5a47f6ea3eaa86e4d4a57fe41e7b2593bf5094b76a551c4c71625bb
---
 src/bench/coin_selection.cpp | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp
index 86b2d8700060..d68795bd0371 100644
--- a/src/bench/coin_selection.cpp
+++ b/src/bench/coin_selection.cpp
@@ -37,24 +37,22 @@ static void CoinSelection(benchmark::State& state)
     std::vector<OutputGroup> groups;
     LOCK(wallet.cs_wallet);
 
-    while (state.KeepRunning()) {
-        // Add coins.
-        for (int i = 0; i < 1000; i++)
-            addCoin(1000 * COIN, wallet, groups);
-        addCoin(3 * COIN, wallet, groups);
+    // Add coins.
+    for (int i = 0; i < 1000; ++i) {
+        addCoin(1000 * COIN, wallet, groups);
+    }
+    addCoin(3 * COIN, wallet, groups);
 
+    const CoinEligibilityFilter filter_standard(1, 6, 0);
+    const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0);
+    while (state.KeepRunning()) {
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;
         bool bnb_used;
-        CoinEligibilityFilter filter_standard(1, 6, 0);
-        CoinSelectionParams coin_selection_params(false, 34, 148, CFeeRate(0), 0);
-        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)
-                       || wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
+        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used);
         assert(success);
         assert(nValueRet == 1003 * COIN);
         assert(setCoinsRet.size() == 2);
-
-        groups.clear();
     }
 }
 

From a53599df247d78b475e998858a817fe74f01068f Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 4 Jul 2018 11:29:46 +0200
Subject: [PATCH 32/33] Merge #13491: Improve handling of INVALID in IsMine

bb582a59c Add P2WSH destination helper and use it instead of manual hashing (Pieter Wuille)
eaba1c111 Add additional unit tests for invalid IsMine combinations (Pieter Wuille)
e6b9730c4 Do not expose invalidity from IsMine (Pieter Wuille)

Pull request description:

  This improves the handling of INVALID in IsMine:
  * Extra INVALID conditions were added to `IsMine` (following https://github.com/bitcoin/bitcoin/pull/13142/files#r185349057), but these were untested. Add unit tests for them.
  * In https://github.com/bitcoin/bitcoin/pull/13142#issuecomment-386396975 it was suggested to merge `isInvalid` into the return status. This PR takes a different approach, and removes the `isInvalid` entirely. It was only ever used inside tests, as normal users of IsMine don't care about the reason for non-mine-ness, only whether it is or not. As the unit tests are extensive enough, it seems sufficient to have a black box text (with tests for both compressed and uncompressed keys).

  Some addition code simplification is done as well.

Tree-SHA512: 3267f8846f3fa4e994f57504b155b0e1bbdf13808c4c04dab7c6886c2c0b88716169cee9c5b350513297e0ca2a00812e3401acf30ac9cde5d892f9fb59ad7fef
---
 src/script/ismine.cpp              |  12 +---
 src/script/ismine.h                |   6 --
 src/test/script_standard_tests.cpp | 109 ++++++++++++-----------------
 3 files changed, 46 insertions(+), 81 deletions(-)

diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp
index cb1ba6fc5bdc..40bb3358c6f6 100644
--- a/src/script/ismine.cpp
+++ b/src/script/ismine.cpp
@@ -37,7 +37,7 @@ enum class IsMineResult
     NO = 0,          //! Not ours
     WATCH_ONLY = 1,  //! Included in watch-only balance
     SPENDABLE = 2,   //! Included in all balances
-    INVALID = 3,     //! Not spendable by anyone
+    INVALID = 3,     //! Not spendable by anyone (P2SH inside P2SH)
 };
 
 bool PermitsUncompressed(IsMineSigVersion sigversion)
@@ -137,12 +137,10 @@ IsMineResult IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey,
 
 } // namespace
 
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid)
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
 {
-    isInvalid = false;
     switch (IsMineInner(keystore, scriptPubKey, IsMineSigVersion::TOP)) {
     case IsMineResult::INVALID:
-        isInvalid = true;
     case IsMineResult::NO:
         return ISMINE_NO;
     case IsMineResult::WATCH_ONLY:
@@ -153,12 +151,6 @@ isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool&
     assert(false);
 }
 
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
-{
-    bool isInvalid = false;
-    return IsMine(keystore, scriptPubKey, isInvalid);
-}
-
 isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest)
 {
     CScript script = GetScriptForDestination(dest);
diff --git a/src/script/ismine.h b/src/script/ismine.h
index 44415a6ce826..c012cc292e3a 100644
--- a/src/script/ismine.h
+++ b/src/script/ismine.h
@@ -24,12 +24,6 @@ enum isminetype
 /** used for bitflags of isminetype */
 typedef uint8_t isminefilter;
 
-/* isInvalid becomes true when the script is found invalid by consensus or policy. This will terminate the recursion
- * and return a ISMINE_NO immediately, as an invalid script should never be considered as "mine". This is needed as
- * different SIGVERSION may have different network rules. Currently there is no use of isInvalid but it could  be
- * used in the future. See https://github.com/bitcoin/bitcoin/pull/8499 (segwit policy limits) as an example.
- */
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid);
 isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey);
 isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest);
 
diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
index 554bb8e1ec97..0a503f391f3c 100644
--- a/src/test/script_standard_tests.cpp
+++ b/src/test/script_standard_tests.cpp
@@ -320,143 +320,132 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
 
     CScript scriptPubKey;
     isminetype result;
-    bool isInvalid;
 
     // P2PK compressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+        scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PK uncompressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << ToByteVector(uncompressedPubkey) << OP_CHECKSIG;
+        scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(uncompressedKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PKH compressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+        scriptPubKey = GetScriptForDestination(pubkeys[0].GetID());
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PKH uncompressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(uncompressedPubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+        scriptPubKey = GetScriptForDestination(uncompressedPubkey.GetID());
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(uncompressedKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2SH
     {
         CBasicKeyStore keystore;
 
-        CScript redeemScript;
-        redeemScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+        CScript redeemScript = GetScriptForDestination(pubkeys[0].GetID());
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
 
         // Keystore does not have redeemScript or key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript but no key
         keystore.AddCScript(redeemScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript and key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
+    }
+
+    // (P2PKH inside) P2SH inside P2SH (invalid)
+    {
+        CBasicKeyStore keystore;
+
+        CScript redeemscript_inner = GetScriptForDestination(pubkeys[0].GetID());
+        CScript redeemscript = GetScriptForDestination(CScriptID(redeemscript_inner));
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemscript));
+
+        keystore.AddCScript(redeemscript);
+        keystore.AddCScript(redeemscript_inner);
+        keystore.AddCScript(scriptPubKey);
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
     }
 
     // scriptPubKey multisig
     {
         CBasicKeyStore keystore;
 
-        scriptPubKey.clear();
-        scriptPubKey << OP_2 <<
-            ToByteVector(uncompressedPubkey) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
+        scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
 
         // Keystore does not have any keys
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has 1/2 keys
         keystore.AddKey(uncompressedKey);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has 2/2 keys
         keystore.AddKey(keys[1]);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has 2/2 keys and the script
         keystore.AddCScript(scriptPubKey);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2SH multisig
@@ -465,25 +454,17 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         keystore.AddKey(uncompressedKey);
         keystore.AddKey(keys[1]);
 
-        CScript redeemScript;
-        redeemScript << OP_2 <<
-            ToByteVector(uncompressedPubkey) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+        CScript redeemScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
 
         // Keystore has no redeemScript
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript
         keystore.AddCScript(redeemScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // OP_RETURN
@@ -494,9 +475,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_RETURN << ToByteVector(pubkeys[0]);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // Nonstandard
@@ -507,9 +487,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 }
 

From 0ef8fae2c06749d75bad8377a81466bfb2892226 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Thu, 5 Jul 2018 16:22:31 +0200
Subject: [PATCH 33/33] Merge #13425: Moving final scriptSig construction from
 CombineSignatures to ProduceSignature (PSBT signer logic)

b81560029 Remove CombineSignatures and replace tests (Andrew Chow)
ed94c8b55 Replace CombineSignatures with ProduceSignature (Andrew Chow)
0422beb9b Make SignatureData able to store signatures and scripts (Andrew Chow)
b6edb4f5e Inline Sign1 and SignN (Andrew Chow)

Pull request description:

  Currently CombineSignatures is used to create the final scriptSig or an input. However ProduceSignature is capable of doing this itself. Using both CombineSignatures and ProduceSignature results in code duplication which is unnecessary.

  To move the scriptSig construction to ProduceSignatures, the SignatureData class contains two maps to hold pubkeys mapped to signatures, and script ids mapped to scripts. DataFromTransaction is extended to be able to extract signatures, their public keys, and scripts from existing ScriptSigs.

  The SignaureData are then passed down to SignStep which can use the aforementioned maps to get the signatures, pubkeys, and scripts that it needs, falling back to the actual SigningProvider and SignatureCreator if the data are not available in the SignatureData.

  Additionally, Sign1 and SignN have been removed and their functionality inlined into SignStep since Sign1 is really just a wrapper around CreateSig.

  Since ProduceSignature can produce the final scriptSig or scriptWitness by using SignatureData which has extracted data from the transaction, CombineSignatures is unnecessary as ProduceSignature is able to replicate all of CombineSignatures' functionality.

  This also furthers BIP 174 support and begins moving towards a BIP 174 style backend.

  The tests have also been updated to use the new combining methodology.

Tree-SHA512: 78cd58a4ebe37f79229bd5eee2958a0bb45cd7f36d0e993eee13ff685b3665dd76ef2dfd5f47d34678995bb587f5594100ee5f6c09b1c69ee96d3684d470d01e
---
 src/dash-tx.cpp            |   4 +-
 src/rpc/rawtransaction.cpp |   9 +-
 src/script/sign.cpp        | 337 +++++++++++++++++++------------------
 src/script/sign.h          |  26 ++-
 src/test/script_tests.cpp  |  86 ++++++----
 5 files changed, 243 insertions(+), 219 deletions(-)

diff --git a/src/dash-tx.cpp b/src/dash-tx.cpp
index 43471825940a..5fe7e7d6f552 100644
--- a/src/dash-tx.cpp
+++ b/src/dash-tx.cpp
@@ -600,13 +600,11 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
         const CScript& prevPubKey = coin.out.scriptPubKey;
         const CAmount& amount = coin.out.nValue;
 
-        SignatureData sigdata;
+        SignatureData sigdata = DataFromTransaction(mergedTx, i, coin.out);
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mergedTx.vout.size()))
             ProduceSignature(keystore, MutableTransactionSignatureCreator(&mergedTx, i, amount, nHashType), prevPubKey, sigdata);
 
-        // ... and merge in other signatures:
-        sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i));
         UpdateInput(txin, sigdata);
     }
 
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 043a955dfef3..1365ba236224 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -734,17 +734,15 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request)
         if (coin.IsSpent()) {
             throw JSONRPCError(RPC_VERIFY_ERROR, "Input not found or already spent");
         }
-        const CScript& prevPubKey = coin.out.scriptPubKey;
-        const CAmount& amount = coin.out.nValue;
-
         SignatureData sigdata;
 
         // ... and merge in other signatures:
         for (const CMutableTransaction& txv : txVariants) {
             if (txv.vin.size() > i) {
-                sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i));
+                sigdata.MergeSignatureData(DataFromTransaction(txv, i, coin.out));
             }
         }
+        ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&mergedTx, i, coin.out.nValue, 1), coin.out.scriptPubKey, sigdata);
 
         UpdateInput(txin, sigdata);
     }
@@ -871,12 +869,11 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival
         const CScript& prevPubKey = coin.out.scriptPubKey;
         const CAmount& amount = coin.out.nValue;
 
-        SignatureData sigdata;
+        SignatureData sigdata = DataFromTransaction(mtx, i, coin.out);
         // Only sign SIGHASH_SINGLE if there's a corresponding output:
         if (!fHashSingle || (i < mtx.vout.size())) {
             ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata);
         }
-        sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(mtx, i));
 
         UpdateInput(txin, sigdata);
 
diff --git a/src/script/sign.cpp b/src/script/sign.cpp
index 6ea649e4670a..4231250bd0eb 100644
--- a/src/script/sign.cpp
+++ b/src/script/sign.cpp
@@ -29,27 +29,48 @@ bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provid
     return true;
 }
 
-static bool Sign1(const SigningProvider& provider, const CKeyID& address, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
+static bool GetCScript(const SigningProvider& provider, const SignatureData& sigdata, const CScriptID& scriptid, CScript& script)
 {
-    std::vector<unsigned char> vchSig;
-    if (!creator.CreateSig(provider, vchSig, address, scriptCode, sigversion))
-        return false;
-    ret.push_back(vchSig);
-    return true;
+    if (provider.GetCScript(scriptid, script)) {
+        return true;
+    }
+    // Look for scripts in SignatureData
+    if (CScriptID(sigdata.redeem_script) == scriptid) {
+        script = sigdata.redeem_script;
+        return true;
+    }
+    return false;
 }
 
-static bool SignN(const SigningProvider& provider, const std::vector<valtype>& multisigdata, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector<valtype>& ret, SigVersion sigversion)
+static bool GetPubKey(const SigningProvider& provider, const SignatureData& sigdata, const CKeyID& address, CPubKey& pubkey)
 {
-    int nSigned = 0;
-    int nRequired = multisigdata.front()[0];
-    for (unsigned int i = 1; i < multisigdata.size()-1 && nSigned < nRequired; i++)
-    {
-        const valtype& pubkey = multisigdata[i];
-        CKeyID keyID = CPubKey(pubkey).GetID();
-        if (Sign1(provider, keyID, creator, scriptCode, ret, sigversion))
-            ++nSigned;
+    if (provider.GetPubKey(address, pubkey)) {
+        return true;
+    }
+    // Look for pubkey in all partial sigs
+    const auto it = sigdata.signatures.find(address);
+    if (it != sigdata.signatures.end()) {
+        pubkey = it->second.first;
+        return true;
+    }
+    return false;
+}
+
+static bool CreateSig(const BaseSignatureCreator& creator, SignatureData& sigdata, const SigningProvider& provider, std::vector<unsigned char>& sig_out, const CKeyID& keyid, const CScript& scriptcode, SigVersion sigversion)
+{
+    const auto it = sigdata.signatures.find(keyid);
+    if (it != sigdata.signatures.end()) {
+        sig_out = it->second.second;
+        return true;
+    }
+    if (creator.CreateSig(provider, sig_out, keyid, scriptcode, sigversion)) {
+        CPubKey pubkey;
+        GetPubKey(provider, sigdata, keyid, pubkey);
+        auto i = sigdata.signatures.emplace(keyid, SigPair(pubkey, sig_out));
+        assert(i.second);
+        return true;
     }
-    return nSigned==nRequired;
+    return false;
 }
 
 /**
@@ -59,46 +80,57 @@ static bool SignN(const SigningProvider& provider, const std::vector<valtype>& m
  * Returns false if scriptPubKey could not be completely satisfied.
  */
 static bool SignStep(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& scriptPubKey,
-                     std::vector<valtype>& ret, txnouttype& whichTypeRet, SigVersion sigversion)
+                     std::vector<valtype>& ret, txnouttype& whichTypeRet, SigVersion sigversion, SignatureData& sigdata)
 {
     CScript scriptRet;
     uint160 h160;
     ret.clear();
+    std::vector<unsigned char> sig;
 
     std::vector<valtype> vSolutions;
     if (!Solver(scriptPubKey, whichTypeRet, vSolutions))
         return false;
 
-    CKeyID keyID;
     switch (whichTypeRet)
     {
     case TX_NONSTANDARD:
     case TX_NULL_DATA:
         return false;
     case TX_PUBKEY:
-        keyID = CPubKey(vSolutions[0]).GetID();
-        return Sign1(provider, keyID, creator, scriptPubKey, ret, sigversion);
-    case TX_PUBKEYHASH:
-        keyID = CKeyID(uint160(vSolutions[0]));
-        if (!Sign1(provider, keyID, creator, scriptPubKey, ret, sigversion))
-            return false;
-        else
-        {
-            CPubKey vch;
-            provider.GetPubKey(keyID, vch);
-            ret.push_back(ToByteVector(vch));
-        }
+        if (!CreateSig(creator, sigdata, provider, sig, CPubKey(vSolutions[0]).GetID(), scriptPubKey, sigversion)) return false;
+        ret.push_back(std::move(sig));
         return true;
+    case TX_PUBKEYHASH: {
+        CKeyID keyID = CKeyID(uint160(vSolutions[0]));
+        if (!CreateSig(creator, sigdata, provider, sig, keyID, scriptPubKey, sigversion)) return false;
+        ret.push_back(std::move(sig));
+        CPubKey pubkey;
+        GetPubKey(provider, sigdata, keyID, pubkey);
+        ret.push_back(ToByteVector(pubkey));
+        return true;
+    }
     case TX_SCRIPTHASH:
-        if (provider.GetCScript(uint160(vSolutions[0]), scriptRet)) {
+        if (GetCScript(provider, sigdata, uint160(vSolutions[0]), scriptRet)) {
             ret.push_back(std::vector<unsigned char>(scriptRet.begin(), scriptRet.end()));
             return true;
         }
         return false;
 
-    case TX_MULTISIG:
+    case TX_MULTISIG: {
+        size_t required = vSolutions.front()[0];
         ret.push_back(valtype()); // workaround CHECKMULTISIG bug
-        return (SignN(provider, vSolutions, creator, scriptPubKey, ret, sigversion));
+        for (size_t i = 1; i < vSolutions.size() - 1; ++i) {
+            CPubKey pubkey = CPubKey(vSolutions[i]);
+            if (ret.size() < required + 1 && CreateSig(creator, sigdata, provider, sig, pubkey.GetID(), scriptPubKey, sigversion)) {
+                ret.push_back(std::move(sig));
+            }
+        }
+        bool ok = ret.size() == required + 1;
+        for (size_t i = 0; i + ret.size() < required + 1; ++i) {
+            ret.push_back(valtype());
+        }
+        return ok;
+    }
 
     default:
         return false;
@@ -122,9 +154,11 @@ static CScript PushAll(const std::vector<valtype>& values)
 
 bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata)
 {
+    if (sigdata.complete) return true;
+
     std::vector<valtype> result;
     txnouttype whichType;
-    bool solved = SignStep(provider, creator, fromPubKey, result, whichType, SigVersion::BASE);
+    bool solved = SignStep(provider, creator, fromPubKey, result, whichType, SigVersion::BASE, sigdata);
     bool P2SH = false;
     CScript subscript;
 
@@ -134,7 +168,8 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
         // the final scriptSig is the signatures from that
         // and then the serialized subscript:
         subscript = CScript(result[0].begin(), result[0].end());
-        solved = solved && SignStep(provider, creator, subscript, result, whichType, SigVersion::BASE) && whichType != TX_SCRIPTHASH;
+        sigdata.redeem_script = subscript;
+        solved = solved && SignStep(provider, creator, subscript, result, whichType, SigVersion::BASE, sigdata) && whichType != TX_SCRIPTHASH;
         P2SH = true;
     }
 
@@ -144,97 +179,29 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
     sigdata.scriptSig = PushAll(result);
 
     // Test solution
-    return solved && VerifyScript(sigdata.scriptSig, fromPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker());
-}
-
-SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn)
-{
-    SignatureData data;
-    assert(tx.vin.size() > nIn);
-    data.scriptSig = tx.vin[nIn].scriptSig;
-    return data;
-}
-
-void UpdateInput(CTxIn& input, const SignatureData& data)
-{
-    input.scriptSig = data.scriptSig;
-}
-
-bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
-{
-    assert(nIn < txTo.vin.size());
-
-    MutableTransactionSignatureCreator creator(&txTo, nIn, amount, nHashType);
-
-    SignatureData sigdata;
-    bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
-    UpdateInput(txTo.vin.at(nIn), sigdata);
-    return ret;
+    sigdata.complete = solved && VerifyScript(sigdata.scriptSig, fromPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker());
+    return sigdata.complete;
 }
 
-bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType)
+class SignatureExtractorChecker final : public BaseSignatureChecker
 {
-    assert(nIn < txTo.vin.size());
-    CTxIn& txin = txTo.vin[nIn];
-    assert(txin.prevout.n < txFrom.vout.size());
-    const CTxOut& txout = txFrom.vout[txin.prevout.n];
+private:
+    SignatureData& sigdata;
+    BaseSignatureChecker& checker;
 
-    return SignSignature(provider, txout.scriptPubKey, txTo, nIn, txout.nValue, nHashType);
-}
+public:
+    SignatureExtractorChecker(SignatureData& sigdata, BaseSignatureChecker& checker) : sigdata(sigdata), checker(checker) {}
+    bool CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
+};
 
-static std::vector<valtype> CombineMultisig(const CScript& scriptPubKey, const BaseSignatureChecker& checker,
-                               const std::vector<valtype>& vSolutions,
-                               const std::vector<valtype>& sigs1, const std::vector<valtype>& sigs2, SigVersion sigversion)
+bool SignatureExtractorChecker::CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const
 {
-    // Combine all the signatures we've got:
-    std::set<valtype> allsigs;
-    for (const valtype& v : sigs1)
-    {
-        if (!v.empty())
-            allsigs.insert(v);
-    }
-    for (const valtype& v : sigs2)
-    {
-        if (!v.empty())
-            allsigs.insert(v);
-    }
-
-    // Build a map of pubkey -> signature by matching sigs to pubkeys:
-    assert(vSolutions.size() > 1);
-    unsigned int nSigsRequired = vSolutions.front()[0];
-    unsigned int nPubKeys = vSolutions.size()-2;
-    std::map<valtype, valtype> sigs;
-    for (const valtype& sig : allsigs)
-    {
-        for (unsigned int i = 0; i < nPubKeys; i++)
-        {
-            const valtype& pubkey = vSolutions[i+1];
-            if (sigs.count(pubkey))
-                continue; // Already got a sig for this pubkey
-
-            if (checker.CheckSig(sig, pubkey, scriptPubKey, sigversion))
-            {
-                sigs[pubkey] = sig;
-                break;
-            }
-        }
-    }
-    // Now build a merged CScript:
-    unsigned int nSigsHave = 0;
-    std::vector<valtype> result; result.push_back(valtype()); // pop-one-too-many workaround
-    for (unsigned int i = 0; i < nPubKeys && nSigsHave < nSigsRequired; i++)
-    {
-        if (sigs.count(vSolutions[i+1]))
-        {
-            result.push_back(sigs[vSolutions[i+1]]);
-            ++nSigsHave;
-        }
+    if (checker.CheckSig(scriptSig, vchPubKey, scriptCode, sigversion)) {
+        CPubKey pubkey(vchPubKey);
+        sigdata.signatures.emplace(pubkey.GetID(), SigPair(pubkey, scriptSig));
+        return true;
     }
-    // Fill any missing with OP_0:
-    for (unsigned int i = nSigsHave; i < nSigsRequired; i++)
-        result.push_back(valtype());
-
-    return result;
+    return false;
 }
 
 namespace
@@ -257,59 +224,98 @@ struct Stacks
 };
 }
 
-static Stacks CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker,
-                                 const txnouttype txType, const std::vector<valtype>& vSolutions,
-                                 Stacks sigs1, Stacks sigs2, SigVersion sigversion)
+// Extracts signatures and scripts from incomplete scriptSigs. Please do not extend this, use PSBT instead
+SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout)
 {
-    switch (txType)
-    {
-    case TX_NONSTANDARD:
-    case TX_NULL_DATA:
-        // Don't know anything about this, assume bigger one is correct:
-        if (sigs1.script.size() >= sigs2.script.size())
-            return sigs1;
-        return sigs2;
-    case TX_PUBKEY:
-    case TX_PUBKEYHASH:
-        // Signatures are bigger than placeholders or empty scripts:
-        if (sigs1.script.empty() || sigs1.script[0].empty())
-            return sigs2;
-        return sigs1;
-    case TX_SCRIPTHASH:
-        if (sigs1.script.empty() || sigs1.script.back().empty())
-            return sigs2;
-        else if (sigs2.script.empty() || sigs2.script.back().empty())
-            return sigs1;
-        else
-        {
-            // Recur to combine:
-            valtype spk = sigs1.script.back();
-            CScript pubKey2(spk.begin(), spk.end());
-
-            txnouttype txType2;
-            std::vector<std::vector<unsigned char> > vSolutions2;
-            Solver(pubKey2, txType2, vSolutions2);
-            sigs1.script.pop_back();
-            sigs2.script.pop_back();
-            Stacks result = CombineSignatures(pubKey2, checker, txType2, vSolutions2, sigs1, sigs2, sigversion);
-            result.script.push_back(spk);
-            return result;
+    SignatureData data;
+    assert(tx.vin.size() > nIn);
+    data.scriptSig = tx.vin[nIn].scriptSig;
+    Stacks stack(data);
+
+    // Get signatures
+    MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue);
+    SignatureExtractorChecker extractor_checker(data, tx_checker);
+    if (VerifyScript(data.scriptSig, txout.scriptPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, extractor_checker)) {
+        data.complete = true;
+        return data;
+    }
+
+    // Get scripts
+    txnouttype script_type;
+    std::vector<std::vector<unsigned char>> solutions;
+    Solver(txout.scriptPubKey, script_type, solutions);
+    SigVersion sigversion = SigVersion::BASE;
+    CScript next_script = txout.scriptPubKey;
+
+    if (script_type == TX_SCRIPTHASH && !stack.script.empty() && !stack.script.back().empty()) {
+        // Get the redeemScript
+        CScript redeem_script(stack.script.back().begin(), stack.script.back().end());
+        data.redeem_script = redeem_script;
+        next_script = std::move(redeem_script);
+
+        // Get redeemScript type
+        Solver(next_script, script_type, solutions);
+        stack.script.pop_back();
+    }
+
+    if (script_type == TX_MULTISIG && !stack.script.empty()) {
+        // Build a map of pubkey -> signature by matching sigs to pubkeys:
+        assert(solutions.size() > 1);
+        unsigned int num_pubkeys = solutions.size()-2;
+        unsigned int last_success_key = 0;
+        for (const valtype& sig : stack.script) {
+            for (unsigned int i = last_success_key; i < num_pubkeys; ++i) {
+                const valtype& pubkey = solutions[i+1];
+                // We either have a signature for this pubkey, or we have found a signature and it is valid
+                if (data.signatures.count(CPubKey(pubkey).GetID()) || extractor_checker.CheckSig(sig, pubkey, next_script, sigversion)) {
+                    last_success_key = i + 1;
+                    break;
+                }
+            }
         }
-    case TX_MULTISIG:
-        return Stacks(CombineMultisig(scriptPubKey, checker, vSolutions, sigs1.script, sigs2.script, sigversion));
-    default:
-        return Stacks();
     }
+
+    return data;
+}
+
+void UpdateInput(CTxIn& input, const SignatureData& data)
+{
+    input.scriptSig = data.scriptSig;
+}
+
+void SignatureData::MergeSignatureData(SignatureData sigdata)
+{
+    if (complete) return;
+    if (sigdata.complete) {
+        *this = std::move(sigdata);
+        return;
+    }
+    if (redeem_script.empty() && !sigdata.redeem_script.empty()) {
+        redeem_script = sigdata.redeem_script;
+    }
+    signatures.insert(std::make_move_iterator(sigdata.signatures.begin()), std::make_move_iterator(sigdata.signatures.end()));
+}
+
+bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
+{
+    assert(nIn < txTo.vin.size());
+
+    MutableTransactionSignatureCreator creator(&txTo, nIn, amount, nHashType);
+
+    SignatureData sigdata;
+    bool ret = ProduceSignature(provider, creator, fromPubKey, sigdata);
+    UpdateInput(txTo.vin.at(nIn), sigdata);
+    return ret;
 }
 
-SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker,
-                          const SignatureData& scriptSig1, const SignatureData& scriptSig2)
+bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType)
 {
-    txnouttype txType;
-    std::vector<std::vector<unsigned char> > vSolutions;
-    Solver(scriptPubKey, txType, vSolutions);
+    assert(nIn < txTo.vin.size());
+    CTxIn& txin = txTo.vin[nIn];
+    assert(txin.prevout.n < txFrom.vout.size());
+    const CTxOut& txout = txFrom.vout[txin.prevout.n];
 
-    return CombineSignatures(scriptPubKey, checker, txType, vSolutions, Stacks(scriptSig1), Stacks(scriptSig2), SigVersion::BASE).Output();
+    return SignSignature(provider, txout.scriptPubKey, txTo, nIn, txout.nValue, nHashType);
 }
 
 namespace {
@@ -345,6 +351,7 @@ class DummySignatureCreator final : public BaseSignatureCreator {
 }
 
 const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator();
+const SigningProvider& DUMMY_SIGNING_PROVIDER = SigningProvider();
 
 bool IsSolvable(const SigningProvider& provider, const CScript& script)
 {
diff --git a/src/script/sign.h b/src/script/sign.h
index 97cb9a7d4c77..fa6c560b7fd0 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -21,11 +21,13 @@ class SigningProvider
 {
 public:
     virtual ~SigningProvider() {}
-    virtual bool GetCScript(const CScriptID &scriptid, CScript& script) const =0;
-    virtual bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const =0;
-    virtual bool GetKey(const CKeyID &address, CKey& key) const =0;
+    virtual bool GetCScript(const CScriptID &scriptid, CScript& script) const { return false; }
+    virtual bool GetPubKey(const CKeyID &address, CPubKey& pubkey) const { return false; }
+    virtual bool GetKey(const CKeyID &address, CKey& key) const { return false; }
 };
 
+extern const SigningProvider& DUMMY_SIGNING_PROVIDER;
+
 /** Interface for signature creators. */
 class BaseSignatureCreator {
 public:
@@ -53,11 +55,20 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator {
 /** A signature creator that just produces 72-byte empty signatures. */
 extern const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR;
 
+typedef std::pair<CPubKey, std::vector<unsigned char>> SigPair;
+
+// This struct contains information from a transaction input and also contains signatures for that input.
+// The information contained here can be used to create a signature and is also filled by ProduceSignature
+// in order to construct final scriptSigs and scriptWitnesses.
 struct SignatureData {
-    CScript scriptSig;
+    bool complete = false; ///< Stores whether the scriptSig is complete
+    CScript scriptSig; ///< The scriptSig of an input. Contains complete signatures or the traditional partial signatures format
+    CScript redeem_script; ///< The redeemScript (if any) for the input
+    std::map<CKeyID, SigPair> signatures; ///< BIP 174 style partial signatures for the input. May contain all signatures necessary for producing a final scriptSig.
 
     SignatureData() {}
     explicit SignatureData(const CScript& script) : scriptSig(script) {}
+    void MergeSignatureData(SignatureData sigdata);
 };
 
 /** Produce a script signature using a generic signature creator. */
@@ -67,11 +78,8 @@ bool ProduceSignature(const SigningProvider& provider, const BaseSignatureCreato
 bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType);
 bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType);
 
-/** Combine two script signatures using a generic signature checker, intelligently, possibly with OP_0 placeholders. */
-SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const SignatureData& scriptSig1, const SignatureData& scriptSig2);
-
-/** Extract signature data from a transaction, and insert it. */
-SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn);
+/** Extract signature data from a transaction input, and insert it. */
+SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout);
 void UpdateInput(CTxIn& input, const SignatureData& data);
 
 /* Check whether we know how to sign for an output like this, assuming we
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 0c14229cd864..8e03aef8508b 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -1106,10 +1106,19 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23)
     BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err));
 }
 
+/* Wrapper around ProduceSignature to combine two scriptsigs */
+SignatureData CombineSignatures(const CTxOut& txout, const CMutableTransaction& tx, const SignatureData& scriptSig1, const SignatureData& scriptSig2)
+{
+    SignatureData data;
+    data.MergeSignatureData(scriptSig1);
+    data.MergeSignatureData(scriptSig2);
+    ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(&tx, 0, txout.nValue), txout.scriptPubKey, data);
+    return data;
+}
+
 BOOST_AUTO_TEST_CASE(script_combineSigs)
 {
-    // Test the CombineSignatures function
-    CAmount amount;
+    // Test the ProduceSignature's ability to combine signatures function
     CBasicKeyStore keystore;
     std::vector<CKey> keys;
     std::vector<CPubKey> pubkeys;
@@ -1125,52 +1134,51 @@ BOOST_AUTO_TEST_CASE(script_combineSigs)
     CMutableTransaction txFrom = BuildCreditingTransaction(GetScriptForDestination(keys[0].GetPubKey().GetID()));
     CMutableTransaction txTo = BuildSpendingTransaction(CScript(), txFrom);
     CScript& scriptPubKey = txFrom.vout[0].scriptPubKey;
-    CScript& scriptSig = txTo.vin[0].scriptSig;
+    SignatureData scriptSig;
 
     SignatureData empty;
-    SignatureData combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, empty);
+    SignatureData combined = CombineSignatures(txFrom.vout[0], txTo, empty, empty);
     BOOST_CHECK(combined.scriptSig.empty());
 
     // Single signature case:
     SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); // changes scriptSig
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty);
-    BOOST_CHECK(combined.scriptSig == scriptSig);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSig);
-    CScript scriptSigCopy = scriptSig;
+    scriptSig = DataFromTransaction(txTo, 0, txFrom.vout[0]);
+    combined = CombineSignatures(txFrom.vout[0], txTo, scriptSig, empty);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
+    combined = CombineSignatures(txFrom.vout[0], txTo, empty, scriptSig);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
+    SignatureData scriptSigCopy = scriptSig;
     // Signing again will give a different, valid signature:
     SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSigCopy || combined.scriptSig == scriptSig);
+    scriptSig = DataFromTransaction(txTo, 0, txFrom.vout[0]);
+    combined = CombineSignatures(txFrom.vout[0], txTo, scriptSigCopy, scriptSig);
+    BOOST_CHECK(combined.scriptSig == scriptSigCopy.scriptSig || combined.scriptSig == scriptSig.scriptSig);
 
     // P2SH, single-signature case:
     CScript pkSingle; pkSingle << ToByteVector(keys[0].GetPubKey()) << OP_CHECKSIG;
     keystore.AddCScript(pkSingle);
     scriptPubKey = GetScriptForDestination(CScriptID(pkSingle));
     SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty);
-    BOOST_CHECK(combined.scriptSig == scriptSig);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSig);
+    scriptSig = DataFromTransaction(txTo, 0, txFrom.vout[0]);
+    combined = CombineSignatures(txFrom.vout[0], txTo, scriptSig, empty);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
+    combined = CombineSignatures(txFrom.vout[0], txTo, empty, scriptSig);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
     scriptSigCopy = scriptSig;
     SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSigCopy || combined.scriptSig == scriptSig);
-    // dummy scriptSigCopy with placeholder, should always choose non-placeholder:
-    scriptSigCopy = CScript() << OP_0 << std::vector<unsigned char>(pkSingle.begin(), pkSingle.end());
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSig);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), SignatureData(scriptSigCopy));
-    BOOST_CHECK(combined.scriptSig == scriptSig);
+    scriptSig = DataFromTransaction(txTo, 0, txFrom.vout[0]);
+    combined = CombineSignatures(txFrom.vout[0], txTo, scriptSigCopy, scriptSig);
+    BOOST_CHECK(combined.scriptSig == scriptSigCopy.scriptSig || combined.scriptSig == scriptSig.scriptSig);
 
     // Hardest case:  Multisig 2-of-3
     scriptPubKey = GetScriptForMultisig(2, pubkeys);
     keystore.AddCScript(scriptPubKey);
     SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty);
-    BOOST_CHECK(combined.scriptSig == scriptSig);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig));
-    BOOST_CHECK(combined.scriptSig == scriptSig);
+    scriptSig = DataFromTransaction(txTo, 0, txFrom.vout[0]);
+    combined = CombineSignatures(txFrom.vout[0], txTo, scriptSig, empty);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
+    combined = CombineSignatures(txFrom.vout[0], txTo, empty, scriptSig);
+    BOOST_CHECK(combined.scriptSig == scriptSig.scriptSig);
 
     // A couple of partially-signed versions:
     std::vector<unsigned char> sig1;
@@ -1197,22 +1205,28 @@ BOOST_AUTO_TEST_CASE(script_combineSigs)
     CScript complete12 = CScript() << OP_0 << sig1 << sig2;
     CScript complete13 = CScript() << OP_0 << sig1 << sig3;
     CScript complete23 = CScript() << OP_0 << sig2 << sig3;
-
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1a), SignatureData(partial1b));
+    SignatureData partial1_sigs;
+    partial1_sigs.signatures.emplace(keys[0].GetPubKey().GetID(), SigPair(keys[0].GetPubKey(), sig1));
+    SignatureData partial2_sigs;
+    partial2_sigs.signatures.emplace(keys[1].GetPubKey().GetID(), SigPair(keys[1].GetPubKey(), sig2));
+    SignatureData partial3_sigs;
+    partial3_sigs.signatures.emplace(keys[2].GetPubKey().GetID(), SigPair(keys[2].GetPubKey(), sig3));
+
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial1_sigs, partial1_sigs);
     BOOST_CHECK(combined.scriptSig == partial1a);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1a), SignatureData(partial2a));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial1_sigs, partial2_sigs);
     BOOST_CHECK(combined.scriptSig == complete12);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial2a), SignatureData(partial1a));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial2_sigs, partial1_sigs);
     BOOST_CHECK(combined.scriptSig == complete12);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1b), SignatureData(partial2b));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial1_sigs, partial2_sigs);
     BOOST_CHECK(combined.scriptSig == complete12);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial1b));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial3_sigs, partial1_sigs);
     BOOST_CHECK(combined.scriptSig == complete13);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial2a), SignatureData(partial3a));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial2_sigs, partial3_sigs);
     BOOST_CHECK(combined.scriptSig == complete23);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial2b));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial3_sigs, partial2_sigs);
     BOOST_CHECK(combined.scriptSig == complete23);
-    combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial3a));
+    combined = CombineSignatures(txFrom.vout[0], txTo, partial3_sigs, partial3_sigs);
     BOOST_CHECK(combined.scriptSig == partial3c);
 }