backport: Merge bitcoin/bitcoin#28264,30291,30397,2279,29459

src/init.cpp

@@ -2653,6 +2653,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     CService onion_service_target;
     if (!connOptions.onion_binds.empty()) {
         onion_service_target = connOptions.onion_binds.front();
+    } else if (!connOptions.vBinds.empty()) {
+        onion_service_target = connOptions.vBinds.front();
     } else {
         onion_service_target = DefaultOnionServiceTarget();
         connOptions.onion_binds.push_back(onion_service_target);

src/logging.h

@@ -257,7 +257,7 @@ std::string SafeStringFormat(const std::string& fmt, const Args&... args)
     }
 }
 
-// Be conservative when using LogPrintf/error or other things which
+// Be conservative when using functions that
 // unconditionally log to debug.log! It should not be the case that an inbound
 // peer can fill up a user's disk with debug.log entries.
 

src/net.cpp

@@ -3942,24 +3942,36 @@ bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlag
 
 bool CConnman::InitBinds(const Options& options)
 {
-    bool fBound = false;
     for (const auto& addrBind : options.vBinds) {
-        fBound |= Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None);
+        if (!Bind(addrBind, BF_REPORT_ERROR, NetPermissionFlags::None)) {
+            return false;
+        }
     }
     for (const auto& addrBind : options.vWhiteBinds) {
-        fBound |= Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags);
+        if (!Bind(addrBind.m_service, BF_REPORT_ERROR, addrBind.m_flags)) {
+            return false;
+        }
     }
     for (const auto& addr_bind : options.onion_binds) {
-        fBound |= Bind(addr_bind, BF_DONT_ADVERTISE, NetPermissionFlags::None);
+        if (!Bind(addr_bind, BF_REPORT_ERROR | BF_DONT_ADVERTISE, NetPermissionFlags::None)) {
+            return false;
+        }
     }
     if (options.bind_on_any) {
+        // Don't consider errors to bind on IPv6 "::" fatal because the host OS
+        // may not have IPv6 support and the user did not explicitly ask us to
+        // bind on that.
+        const CService ipv6_any{in6_addr(IN6ADDR_ANY_INIT), GetListenPort()}; // ::
+        Bind(ipv6_any, BF_NONE, NetPermissionFlags::None);
+
         struct in_addr inaddr_any;
         inaddr_any.s_addr = htonl(INADDR_ANY);
-        struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT;
-        fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::None);
-        fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::None);
+        const CService ipv4_any{inaddr_any, GetListenPort()}; // 0.0.0.0
+        if (!Bind(ipv4_any, BF_REPORT_ERROR, NetPermissionFlags::None)) {
+            return false;
+        }
     }
-    return fBound;
+    return true;
 }
 
 bool CConnman::Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync,

src/test/util/net.cpp

@@ -118,20 +118,20 @@ std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candida
     std::vector<NodeEvictionCandidate> candidates;
     for (int id = 0; id < n_candidates; ++id) {
         candidates.push_back({
-            /*id=*/id,
-            /*m_connected=*/std::chrono::seconds{random_context.randrange(100)},
-            /*m_min_ping_time=*/std::chrono::microseconds{random_context.randrange(100)},
-            /*m_last_block_time=*/std::chrono::seconds{random_context.randrange(100)},
-            /*m_last_tx_time=*/std::chrono::seconds{random_context.randrange(100)},
-            /*fRelevantServices=*/random_context.randbool(),
-            /*m_relay_txs=*/random_context.randbool(),
-            /*fBloomFilter=*/random_context.randbool(),
-            /*nKeyedNetGroup=*/random_context.randrange(100),
-            /*prefer_evict=*/random_context.randbool(),
-            /*m_is_local=*/random_context.randbool(),
-            /*m_network=*/ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],
-            /*m_noban=*/false,
-            /*m_conn_type=*/ConnectionType::INBOUND,
+            .id=id,
+            .m_connected=std::chrono::seconds{random_context.randrange(100)},
+            .m_min_ping_time=std::chrono::microseconds{random_context.randrange(100)},
+            .m_last_block_time=std::chrono::seconds{random_context.randrange(100)},
+            .m_last_tx_time=std::chrono::seconds{random_context.randrange(100)},
+            .fRelevantServices=random_context.randbool(),
+            .m_relay_txs=random_context.randbool(),
+            .fBloomFilter=random_context.randbool(),
+            .nKeyedNetGroup=random_context.randrange(100),
+            .prefer_evict=random_context.randbool(),
+            .m_is_local=random_context.randbool(),
+            .m_network=ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],
+            .m_noban=false,
+            .m_conn_type=ConnectionType::INBOUND,
         });
     }
     return candidates;

test/functional/feature_bind_extra.py

@@ -30,7 +30,7 @@ def set_test_params(self):
         # Avoid any -bind= on the command line. Force the framework to avoid
         # adding -bind=127.0.0.1.
         self.bind_to_localhost_only = False
-        self.num_nodes = 2
+        self.num_nodes = 3
 
     def setup_network(self):
         # Due to OS-specific network stats queries, we only run on Linux.
@@ -64,14 +64,21 @@ def setup_network(self):
         )
         port += 2
 
+        # Node2, no -bind=...=onion, thus no extra port for Tor target.
+        self.expected.append(
+            [
+                [f"-bind=127.0.0.1:{port}"],
+                [(loopback_ipv4, port)]
+            ],
+        )
+        port += 1
+
         self.extra_args = list(map(lambda e: e[0], self.expected))
-        self.add_nodes(self.num_nodes, self.extra_args)
-        # Don't start the nodes, as some of them would collide trying to bind on the same port.
+        self.setup_nodes()
 
     def run_test(self):
-        for i in range(len(self.expected)):
-            self.log.info(f"Starting node {i} with {self.expected[i][0]}")
-            self.start_node(i)
+        for i, (args, expected_services) in enumerate(self.expected):
+            self.log.info(f"Checking listening ports of node {i} with {args}")
             pid = self.nodes[i].process.pid
             binds = set(get_bind_addrs(pid))
             # Remove IPv6 addresses because on some CI environments "::1" is not configured
@@ -82,9 +89,7 @@ def run_test(self):
             binds = set(filter(lambda e: len(e[0]) != ipv6_addr_len_bytes, binds))
             # Remove RPC ports. They are not relevant for this test.
             binds = set(filter(lambda e: e[1] != rpc_port(i), binds))
-            assert_equal(binds, set(self.expected[i][1]))
-            self.stop_node(i)
-            self.log.info(f"Stopped node {i}")
+            assert_equal(binds, set(expected_services))
 
 if __name__ == '__main__':
     BindExtraTest().main()

test/functional/mempool_accept.py

@@ -77,6 +77,13 @@ def run_test(self):
         txid_in_block = self.wallet.sendrawtransaction(from_node=node, tx_hex=raw_tx_in_block)
         self.generate(node, 1)
         self.mempool_size = 0
+        # Check negative feerate
+        assert_raises_rpc_error(-3, "Amount out of range", lambda: self.check_mempool_result(
+            result_expected=None,
+            rawtxs=[raw_tx_in_block],
+            maxfeerate=-0.01,
+        ))
+        # ... 0.99 passes
         self.check_mempool_result(
             result_expected=[{'txid': txid_in_block, 'allowed': False, 'reject-reason': 'txn-already-known'}],
             rawtxs=[raw_tx_in_block],

test/functional/rpc_psbt.py

@@ -31,7 +31,6 @@
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    find_output,
     random_bytes,
 )
 from test_framework.wallet_util import bytes_to_wif
@@ -253,16 +252,17 @@ def run_test(self):
         self.nodes[0].converttopsbt(hexstring=signedtx['hex'], permitsigdata=True)
 
         # Create outputs to nodes 1 and 2
+        # (note that we intentionally create two different txs here, as we want
+        #  to check that each node is missing prevout data for one of the two
+        #  utxos, see "should only have data for one input" test below)
         node1_addr = self.nodes[1].getnewaddress()
         node2_addr = self.nodes[2].getnewaddress()
-        txid1 = self.nodes[0].sendtoaddress(node1_addr, 13)
-        txid2 = self.nodes[0].sendtoaddress(node2_addr, 13)
-        blockhash = self.generate(self.nodes[0], 6)[0]
-        vout1 = find_output(self.nodes[1], txid1, 13, blockhash=blockhash)
-        vout2 = find_output(self.nodes[2], txid2, 13, blockhash=blockhash)
+        utxo1 = self.create_outpoints(self.nodes[0], outputs=[{node1_addr: 13}])[0]
+        utxo2 = self.create_outpoints(self.nodes[0], outputs=[{node2_addr: 13}])[0]
+        self.generate(self.nodes[0], 6)[0]
 
         # Create a psbt spending outputs from nodes 1 and 2
-        psbt_orig = self.nodes[0].createpsbt([{"txid":txid1,  "vout":vout1}, {"txid":txid2, "vout":vout2}], {self.nodes[0].getnewaddress():25.999})
+        psbt_orig = self.nodes[0].createpsbt([utxo1, utxo2], {self.nodes[0].getnewaddress():25.999})
 
         # Update psbts, should only have data for one input and not the other
         psbt1 = self.nodes[1].walletprocesspsbt(psbt_orig, False, "ALL")['psbt']
@@ -377,23 +377,18 @@ def run_test(self):
         assert_raises_rpc_error(-22, "TX decode failed invalid base64", self.nodes[0].decodepsbt, ";definitely not base64;")
 
         # Send to all types of addresses
-        addr1 = self.nodes[1].getnewaddress()
-        txid1 = self.nodes[0].sendtoaddress(addr1, 11)
-        vout1 = find_output(self.nodes[0], txid1, 11)
-        addr2 = self.nodes[1].getnewaddress()
-        txid2 = self.nodes[0].sendtoaddress(addr2, 11)
-        vout2 = find_output(self.nodes[0], txid2, 11)
-        addr3 = self.nodes[1].getnewaddress()
-        txid3 = self.nodes[0].sendtoaddress(addr3, 11)
-        vout3 = find_output(self.nodes[0], txid3, 11)
+        addr1 = self.nodes[1].getnewaddress("", "bech32")
+        addr2 = self.nodes[1].getnewaddress("", "legacy")
+        addr3 = self.nodes[1].getnewaddress("", "p2sh-segwit")
+        utxo1, utxo2, utxo3 = self.create_outpoints(self.nodes[1], outputs=[{addr1: 11}, {addr2: 11}, {addr3: 11}])
         self.sync_all()
 
         def test_psbt_input_keys(psbt_input, keys):
             """Check that the psbt input has only the expected keys."""
             assert_equal(set(keys), set(psbt_input.keys()))
 
         # Create a PSBT. None of the inputs are filled initially
-        psbt = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1},{"txid":txid2, "vout":vout2},{"txid":txid3, "vout":vout3}], {self.nodes[0].getnewaddress():32.999})
+        psbt = self.nodes[1].createpsbt([utxo1, utxo2, utxo3], {self.nodes[0].getnewaddress():32.999})
         decoded = self.nodes[1].decodepsbt(psbt)
         test_psbt_input_keys(decoded['inputs'][0], [])
         test_psbt_input_keys(decoded['inputs'][1], [])
@@ -416,15 +411,14 @@ def test_psbt_input_keys(psbt_input, keys):
         test_psbt_input_keys(decoded['inputs'][2], ['non_witness_utxo', 'bip32_derivs'])
 
         # Two PSBTs with a common input should not be joinable
-        psbt1 = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1}], {self.nodes[0].getnewaddress():Decimal('10.999')})
+        psbt1 = self.nodes[1].createpsbt([utxo1], {self.nodes[0].getnewaddress():Decimal('10.999')})
         assert_raises_rpc_error(-8, "exists in multiple PSBTs", self.nodes[1].joinpsbts, [psbt1, updated])
 
         # Join two distinct PSBTs
-        addr4 = self.nodes[1].getnewaddress()
-        txid4 = self.nodes[0].sendtoaddress(addr4, 5)
-        vout4 = find_output(self.nodes[0], txid4, 5)
+        addr4 = self.nodes[1].getnewaddress("", "p2sh-segwit")
+        utxo4 = self.create_outpoints(self.nodes[0], outputs=[{addr4: 5}])[0]
         self.generate(self.nodes[0], 6)
-        psbt2 = self.nodes[1].createpsbt([{"txid":txid4, "vout":vout4}], {self.nodes[0].getnewaddress():Decimal('4.999')})
+        psbt2 = self.nodes[1].createpsbt([utxo4], {self.nodes[0].getnewaddress():Decimal('4.999')})
         psbt2 = self.nodes[1].walletprocesspsbt(psbt2)['psbt']
         psbt2_decoded = self.nodes[0].decodepsbt(psbt2)
         assert "final_scriptwitness" not in psbt2_decoded['inputs'][0] and "final_scriptSig" in psbt2_decoded['inputs'][0]
@@ -433,11 +427,11 @@ def test_psbt_input_keys(psbt_input, keys):
         assert len(joined_decoded['inputs']) == 4 and len(joined_decoded['outputs']) == 2 and "final_scriptwitness" not in joined_decoded['inputs'][3] and "final_scriptSig" not in joined_decoded['inputs'][3]
 
         # Newly created PSBT needs UTXOs and updating
-        addr = self.nodes[1].getnewaddress()
-        txid = self.nodes[0].sendtoaddress(addr, 7)
-        self.generate(self.nodes[0], 6)
-        vout = find_output(self.nodes[0], txid, 7)
-        psbt = self.nodes[1].createpsbt([{"txid":txid, "vout":vout}], {self.nodes[0].getnewaddress():Decimal('6.999')})
+        addr = self.nodes[1].getnewaddress("", "p2sh-segwit")
+        utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 7}])[0]
+        addrinfo = self.nodes[1].getaddressinfo(addr)
+        self.generate(self.nodes[0], 6)[0]
+        psbt = self.nodes[1].createpsbt([utxo], {self.nodes[0].getnewaddress("", "p2sh-segwit"):Decimal('6.999')})
         analyzed = self.nodes[0].analyzepsbt(psbt)
         assert not analyzed['inputs'][0]['has_utxo'] and not analyzed['inputs'][0]['is_final'] and analyzed['inputs'][0]['next'] == 'updater' and analyzed['next'] == 'updater'
 
@@ -637,6 +631,5 @@ def test_psbt_input_keys(psbt_input, keys):
         assert_raises_rpc_error(-8, "PSBTs not compatible (different transactions)", self.nodes[0].combinepsbt, [psbt1, psbt2])
         assert_equal(self.nodes[0].combinepsbt([psbt1, psbt1]), psbt1)
 
-
 if __name__ == '__main__':
     PSBTTest().main()

test/functional/test-shell.md

@@ -169,7 +169,7 @@ can be called after the TestShell is shut down.
 
 | Test parameter key | Default Value | Description |
 |---|---|---|
-| `bind_to_localhost_only` | `True` | Binds bitcoind RPC services to `127.0.0.1` if set to `True`.|
+| `bind_to_localhost_only` | `True` | Binds bitcoind P2P services to `127.0.0.1` if set to `True`.|
 | `cachedir` | `"/path/to/bitcoin/test/cache"` | Sets the bitcoind datadir directory. |
 | `chain`  | `"regtest"` | Sets the chain-type for the underlying test bitcoind processes. |
 | `configfile` | `"/path/to/bitcoin/test/config.ini"` | Sets the location of the test framework config file. |

test/functional/test_framework/test_framework.py

@@ -48,6 +48,7 @@
     check_json_precision,
     copy_datadir,
     force_finish_mnsync,
+    find_vout_for_address,
     get_datadir_path,
     initialize_datadir,
     p2p_port,
@@ -823,6 +824,22 @@ def generatetodescriptor(self, generator, *args, sync_fun=None, **kwargs):
         sync_fun() if sync_fun else self.sync_all()
         return blocks
 
+    def create_outpoints(self, node, *, outputs):
+        """Send funds to a given list of `{address: amount}` targets using the bitcoind
+        wallet and return the corresponding outpoints as a list of dictionaries
+        `[{"txid": txid, "vout": vout1}, {"txid": txid, "vout": vout2}, ...]`.
+        The result can be used to specify inputs for RPCs like `createrawtransaction`,
+        `createpsbt`, `lockunspent` etc."""
+        assert all(len(output.keys()) == 1 for output in outputs)
+        send_res = node.send(outputs)
+        assert send_res["complete"]
+        utxos = []
+        for output in outputs:
+            address = list(output.keys())[0]
+            vout = find_vout_for_address(node, send_res["txid"], address)
+            utxos.append({"txid": send_res["txid"], "vout": vout})
+        return utxos
+
     def sync_blocks(self, nodes=None, wait=1, timeout=60):
         """
         Wait until everybody has the same tip.

test/functional/test_framework/test_node.py

@@ -40,6 +40,7 @@
     wait_until_helper,
     p2p_port,
     get_chain_folder,
+    tor_port,
 )
 
 BITCOIND_PROC_WAIT_TIMEOUT = 60
@@ -90,8 +91,11 @@ def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timew
         self.cwd = cwd
         self.mocktime = mocktime
         self.descriptors = descriptors
+        self.has_explicit_bind = False
         if extra_conf is not None:
             append_config(datadir, extra_conf)
+            # Remember if there is bind=... in the config file.
+            self.has_explicit_bind = any(e.startswith("bind=") for e in extra_conf)
         # Most callers will just need to add extra args to the standard list below.
         # For those callers that need more flexibity, they can just set the args property directly.
         # Note that common args are set in the config file (see initialize_datadir)
@@ -227,6 +231,17 @@ def start(self, extra_args=None, *, cwd=None, stdout=None, stderr=None, **kwargs
         if extra_args is None:
             extra_args = self.extra_args
 
+        # If listening and no -bind is given, then bitcoind would bind P2P ports on
+        # 0.0.0.0:P and 127.0.0.1:18445 (for incoming Tor connections), where P is
+        # a unique port chosen by the test framework and configured as port=P in
+        # bitcoin.conf. To avoid collisions on 127.0.0.1:18445, change it to
+        # 127.0.0.1:tor_port().
+        will_listen = all(e != "-nolisten" and e != "-listen=0" for e in extra_args)
+        has_explicit_bind = self.has_explicit_bind or any(e.startswith("-bind=") for e in extra_args)
+        if will_listen and not has_explicit_bind:
+            extra_args.append(f"-bind=0.0.0.0:{p2p_port(self.index)}")
+            extra_args.append(f"-bind=127.0.0.1:{tor_port(self.index)}=onion")
+
         self.use_v2transport = "-v2transport=1" in extra_args or (self.default_to_v2 and "-v2transport=0" not in extra_args)
 
         # Add a new stdout and stderr file each time dashd is started

test/functional/test_framework/util.py

@@ -320,9 +320,9 @@ def random_bytes(n):
 
 # The maximum number of nodes a single test can spawn
 MAX_NODES = 20
-# Don't assign rpc or p2p ports lower than this
+# Don't assign p2p, rpc or tor ports lower than this
 PORT_MIN = int(os.getenv('TEST_RUNNER_PORT_MIN', default=11000))
-# The number of ports to "reserve" for p2p and rpc, each
+# The number of ports to "reserve" for p2p, rpc and tor, each
 PORT_RANGE = 10000
 
 
@@ -362,7 +362,11 @@ def p2p_port(n):
 
 
 def rpc_port(n):
-    return PORT_MIN + PORT_RANGE + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
+    return p2p_port(n) + PORT_RANGE
+
+
+def tor_port(n):
+    return p2p_port(n) + PORT_RANGE * 2
 
 
 def rpc_url(datadir, i, chain, rpchost=None):
@@ -579,18 +583,6 @@ def get_mnemonic(node):
 #############################
 
 
-def find_output(node, txid, amount, *, blockhash=None):
-    """
-    Return index to output of txid with value amount
-    Raises exception if there is none.
-    """
-    txdata = node.getrawtransaction(txid, 1, blockhash)
-    for i in range(len(txdata["vout"])):
-        if txdata["vout"][i]["value"] == amount:
-            return i
-    raise RuntimeError("find_output txid %s : %s not found" % (txid, str(amount)))
-
-
 # Create large OP_RETURN txouts that can be appended to a transaction
 # to make it large (helper for constructing large transactions). The
 # total serialized size of the txouts is about 66k vbytes.