Merge bitcoin/bitcoin#28331: BIP324 integration

75a3291 doc: mention BIP324 support in bips.md (Pieter Wuille)
64ca721 test: enable v2 transport between nodes in some functional tests (Pieter Wuille)
05d19fb test: Functional test for opportunistic encryption (dhruv)
b815cce net: expose transport types/session IDs of connections in RPC and logs (Pieter Wuille)
432a62c net: reconnect with V1Transport under certain conditions (Pieter Wuille)
4d265d0 sync: modernize CSemaphore / CSemaphoreGrant (Pieter Wuille)
c73cd42 rpc: addnode arg to use BIP324 v2 p2p (dhruv)
62d21ee net: use V2Transport when NODE_P2P_V2 service flag is present (Pieter Wuille)
a4706bc rpc: don't report v2 handshake bytes in the per-type sent byte statistics (Sebastian Falbesoner)
abf343b net: advertise NODE_P2P_V2 if CLI arg -v2transport is on (Pieter Wuille)

Pull request description:

  Part of #27634.

  This makes BIP324 support feature complete, through a (default off) `-v2transport` option for enabling V2 connections. If it is enabled:
  * The `NODE_P2P_V2` service flag (*1 << 11*) is advertized.
  * Inbound connections can use V1 or V2 (automatically detected based on the protocol used by the peer)
  * V2 connections are used on outbound when the `NODE_P2P_V2` service is available (or the new `use_v2` parameter is set on the `addnode` RPC).
  * V2 outbound connections that instantly fail get retried as V1.

  There are two new RPC fields, `"transport_protocol_type"` and `"session_id"`, in `getpeerinfo`.

ACKs for top commit:
  mzumsande:
    re-ACK 75a3291
  theStack:
    re-ACK 75a3291

Tree-SHA512: 90ea1cd37f3dce410a59ff5de1c2405891e8aa62318d0e06dcb68b21603fb0c061631526633f3d4fb630e63d2b8db407eed48e246befcbef3503bea893a4ff15

doc/bips.md

@@ -49,6 +49,7 @@ BIPs that are implemented by Bitcoin Core:
 * [`BIP 173`](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki): Bech32 addresses for native Segregated Witness outputs are supported as of **v0.16.0** ([PR 11167](https://github.com/bitcoin/bitcoin/pull/11167)). Bech32 addresses are generated by default as of **v0.20.0** ([PR 16884](https://github.com/bitcoin/bitcoin/pull/16884)).
 * [`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 **v0.17.0** ([PR 13557](https://github.com/bitcoin/bitcoin/pull/13557)).
 * [`BIP 176`](https://github.com/bitcoin/bips/blob/master/bip-0176.mediawiki): Bits Denomination [QT only] is supported as of **v0.16.0** ([PR 12035](https://github.com/bitcoin/bitcoin/pull/12035)).
+* [`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 **v26.0**, but off by default ([PR 28331](https://github.com/bitcoin/bitcoin/pull/28331)).
 * [`BIP 325`](https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki): Signet test network is supported as of **v0.21.0** ([PR 18267](https://github.com/bitcoin/bitcoin/pull/18267)).
 * [`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)).
 * [`BIP 340`](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki)

src/init.cpp

@@ -498,6 +498,7 @@ void SetupServerArgs(ArgsManager& argsman)
  argsman.AddArg("-i2psam=<ip:port>", "I2P SAM proxy to reach I2P peers and accept I2P connections (default: none)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
  argsman.AddArg("-i2pacceptincoming", strprintf("Whether to accept inbound I2P connections (default: %i). Ignored if -i2psam is not set. Listening for inbound I2P connections is done through the SAM proxy, not by binding to a local address and port.", DEFAULT_I2P_ACCEPT_INCOMING), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
  argsman.AddArg("-onlynet=<net>", "Make automatic outbound connections only to network <net> (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+ argsman.AddArg("-v2transport", strprintf("Support v2 transport (default: %u)", DEFAULT_V2_TRANSPORT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
  argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
  argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
  argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
@@ -893,6 +894,11 @@ bool AppInitParameterInteraction(const ArgsManager& args)
  }
  }
 
+ // Signal NODE_P2P_V2 if BIP324 v2 transport is enabled.
+ if (args.GetBoolArg("-v2transport", DEFAULT_V2_TRANSPORT)) {
+ nLocalServices = ServiceFlags(nLocalServices | NODE_P2P_V2);
+ }
+
  // Signal NODE_COMPACT_FILTERS if peerblockfilters and basic filters index are both enabled.
  if (args.GetBoolArg("-peerblockfilters", DEFAULT_PEERBLOCKFILTERS)) {
  if (g_enabled_filter_types.count(BlockFilterType::BASIC) != 1) {

src/net.h

@@ -94,11 +94,17 @@ static constexpr 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};
+
 typedef int64_t NodeId;
 
-struct AddedNodeInfo
-{
- std::string strAddedNode;
+struct AddedNodeParams {
+ std::string m_added_node;
+ bool m_use_v2transport;
+};
+
+struct AddedNodeInfo {
+ AddedNodeParams m_params;
  CService resolvedAddress;
  bool fConnected;
  bool fInbound;
@@ -226,6 +232,10 @@ class CNodeStats
  Network m_network;
  uint32_t m_mapped_as;
  ConnectionType m_conn_type;
+ /** Transport protocol type. */
+ TransportProtocolType m_transport_type;
+ /** BIP324 session id string in hex, if any. */
+ std::string m_session_id;
 };
 
 
@@ -262,6 +272,15 @@ class Transport {
 public:
  virtual ~Transport() {}
 
+ struct Info
+ {
+ TransportProtocolType transport_type;
+ std::optional<uint256> session_id;
+ };
+
+ /** Retrieve information about this transport. */
+ virtual Info GetInfo() const noexcept = 0;
+
  // 1. Receiver side functions, for decoding bytes received on the wire into transport protocol
  // agnostic CNetMessage (message type & payload) objects.
 
@@ -355,6 +374,11 @@ class Transport {
 
  /** Return the memory usage of this transport attributable to buffered data to send. */
  virtual size_t GetSendMemoryUsage() const noexcept = 0;
+
+ // 3. Miscellaneous functions.
+
+ /** Whether upon disconnections, a reconnect with V1 is warranted. */
+ virtual bool ShouldReconnectV1() const noexcept = 0;
 };
 
 class V1Transport final : public Transport
@@ -415,6 +439,8 @@ class V1Transport final : public Transport
  return WITH_LOCK(m_recv_mutex, return CompleteInternal());
  }
 
+ Info GetInfo() const noexcept override;
+
  bool ReceivedBytes(Span<const uint8_t>& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex)
  {
  AssertLockNotHeld(m_recv_mutex);
@@ -434,6 +460,7 @@ class V1Transport final : public Transport
  BytesToSend GetBytesToSend(bool have_next_message) const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
  void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
  size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
+ bool ShouldReconnectV1() const noexcept override { return false; }
 };
 
 class V2Transport final : public Transport
@@ -602,6 +629,8 @@ class V2Transport final : public Transport
  std::string m_send_type GUARDED_BY(m_send_mutex);
  /** Current sender state. */
  SendState m_send_state GUARDED_BY(m_send_mutex);
+ /** Whether we've sent at least 24 bytes (which would trigger disconnect for V1 peers). */
+ bool m_sent_v1_header_worth GUARDED_BY(m_send_mutex) {false};
 
  /** Change the receive state. */
  void SetReceiveState(RecvState recv_state) noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex);
@@ -647,6 +676,10 @@ class V2Transport final : public Transport
  BytesToSend GetBytesToSend(bool have_next_message) const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
  void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
  size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
+
+ // Miscellaneous functions.
+ bool ShouldReconnectV1() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex);
+ Info GetInfo() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex);
 };
 
 struct CNodeOptions
@@ -655,6 +688,7 @@ struct CNodeOptions
  std::unique_ptr<i2p::sam::Session> i2p_sam_session = nullptr;
  bool prefer_evict = false;
  size_t recv_flood_size{DEFAULT_MAXRECEIVEBUFFER * 1000};
+ bool use_v2transport = false;
 };
 
 /** Information about a peer */
@@ -699,6 +733,8 @@ class CNode
  // Bind address of our side of the connection
  const CAddress addrBind;
  const std::string m_addr_name;
+ /** The pszDest argument provided to ConnectNode(). Only used for reconnections. */
+ const std::string m_dest;
  //! Whether this peer is an inbound onion, i.e. connected via our Tor onion service.
  const bool m_inbound_onion;
  std::atomic<int> nVersion{0};
@@ -1072,7 +1108,11 @@ class CConnman
  vWhitelistedRange = connOptions.vWhitelistedRange;
  {
  LOCK(m_added_nodes_mutex);
- m_added_nodes = connOptions.m_added_nodes;
+
+ for (const std::string& added_node : connOptions.m_added_nodes) {
+ // -addnode cli arg does not currently have a way to signal BIP324 support
+ m_added_node_params.push_back({added_node, false});
+ }
  }
  m_onion_binds = connOptions.onion_binds;
  }
@@ -1096,7 +1136,7 @@ class CConnman
  bool GetNetworkActive() const { return fNetworkActive; };
  bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; };
  void SetNetworkActive(bool active);
- void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
+ void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
  bool CheckIncomingNonce(uint64_t nonce);
 
  // alias for thread safety annotations only, not defined
@@ -1159,7 +1199,7 @@ class CConnman
  // Count the number of block-relay-only peers we have over our limit.
  int GetExtraBlockRelayCount() const;
 
- bool AddNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
+ bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
  bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
  std::vector<AddedNodeInfo> GetAddedNodeInfo() const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
 
@@ -1242,10 +1282,10 @@ class CConnman
  bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions);
  bool InitBinds(const Options& options);
 
- void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex);
+ void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex);
  void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex);
  void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_unused_i2p_sessions_mutex);
- void ThreadOpenConnections(std::vector<std::string> connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex);
+ void ThreadOpenConnections(std::vector<std::string> connect) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !m_unused_i2p_sessions_mutex, !m_reconnections_mutex);
  void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
  void ThreadI2PAcceptIncoming();
  void AcceptConnection(const ListenSocket& hListenSocket);
@@ -1263,7 +1303,7 @@ class CConnman
  const CAddress& addr_bind,
  const CAddress& addr);
 
- void DisconnectNodes();
+ void DisconnectNodes() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_nodes_mutex);
  void NotifyNumConnectionsChanged();
  /** Return true if the peer is inactive and should be disconnected. */
  bool InactivityCheck(const CNode& node) const;
@@ -1295,7 +1335,7 @@ class CConnman
  */
  void SocketHandlerListening(const Sock::EventsPerSock& events_per_sock);
 
- void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc);
+ void ThreadSocketHandler() EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc, !m_nodes_mutex, !m_reconnections_mutex);
  void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex);
 
  uint64_t CalculateKeyedNetGroup(const CAddress& ad) const;
@@ -1312,7 +1352,7 @@ class CConnman
  bool AlreadyConnectedToAddress(const CAddress& addr);
 
  bool AttemptToEvictConnection();
- CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
+ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex);
  void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const;
 
  void DeleteNode(CNode* pnode);
@@ -1384,7 +1424,10 @@ class CConnman
  const NetGroupManager& m_netgroupman;
  std::deque<std::string> m_addr_fetches GUARDED_BY(m_addr_fetches_mutex);
  Mutex m_addr_fetches_mutex;
- std::vector<std::string> m_added_nodes GUARDED_BY(m_added_nodes_mutex);
+
+ // connection string and whether to use v2 p2p
+ std::vector<AddedNodeParams> m_added_node_params GUARDED_BY(m_added_nodes_mutex);
+
  mutable Mutex m_added_nodes_mutex;
  std::vector<CNode*> m_nodes GUARDED_BY(m_nodes_mutex);
  std::list<CNode*> m_nodes_disconnected;
@@ -1523,6 +1566,29 @@ class CConnman
  */
  std::queue<std::unique_ptr<i2p::sam::Session>> m_unused_i2p_sessions GUARDED_BY(m_unused_i2p_sessions_mutex);
 
+ /**
+ * Mutex protecting m_reconnections.
+ */
+ Mutex m_reconnections_mutex;
+
+ /** Struct for entries in m_reconnections. */
+ struct ReconnectionInfo
+ {
+ CAddress addr_connect;
+ CSemaphoreGrant grant;
+ std::string destination;
+ ConnectionType conn_type;
+ bool use_v2transport;
+ };
+
+ /**
+ * List of reconnections we have to make.
+ */
+ std::list<ReconnectionInfo> m_reconnections GUARDED_BY(m_reconnections_mutex);
+
+ /** Attempt reconnections, if m_reconnections non-empty. */
+ void PerformReconnections() EXCLUSIVE_LOCKS_REQUIRED(!m_reconnections_mutex, !m_unused_i2p_sessions_mutex);
+
  /**
  * Cap on the size of `m_unused_i2p_sessions`, to ensure it does not
  * unexpectedly use too much memory.

src/net_processing.cpp

@@ -3585,13 +3585,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
  return;
  }
 
- if (!pfrom.IsInboundConn()) {
+ // Log succesful connections unconditionally for outbound, but not for inbound as those
+ // can be triggered by an attacker at high rate.
+ if (!pfrom.IsInboundConn() || LogAcceptCategory(BCLog::NET, BCLog::Level::Debug)) {
  const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
- LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s%s (%s)\n",
+ LogPrintf("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s%s\n",
+ pfrom.ConnectionTypeAsString(),
+ TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
  pfrom.nVersion.load(), peer->m_starting_height,
  pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""),
- (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""),
- pfrom.ConnectionTypeAsString());
+ (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
  }
 
  if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) {

src/node/connection_types.cpp

@@ -24,3 +24,17 @@ std::string ConnectionTypeAsString(ConnectionType conn_type)
 
  assert(false);
 }
+
+std::string TransportTypeAsString(TransportProtocolType transport_type)
+{
+ switch (transport_type) {
+ case TransportProtocolType::DETECTING:
+ return "detecting";
+ case TransportProtocolType::V1:
+ return "v1";
+ case TransportProtocolType::V2:
+ return "v2";
+ } // no default case, so the compiler can warn about missing cases
+
+ assert(false);
+}

src/node/connection_types.h

@@ -6,6 +6,7 @@
 #define BITCOIN_NODE_CONNECTION_TYPES_H
 
 #include <string>
+#include <stdint.h>
 
 /** Different types of connections to a peer. This enum encapsulates the
  * information we have available at the time of opening or accepting the
@@ -79,4 +80,14 @@ enum class ConnectionType {
 /** Convert ConnectionType enum to a string value */
 std::string ConnectionTypeAsString(ConnectionType conn_type);
 
+/** Transport layer version */
+enum class TransportProtocolType : uint8_t {
+ DETECTING, //!< Peer could be v1 or v2
+ V1, //!< Unencrypted, plaintext protocol
+ V2, //!< BIP324 protocol
+};
+
+/** Convert TransportProtocolType enum to a string value */
+std::string TransportTypeAsString(TransportProtocolType transport_type);
+
 #endif // BITCOIN_NODE_CONNECTION_TYPES_H

src/protocol.cpp

@@ -199,6 +199,7 @@ static std::string serviceFlagToStr(size_t bit)
  case NODE_WITNESS: return "WITNESS";
  case NODE_COMPACT_FILTERS: return "COMPACT_FILTERS";
  case NODE_NETWORK_LIMITED: return "NETWORK_LIMITED";
+ case NODE_P2P_V2: return "P2P_V2";
  // Not using default, so we get warned when a case is missing
  }
 

src/protocol.h

@@ -291,6 +291,9 @@ enum ServiceFlags : uint64_t {
  // See BIP159 for details on how this is implemented.
  NODE_NETWORK_LIMITED = (1 << 10),
 
+ // NODE_P2P_V2 means the node supports BIP324 transport
+ NODE_P2P_V2 = (1 << 11),
+
  // Bits 24-31 are reserved for temporary experiments. Just pick a bit that
  // isn't getting used, or one not being used much, and notify the
  // bitcoin-development mailing list. Remember that service bits are just

src/rpc/client.cpp

@@ -301,6 +301,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
  { "addpeeraddress", 2, "tried"},
  { "sendmsgtopeer", 0, "peer_id" },
  { "stop", 0, "wait" },
+ { "addnode", 2, "v2transport" },
 };
 // clang-format on