Merge #6488: backport: merge bitcoin#27452, bitcoin#29347, bitcoin#29356

, bitcoin#29353, bitcoin#29452, bitcoin#29483, bitcoin#30545, bitcoin#31383 (BIP324 backports: part 5)

c6f23a7 merge bitcoin#31383: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py (Kittywhiskers Van Gogh)
9072a10 merge bitcoin#30545: fix intermittent failures in feature_proxy.py (Kittywhiskers Van Gogh)
7e2d435 merge bitcoin#29483: add --v1transport option, add --v2transport to a CI task (Kittywhiskers Van Gogh)
7e59a96 merge bitcoin#29452: document that BIP324 on by default (Kittywhiskers Van Gogh)
0f3b5e0 merge bitcoin#29353: adhere to typical VERSION message protocol flow (Kittywhiskers Van Gogh)
dfdddfd rpc: enable `v2transport` for `masternode connect` when capable (Kittywhiskers Van Gogh)
3c16361 merge bitcoin#29356: make v2transport arg in addconnection mandatory and few cleanups (Kittywhiskers Van Gogh)
c53cd93 merge bitcoin#29347: enable v2transport by default (Kittywhiskers Van Gogh)
7dcf561 merge bitcoin#27452: cover addrv2 anchors by adding TorV3 to CAddress in messages.py (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * When backporting [bitcoin#27452](bitcoin#27452) in `feature_anchors.py`, `P2P_SERVICES` (`NODE_NETWORK | NODE_HEADERS_COMPRESSED`) has been replaced with `NODE_NETWORK` as the former evaluates to a value greater than `256` (specifically `2049`), which causes test failure. The replacement value is acceptable as `NODE_NETWORK` is the desired service flag expected by Dash Core ([source](https://github.com/dashpay/dash/blob/779e4295ad253bf909a7c45ec02743e580abc61b/src/protocol.cpp#L249-L254)).

    <details>

    <summary>Test failure:</summary>

    ```
    dash@89afd55ae77e:/src/dash$ ./test/functional/feature_anchors.py
    2024-12-14T12:31:22.244000Z TestFramework (INFO): PRNG seed is: 8365703189892653614
    2024-12-14T12:31:22.244000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_j0ya02yu
    2024-12-14T12:31:22.776000Z TestFramework (INFO): When node starts, check if anchors.dat doesn't exist
    2024-12-14T12:31:22.776000Z TestFramework (INFO): Add 2 block-relay-only connections to node
    2024-12-14T12:31:24.781000Z TestFramework (INFO): Add 5 inbound connections to node
    2024-12-14T12:31:29.843000Z TestFramework (INFO): Check node connections
    2024-12-14T12:31:30.848000Z TestFramework (INFO): Check the addresses in anchors.dat
    2024-12-14T12:31:30.848000Z TestFramework (INFO): Perturb anchors.dat to test it doesn't throw an error during initialization
    2024-12-14T12:31:31.356000Z TestFramework (INFO): When node starts, check if anchors.dat doesn't exist anymore
    2024-12-14T12:31:31.357000Z TestFramework (INFO): Ensure addrv2 support
    2024-12-14T12:31:32.364000Z TestFramework (INFO): Add 256-bit-address block-relay-only connections to node
    2024-12-14T12:31:33.368000Z TestFramework (INFO): Check for addrv2 addresses in anchors.dat
    2024-12-14T12:31:33.369000Z TestFramework (ERROR): Unexpected exception caught during testing
    Traceback (most recent call last):
      File "/src/dash/test/functional/test_framework/test_framework.py", line 161, in main
        self.run_test()
      File "/src/dash/./test/functional/feature_anchors.py", line 135, in run_test
        new_data[services_index] = P2P_SERVICES
    ValueError: byte must be in range(0, 256)
    2024-12-14T12:31:33.870000Z TestFramework (INFO): Stopping nodes
    2024-12-14T12:31:33.871000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_j0ya02yu
    2024-12-14T12:31:33.871000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_j0ya02yu/test_framework.log
    ```

    </details>

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK c6f23a7
  PastaPastaPasta:
    utACK c6f23a7;

Tree-SHA512: ca134432d000d521827a20c75c03913421fe52a31fda1cbf632e0b207c31728406feb090469d592d8e2fd8d64298faa2752ff703de79f737a62c276c6a231097

ci/test/00_setup_env_native_multiprocess.sh

@@ -10,5 +10,6 @@ export CONTAINER_NAME=ci_native_multiprocess
 export PACKAGES="cmake python3 llvm clang"
 export DEP_OPTS="DEBUG=1 MULTIPROCESS=1"
 export GOAL="install"
+export TEST_RUNNER_EXTRA="--v2transport"
 export BITCOIN_CONFIG="--with-boost-process --enable-debug CC=clang CXX=clang++" # Use clang to avoid OOM
 export TEST_RUNNER_ENV="BITCOIND=dash-node"

doc/bips.md

@@ -41,5 +41,5 @@ Versions and PRs are relevant to Bitcoin's core if not mentioned other.
 * [`BIP 158`](https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki): Compact Block Filters for Light Clients can be indexed as of **Dash Core v18.0** ([PR dash#4314](https://github.com/dashpay/dash/pull/4314), [PR #14121](https://github.com/bitcoin/bitcoin/pull/14121)).
 * [`BIP 159`](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki): The `NODE_NETWORK_LIMITED` service bit is signalled as of **v0.16.0** ([PR 11740](https://github.com/bitcoin/bitcoin/pull/11740)), and such nodes are connected to as of **v0.17.0** ([PR 10387](https://github.com/bitcoin/bitcoin/pull/10387)).
 * [`BIP 174`](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki): RPCs to operate on Partially Signed Bitcoin Transactions (PSBT) are present as of **v18.0** ([PR 13557](https://github.com/bitcoin/bitcoin/pull/13557)).
-* [`BIP 324`](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki): The v2 transport protocol specified by BIP324 and the associated `NODE_P2P_V2` service bit are supported as of **v22.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)).
+* [`BIP 324`](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki): The v2 transport protocol specified by BIP324 and the associated `NODE_P2P_V2` service bit are supported as of **v22.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)). On by default as of **v22.1** ([PR 29347](https://github.com/bitcoin/bitcoin/pull/29347)).
 * [`BIP 339`](https://github.com/bitcoin/bips/blob/master/bip-0339.mediawiki): Relay of transactions by wtxid is supported as of **v0.21.0** ([PR 18044](https://github.com/bitcoin/bitcoin/pull/18044)).

src/net.h

@@ -117,7 +117,7 @@ static const bool DEFAULT_FIXEDSEEDS = true;
 static const size_t DEFAULT_MAXRECEIVEBUFFER = 5 * 1000;
 static const size_t DEFAULT_MAXSENDBUFFER    = 1 * 1000;
 
-static constexpr bool DEFAULT_V2_TRANSPORT{false};
+static constexpr bool DEFAULT_V2_TRANSPORT{true};
 
 #if defined USE_KQUEUE
 #define DEFAULT_SOCKETEVENTS "kqueue"

src/rpc/masternode.cpp

@@ -38,7 +38,7 @@ static RPCHelpMan masternode_connect()
         "Connect to given masternode\n",
         {
             {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The address of the masternode to connect"},
-            {"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol"},
+            {"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport"}, "Attempt to connect using BIP324 v2 transport protocol"},
         },
         RPCResults{},
         RPCExamples{""},
@@ -51,12 +51,13 @@ static RPCHelpMan masternode_connect()
         throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Incorrect masternode address %s", strAddress));
     }
 
-    bool use_v2transport = !request.params[1].isNull() && ParseBoolV(request.params[1], "v2transport");
-
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     CConnman& connman = EnsureConnman(node);
 
-    if (use_v2transport && !(connman.GetLocalServices() & NODE_P2P_V2)) {
+    bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
+    bool use_v2transport = request.params[1].isNull() ? node_v2transport : ParseBoolV(request.params[1], "v2transport");
+
+    if (use_v2transport && !node_v2transport) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Adding v2transport connections requires -v2transport init flag to be set.");
     }
 

src/rpc/net.cpp

@@ -360,7 +360,7 @@ static RPCHelpMan addconnection()
         {
             {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."},
             {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."},
-            {"v2transport", RPCArg::Type::BOOL, RPCArg::Default{false}, "Attempt to connect using BIP324 v2 transport protocol"},
+            {"v2transport", RPCArg::Type::BOOL, RPCArg::Optional::NO, "Attempt to connect using BIP324 v2 transport protocol"},
         },
         RPCResult{
             RPCResult::Type::OBJ, "", "",

test/functional/feature_anchors.py

@@ -7,11 +7,14 @@
 import os
 
 from test_framework.p2p import P2PInterface
+from test_framework.socks5 import Socks5Configuration, Socks5Server
+from test_framework.messages import CAddress, hash256, NODE_NETWORK
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import check_node_connections
+from test_framework.util import check_node_connections, assert_equal, p2p_port
 
 INBOUND_CONNECTIONS = 5
 BLOCK_RELAY_CONNECTIONS = 2
+ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333"
 
 
 class AnchorsTest(BitcoinTestFramework):
@@ -55,7 +58,7 @@ def run_test(self):
             else:
                 inbound_nodes_port.append(hex(int(addr_split[1]))[2:])
 
-        self.log.info("Stop node 0")
+        self.log.debug("Stop node")
         self.stop_node(0)
 
         # It should contain only the block-relay-only addresses
@@ -79,12 +82,64 @@ def run_test(self):
                 tweaked_contents[20:20] = b'1'
                 out_file_handler.write(bytes(tweaked_contents))
 
-            self.log.info("Start node")
+            self.log.debug("Start node")
             self.start_node(0)
 
         self.log.info("When node starts, check if anchors.dat doesn't exist anymore")
         assert not os.path.exists(node_anchors_path)
 
+        self.log.info("Ensure addrv2 support")
+        # Use proxies to catch outbound connections to networks with 256-bit addresses
+        onion_conf = Socks5Configuration()
+        onion_conf.auth = True
+        onion_conf.unauth = True
+        onion_conf.addr = ('127.0.0.1', p2p_port(self.num_nodes))
+        onion_conf.keep_alive = True
+        onion_proxy = Socks5Server(onion_conf)
+        onion_proxy.start()
+        self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])
+
+        self.log.info("Add 256-bit-address block-relay-only connections to node")
+        self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only', v2transport=False)
+
+        self.log.debug("Stop node")
+        with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]):
+            self.stop_node(0)
+        # Manually close keep_alive proxy connection
+        onion_proxy.stop()
+
+        self.log.info("Check for addrv2 addresses in anchors.dat")
+        caddr = CAddress()
+        caddr.net = CAddress.NET_TORV3
+        caddr.ip, port_str = ONION_ADDR.split(":")
+        caddr.port = int(port_str)
+        # TorV3 addrv2 serialization:
+        # time(4) | services(1) | networkID(1) | address length(1) | address(32)
+        expected_pubkey = caddr.serialize_v2()[7:39].hex()
+
+        # position of services byte of first addr in anchors.dat
+        # network magic, vector length, version, nTime
+        services_index = 4 + 1 + 4 + 4
+        data = bytes()
+        with open(node_anchors_path, "rb") as file_handler:
+            data = file_handler.read()
+            assert_equal(data[services_index], 0x00)  # services == NONE
+            anchors2 = data.hex()
+            assert expected_pubkey in anchors2
+
+        with open(node_anchors_path, "wb") as file_handler:
+            # Modify service flags for this address even though we never connected to it.
+            # This is necessary because on restart we will not attempt an anchor connection
+            # to a host without our required services, even if its address is in the anchors.dat file
+            new_data = bytearray(data)[:-32]
+            new_data[services_index] = NODE_NETWORK
+            new_data_hash = hash256(new_data)
+            file_handler.write(new_data + new_data_hash)
+
+        self.log.info("Restarting node attempts to reconnect to anchors")
+        with self.nodes[0].assert_debug_log([f"Trying to make an anchor connection to {ONION_ADDR}"]):
+            self.start_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])
+
 
 if __name__ == "__main__":
     AnchorsTest().main()

test/functional/feature_proxy.py

@@ -125,7 +125,8 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
         rv = []
         addr = "15.61.23.23:1234"
         self.log.debug(f"Test: outgoing IPv4 connection through node for address {addr}")
-        node.addnode(addr, "onetry")
+        # v2transport=False is used to avoid reconnections with v1 being scheduled. These could interfere with later checks.
+        node.addnode(addr, "onetry", v2transport=False)
         cmd = proxies[0].queue.get()
         assert isinstance(cmd, Socks5Command)
         # Note: dashd's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
@@ -141,7 +142,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
         if self.have_ipv6:
             addr = "[1233:3432:2434:2343:3234:2345:6546:4534]:5443"
             self.log.debug(f"Test: outgoing IPv6 connection through node for address {addr}")
-            node.addnode(addr, "onetry")
+            node.addnode(addr, "onetry", v2transport=False)
             cmd = proxies[1].queue.get()
             assert isinstance(cmd, Socks5Command)
             # Note: dashd's SOCKS5 implementation only sends atyp DOMAINNAME, even if connecting directly to IPv4/IPv6
@@ -157,7 +158,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
         if test_onion:
             addr = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333"
             self.log.debug(f"Test: outgoing onion connection through node for address {addr}")
-            node.addnode(addr, "onetry")
+            node.addnode(addr, "onetry", v2transport=False)
             cmd = proxies[2].queue.get()
             assert isinstance(cmd, Socks5Command)
             assert_equal(cmd.atyp, AddressType.DOMAINNAME)
@@ -172,7 +173,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
         if test_cjdns:
             addr = "[fc00:1:2:3:4:5:6:7]:8888"
             self.log.debug(f"Test: outgoing CJDNS connection through node for address {addr}")
-            node.addnode(addr, "onetry")
+            node.addnode(addr, "onetry", v2transport=False)
             cmd = proxies[1].queue.get()
             assert isinstance(cmd, Socks5Command)
             assert_equal(cmd.atyp, AddressType.DOMAINNAME)
@@ -186,7 +187,7 @@ def node_test(self, node, *, proxies, auth, test_onion, test_cjdns):
 
         addr = "node.noumenon:8333"
         self.log.debug(f"Test: outgoing DNS name connection through node for address {addr}")
-        node.addnode(addr, "onetry")
+        node.addnode(addr, "onetry", v2transport=False)
         cmd = proxies[3].queue.get()
         assert isinstance(cmd, Socks5Command)
         assert_equal(cmd.atyp, AddressType.DOMAINNAME)

test/functional/p2p_add_connections.py

@@ -17,7 +17,7 @@ def on_version(self, message):
         # message is received from the test framework. Don't send any responses
         # to the node's version message since the connection will already be
         # closed.
-        pass
+        self.send_version()
 
 class P2PAddConnections(BitcoinTestFramework):
     def set_test_params(self):

test/functional/p2p_addr_relay.py

@@ -74,6 +74,7 @@ def addr_received(self):
         return self.num_ipv4_received != 0
 
     def on_version(self, message):
+        self.send_version()
         self.send_message(msg_verack())
         if (self.send_getaddr):
             self.send_message(msg_getaddr())

test/functional/p2p_addrv2_relay.py

@@ -18,6 +18,7 @@
 from test_framework.util import assert_equal
 
 I2P_ADDR = "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p"
+ONION_ADDR = "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"
 
 ADDRS: List[CAddress] = []
 
@@ -37,6 +38,16 @@ def on_addrv2(self, message):
     def wait_for_addrv2(self):
         self.wait_until(lambda: "addrv2" in self.last_message)
 
+def calc_addrv2_msg_size(addrs):
+    size = 1  # vector length byte
+    for addr in addrs:
+        size += 4  # time
+        size += 1  # services, COMPACTSIZE(P2P_SERVICES)
+        size += 1  # network id
+        size += 1  # address length byte
+        size += addr.ADDRV2_ADDRESS_LENGTH[addr.net]  # address
+        size += 2  # port
+    return size
 
 class AddrTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -48,14 +59,18 @@ def run_test(self):
         for i in range(10):
             addr = CAddress()
             addr.time = int(self.mocktime) + i
+            addr.port = 8333 + i
             addr.nServices = NODE_NETWORK
-            # Add one I2P address at an arbitrary position.
+            # Add one I2P and one onion V3 address at an arbitrary position.
             if i == 5:
                 addr.net = addr.NET_I2P
                 addr.ip = I2P_ADDR
+                addr.port = 0
+            elif i == 8:
+                addr.net = addr.NET_TORV3
+                addr.ip = ONION_ADDR
             else:
                 addr.ip = f"123.123.123.{i % 256}"
-            addr.port = 8333 + i
             ADDRS.append(addr)
 
         self.log.info('Create connection that sends addrv2 messages')
@@ -73,14 +88,15 @@ def run_test(self):
         addr_source = self.nodes[0].add_p2p_connection(P2PInterface())
         addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver())
         msg.addrs = ADDRS
+        msg_size = calc_addrv2_msg_size(ADDRS)
         with self.nodes[0].assert_debug_log([
-                'received: addrv2 (159 bytes) peer=1',
+                f'received: addrv2 ({msg_size} bytes) peer=1',
         ]):
             addr_source.send_and_ping(msg)
 
         # Wait until "Added ..." before bumping mocktime to make sure addv2 is (almost) fully processed
         with self.nodes[0].assert_debug_log([
-                'sending addrv2 (159 bytes) peer=2',
+                f'sending addrv2 ({msg_size} bytes) peer=2',
         ]):
             self.bump_mocktime(30 * 60)
             addr_receiver.wait_for_addrv2()

test/functional/p2p_ibd_stalling.py

@@ -77,6 +77,7 @@ def run_test(self):
 
         self.log.info("Check that a staller does not get disconnected if the 1024 block lookahead buffer is filled")
         self.mocktime = int(time.time()) + 1
+        node.setmocktime(self.mocktime)
         for id in range(NUM_PEERS):
             peers.append(node.add_outbound_p2p_connection(P2PStaller(stall_block), services = NODE_NETWORK | NODE_BLOOM, p2p_idx=id, connection_type="outbound-full-relay"))
             peers[-1].block_store = block_dict

test/functional/p2p_sendtxrcncl.py

@@ -25,7 +25,7 @@ def __init__(self,):
 
     def on_version(self, message):
         # Avoid sending verack in response to version.
-        pass
+        self.send_version()
 
 class SendTxrcnclReceiver(P2PInterface):
     def __init__(self):
@@ -38,7 +38,8 @@ def on_sendtxrcncl(self, message):
 
 class P2PFeelerReceiver(SendTxrcnclReceiver):
     def on_version(self, message):
-        pass  # feeler connections can not send any message other than their own version
+        # feeler connections can not send any message other than their own version
+        self.send_version()
 
 
 class PeerTrackMsgOrder(P2PInterface):