diff --git a/doc/bips.md b/doc/bips.md index ebf6b8fcd7d7..6f9b322e33da 100644 --- a/doc/bips.md +++ b/doc/bips.md @@ -79,3 +79,4 @@ BIPs that are implemented by Bitcoin Core: * [`BIP 390`](https://github.com/bitcoin/bips/blob/master/bip-0390.mediawiki): MuSig2 Descriptor parsing is implemented in **v30.0** ([PR 31244](https://github.com/bitcoin/bitcoin/pull/31244)) and signing in **v31.0** ([PR 29675](https://github.com/bitcoin/bitcoin/pull/29675)) * [`BIP 431`](https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki): transactions with nVersion=3 are standard and treated as Topologically Restricted Until Confirmation as of **v28.0** ([PR 29496](https://github.com/bitcoin/bitcoin/pull/29496)). * [`BIP 433`](https://github.com/bitcoin/bips/blob/master/bip-0433.mediawiki): Spending of Pay to Anchor (P2A) outputs is standard as of **v28.0** ([PR 30352](https://github.com/bitcoin/bitcoin/pull/30352)). +* [`BIP 434`](https://github.com/bitcoin/bips/blob/master/bip-0434.md): Peer Feature Negotiation as of **v32.0** ([PR 35221](https://github.com/bitcoin/bitcoin/pull/35221)). diff --git a/src/bip324.h b/src/bip324.h index 396a28a44894..821cc3f759fc 100644 --- a/src/bip324.h +++ b/src/bip324.h @@ -15,6 +15,8 @@ #include #include +static constexpr unsigned BIP324_SHORTIDS_IMPLEMENTED{38}; + /** The BIP324 packet cipher, encapsulating its key derivation, stream cipher, and AEAD. */ class BIP324Cipher { diff --git a/src/net.cpp b/src/net.cpp index b22b503997b0..951e804f8877 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -922,7 +922,7 @@ namespace { * Only message types that are actually implemented in this codebase need to be listed, as other * messages get ignored anyway - whether we know how to decode them or not. */ -const std::array V2_MESSAGE_IDS = { +const std::array V2_MESSAGE_IDS = { "", // 12 bytes follow encoding the message type like in V1 NetMsgType::ADDR, NetMsgType::BLOCK, @@ -952,11 +952,10 @@ const std::array V2_MESSAGE_IDS = { NetMsgType::GETCFCHECKPT, NetMsgType::CFCHECKPT, NetMsgType::ADDRV2, - // Unimplemented message types that are assigned in BIP324: - "", - "", - "", - "" + "", "", "", // Unimplemented message types 29-31 + "", "", "", "", // Unimplemented message types 32-35 + "", // Unimplemented message type 36 + NetMsgType::FEATURE, }; class V2MessageMap diff --git a/src/net.h b/src/net.h index f54a8e0cdb72..d0cfe7c9ac31 100644 --- a/src/net.h +++ b/src/net.h @@ -831,6 +831,13 @@ class CNode return m_conn_type == ConnectionType::PRIVATE_BROADCAST; } + /** Protocol version advertised in our VERSION message. + * Private broadcast connections use a fixed version to maximise anonymity. */ + int AdvertisedVersion() const + { + return IsPrivateBroadcastConn() ? WTXID_RELAY_VERSION : PROTOCOL_VERSION; + } + bool IsInboundConn() const { return m_conn_type == ConnectionType::INBOUND; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 71cc6ff7baf4..17e078615b0c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -725,6 +725,15 @@ class PeerManagerImpl final : public PeerManager { m_connman.PushMessage(&node, NetMsg::Make(std::move(msg_type), std::forward(args)...)); } + template + void MakeAndPushFeature(CNode& node, std::string_view feature_id, Args&&... args) const + { + if (!Assume(feature_id.size() >= 4 && feature_id.size() <= MAX_FEATUREID_LENGTH)) return; + std::vector feature_data; + VectorWriter{feature_data, 0, std::forward(args)...}; + if (!Assume(feature_data.size() <= MAX_FEATUREDATA_LENGTH)) return; + MakeAndPushMessage(node, NetMsgType::FEATURE, feature_id, std::move(feature_data)); + } /** Send a version message to a peer */ void PushNodeVersion(CNode& pnode, const Peer& peer); @@ -1578,7 +1587,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) MakeAndPushMessage( pnode, NetMsgType::VERSION, - PROTOCOL_VERSION, + pnode.AdvertisedVersion(), my_services, my_time, // your_services + CNetAddr::V1(your_addr) is the pre-version-31402 serialization of your_addr (without nTime) @@ -1592,7 +1601,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) LogDebug( BCLog::NET, "send version message: version=%d, blocks=%d%s, txrelay=%d, peer=%d\n", - PROTOCOL_VERSION, my_height, + pnode.AdvertisedVersion(), my_height, fLogIPs ? strprintf(", them=%s", your_addr.ToStringAddrPort()) : "", my_tx_relay, pnode.GetId()); } @@ -3679,7 +3688,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string } // Change version - const int greatest_common_version = std::min(nVersion, PROTOCOL_VERSION); + const int greatest_common_version = std::min(nVersion, pfrom.AdvertisedVersion()); pfrom.SetCommonVersion(greatest_common_version); pfrom.nVersion = nVersion; @@ -3754,6 +3763,11 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string } } + if (greatest_common_version >= FEATURE_VERSION) { + // announce supported features + // MakeAndPushFeature(pfrom, NetMsgFeature::FOO, uint32_t{1}); + } + MakeAndPushMessage(pfrom, NetMsgType::VERACK); // Potentially mark this peer as a preferred download peer. @@ -3969,6 +3983,45 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string return; } + if (msg_type == NetMsgType::FEATURE) { + if (pfrom.fSuccessfullyConnected) { + // Disconnect peers that send a FEATURE message after VERACK. + LogDebug(BCLog::NET, "feature received after verack, %s", pfrom.DisconnectMsg()); + pfrom.fDisconnect = true; + return; + } else if (pfrom.GetCommonVersion() < FEATURE_VERSION) { + // Disconnect peers that send a FEATURE message without valid version negotiation. + LogDebug(BCLog::NET, "feature received with incompatible version %d, %s", pfrom.GetCommonVersion(), pfrom.DisconnectMsg()); + pfrom.fDisconnect = true; + return; + } + + std::string feature_id; + DataStream feature_data; + try { + vRecv >> LIMITED_STRING(feature_id, MAX_FEATUREID_LENGTH); + std::vector feature_data_vec; + vRecv >> LIMITED_VECTOR(feature_data_vec, MAX_FEATUREDATA_LENGTH); + feature_data = DataStream(feature_data_vec); + } catch (const std::exception&) { + feature_id.clear(); // use empty feature_id as error indicator + } + if (feature_id.size() < 4 || !vRecv.empty()) { + LogDebug(BCLog::NET, "invalid feature payload, %s", pfrom.DisconnectMsg()); + pfrom.fDisconnect = true; + return; + } + + // if (feature_id == NetMsgFeature::FOO) { + // ... + // return; + // } + + // ignore unknown feature_id + LogDebug(BCLog::NET, "unknown feature advertised: %s", SanitizeString(feature_id)); + return; + } + // Received from a peer demonstrating readiness to announce transactions via reconciliations. // This feature negotiation must happen between VERSION and VERACK to avoid relay problems // from switching announcement protocols after the connection is up. diff --git a/src/node/caches.cpp b/src/node/caches.cpp index 94d0ced88f69..c98b8ce60407 100644 --- a/src/node/caches.cpp +++ b/src/node/caches.cpp @@ -58,16 +58,27 @@ CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes) { size_t total_cache{CalculateDbCacheBytes(args)}; + // Allocate proportional to usage pattern benefit: + // - txindex (10%): serves getrawtransaction RPCs with mostly unique, + // non-repetitive lookups across the entire blockchain. + // - blockfilterindex (5%): serves BIP 157 light clients that repeatedly + // query recent blocks, benefiting from LevelDB cache, but the + // working set for a typical 2-week offline gap is ~200kiB, well within 5% + // of the total cache. + // - txospenderindex (5%): serves gettxspendingprevout RPCs with very + // specific, rarely repeated outpoint queries. + // - coinstatsindex: intentionally not included here, since usage pattern + // does not seem to suggest it would be necessary to cache. IndexCacheSizes index_sizes; - index_sizes.tx_index = std::min(total_cache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); - total_cache -= index_sizes.tx_index; - index_sizes.txospender_index = std::min(total_cache / 8, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0); - total_cache -= index_sizes.txospender_index; + index_sizes.tx_index = std::min(total_cache * 10 / 100, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? MAX_TX_INDEX_CACHE : 0); + index_sizes.txospender_index = std::min(total_cache * 5 / 100, args.GetBoolArg("-txospenderindex", DEFAULT_TXOSPENDERINDEX) ? MAX_TXOSPENDER_INDEX_CACHE : 0); if (n_indexes > 0) { - size_t max_cache = std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE); + size_t max_cache = std::min(total_cache * 5 / 100, MAX_FILTER_INDEX_CACHE); index_sizes.filter_index = max_cache / n_indexes; total_cache -= index_sizes.filter_index * n_indexes; } + total_cache -= index_sizes.tx_index; + total_cache -= index_sizes.txospender_index; return {index_sizes, kernel::CacheSizes{total_cache}}; } diff --git a/src/node/protocol_version.h b/src/node/protocol_version.h index 7904086fe555..a72ac7774655 100644 --- a/src/node/protocol_version.h +++ b/src/node/protocol_version.h @@ -9,7 +9,7 @@ * network protocol versioning */ -static const int PROTOCOL_VERSION = 70016; +static const int PROTOCOL_VERSION = 70017; //! initial proto version, to be increased after version/verack negotiation static const int INIT_PROTO_VERSION = 209; @@ -35,4 +35,7 @@ static const int INVALID_CB_NO_BAN_VERSION = 70015; //! "wtxidrelay" message type for wtxid-based relay starts with this version static const int WTXID_RELAY_VERSION = 70016; +//! "feature" message type for feature negotiation starts with this version +static const int FEATURE_VERSION = 70017; + #endif // BITCOIN_NODE_PROTOCOL_VERSION_H diff --git a/src/protocol.h b/src/protocol.h index 8ed90dd3f8ef..24851e2fc49d 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -264,6 +264,10 @@ inline constexpr const char* WTXIDRELAY{"wtxidrelay"}; * txreconciliation, as described by BIP 330. */ inline constexpr const char* SENDTXRCNCL{"sendtxrcncl"}; +/** + * BIP 434 Peer feature negotiation + */ +inline constexpr const char* FEATURE{"feature"}; }; // namespace NetMsgType /** All known message types (see above). Keep this in the same order as the list of messages above. */ @@ -303,8 +307,16 @@ inline const std::array ALL_NET_MESSAGE_TYPES{std::to_array({ NetMsgType::CFCHECKPT, NetMsgType::WTXIDRELAY, NetMsgType::SENDTXRCNCL, + NetMsgType::FEATURE, })}; +static constexpr size_t MAX_FEATUREID_LENGTH{80}; +static constexpr size_t MAX_FEATUREDATA_LENGTH{512}; + +namespace NetMsgFeature { +//inline constexpr std::string_view FOO{"BIP-FOO"}; +} + /** nServices flags */ enum ServiceFlags : uint64_t { // NOTE: When adding here, be sure to update serviceFlagToStr too diff --git a/src/serialize.h b/src/serialize.h index 5cef72a9e03d..5d38a5100760 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -493,6 +494,7 @@ static inline Wrapper Using(T&& t) { return Wrapper>(obj) #define COMPACTSIZE(obj) Using>(obj) #define LIMITED_STRING(obj,n) Using>(obj) +#define LIMITED_VECTOR(obj,n) Using>(obj) /** Serialization wrapper class for integers in VarInt format. */ template @@ -705,6 +707,12 @@ struct VectorFormatter template void Serialize(Stream& os, const std::basic_string& str); template void Unserialize(Stream& is, std::basic_string& str); +/** + * string_view + */ +template void Serialize(Stream& os, const std::basic_string_view& str); +template void Unserialize(Stream& is, std::basic_string_view& str) = delete; + /** * prevector */ @@ -783,6 +791,35 @@ struct DefaultFormatter static void Unser(Stream& s, T& t) { Unserialize(s, t); } }; +/** + * Limited vector formatter. Throws an error if a vector is oversized. + */ + +template +struct LimitedVectorFormatter +{ + template + void Unser(Stream& s, V& v) + { + Formatter formatter; + v.clear(); + size_t size = ReadCompactSize(s); + if (size > Limit) { + throw std::ios_base::failure("Vector length limit exceeded"); + } + v.reserve(size); + for (size_t i = 0; i < size; ++i) { + v.emplace_back(); + formatter.Unser(s, v.back()); + } + } + + template + void Ser(Stream& s, const V& v) + { + VectorFormatter{}.Ser(s, v); + } +}; @@ -807,7 +844,17 @@ void Unserialize(Stream& is, std::basic_string& str) is.read(MakeWritableByteSpan(str)); } - +/** + * string_view + */ +template +void Serialize(Stream& os, const std::basic_string_view& str) +{ + WriteCompactSize(os, str.size()); + if (!str.empty()) { + os.write(MakeByteSpan(str)); + } +} /** * prevector diff --git a/src/test/feerounder_tests.cpp b/src/test/feerounder_tests.cpp index 82cd7d65b409..d88a4f771b6c 100644 --- a/src/test/feerounder_tests.cpp +++ b/src/test/feerounder_tests.cpp @@ -9,7 +9,7 @@ #include -BOOST_AUTO_TEST_SUITE(fee_rounder_tests) +BOOST_AUTO_TEST_SUITE(feerounder_tests) BOOST_AUTO_TEST_CASE(FeeRounder) { diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 7168d3dcaec2..a2b647509a95 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -1568,7 +1569,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test) tester.CompareSessionIDs(); auto msg_data_1 = m_rng.randbytes(4000000); // test that receiving 4M payload works auto msg_data_2 = m_rng.randbytes(4000000); // test that sending 4M payload works - tester.SendMessage(uint8_t(m_rng.randrange(223) + 33), {}); // unknown short id + tester.SendMessage(uint8_t(m_rng.randrange(256 - BIP324_SHORTIDS_IMPLEMENTED) + BIP324_SHORTIDS_IMPLEMENTED), {}); // unknown short id tester.SendMessage(uint8_t(2), msg_data_1); // "block" short id tester.AddMessage("blocktxn", msg_data_2); // schedule blocktxn to be sent to us ret = tester.Interact(); diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index f646ba5fafbd..9c2f16f03e30 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -241,6 +242,40 @@ BOOST_AUTO_TEST_CASE(noncanonical) BOOST_CHECK_EXCEPTION(ReadCompactSize(ss), std::ios_base::failure, isCanonicalException); } +BOOST_AUTO_TEST_CASE(string_view) +{ + const std::string_view sv{"hello, world"}; + DataStream ss; + ss << sv; + std::string s; + ss >> s; + BOOST_CHECK_EQUAL(sv, s); +} + +BOOST_AUTO_TEST_CASE(limited_vector) +{ + const std::vector v = {1,2,3,4,-5,-6,-7,-8,-9,-10,10000,20000,-30000}; + + auto check = [&]() { + DataStream ss; + ss << v; + try { + std::vector r; + ss >> LIMITED_VECTOR(r, N); + BOOST_CHECK_LE(r.size(), N); + BOOST_CHECK(std::ranges::equal(r, v)); + } catch (const std::ios_base::failure&) { + BOOST_CHECK_GT(v.size(), N); + } + }; + check.operator()<0>(); + check.operator()<10>(); + check.operator()<12>(); + check.operator()<13>(); + check.operator()<14>(); + check.operator()<100>(); +} + BOOST_AUTO_TEST_CASE(class_methods) { int intval(100); diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index c4850c64cfd3..1415c8d4968e 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -56,7 +56,7 @@ void ConnmanTestMsg::Handshake(CNode& node, FlushSendBuffer(node); // Drop the verack message added by SendMessages. if (node.fDisconnect) return; assert(node.nVersion == version); - assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); + assert(node.GetCommonVersion() == std::min(version, node.AdvertisedVersion())); CNodeStateStats statestats; assert(peerman.GetNodeStateStats(node.GetId(), statestats)); assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); diff --git a/test/functional/p2p_bip434_feature.py b/test/functional/p2p_bip434_feature.py new file mode 100755 index 000000000000..ccb8144a91b3 --- /dev/null +++ b/test/functional/p2p_bip434_feature.py @@ -0,0 +1,242 @@ +#!/usr/bin/env python3 +# Copyright (c) The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +"""Test BIP434 feature negotiation.""" + +from test_framework.messages import ( + msg_version, + ser_compact_size, +) +from test_framework.p2p import ( + P2PInterface, + P2P_SERVICES, + P2P_SUBVERSION, + P2P_VERSION_RELAY, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +# Pre-BIP434 protocol version +PRE_FEATURE_VERSION = 70016 +# Protocol version which enables BIP434 FEATURE negotiation +FEATURE_VERSION = 70017 +# BIP434 wire-format limits +MIN_FEATUREID_LENGTH = 4 +MAX_FEATUREID_LENGTH = 80 +MAX_FEATUREDATA_LENGTH = 512 + + +class RawFeature: + """A FEATURE message with a hand-crafted payload.""" + msgtype = b"feature" + + def __init__(self, payload): + self.payload = payload + + def serialize(self): + return self.payload + + def __repr__(self): + return f"RawFeature(payload_len={len(self.payload)})" + + +def feature_wire(feature_id, feature_data, *, trailing=b""): + """Build a FEATURE payload plus optional trailing bytes.""" + if isinstance(feature_id, str): + feature_id = feature_id.encode() + return (ser_compact_size(len(feature_id)) + feature_id + + ser_compact_size(len(feature_data)) + feature_data + + trailing) + + +def _version_msg(nversion): + v = msg_version() + v.nVersion = nversion + v.strSubVer = P2P_SUBVERSION + v.nServices = P2P_SERVICES + v.relay = P2P_VERSION_RELAY + return v + + +class FeaturePeer(P2PInterface): + """P2PInterface that counts FEATURE messages received.""" + + def __init__(self): + super().__init__() + self.got_feature_count = 0 + self.last_feature = None + + def on_feature(self, message): + self.got_feature_count += 1 + self.last_feature = message + + +class FeaturePeerNoVerack(FeaturePeer): + """Peer that records but does not auto-reply to the node's VERSION, so the + node stays in the post-VERSION / pre-VERACK window where FEATURE messages + are valid to send.""" + + def on_version(self, message): + self.nServices = message.nServices + self.relay = message.relay + + +class P2PBIP434FeatureTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + # peertimeout=999 prevents the node from kicking the peer for being idle + self.extra_args = [["-debug=net", "-peertimeout=999"]] + + def run_test(self): + self.test_advertised_version() + self.test_no_feature_to_pre_70017_peer() + self.test_features_announced_to_modern_peer() + self.test_feature_after_verack_disconnects() + self.test_feature_before_version_ignored() + self.test_feature_id_length_boundaries() + self.test_feature_data_length_boundaries() + self.test_trailing_bytes_disconnect() + self.test_truncated_feature_id_disconnect() + self.test_truncated_feature_data_disconnect() + self.test_non_ascii_feature_id_accepted() + self.test_many_features_in_handshake() + self.test_recv_feature_from_pre_70017_peer() + + def _silent_peer(self, *, nversion=FEATURE_VERSION): + peer = self.nodes[0].add_p2p_connection( + FeaturePeerNoVerack(), + send_version=False, wait_for_verack=False, + ) + peer.send_without_ping(_version_msg(nversion)) + peer.wait_for_verack() + return peer + + def _expect_accept(self, payload, *, log_substring="unknown feature advertised", + nversion=FEATURE_VERSION): + peer = self._silent_peer(nversion=nversion) + with self.nodes[0].assert_debug_log([log_substring], timeout=2): + peer.send_without_ping(RawFeature(payload)) + assert peer.is_connected, "peer disconnected after a well-formed FEATURE" + self.nodes[0].disconnect_p2ps() + + def _expect_disconnect(self, payload, *, log_substring="invalid feature payload", + nversion=FEATURE_VERSION): + peer = self._silent_peer(nversion=nversion) + with self.nodes[0].assert_debug_log([log_substring], timeout=2): + peer.send_without_ping(RawFeature(payload)) + peer.wait_for_disconnect() + + def test_advertised_version(self): + self.log.info("Test that node advertises FEATURE to peer with protocol version 70017") + peer = self.nodes[0].add_p2p_connection(FeaturePeer()) + assert_equal(peer.last_message["version"].nVersion, FEATURE_VERSION) + self.nodes[0].disconnect_p2ps() + + def test_no_feature_to_pre_70017_peer(self): + self.log.info("Test that node doesn't send FEATURE to a peer with protocol version <70017") + peer = self.nodes[0].add_p2p_connection( + FeaturePeer(), send_version=False, wait_for_verack=False, + ) + peer.send_without_ping(_version_msg(PRE_FEATURE_VERSION)) + peer.wait_for_verack() + # The node's full handshake has now been delivered to us; if any + # FEATURE would have been sent it would be in last_message by now. + assert_equal(peer.got_feature_count, 0) + self.nodes[0].disconnect_p2ps() + + def test_features_announced_to_modern_peer(self): + self.log.info("Test that node announces correct number of features to a 70017 peer") + peer = self.nodes[0].add_p2p_connection(FeaturePeer()) + assert_equal(peer.got_feature_count, 0) + self.nodes[0].disconnect_p2ps() + + def test_feature_after_verack_disconnects(self): + self.log.info("Test that FEATURE after VERACK triggers disconnect") + peer = self.nodes[0].add_p2p_connection(FeaturePeer()) + peer.sync_with_ping() # ensure node has set fSuccessfullyConnected + with self.nodes[0].assert_debug_log(["feature received after verack"], timeout=2): + peer.send_without_ping(RawFeature(feature_wire(b"abcd", b""))) + peer.wait_for_disconnect() + + def test_feature_before_version_ignored(self): + self.log.info("Test that FEATURE before any VERSION is silently ignored") + peer = self.nodes[0].add_p2p_connection( + FeaturePeer(), send_version=False, wait_for_verack=False, + ) + with self.nodes[0].assert_debug_log( + ["non-version message before version handshake"], timeout=2, + ): + peer.send_without_ping(RawFeature(feature_wire(b"abcd", b""))) + assert peer.is_connected + self.nodes[0].disconnect_p2ps() + + def test_feature_id_length_boundaries(self): + self.log.info("Test feature_id length boundaries") + for length, accept in [(0, False), + (3, False), + (MIN_FEATUREID_LENGTH, True), + (MAX_FEATUREID_LENGTH, True), + (MAX_FEATUREID_LENGTH + 1, False)]: + payload = feature_wire(b"a" * length, b"") + if accept: + self._expect_accept(payload) + else: + self._expect_disconnect(payload) + + def test_feature_data_length_boundaries(self): + self.log.info("Test feature_data length boundaries") + for length, accept in [(0, True), + (MAX_FEATUREDATA_LENGTH, True), + (MAX_FEATUREDATA_LENGTH + 1, False)]: + payload = feature_wire(b"abcd", b"\x00" * length) + if accept: + self._expect_accept(payload) + else: + self._expect_disconnect(payload) + + def test_trailing_bytes_disconnect(self): + self.log.info("Test that trailing bytes after data triggers disconnect") + self._expect_disconnect( + feature_wire(b"abcd", b"", trailing=b"\x00"), + ) + + def test_truncated_feature_id_disconnect(self): + self.log.info("Test that truncated feature_id triggers disconnect") + payload = ser_compact_size(10) + b"abcde" + self._expect_disconnect(payload) + + def test_truncated_feature_data_disconnect(self): + self.log.info("Test that truncated feature_data triggers disconnect") + payload = (ser_compact_size(MIN_FEATUREID_LENGTH) + b"abcd" + + ser_compact_size(10) + b"xx") + self._expect_disconnect(payload) + + def test_non_ascii_feature_id_accepted(self): + self.log.info("Test that feature_id with non-ASCII bytes is still accepted") + # BIP says SHOULD, not MUST, on this + self._expect_accept(feature_wire(b"\x00\xff\x01\x7f", b"")) + + def test_many_features_in_handshake(self): + self.log.info("Test multiple FEATURE advertisements") + peer = self._silent_peer() + with self.nodes[0].assert_debug_log(["unknown feature advertised"], timeout=2): + for i in range(16): + peer.send_without_ping( + RawFeature(feature_wire(f"feat{i:04d}".encode(), b"")) + ) + assert peer.is_connected + self.nodes[0].disconnect_p2ps() + + def test_recv_feature_from_pre_70017_peer(self): + self.log.info("Test that FEATURE from <70017 peer triggers disconnect") + self._expect_disconnect( + feature_wire(b"abcd", b""), + log_substring="feature received with incompatible version", + nversion=PRE_FEATURE_VERSION, + ) + + +if __name__ == "__main__": + P2PBIP434FeatureTest(__file__).main() diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index ec7b9dc209fc..71046edb4b9c 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -5,7 +5,7 @@ """Test message sending before handshake completion. Before receiving a VERACK, a node should not send anything but VERSION/VERACK -and feature negotiation messages (WTXIDRELAY, SENDADDRV2). +and feature negotiation messages (WTXIDRELAY, SENDADDRV2, FEATURE). This test connects to a node and sends it a few messages, trying to entice it into sending us something it shouldn't.""" @@ -39,6 +39,7 @@ def __init__(self): self.ever_connected = False self.got_wtxidrelay = False self.got_sendaddrv2 = False + self.got_feature = False def bad_message(self, message): self.unexpected_msg = True @@ -70,6 +71,7 @@ def on_getblocktxn(self, message): self.bad_message(message) def on_blocktxn(self, message): self.bad_message(message) def on_wtxidrelay(self, message): self.got_wtxidrelay = True def on_sendaddrv2(self, message): self.got_sendaddrv2 = True + def on_feature(self, message): self.got_feature = True # Peer that sends a version but not a verack. @@ -119,7 +121,7 @@ def run_test(self): no_verack_idle_peer = self.nodes[0].add_p2p_connection(NoVerackIdlePeer(), wait_for_verack=False) # Pre-wtxidRelay peer that sends a version but not a verack and does not support feature negotiation - # messages which start at nVersion == 70016 + # messages which start at nVersion >= 70016 pre_wtxidrelay_peer = self.nodes[0].add_p2p_connection(NoVerackIdlePeer(), send_version=False, wait_for_verack=False) pre_wtxidrelay_peer.send_without_ping(self.create_old_version(70015)) @@ -145,14 +147,17 @@ def run_test(self): assert not no_version_idle_peer.unexpected_msg assert not no_version_idle_peer.got_wtxidrelay assert not no_version_idle_peer.got_sendaddrv2 + assert not no_version_idle_peer.got_feature assert not no_verack_idle_peer.unexpected_msg assert no_verack_idle_peer.got_wtxidrelay assert no_verack_idle_peer.got_sendaddrv2 + #assert no_verack_idle_peer.got_feature ## uncomment once a feature message is supported assert not pre_wtxidrelay_peer.unexpected_msg assert not pre_wtxidrelay_peer.got_wtxidrelay assert not pre_wtxidrelay_peer.got_sendaddrv2 + assert not pre_wtxidrelay_peer.got_feature # Expect peers to be disconnected due to timeout assert not no_version_idle_peer.is_connected diff --git a/test/functional/p2p_private_broadcast.py b/test/functional/p2p_private_broadcast.py index 2c581df17795..638028af192a 100755 --- a/test/functional/p2p_private_broadcast.py +++ b/test/functional/p2p_private_broadcast.py @@ -14,7 +14,6 @@ P2PDataStore, P2PInterface, P2P_SERVICES, - P2P_VERSION, start_p2p_listener, ) from test_framework.messages import ( @@ -46,6 +45,7 @@ MiniWallet, ) +P2P_PRIVATE_VERSION = 70016 NUM_PRIVATE_BROADCAST_PER_TX = 3 @@ -195,7 +195,7 @@ def get_destinations_len(): }) dummy_address = CAddress() dummy_address.nServices = 0 - assert_equal(peer.last_message["version"].nVersion, P2P_VERSION) + assert_equal(peer.last_message["version"].nVersion, P2P_PRIVATE_VERSION) assert_equal(peer.last_message["version"].nServices, 0) assert_equal(peer.last_message["version"].nTime, 0) assert_equal(peer.last_message["version"].addrTo, dummy_address) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index a0f2a1740236..07abd5b6eb09 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1928,6 +1928,28 @@ def __repr__(self): return "msg_sendtxrcncl(version=%lu, salt=%lu)" %\ (self.version, self.salt) +class msg_feature: + """FEATURE message for negotiating optional features.""" + __slots__ = ("feature_id", "feature_data") + msgtype = b"feature" + + def __init__(self, feature_id="", feature_data=b""): + self.feature_id = feature_id + self.feature_data = feature_data + + def deserialize(self, f): + self.feature_id = deser_string(f).decode() + self.feature_data = deser_string(f) + + def serialize(self): + r = ser_string(self.feature_id.encode()) + r += ser_string(self.feature_data) + return r + + def __repr__(self): + return f"msg_feature(feature_id={self.feature_id}, data={self.feature_data.hex()})" + + class TestFrameworkScript(unittest.TestCase): def test_addrv2_encode_decode(self): def check_addrv2(ip, net): diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 4d812ce3b19d..803194791bfa 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -43,6 +43,7 @@ msg_cfheaders, msg_cfilter, msg_cmpctblock, + msg_feature, msg_feefilter, msg_filteradd, msg_filterclear, @@ -99,7 +100,8 @@ MIN_P2P_VERSION_SUPPORTED = 60001 # The P2P version that this test framework implements and sends in its `version` message # Version 70016 supports wtxid relay -P2P_VERSION = 70016 +# Version 70017 supports feature +P2P_VERSION = 70017 # The services that this test framework offers in its `version` message P2P_SERVICES = NODE_NETWORK | NODE_WITNESS # The P2P user agent string that this test framework sends in its `version` message @@ -124,6 +126,7 @@ b"cfheaders": msg_cfheaders, b"cfilter": msg_cfilter, b"cmpctblock": msg_cmpctblock, + b"feature": msg_feature, b"feefilter": msg_feefilter, b"filteradd": msg_filteradd, b"filterclear": msg_filterclear, @@ -543,6 +546,7 @@ def on_cfcheckpt(self, message): pass def on_cfheaders(self, message): pass def on_cfilter(self, message): pass def on_cmpctblock(self, message): pass + def on_feature(self, message): pass def on_feefilter(self, message): pass def on_filteradd(self, message): pass def on_filterclear(self, message): pass diff --git a/test/functional/test_framework/v2_p2p.py b/test/functional/test_framework/v2_p2p.py index 087b885ae027..4057fd9a8823 100644 --- a/test/functional/test_framework/v2_p2p.py +++ b/test/functional/test_framework/v2_p2p.py @@ -50,6 +50,7 @@ 26: b"getcfcheckpt", 27: b"cfcheckpt", 28: b"addrv2", + 37: b"feature", } # Dictionary which contains short message type ID for the P2P message diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4f59f0efee72..5bfc7d86239b 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -301,6 +301,8 @@ 'wallet_listsinceblock.py', 'wallet_listdescriptors.py', 'p2p_leak.py', + 'p2p_bip434_feature.py', + 'p2p_bip434_feature.py --v2transport', 'wallet_encryption.py', 'feature_dersig.py', 'feature_reindex_init.py', diff --git a/test/lint/lint-tests.py b/test/lint/lint-tests.py index c7354bc41228..75e4eb54e12d 100755 --- a/test/lint/lint-tests.py +++ b/test/lint/lint-tests.py @@ -13,12 +13,12 @@ import sys -def grep_boost_fixture_test_suite(): +def grep_boost_test_suites(): command = [ "git", "grep", "-E", - r"^BOOST_FIXTURE_TEST_SUITE\(", + r"^(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\(", "--", "src/ipc/test/**.cpp", "src/test/**.cpp", @@ -31,7 +31,7 @@ def check_matching_test_names(test_suite_list): not_matching = [ x for x in test_suite_list - if re.search(r"/(.*?)\.cpp:BOOST_FIXTURE_TEST_SUITE\(\1(_[a-z0-9]+)?, .*\)", x) is None + if re.search(r"/(.*?)\.cpp:(?:BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\(\1(_[a-z0-9]+)?[,)]", x) is None ] if len(not_matching) > 0: not_matching = "\n".join(not_matching) @@ -61,7 +61,7 @@ def get_duplicates(input_list): def check_unique_test_names(test_suite_list): - output = [re.search(r"\((.*?),", x) for x in test_suite_list] + output = [re.search(r"\((.*?)[,)]", x) for x in test_suite_list] output = [x.group(1) for x in output if x is not None] output = get_duplicates(output) output = sorted(list(output)) @@ -78,7 +78,7 @@ def check_unique_test_names(test_suite_list): def main(): - test_suite_list = grep_boost_fixture_test_suite().splitlines() + test_suite_list = grep_boost_test_suites().splitlines() exit_code = check_matching_test_names(test_suite_list) exit_code |= check_unique_test_names(test_suite_list) sys.exit(exit_code)