From 0f6b385e3bdb495dd80eeb105f6b1a91cad3be44 Mon Sep 17 00:00:00 2001 From: echennells Date: Tue, 1 Sep 2026 03:10:07 +0000 Subject: [PATCH 1/2] Add not_found protocol overrides at bip37. --- builds/gnu/Makefile.am | 4 + .../libbitcoin-node/libbitcoin-node.vcxproj | 4 + .../libbitcoin-node.vcxproj.filters | 12 +++ .../libbitcoin-node/libbitcoin-node.vcxproj | 4 + .../libbitcoin-node.vcxproj.filters | 12 +++ .../node/impl/sessions/session_peer.ipp | 17 ++-- .../node/protocols/protocol_block_out_106.hpp | 5 +- .../protocols/protocol_block_out_70001.hpp | 55 ++++++++++++ .../protocols/protocol_block_out_70012.hpp | 6 +- .../protocol_transaction_out_106.hpp | 5 ++ .../protocol_transaction_out_70001.hpp | 58 +++++++++++++ include/bitcoin/node/protocols/protocols.hpp | 2 + src/protocols/protocol_block_out_106.cpp | 32 ++++++- src/protocols/protocol_block_out_70001.cpp | 59 +++++++++++++ src/protocols/protocol_block_out_70012.cpp | 2 +- .../protocol_transaction_out_106.cpp | 10 ++- .../protocol_transaction_out_70001.cpp | 60 +++++++++++++ test/functional/p2p.cpp | 85 +++++++++++++++++++ test/functional/p2p_setup_fixture.cpp | 19 ++++- test/functional/p2p_setup_fixture.hpp | 64 +++++++++++++- 20 files changed, 495 insertions(+), 20 deletions(-) create mode 100644 include/bitcoin/node/protocols/protocol_block_out_70001.hpp create mode 100644 include/bitcoin/node/protocols/protocol_transaction_out_70001.hpp create mode 100644 src/protocols/protocol_block_out_70001.cpp create mode 100644 src/protocols/protocol_transaction_out_70001.cpp diff --git a/builds/gnu/Makefile.am b/builds/gnu/Makefile.am index 8daa8051..753dc613 100644 --- a/builds/gnu/Makefile.am +++ b/builds/gnu/Makefile.am @@ -77,6 +77,7 @@ src_libbitcoin_node_la_SOURCES = \ ${srcdir}/../../src/protocols/protocol_block_in_106.cpp \ ${srcdir}/../../src/protocols/protocol_block_in_31800.cpp \ ${srcdir}/../../src/protocols/protocol_block_out_106.cpp \ + ${srcdir}/../../src/protocols/protocol_block_out_70001.cpp \ ${srcdir}/../../src/protocols/protocol_block_out_70012.cpp \ ${srcdir}/../../src/protocols/protocol_filter_out_70015.cpp \ ${srcdir}/../../src/protocols/protocol_header_in_31800.cpp \ @@ -88,6 +89,7 @@ src_libbitcoin_node_la_SOURCES = \ ${srcdir}/../../src/protocols/protocol_performer.cpp \ ${srcdir}/../../src/protocols/protocol_transaction_in_106.cpp \ ${srcdir}/../../src/protocols/protocol_transaction_out_106.cpp \ + ${srcdir}/../../src/protocols/protocol_transaction_out_70001.cpp \ ${srcdir}/../../src/sessions/session.cpp \ ${srcdir}/../../src/sessions/session_inbound.cpp \ ${srcdir}/../../src/sessions/session_manual.cpp \ @@ -162,6 +164,7 @@ include_bitcoin_node_protocols_HEADERS = \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_block_in_106.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_block_in_31800.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_block_out_106.hpp \ + ${srcdir}/../../include/bitcoin/node/protocols/protocol_block_out_70001.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_block_out_70012.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_filter_out_70015.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_header_in_31800.hpp \ @@ -173,6 +176,7 @@ include_bitcoin_node_protocols_HEADERS = \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_performer.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_transaction_in_106.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocol_transaction_out_106.hpp \ + ${srcdir}/../../include/bitcoin/node/protocols/protocol_transaction_out_70001.hpp \ ${srcdir}/../../include/bitcoin/node/protocols/protocols.hpp include_bitcoin_node_sessionsdir = \ diff --git a/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj b/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj index c355ac35..750d08b1 100644 --- a/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj +++ b/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj @@ -146,6 +146,7 @@ + @@ -157,6 +158,7 @@ + @@ -196,6 +198,7 @@ + @@ -207,6 +210,7 @@ + diff --git a/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj.filters b/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj.filters index a6dbdb30..10876c36 100644 --- a/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj.filters +++ b/builds/msvc/vs2022/libbitcoin-node/libbitcoin-node.vcxproj.filters @@ -132,6 +132,9 @@ src\protocols + + src\protocols + src\protocols @@ -165,6 +168,9 @@ src\protocols + + src\protocols + src\sessions @@ -278,6 +284,9 @@ include\bitcoin\node\protocols + + include\bitcoin\node\protocols + include\bitcoin\node\protocols @@ -311,6 +320,9 @@ include\bitcoin\node\protocols + + include\bitcoin\node\protocols + include\bitcoin\node\protocols diff --git a/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj b/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj index 2d05d8d6..c43c94a9 100644 --- a/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj +++ b/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj @@ -146,6 +146,7 @@ + @@ -157,6 +158,7 @@ + @@ -196,6 +198,7 @@ + @@ -207,6 +210,7 @@ + diff --git a/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj.filters b/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj.filters index a6dbdb30..10876c36 100644 --- a/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj.filters +++ b/builds/msvc/vs2026/libbitcoin-node/libbitcoin-node.vcxproj.filters @@ -132,6 +132,9 @@ src\protocols + + src\protocols + src\protocols @@ -165,6 +168,9 @@ src\protocols + + src\protocols + src\sessions @@ -278,6 +284,9 @@ include\bitcoin\node\protocols + + include\bitcoin\node\protocols + include\bitcoin\node\protocols @@ -311,6 +320,9 @@ include\bitcoin\node\protocols + + include\bitcoin\node\protocols + include\bitcoin\node\protocols diff --git a/include/bitcoin/node/impl/sessions/session_peer.ipp b/include/bitcoin/node/impl/sessions/session_peer.ipp index fde9fa10..c45216ee 100644 --- a/include/bitcoin/node/impl/sessions/session_peer.ipp +++ b/include/bitcoin/node/impl/sessions/session_peer.ipp @@ -140,22 +140,25 @@ inline void CLASS::attach_protocols(const channel_ptr& channel) NOEXCEPT channel->attach(self)->start(); channel->attach(self)->start(); } - else if (headers && peer->is_negotiated(level::headers_protocol)) - { - channel->attach(self)->start(); - channel->attach(self)->start(); - } else { - channel->attach(self)->start(); + if (headers && peer->is_negotiated(level::headers_protocol)) + channel->attach(self)->start(); + + // not_found is defined at bip37. + if (peer->is_negotiated(level::bip37)) + channel->attach(self)->start(); + else + channel->attach(self)->start(); } } // Relay is configured, active, and txs are ready (txs in/out). if (txs_in_out) { + // Attached above bip37, where not_found is defined. if (peer->peer_version()->relay) - channel->attach(self)->start(); + channel->attach(self)->start(); } } diff --git a/include/bitcoin/node/protocols/protocol_block_out_106.hpp b/include/bitcoin/node/protocols/protocol_block_out_106.hpp index 7c254ce7..df64b055 100644 --- a/include/bitcoin/node/protocols/protocol_block_out_106.hpp +++ b/include/bitcoin/node/protocols/protocol_block_out_106.hpp @@ -54,6 +54,7 @@ class BCN_API protocol_block_out_106 protected: using get_data = network::messages::peer::get_data; using get_blocks = network::messages::peer::get_blocks; + using inventory_item = network::messages::peer::inventory_item; /// Block announcements are superseded by send_headers. virtual bool superseded() const NOEXCEPT; @@ -71,9 +72,11 @@ class BCN_API protocol_block_out_106 const get_data::cptr& message) NOEXCEPT; virtual void send_block(const code& ec) NOEXCEPT; + /// The requested item cannot be served, stops the channel. + virtual void handle_unservable(const inventory_item& item) NOEXCEPT; + private: using inventory = network::messages::peer::inventory; - using inventory_item = network::messages::peer::inventory_item; using inventory_items = network::messages::peer::inventory_items; bool is_under_checkpoint(const database::header_link& link) NOEXCEPT; diff --git a/include/bitcoin/node/protocols/protocol_block_out_70001.hpp b/include/bitcoin/node/protocols/protocol_block_out_70001.hpp new file mode 100644 index 00000000..b764bca6 --- /dev/null +++ b/include/bitcoin/node/protocols/protocol_block_out_70001.hpp @@ -0,0 +1,55 @@ +/** + * Copyright (c) 2011-2026 libbitcoin developers + * + * This file is part of libbitcoin. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +#ifndef LIBBITCOIN_NODE_PROTOCOLS_PROTOCOL_BLOCK_OUT_70001_HPP +#define LIBBITCOIN_NODE_PROTOCOLS_PROTOCOL_BLOCK_OUT_70001_HPP + +#include +#include + +namespace libbitcoin { +namespace node { + +class BCN_API protocol_block_out_70001 + : public protocol_block_out_106, + protected network::tracker +{ +public: + typedef std::shared_ptr ptr; + + protocol_block_out_70001(const auto& session, + const network::channel::ptr& channel) NOEXCEPT + : protocol_block_out_106(session, channel), + enable_not_found_(session->network_settings().enable_not_found), + network::tracker(session->log) + { + } + +protected: + /// The requested item cannot be served, replies not_found. + void handle_unservable(const inventory_item& item) NOEXCEPT override; + +private: + // This is thread safe. + const bool enable_not_found_; +}; + +} // namespace node +} // namespace libbitcoin + +#endif diff --git a/include/bitcoin/node/protocols/protocol_block_out_70012.hpp b/include/bitcoin/node/protocols/protocol_block_out_70012.hpp index 4cbe70e3..41265988 100644 --- a/include/bitcoin/node/protocols/protocol_block_out_70012.hpp +++ b/include/bitcoin/node/protocols/protocol_block_out_70012.hpp @@ -20,13 +20,13 @@ #define LIBBITCOIN_NODE_PROTOCOLS_PROTOCOL_BLOCK_OUT_70012_HPP #include -#include +#include namespace libbitcoin { namespace node { class BCN_API protocol_block_out_70012 - : public protocol_block_out_106, + : public protocol_block_out_70001, protected network::tracker { public: @@ -34,7 +34,7 @@ class BCN_API protocol_block_out_70012 protocol_block_out_70012(const auto& session, const network::channel::ptr& channel) NOEXCEPT - : protocol_block_out_106(session, channel), + : protocol_block_out_70001(session, channel), network::tracker(session->log) { } diff --git a/include/bitcoin/node/protocols/protocol_transaction_out_106.hpp b/include/bitcoin/node/protocols/protocol_transaction_out_106.hpp index 23aa4c44..05dcf659 100644 --- a/include/bitcoin/node/protocols/protocol_transaction_out_106.hpp +++ b/include/bitcoin/node/protocols/protocol_transaction_out_106.hpp @@ -63,6 +63,11 @@ class BCN_API protocol_transaction_out_106 virtual void send_transaction(const code& ec, size_t index, const network::messages::peer::get_data::cptr& message) NOEXCEPT; + /// The requested item cannot be served, stops the channel. + virtual void handle_unservable( + const network::messages::peer::inventory_item& item, size_t index, + const network::messages::peer::get_data::cptr& message) NOEXCEPT; + virtual bool announce(const system::hash_digest& hash) NOEXCEPT; private: diff --git a/include/bitcoin/node/protocols/protocol_transaction_out_70001.hpp b/include/bitcoin/node/protocols/protocol_transaction_out_70001.hpp new file mode 100644 index 00000000..d37d2960 --- /dev/null +++ b/include/bitcoin/node/protocols/protocol_transaction_out_70001.hpp @@ -0,0 +1,58 @@ +/** + * Copyright (c) 2011-2026 libbitcoin developers + * + * This file is part of libbitcoin. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +#ifndef LIBBITCOIN_NODE_PROTOCOLS_PROTOCOL_TRANSACTION_OUT_70001_HPP +#define LIBBITCOIN_NODE_PROTOCOLS_PROTOCOL_TRANSACTION_OUT_70001_HPP + +#include +#include + +namespace libbitcoin { +namespace node { + +class BCN_API protocol_transaction_out_70001 + : public protocol_transaction_out_106, + protected network::tracker +{ +public: + typedef std::shared_ptr ptr; + + protocol_transaction_out_70001(const auto& session, + const network::channel::ptr& channel) NOEXCEPT + : protocol_transaction_out_106(session, channel), + enable_not_found_(session->network_settings().enable_not_found), + network::tracker(session->log) + { + } + +protected: + /// The requested item cannot be served, replies not_found. + void handle_unservable( + const network::messages::peer::inventory_item& item, size_t index, + const network::messages::peer::get_data::cptr& message) NOEXCEPT + override; + +private: + // This is thread safe. + const bool enable_not_found_; +}; + +} // namespace node +} // namespace libbitcoin + +#endif diff --git a/include/bitcoin/node/protocols/protocols.hpp b/include/bitcoin/node/protocols/protocols.hpp index 21ded064..9ff2dc25 100644 --- a/include/bitcoin/node/protocols/protocols.hpp +++ b/include/bitcoin/node/protocols/protocols.hpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -34,5 +35,6 @@ #include #include #include +#include #endif diff --git a/src/protocols/protocol_block_out_106.cpp b/src/protocols/protocol_block_out_106.cpp index 51495482..07e0d01b 100644 --- a/src/protocols/protocol_block_out_106.cpp +++ b/src/protocols/protocol_block_out_106.cpp @@ -205,7 +205,9 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT return; if (backlog_.empty()) return; - const auto& item = backlog_.front(); + + // Copied because the item is answered after the backlog is popped. + const auto item = backlog_.front(); const auto witness = item.is_witness_type(); if (witness && !node_witness_) { @@ -216,11 +218,26 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT const auto& query = archive(); const auto link = query.to_header(item.hash); + + // A hash that resolves to no header is ordinary peer input, and the + // checkpoint height query faults the store on a terminal link. + if (link.is_terminal()) + { + LOGR("Requested block " << encode_hash(item.hash) << " from [" + << opposite() << "] not stored."); + + backlog_.pop_front(); + handle_unservable(item); + return; + } + if (node_pruned_ && (is_under_checkpoint(link) || query.is_milestone(link))) { LOGR("Requested pruned block " << encode_hash(item.hash) << " from [" << opposite() << "]."); - stop(system::error::not_found); + + backlog_.pop_front(); + handle_unservable(item); return; } @@ -235,8 +252,8 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT << opposite() << "] not found."); // This block could not have been advertised to the peer. - // TODO: send not_found message in protocol override. - stop(system::error::not_found); + backlog_.pop_front(); + handle_unservable(item); return; } @@ -245,6 +262,13 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT SEND(std::move(out), send_block, _1); } +// not_found is undefined below bip37, so the channel is stopped instead. +void protocol_block_out_106::handle_unservable(const inventory_item&) NOEXCEPT +{ + BC_ASSERT(stranded()); + stop(system::error::not_found); +} + // utilities // ---------------------------------------------------------------------------- diff --git a/src/protocols/protocol_block_out_70001.cpp b/src/protocols/protocol_block_out_70001.cpp new file mode 100644 index 00000000..69710684 --- /dev/null +++ b/src/protocols/protocol_block_out_70001.cpp @@ -0,0 +1,59 @@ +/** + * Copyright (c) 2011-2026 libbitcoin developers + * + * This file is part of libbitcoin. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +#include + +#include + +namespace libbitcoin { +namespace node { + +#define CLASS protocol_block_out_70001 + +using namespace system; +using namespace network; +using namespace network::messages::peer; +using namespace std::placeholders; + +BC_PUSH_WARNING(SMART_PTR_NOT_NEEDED) +BC_PUSH_WARNING(NO_VALUE_OR_CONST_REF_SHARED_PTR) + +// Outbound (not_found). +// ---------------------------------------------------------------------------- + +// The item is answered and the send loop resumed, as with a block, so nothing +// is produced until the prior write completes. +void protocol_block_out_70001::handle_unservable( + const inventory_item& item) NOEXCEPT +{ + BC_ASSERT(stranded()); + + if (!enable_not_found_) + { + protocol_block_out_106::handle_unservable(item); + return; + } + + SEND(not_found{ { item } }, send_block, _1); +} + +BC_POP_WARNING() +BC_POP_WARNING() + +} // namespace node +} // namespace libbitcoin diff --git a/src/protocols/protocol_block_out_70012.cpp b/src/protocols/protocol_block_out_70012.cpp index ccd750cb..f341d534 100644 --- a/src/protocols/protocol_block_out_70012.cpp +++ b/src/protocols/protocol_block_out_70012.cpp @@ -44,7 +44,7 @@ void protocol_block_out_70012::start() NOEXCEPT return; SUBSCRIBE_CHANNEL(send_headers, handle_receive_send_headers, _1, _2); - protocol_block_out_106::start(); + protocol_block_out_70001::start(); } // Inbound (send_headers). diff --git a/src/protocols/protocol_transaction_out_106.cpp b/src/protocols/protocol_transaction_out_106.cpp index 9f2423f3..3d7c5f41 100644 --- a/src/protocols/protocol_transaction_out_106.cpp +++ b/src/protocols/protocol_transaction_out_106.cpp @@ -203,13 +203,21 @@ void protocol_transaction_out_106::send_transaction(const code& ec, << " from [" << opposite() << "] not found."); // This tx could not have been advertised to the peer. - stop(system::error::not_found); + handle_unservable(item, index, message); return; } SEND(transaction{ ptr }, send_transaction, _1, add1(index), message); } +// not_found is undefined below bip37, so the channel is stopped instead. +void protocol_transaction_out_106::handle_unservable(const inventory_item&, + size_t, const get_data::cptr&) NOEXCEPT +{ + BC_ASSERT(stranded()); + stop(system::error::not_found); +} + BC_POP_WARNING() BC_POP_WARNING() diff --git a/src/protocols/protocol_transaction_out_70001.cpp b/src/protocols/protocol_transaction_out_70001.cpp new file mode 100644 index 00000000..76008da8 --- /dev/null +++ b/src/protocols/protocol_transaction_out_70001.cpp @@ -0,0 +1,60 @@ +/** + * Copyright (c) 2011-2026 libbitcoin developers + * + * This file is part of libbitcoin. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +#include + +#include + +namespace libbitcoin { +namespace node { + +#define CLASS protocol_transaction_out_70001 + +using namespace system; +using namespace network::messages::peer; +using namespace std::placeholders; + +// Shared pointers required for lifetime in handler parameters. +BC_PUSH_WARNING(SMART_PTR_NOT_NEEDED) +BC_PUSH_WARNING(NO_VALUE_OR_CONST_REF_SHARED_PTR) + +// Outbound (not_found). +// ---------------------------------------------------------------------------- + +// The item is answered and the send loop resumed, as with a transaction, so +// nothing is produced until the prior write completes. +void protocol_transaction_out_70001::handle_unservable( + const inventory_item& item, size_t index, + const get_data::cptr& message) NOEXCEPT +{ + BC_ASSERT(stranded()); + + if (!enable_not_found_) + { + protocol_transaction_out_106::handle_unservable(item, index, message); + return; + } + + SEND(not_found{ { item } }, send_transaction, _1, add1(index), message); +} + +BC_POP_WARNING() +BC_POP_WARNING() + +} // namespace node +} // namespace libbitcoin diff --git a/test/functional/p2p.cpp b/test/functional/p2p.cpp index 61fffefa..813dbbf3 100644 --- a/test/functional/p2p.cpp +++ b/test/functional/p2p.cpp @@ -58,4 +58,89 @@ BOOST_AUTO_TEST_CASE(functional_p2p__get_data__genesis_block__expected_bytes) BOOST_REQUIRE(payload == expected); } +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_block__not_found, + p2p_not_found_setup_fixture) +{ + BOOST_REQUIRE(handshake()); + + const get_data get{ { { inventory_item::type_id::block, system::one_hash } } }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); + BOOST_REQUIRE(message->items.front().hash == system::one_hash); +} + +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__pruned_block__not_found, + p2p_limited_setup_fixture) +{ + BOOST_REQUIRE(handshake()); + + const system::chain::block& genesis = config_.bitcoin.genesis_block; + const get_data get{ { { inventory_item::type_id::block, genesis.hash() } } }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); +} + +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unassociated_block__not_found, + p2p_unassociated_setup_fixture) +{ + BOOST_REQUIRE(handshake()); + + const auto hash = unassociated().hash(); + const get_data get{ { { inventory_item::type_id::block, hash } } }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); + BOOST_REQUIRE(message->items.front().hash == hash); +} + +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_transaction__not_found, + p2p_relay_setup_fixture) +{ + BOOST_REQUIRE(handshake(0, level::maximum_protocol, true)); + + const get_data get{ { { inventory_item::type_id::transaction, system::one_hash } } }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); + BOOST_REQUIRE(message->items.front().hash == system::one_hash); +} + +// not_found is undefined below bip37, so the channel is stopped instead. +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_block_106__stopped, + p2p_not_found_setup_fixture) +{ + BOOST_REQUIRE(handshake(0, level::bip35)); + + const get_data get{ { { inventory_item::type_id::block, system::one_hash } } }; + send(get, level::bip35); + + // The channel is stopped, so the socket closes without a not_found. + BOOST_REQUIRE_THROW(receive(not_found::command), boost::system::system_error); +} + +// The option is off by default, so the channel is stopped instead. +BOOST_AUTO_TEST_CASE(functional_p2p__get_data__unknown_block_disabled__stopped) +{ + BOOST_REQUIRE(handshake()); + + const get_data get{ { { inventory_item::type_id::block, system::one_hash } } }; + send(get, node_version->value); + + BOOST_REQUIRE_THROW(receive(not_found::command), boost::system::system_error); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/p2p_setup_fixture.cpp b/test/functional/p2p_setup_fixture.cpp index a4acf10d..ddfddb1f 100644 --- a/test/functional/p2p_setup_fixture.cpp +++ b/test/functional/p2p_setup_fixture.cpp @@ -134,7 +134,8 @@ data_chunk p2p_setup_fixture::receive(const std::string& command) } } -bool p2p_setup_fixture::handshake(uint64_t services, uint32_t value) +bool p2p_setup_fixture::handshake(uint64_t services, uint32_t value, + bool relay) { version out{}; out.value = value; @@ -143,7 +144,7 @@ bool p2p_setup_fixture::handshake(uint64_t services, uint32_t value) out.nonce = 42424242; out.user_agent = "/test/"; out.start_height = 0; - out.relay = false; + out.relay = relay; send(out, value); // The node sends its version upon attach and verack upon our version. @@ -170,4 +171,18 @@ bool p2p_setup_fixture::handshake(uint64_t services, uint32_t value) return true; } +system::chain::header p2p_unassociated_setup_fixture::unassociated() NOEXCEPT +{ + const system::settings bitcoin{ chain::selection::mainnet }; + return + { + 1u, + bitcoin.genesis_block.hash(), + system::null_hash, + 0u, + 0u, + 0u + }; +} + BC_POP_WARNING() diff --git a/test/functional/p2p_setup_fixture.hpp b/test/functional/p2p_setup_fixture.hpp index b4a7f80c..ae12f910 100644 --- a/test/functional/p2p_setup_fixture.hpp +++ b/test/functional/p2p_setup_fixture.hpp @@ -55,7 +55,8 @@ struct p2p_setup_fixture /// Perform the version handshake, retains the node's version message. bool handshake(uint64_t services=0, - uint32_t version=network::messages::peer::level::maximum_protocol); + uint32_t version=network::messages::peer::level::maximum_protocol, + bool relay=false); /// The node's version message (set by handshake). network::messages::peer::version::cptr node_version{}; @@ -72,4 +73,65 @@ struct p2p_setup_fixture boost::asio::ip::tcp::socket socket_{ io_ }; }; +// A node configured to reply not_found. +struct p2p_not_found_setup_fixture + : p2p_setup_fixture +{ + inline p2p_not_found_setup_fixture() + : p2p_setup_fixture({}, [](configuration& config) + { + config.network.enable_not_found = true; + }) + { + } +}; + +// A node that relays transactions and replies not_found. +struct p2p_relay_setup_fixture + : p2p_setup_fixture +{ + inline p2p_relay_setup_fixture() + : p2p_setup_fixture({}, [](configuration& config) + { + config.network.enable_relay = true; + config.network.enable_not_found = true; + }) + { + } +}; + +// A node that does not store the blocks it has pruned. +struct p2p_limited_setup_fixture + : p2p_setup_fixture +{ + inline p2p_limited_setup_fixture() + : p2p_setup_fixture({}, [](configuration& config) + { + config.node.limited_blocks = true; + config.network.enable_not_found = true; + }) + { + } +}; + +// A header is archived before its block is associated, so the header link +// resolves while the block remains absent from the archive. This is the +// steady state of headers-first synchronization. +struct p2p_unassociated_setup_fixture + : p2p_setup_fixture +{ + static system::chain::header unassociated() NOEXCEPT; + + inline p2p_unassociated_setup_fixture() + : p2p_setup_fixture([](node::query& query) + { + return query.set(unassociated(), database::context{}, false); + }, [](configuration& config) + { + config.network.enable_not_found = true; + }) + { + } +}; + #endif From 756eca7972dce7d2bc0d8574765322d3c183e57a Mon Sep 17 00:00:00 2001 From: Eric Voskuil Date: Thu, 10 Sep 2026 21:28:07 -0400 Subject: [PATCH 2/2] Batch not_found, demote unservable logging, order the protocol ladder. Report a run of unservable block requests with one not_found rather than one per item, testing servability with is_associated so that the block is not fetched twice at a run boundary. The run is accumulated by the 70001 protocol, which knows the message can carry more than one item; the 106 protocol stops the channel on the first, so the send loop asks before it continues and nothing is accumulated where nothing is reported. Move the per-item reason to verbose. The channel is no longer stopped, so an unservable request is ordinary peer input and the log is otherwise trivially overflowed. The stopping case logs once, in the override that decides to stop. Select the transaction protocol by negotiated version, as with blocks, rather than replacing the 106 protocol outright. That tier is not yet reachable, as version.relay is read as false below bip37, where the peer has no way to decline relay. Add the two protocol headers to node.hpp, and cover the 70001 protocol where it is attached directly rather than as the 70012 base. Co-Authored-By: Claude Opus 5 --- include/bitcoin/node.hpp | 2 + .../node/impl/sessions/session_peer.ipp | 11 +- .../node/protocols/protocol_block_out_106.hpp | 11 +- .../protocols/protocol_block_out_70001.hpp | 10 +- src/protocols/protocol_block_out_106.cpp | 111 ++++++++++++------ src/protocols/protocol_block_out_70001.cpp | 30 +++-- .../protocol_transaction_out_106.cpp | 11 +- test/functional/p2p.cpp | 65 ++++++++++ 8 files changed, 196 insertions(+), 55 deletions(-) diff --git a/include/bitcoin/node.hpp b/include/bitcoin/node.hpp index b7d31740..7ce08dfe 100644 --- a/include/bitcoin/node.hpp +++ b/include/bitcoin/node.hpp @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -59,6 +60,7 @@ #include #include #include +#include #include #include #include diff --git a/include/bitcoin/node/impl/sessions/session_peer.ipp b/include/bitcoin/node/impl/sessions/session_peer.ipp index c45216ee..d766ee94 100644 --- a/include/bitcoin/node/impl/sessions/session_peer.ipp +++ b/include/bitcoin/node/impl/sessions/session_peer.ipp @@ -107,8 +107,7 @@ inline void CLASS::attach_protocols(const channel_ptr& channel) NOEXCEPT return; // Ready to relay transactions. - const auto txs_in_out = relay && peer->is_negotiated(level::bip37) && - (!delay || is_current_chain(true)); + const auto txs_in_out = relay && (!delay || is_current_chain(true)); // Peer advertises chain (blocks in). if (peer->is_peer_service(service::node_network)) @@ -145,7 +144,6 @@ inline void CLASS::attach_protocols(const channel_ptr& channel) NOEXCEPT if (headers && peer->is_negotiated(level::headers_protocol)) channel->attach(self)->start(); - // not_found is defined at bip37. if (peer->is_negotiated(level::bip37)) channel->attach(self)->start(); else @@ -154,11 +152,12 @@ inline void CLASS::attach_protocols(const channel_ptr& channel) NOEXCEPT } // Relay is configured, active, and txs are ready (txs in/out). - if (txs_in_out) + if (txs_in_out && peer->peer_version()->relay) { - // Attached above bip37, where not_found is defined. - if (peer->peer_version()->relay) + if (peer->is_negotiated(level::bip37)) channel->attach(self)->start(); + else + channel->attach(self)->start(); } } diff --git a/include/bitcoin/node/protocols/protocol_block_out_106.hpp b/include/bitcoin/node/protocols/protocol_block_out_106.hpp index df64b055..7e3c8f96 100644 --- a/include/bitcoin/node/protocols/protocol_block_out_106.hpp +++ b/include/bitcoin/node/protocols/protocol_block_out_106.hpp @@ -55,6 +55,7 @@ class BCN_API protocol_block_out_106 using get_data = network::messages::peer::get_data; using get_blocks = network::messages::peer::get_blocks; using inventory_item = network::messages::peer::inventory_item; + using inventory_items = network::messages::peer::inventory_items; /// Block announcements are superseded by send_headers. virtual bool superseded() const NOEXCEPT; @@ -72,13 +73,17 @@ class BCN_API protocol_block_out_106 const get_data::cptr& message) NOEXCEPT; virtual void send_block(const code& ec) NOEXCEPT; - /// The requested item cannot be served, stops the channel. - virtual void handle_unservable(const inventory_item& item) NOEXCEPT; + /// The item cannot be served, stops the channel and returns false. + virtual bool handle_unservable(const inventory_item& item) NOEXCEPT; + + /// Send any unservable items accumulated above, false if none. + virtual bool report_unservable() NOEXCEPT; private: using inventory = network::messages::peer::inventory; - using inventory_items = network::messages::peer::inventory_items; + bool is_servable(const inventory_item& item, + const database::header_link& link) NOEXCEPT; bool is_under_checkpoint(const database::header_link& link) NOEXCEPT; inventory create_inventory(const get_blocks& locator) const NOEXCEPT; void merge_inventory(const inventory_items& items) NOEXCEPT; diff --git a/include/bitcoin/node/protocols/protocol_block_out_70001.hpp b/include/bitcoin/node/protocols/protocol_block_out_70001.hpp index b764bca6..09104759 100644 --- a/include/bitcoin/node/protocols/protocol_block_out_70001.hpp +++ b/include/bitcoin/node/protocols/protocol_block_out_70001.hpp @@ -41,12 +41,18 @@ class BCN_API protocol_block_out_70001 } protected: - /// The requested item cannot be served, replies not_found. - void handle_unservable(const inventory_item& item) NOEXCEPT override; + /// The item cannot be served, accumulates it for the not_found reply. + bool handle_unservable(const inventory_item& item) NOEXCEPT override; + + /// Replies not_found with the accumulated items, false if none. + bool report_unservable() NOEXCEPT override; private: // This is thread safe. const bool enable_not_found_; + + // This is protected by strand. + inventory_items unservable_{}; }; } // namespace node diff --git a/src/protocols/protocol_block_out_106.cpp b/src/protocols/protocol_block_out_106.cpp index 07e0d01b..aaad09c7 100644 --- a/src/protocols/protocol_block_out_106.cpp +++ b/src/protocols/protocol_block_out_106.cpp @@ -204,56 +204,55 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT if (stopped(ec)) return; - if (backlog_.empty()) return; - - // Copied because the item is answered after the backlog is popped. - const auto item = backlog_.front(); - const auto witness = item.is_witness_type(); - if (witness && !node_witness_) - { - LOGR("Unsupported witness get_data from [" << opposite() << "]."); - stop(network::error::protocol_violation); - return; - } - const auto& query = archive(); - const auto link = query.to_header(item.hash); - // A hash that resolves to no header is ordinary peer input, and the - // checkpoint height query faults the store on a terminal link. - if (link.is_terminal()) + // Drain unservable items from the front of the backlog. The derived + // protocol accumulates them if it reports them, and otherwise stops the + // channel on the first. Copied because each is handled after the pop. + database::header_link link{}; + while (!backlog_.empty()) { - LOGR("Requested block " << encode_hash(item.hash) << " from [" - << opposite() << "] not stored."); + const auto item = backlog_.front(); + if (item.is_witness_type() && !node_witness_) + { + LOGR("Unsupported witness get_data from [" << opposite() << "]."); + stop(network::error::protocol_violation); + return; + } + + link = query.to_header(item.hash); + if (is_servable(item, link)) + break; backlog_.pop_front(); - handle_unservable(item); - return; + if (!handle_unservable(item)) + return; } - if (node_pruned_ && (is_under_checkpoint(link) || query.is_milestone(link))) - { - LOGR("Requested pruned block " << encode_hash(item.hash) - << " from [" << opposite() << "]."); - - backlog_.pop_front(); - handle_unservable(item); + // The report resumes this loop on completion, so it precedes the block. + if (report_unservable()) return; - } + if (backlog_.empty()) return; + + const auto item = backlog_.front(); + const auto witness = item.is_witness_type(); const auto start = logger::now(); messages::peer::block out { { query.get_wire_block(link, witness), witness } }; + + // Association is verified above, so this is not ordinary peer input. if (!out.block.is_valid()) { - LOGR("Requested block " << encode_hash(item.hash) << " from [" - << opposite() << "] not found."); + LOGV("Requested block " << encode_hash(item.hash) << " from [" + << opposite() << "] not obtained."); - // This block could not have been advertised to the peer. backlog_.pop_front(); - handle_unservable(item); + if (handle_unservable(item)) + report_unservable(); + return; } @@ -262,11 +261,57 @@ void protocol_block_out_106::send_block(const code& ec) NOEXCEPT SEND(std::move(out), send_block, _1); } +// The checkpoint, milestone and association queries assume an archived header. +bool protocol_block_out_106::is_servable(const inventory_item& LOG_ONLY(item), + const database::header_link& link) NOEXCEPT +{ + BC_ASSERT(stranded()); + + // A hash that resolves to no header is ordinary peer input. + if (link.is_terminal()) + { + LOGV("Requested block " << encode_hash(item.hash) << " from [" + << opposite() << "] not stored."); + return false; + } + + const auto& query = archive(); + if (node_pruned_ && (is_under_checkpoint(link) || query.is_milestone(link))) + { + LOGV("Requested pruned block " << encode_hash(item.hash) + << " from [" << opposite() << "]."); + return false; + } + + // This block could not have been advertised to the peer. + if (!query.is_associated(link)) + { + LOGV("Requested block " << encode_hash(item.hash) << " from [" + << opposite() << "] not found."); + return false; + } + + return true; +} + // not_found is undefined below bip37, so the channel is stopped instead. -void protocol_block_out_106::handle_unservable(const inventory_item&) NOEXCEPT +bool protocol_block_out_106::handle_unservable( + const inventory_item& LOG_ONLY(item)) NOEXCEPT { BC_ASSERT(stranded()); + + LOGR("Unservable block " << encode_hash(item.hash) << " from [" + << opposite() << "], stopping."); + stop(system::error::not_found); + return false; +} + +// There is nothing to report below bip37, the channel is stopped above. +bool protocol_block_out_106::report_unservable() NOEXCEPT +{ + BC_ASSERT(stranded()); + return false; } // utilities diff --git a/src/protocols/protocol_block_out_70001.cpp b/src/protocols/protocol_block_out_70001.cpp index 69710684..06bc39b0 100644 --- a/src/protocols/protocol_block_out_70001.cpp +++ b/src/protocols/protocol_block_out_70001.cpp @@ -30,26 +30,40 @@ using namespace network; using namespace network::messages::peer; using namespace std::placeholders; +// Shared pointers required for lifetime in handler parameters. BC_PUSH_WARNING(SMART_PTR_NOT_NEEDED) BC_PUSH_WARNING(NO_VALUE_OR_CONST_REF_SHARED_PTR) // Outbound (not_found). // ---------------------------------------------------------------------------- -// The item is answered and the send loop resumed, as with a block, so nothing -// is produced until the prior write completes. -void protocol_block_out_70001::handle_unservable( +// Accumulate the run, so that it is reported by one message. +bool protocol_block_out_70001::handle_unservable( const inventory_item& item) NOEXCEPT { BC_ASSERT(stranded()); if (!enable_not_found_) - { - protocol_block_out_106::handle_unservable(item); - return; - } + return protocol_block_out_106::handle_unservable(item); - SEND(not_found{ { item } }, send_block, _1); + unservable_.push_back(item); + return true; +} + +// The items are answered and the send loop resumed, as with a block, so +// nothing is produced until the prior write completes. +bool protocol_block_out_70001::report_unservable() NOEXCEPT +{ + BC_ASSERT(stranded()); + + if (unservable_.empty()) + return false; + + auto items = std::move(unservable_); + unservable_.clear(); + + SEND(not_found{ std::move(items) }, send_block, _1); + return true; } BC_POP_WARNING() diff --git a/src/protocols/protocol_transaction_out_106.cpp b/src/protocols/protocol_transaction_out_106.cpp index 3d7c5f41..7d0d8e57 100644 --- a/src/protocols/protocol_transaction_out_106.cpp +++ b/src/protocols/protocol_transaction_out_106.cpp @@ -199,7 +199,7 @@ void protocol_transaction_out_106::send_transaction(const code& ec, const auto ptr = query.get_transaction(query.to_tx(item.hash), witness); if (!ptr) { - LOGR("Requested tx " << encode_hash(item.hash) + LOGV("Requested tx " << encode_hash(item.hash) << " from [" << opposite() << "] not found."); // This tx could not have been advertised to the peer. @@ -211,10 +211,15 @@ void protocol_transaction_out_106::send_transaction(const code& ec, } // not_found is undefined below bip37, so the channel is stopped instead. -void protocol_transaction_out_106::handle_unservable(const inventory_item&, - size_t, const get_data::cptr&) NOEXCEPT +void protocol_transaction_out_106::handle_unservable( + const inventory_item& LOG_ONLY(item), size_t, + const get_data::cptr&) NOEXCEPT { BC_ASSERT(stranded()); + + LOGR("Unservable tx " << encode_hash(item.hash) << " from [" + << opposite() << "], stopping."); + stop(system::error::not_found); } diff --git a/test/functional/p2p.cpp b/test/functional/p2p.cpp index 813dbbf3..b56fe1cc 100644 --- a/test/functional/p2p.cpp +++ b/test/functional/p2p.cpp @@ -119,6 +119,71 @@ BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_transaction__not_found BOOST_REQUIRE(message->items.front().hash == system::one_hash); } +// Below bip130 the 70001 protocol is attached directly (not as 70012 base). +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_block_70001__not_found, + p2p_not_found_setup_fixture) +{ + BOOST_REQUIRE(handshake(0, level::bip61)); + + const get_data get{ { { inventory_item::type_id::block, system::one_hash } } }; + send(get, level::bip61); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(level::bip61, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); + BOOST_REQUIRE(message->items.front().hash == system::one_hash); +} + +// A run of unservable items is answered by one message. +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_blocks__one_not_found, + p2p_not_found_setup_fixture) +{ + BOOST_REQUIRE(handshake()); + + const get_data get + { + { + { inventory_item::type_id::block, system::one_hash }, + { inventory_item::type_id::block, system::null_hash } + } + }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), two); + BOOST_REQUIRE(message->items.front().hash == system::one_hash); + BOOST_REQUIRE(message->items.back().hash == system::null_hash); +} + +// The unservable run is flushed before the send loop resumes. +BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_then_genesis__not_found_then_block, + p2p_not_found_setup_fixture) +{ + BOOST_REQUIRE(handshake()); + + const system::chain::block& genesis = config_.bitcoin.genesis_block; + const get_data get + { + { + { inventory_item::type_id::block, system::one_hash }, + { inventory_item::type_id::block, genesis.hash() } + } + }; + send(get, node_version->value); + + const auto payload = receive(not_found::command); + const auto message = not_found::deserialize(node_version->value, payload); + BOOST_REQUIRE(message); + BOOST_REQUIRE_EQUAL(message->items.size(), one); + BOOST_REQUIRE(message->items.front().hash == system::one_hash); + + // The send loop resumes and serves the item that follows the run. + BOOST_REQUIRE_EQUAL(receive(block::command).size(), genesis.to_data(true).size()); +} + // not_found is undefined below bip37, so the channel is stopped instead. BOOST_FIXTURE_TEST_CASE(functional_p2p__get_data__unknown_block_106__stopped, p2p_not_found_setup_fixture)