Merge bitcoin/bitcoin#29356: test: make v2transport arg in addconnection mandatory and few cleanups

e7fd70f4b6 [test] make v2transport arg in addconnection mandatory and few cleanups (stratospher)

Pull request description:

  - make `v2transport` argument in `addconnection` regression-testing only RPC mandatory. https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1470738750
  - previously it was an optional arg with default `false` value.
  - only place this RPC is used is in the [functional tests](11b436a66a/test/functional/test_framework/test_node.py (L742)) where we always pass the appropriate `v2transport` option to the RPC anyways. (and that too just for python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) interactions)
  - rename `v2_handshake()` to `_on_data_v2_handshake()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466958424
  - more compact return statement in `wait_for_reconnect()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466979708
  - assertion to check that empty version packets are received from `TestNode`.

ACKs for top commit:
  glozow:
    ACK e7fd70f4b6
  theStack:
    Code-review ACK e7fd70f4b6
  mzumsande:
    Code Review ACK e7fd70f4b6

Tree-SHA512: e66e29baccd91e1e4398b91f7d45c5fc7c2841d77d8a6178734586017bf2be63496721649da91848dec71da605ee31664352407d5bb896e624cc693767c61a1f
This commit is contained in:
glozow 2024-02-06 10:56:51 +00:00
commit 4de84557d6
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4
4 changed files with 7 additions and 8 deletions

View file

@ -371,7 +371,7 @@ static RPCHelpMan addconnection()
{ {
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."}, {"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\")."}, {"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{
RPCResult::Type::OBJ, "", "", RPCResult::Type::OBJ, "", "",
@ -403,7 +403,7 @@ static RPCHelpMan addconnection()
} else { } else {
throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString()); throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString());
} }
bool use_v2transport = !request.params[2].isNull() && request.params[2].get_bool(); bool use_v2transport = self.Arg<bool>(2);
NodeContext& node = EnsureAnyNodeContext(request.context); NodeContext& node = EnsureAnyNodeContext(request.context);
CConnman& connman = EnsureConnman(node); CConnman& connman = EnsureConnman(node);

View file

@ -99,7 +99,7 @@ class AnchorsTest(BitcoinTestFramework):
self.restart_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"]) 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.log.info("Add 256-bit-address block-relay-only connections to node")
self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only') self.nodes[0].addconnection(ONION_ADDR, 'block-relay-only', v2transport=False)
self.log.debug("Stop node") 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"]): with self.nodes[0].assert_debug_log([f"DumpAnchors: Flush 1 outbound block-relay-only peer addresses to anchors.dat"]):

View file

@ -242,7 +242,7 @@ class P2PConnection(asyncio.Protocol):
self.on_close() self.on_close()
# v2 handshake method # v2 handshake method
def v2_handshake(self): def _on_data_v2_handshake(self):
"""v2 handshake performed before P2P messages are exchanged (see BIP324). P2PConnection is the initiator """v2 handshake performed before P2P messages are exchanged (see BIP324). P2PConnection is the initiator
(in inbound connections to TestNode) and the responder (in outbound connections from TestNode). (in inbound connections to TestNode) and the responder (in outbound connections from TestNode).
Performed by: Performed by:
@ -298,7 +298,7 @@ class P2PConnection(asyncio.Protocol):
if len(t) > 0: if len(t) > 0:
self.recvbuf += t self.recvbuf += t
if self.supports_v2_p2p and not self.v2_state.tried_v2_handshake: if self.supports_v2_p2p and not self.v2_state.tried_v2_handshake:
self.v2_handshake() self._on_data_v2_handshake()
else: else:
self._on_data() self._on_data()
@ -595,9 +595,7 @@ class P2PInterface(P2PConnection):
def wait_for_reconnect(self, timeout=60): def wait_for_reconnect(self, timeout=60):
def test_function(): def test_function():
if not (self.is_connected and self.last_message.get('version') and self.v2_state is None): return self.is_connected and self.last_message.get('version') and not self.supports_v2_p2p
return False
return True
self.wait_until(test_function, timeout=timeout, check_connected=False) self.wait_until(test_function, timeout=timeout, check_connected=False)
# Message receiving helper methods # Message receiving helper methods

View file

@ -220,6 +220,7 @@ class EncryptedP2PState:
# decoy packets have contents = None. v2 handshake is complete only when version packet # decoy packets have contents = None. v2 handshake is complete only when version packet
# (can be empty with contents = b"") with contents != None is received. # (can be empty with contents = b"") with contents != None is received.
if contents is not None: if contents is not None:
assert contents == b"" # currently TestNode sends an empty version packet
self.tried_v2_handshake = True self.tried_v2_handshake = True
return processed_length, True return processed_length, True
response = response[length:] response = response[length:]