diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 2b696f56f8..284fc9cdb7 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -112,6 +112,7 @@ test.jtx > xrpl.config test.jtx > xrpl.core test.jtx > xrpld.app test.jtx > xrpld.core +test.jtx > xrpld.overlay test.jtx > xrpld.rpc test.jtx > xrpl.json test.jtx > xrpl.ledger diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index 0853affab7..bb0c790e37 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -21,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -29,19 +29,14 @@ #include #include #include -#include #include #include #include #include #include -#include -#include #include -#include #include #include -#include #include #include @@ -268,136 +263,33 @@ enum class PeerFeature { * Simulate a network peer. * Depending on the configured PeerFeature, * it either supports the ProtocolFeature::LedgerReplay or not + * + * `PeerStub` supplies the rest of the `Peer` interface as no-ops. */ -class TestPeer : public Peer +class TestPeer : public PeerStub { public: - TestPeer(bool enableLedgerReplay) - : ledgerReplayEnabled_(enableLedgerReplay) - , nodePublicKey_(derivePublicKey(KeyType::Ed25519, randomSecretKey())) + // Arbitrary but fixed: the replay code only compares ids. + explicit TestPeer(bool enableLedgerReplay) + : PeerStub(1234), ledgerReplayEnabled_(enableLedgerReplay) { } - void - send(std::shared_ptr const& m) override - { - } - [[nodiscard]] beast::ip::Endpoint - getRemoteAddress() const override - { - return {}; - } - void - charge(resource::Charge const& fee, std::string const& context = {}) override - { - } - [[nodiscard]] id_t - id() const override - { - return 1234; - } - [[nodiscard]] bool - cluster() const override - { - return false; - } - [[nodiscard]] bool - isHighLatency() const override - { - return false; - } - [[nodiscard]] int - getScore(bool) const override - { - return 0; - } - [[nodiscard]] PublicKey const& - getNodePublic() const override - { - return nodePublicKey_; - } - json::Value - json() override - { - return {}; - } [[nodiscard]] bool supportsFeature(ProtocolFeature f) const override { return f == ProtocolFeature::LedgerReplay && ledgerReplayEnabled_; } - [[nodiscard]] std::optional - publisherListSequence(PublicKey const&) const override - { - return {}; - } - void - setPublisherListSequence(PublicKey const&, std::size_t const) override - { - } - [[nodiscard]] uint256 - getClosedLedgerHash() const override - { - static uint256 const kHash{}; - return kHash; - } + + // The replay code only asks peers that already have the ledger. [[nodiscard]] bool - hasLedger(uint256 const& hash, std::uint32_t seq) const override + hasLedger(uint256 const&, std::uint32_t) const override { return true; } - void - ledgerRange(std::uint32_t& minSeq, std::uint32_t& maxSeq) const override - { - } - [[nodiscard]] bool - hasTxSet(uint256 const& hash) const override - { - return false; - } - void - cycleStatus() override - { - } - bool - hasRange(std::uint32_t uMin, std::uint32_t uMax) override - { - return false; - } - [[nodiscard]] bool - compressionEnabled() const override - { - return false; - } - void - sendTxQueue() override - { - } - void - addTxQueue(uint256 const&) override - { - } - void - removeTxQueue(uint256 const&) override - { - } - [[nodiscard]] bool - txReduceRelayEnabled() const override - { - return false; - } - [[nodiscard]] std::string const& - fingerprint() const override - { - return fingerprint_; - } - - // NOLINTBEGIN(readability-identifier-naming) - std::string fingerprint_; +private: bool ledgerReplayEnabled_; - PublicKey nodePublicKey_; - // NOLINTEND(readability-identifier-naming) }; enum class PeerSetBehavior { diff --git a/src/test/jtx/PeerStub.h b/src/test/jtx/PeerStub.h new file mode 100644 index 0000000000..cabe94f351 --- /dev/null +++ b/src/test/jtx/PeerStub.h @@ -0,0 +1,185 @@ +#pragma once + +#include +#include + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace xrpl::test { + +/** + * A `Peer` whose every method is a no-op returning a default. + * + * Derive from this and override only the methods a test cares about. Adding a + * method to `Peer` then costs one stub here, not one per double. + * + * The id and the node public key are real, because the code under test routes + * and deduplicates on both. + */ +class PeerStub : public Peer +{ +public: + /** + * @param id The connection id reported by `id()`. + */ + explicit PeerStub(id_t id = 0) + : id_(id), nodePublicKey_(derivePublicKey(KeyType::Ed25519, randomSecretKey())) + { + } + + ~PeerStub() override = default; + + void + send(std::shared_ptr const&) override + { + } + + [[nodiscard]] beast::ip::Endpoint + getRemoteAddress() const override + { + return {}; + } + + void + sendTxQueue() override + { + } + + void + addTxQueue(uint256 const&) override + { + } + + void + removeTxQueue(uint256 const&) override + { + } + + void + charge(resource::Charge const&, std::string const&) override + { + } + + [[nodiscard]] id_t + id() const override + { + return id_; + } + + [[nodiscard]] bool + cluster() const override + { + return false; + } + + [[nodiscard]] bool + isHighLatency() const override + { + return false; + } + + [[nodiscard]] int + getScore(bool) const override + { + return 0; + } + + [[nodiscard]] PublicKey const& + getNodePublic() const override + { + return nodePublicKey_; + } + + json::Value + json() override + { + return {}; + } + + [[nodiscard]] bool + supportsFeature(ProtocolFeature) const override + { + return false; + } + + [[nodiscard]] std::optional + publisherListSequence(PublicKey const&) const override + { + return {}; + } + + void + setPublisherListSequence(PublicKey const&, std::size_t const) override + { + } + + [[nodiscard]] std::string const& + fingerprint() const override + { + return fingerprint_; + } + + [[nodiscard]] uint256 + getClosedLedgerHash() const override + { + return {}; + } + + [[nodiscard]] bool + hasLedger(uint256 const&, std::uint32_t) const override + { + return false; + } + + void + ledgerRange(std::uint32_t&, std::uint32_t&) const override + { + } + + [[nodiscard]] bool + hasTxSet(uint256 const&) const override + { + return false; + } + + void + cycleStatus() override + { + } + + bool + hasRange(std::uint32_t, std::uint32_t) override + { + return false; + } + + [[nodiscard]] bool + compressionEnabled() const override + { + return false; + } + + [[nodiscard]] bool + txReduceRelayEnabled() const override + { + return false; + } + +private: + id_t const id_; + PublicKey const nodePublicKey_; + std::string const fingerprint_; +}; + +} // namespace xrpl::test diff --git a/src/test/overlay/CapturePeer.h b/src/test/overlay/CapturePeer.h new file mode 100644 index 0000000000..28a26d01e7 --- /dev/null +++ b/src/test/overlay/CapturePeer.h @@ -0,0 +1,252 @@ +#pragma once + +#include + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl::test { + +/** + * A real `PeerImp` that captures the messages it would have sent. + * + * Only `send` and `run` are overridden, so `onMessage` runs production code. + * Derive from this to reach a `protected` `PeerImp` member. + */ +class CapturePeer : public PeerImp +{ +public: + using MiddleType = boost::beast::tcp_stream; + using StreamType = boost::beast::ssl_stream; + using SocketType = boost::asio::ip::tcp::socket; + + /** + * Takes `PeerImp`'s two rvalue-reference parameters by value instead, so a + * derived double can inherit this constructor without a never-moved-from + * warning. + * + * @param app The application owning the peer. + * @param id The connection id, unique among the overlay's peers. + * @param slot The peer finder slot; must be seated. + * @param request The handshake request. + * @param publicKey The peer's node public key. + * @param protocol The negotiated protocol version. + * @param consumer The resource manager endpoint for the peer. + * @param streamPtr The connection's ssl stream. + * @param overlay The overlay to register with. + */ + CapturePeer( + Application& app, + Peer::id_t id, + std::shared_ptr const& slot, + http_request_type request, + PublicKey const& publicKey, + ProtocolVersion protocol, + resource::Consumer consumer, + std::unique_ptr streamPtr, + OverlayImpl& overlay) + : PeerImp( + app, + id, + slot, + std::move(request), + publicKey, + protocol, + consumer, // copy-only, so `std::move` would be a copy anyway + std::move(streamPtr), + overlay) + { + } + + ~CapturePeer() override = default; + + /** + * Does nothing, so the peer stays registered. The real `run()` reaches + * `PeerImp::doAccept`, which fails on an unconnected socket and detaches. + */ + void + run() override + { + } + + /** + * Captures the message instead of writing it, so replies are observable. + */ + void + send(std::shared_ptr const& m) override + { + sent_.push_back(m); + } + + /** + * @return Every message sent to this peer, in order. + */ + std::vector> const& + sent() const + { + return sent_; + } + + /** + * @return The most recent message sent, or null if there was none. + */ + std::shared_ptr + lastSent() const + { + return sent_.empty() ? nullptr : sent_.back(); + } + + /** + * Reads the accumulated charge without draining it through `charge()`. + * + * @return The charge accumulated on the peer so far. + */ + resource::Charge + feeCharge() const + { + return currentFeeCharge(); + } + +private: + std::vector> sent_; +}; + +namespace detail { + +// `inline` so the functions below name one entity across translation units. +inline constexpr std::uint16_t kCapturePeerPort = 51235; + +/** + * Non-template, so all `makeCapturePeer` instantiations share one counter. A + * per-instantiation counter would give two peer types the same id, and + * `addActive` would silently drop the second from `ids_`. + * + * @return The next unused connection id. + */ +inline Peer::id_t +nextCapturePeerId() +{ + static Peer::id_t id{0}; + return ++id; +} + +/** + * Non-template for the same reason as `nextCapturePeerId`. Each peer needs its + * own address, not just its own port: the peer finder caps inbound connections + * per address at `ipLimit`, which is at most 2 unless configured. + * + * @return The next unused remote endpoint. + */ +inline beast::ip::Endpoint +nextCapturePeerRemote() +{ + // From 172.2.0.1 upward, so ~900k fit before reaching 172.16/12, where the + // peer finder would treat them as private rather than as real inbound. + static std::uint32_t next{0xAC020001}; + return beast::ip::Endpoint(boost::asio::ip::address_v4(next++), kCapturePeerPort); +} + +/** + * Outlives every peer, whose stream keeps a reference to it. `PeerImp::charge` + * posts a handler holding the peer, so a peer can outlive its caller's scope. + * + * @return The ssl context every test peer's stream is built on. + */ +inline boost::asio::ssl::context& +capturePeerSslContext() +{ + static std::shared_ptr const kContext{makeSslContext("")}; + return *kContext; +} + +} // namespace detail + +/** + * Build an active `CapturePeer` and register it with the overlay. + * + * @tparam PeerType The peer class to build; must derive from `CapturePeer` and + * inherit its constructor. + * @param env The environment owning the overlay. + * @param key The peer's node public key, or unseated for a fresh random + * one. + * @param request The handshake request. `PeerImp` reads its `X-Protocol-Ctl` + * header in the constructor to negotiate features. + * @return The peer, already registered with the overlay. Throws if the peer + * finder refused a slot. + */ +template +std::shared_ptr +makeCapturePeer( + jtx::Env& env, + std::optional key = std::nullopt, + http_request_type request = {}) +{ + auto& overlay = dynamic_cast(env.app().getOverlay()); + auto streamPtr = std::make_unique( + CapturePeer::SocketType(env.app().getIOContext()), detail::capturePeerSslContext()); + + beast::ip::Endpoint const local( + boost::asio::ip::make_address("172.1.1.1"), detail::kCapturePeerPort); + auto const remote = detail::nextCapturePeerRemote(); + + auto consumer = overlay.resourceManager().newInboundEndpoint(remote); + auto [slot, _] = overlay.peerFinder().newInboundSlot(local, remote); + + // Unseated when the endpoint is already connected or at the per-address + // limit. `PeerImp` dereferences the slot, so fail here, not there. + if (!slot) + { + Throw("makeCapturePeer: no slot for " + to_string(remote)); + } + + if (!key) + key = PublicKey(std::get<0>(randomKeyPair(KeyType::Ed25519))); + + auto peer = std::make_shared( + env.app(), + detail::nextCapturePeerId(), + slot, + std::move(request), + *key, + // An unsupported version fails every `supportsFeature` test, so a + // version-gated reply would only ever take its legacy branch. + newestSupportedProtocolVersion(), + consumer, + std::move(streamPtr), + overlay); + + overlay.addActive(peer); + return peer; +} + +} // namespace xrpl::test diff --git a/src/test/overlay/PeerTest.cpp b/src/test/overlay/PeerTest.cpp deleted file mode 100644 index 341febb25b..0000000000 --- a/src/test/overlay/PeerTest.cpp +++ /dev/null @@ -1,166 +0,0 @@ -#include - -#include - -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -#include - -#include -#include -#include -#include - -namespace xrpl::test { - -PeerTest::PeerTest( - Application& app, - std::shared_ptr const& slot, - http_request_type&& request, - PublicKey const& publicKey, - ProtocolVersion protocol, - resource::Consumer consumer, - std::unique_ptr&& streamPtr, - OverlayImpl& overlay) - : PeerImp{ - app, - id++, - slot, - std::move(request), - publicKey, - protocol, - consumer, - std::move(streamPtr), - overlay} -{ -} - -void -PeerTest::run() -{ -} - -void -PeerTest::send(std::shared_ptr const& message) -{ - lastSentMessage_ = message; -} - -std::shared_ptr -PeerTest::getLastSentMessage() const -{ - return lastSentMessage_; -} - -void -PeerTest::runProcessGetObjectByHash(std::shared_ptr const& message) -{ - PeerImp::processGetObjectByHash(message); -} - -void -PeerTest::runProcessLedgerRequest( - std::shared_ptr const& message, - std::vector nodeIDs) -{ - PeerImp::processLedgerRequest(message, std::move(nodeIDs)); -} - -resource::Charge -PeerTest::getCurrentFeeCharge() const -{ - return PeerImp::currentFeeCharge(); -} - -void -PeerTest::resetId() -{ - id = 0; -} - -bool -PeerTest::compressionEnabled() const -{ - if (compressionEnabled_.has_value()) - { - return *compressionEnabled_; - } - return PeerImp::compressionEnabled(); -} - -void -PeerTest::compressionEnabled(std::optional enabled) -{ - compressionEnabled_ = enabled; -} - -bool -PeerTest::txReduceRelayEnabled() const -{ - if (reduceRelayEnabled_.has_value()) - { - return *reduceRelayEnabled_; - } - return PeerImp::txReduceRelayEnabled(); -} - -void -PeerTest::txReduceRelayEnabled(std::optional enabled) -{ - reduceRelayEnabled_ = enabled; -} - -std::shared_ptr -makePeerTest(jtx::Env& env, PeerTest::SharedContext const& context, ProtocolVersion protocolVersion) -{ - using SocketType = boost::asio::ip::tcp::socket; - - auto& overlay = dynamic_cast(env.app().getOverlay()); - boost::beast::http::request request; - auto streamPtr = - std::make_unique(SocketType(env.app().getIOContext()), *context); - - beast::ip::Endpoint const local(boost::asio::ip::make_address("172.1.1.1"), 51235); - beast::ip::Endpoint const remote(boost::asio::ip::make_address("172.1.1.2"), 51235); - - PublicKey const key{std::get<0>(randomKeyPair(KeyType::Ed25519))}; - auto consumer = overlay.resourceManager().newInboundEndpoint(remote); - auto [slot, _] = overlay.peerFinder().newInboundSlot(local, remote); - - auto peer = std::make_shared( - env.app(), - slot, - std::move(request), - key, - protocolVersion, - consumer, - std::move(streamPtr), - overlay); - - overlay.addActive(peer); - return peer; -} - -} // namespace xrpl::test diff --git a/src/test/overlay/PeerTest.h b/src/test/overlay/PeerTest.h deleted file mode 100644 index f7b2815da3..0000000000 --- a/src/test/overlay/PeerTest.h +++ /dev/null @@ -1,108 +0,0 @@ -#pragma once - -#include - -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -#include - -#include -#include -#include - -namespace xrpl::test { - -/** - * Test peer that captures sent messages for verification. - */ -class PeerTest : public PeerImp -{ - inline static Peer::id_t id{}; - std::shared_ptr lastSentMessage_; - std::optional compressionEnabled_; - std::optional reduceRelayEnabled_; - -public: - using MiddleType = boost::beast::tcp_stream; - using SharedContext = std::shared_ptr; - using StreamType = boost::beast::ssl_stream; - - PeerTest( - Application& app, - std::shared_ptr const& slot, - http_request_type&& request, - PublicKey const& publicKey, - ProtocolVersion protocol, - resource::Consumer consumer, - std::unique_ptr&& streamPtr, - OverlayImpl& overlay); - - ~PeerTest() override = default; - - void - run() override; - - void - send(std::shared_ptr const& m) override; - - std::shared_ptr - getLastSentMessage() const; - - // Synchronous test access to the JobQueue-dispatched processor. - // The production path runs this on JtLedgerReq; tests need a - // synchronous entry point to inspect the reply via send(). - // PeerImp::processGetObjectByHash is `protected` so the derived - // test subclass can call it directly. - void - runProcessGetObjectByHash(std::shared_ptr const& m); - - void - runProcessLedgerRequest( - std::shared_ptr const& m, - std::vector nodeIDs); - - resource::Charge - getCurrentFeeCharge() const; - - static void - resetId(); - - bool - compressionEnabled() const override; - - void - compressionEnabled(std::optional enabled); - - bool - txReduceRelayEnabled() const override; - - void - txReduceRelayEnabled(std::optional enabled); -}; - -std::shared_ptr -makePeerTest( - jtx::Env& env, - PeerTest::SharedContext const& context, - ProtocolVersion protocolVersion); - -} // namespace xrpl::test diff --git a/src/test/overlay/TMGetLedger_test.cpp b/src/test/overlay/TMGetLedger_test.cpp index 9088e6fa65..eac1a24e24 100644 --- a/src/test/overlay/TMGetLedger_test.cpp +++ b/src/test/overlay/TMGetLedger_test.cpp @@ -1,30 +1,19 @@ #include -#include +#include #include -#include -#include -#include -#include #include #include -#include #include -#include #include #include -#include -#include -#include -#include -#include - #include #include #include +#include #include namespace xrpl::test { @@ -33,8 +22,23 @@ using namespace jtx; class TMGetLedger_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; + /** + * Calls the JtLedgerReq-dispatched processor synchronously, so the reply is + * visible through `lastSent()`. + */ + class GetLedgerPeer : public CapturePeer + { + public: + using CapturePeer::CapturePeer; + + void + runProcessLedgerRequest( + std::shared_ptr const& m, + std::vector nodeIDs) + { + processLedgerRequest(m, std::move(nodeIDs)); + } + }; // Build a well-formed TMGetLedger node request carrying `numNodeIds` node // IDs. @@ -64,17 +68,16 @@ class TMGetLedger_test : public beast::unit_test::Suite testcase("Node ID Count Accepted"); Env env{*this}; - PeerTest::resetId(); - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = makeCapturePeer(env); peer->onMessage(createRequest(numNodeIds)); // A request outside the accepted node-ID count is charged kFeeInvalidData; one inside // it is not. The JobQueue handler may run concurrently and update the fee in the // accepted case. BEAST_EXPECT( - expectRejected ? (peer->getCurrentFeeCharge() == resource::kFeeInvalidData) - : !(peer->getCurrentFeeCharge() == resource::kFeeInvalidData)); + expectRejected ? (peer->feeCharge() == resource::kFeeInvalidData) + : !(peer->feeCharge() == resource::kFeeInvalidData)); } void @@ -84,9 +87,8 @@ class TMGetLedger_test : public beast::unit_test::Suite Env env{*this}; env.close(); - PeerTest::resetId(); - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = makeCapturePeer(env); // Ask for the account-state root node of the closed ledger. auto request = createRequest(numNodeIds); @@ -96,7 +98,7 @@ class TMGetLedger_test : public beast::unit_test::Suite peer->runProcessLedgerRequest(request, std::vector(numNodeIds)); - auto sentMessage = peer->getLastSentMessage(); + auto sentMessage = peer->lastSent(); BEAST_EXPECT(sentMessage != nullptr); if (!sentMessage) { diff --git a/src/test/overlay/TMGetObjectByHash_test.cpp b/src/test/overlay/TMGetObjectByHash_test.cpp index 84495fec37..1aafd77f2f 100644 --- a/src/test/overlay/TMGetObjectByHash_test.cpp +++ b/src/test/overlay/TMGetObjectByHash_test.cpp @@ -1,26 +1,17 @@ #include -#include +#include #include #include #include -#include -#include #include #include #include -#include #include #include #include -#include -#include -#include -#include -#include - #include #include @@ -41,8 +32,21 @@ using namespace jtx; */ class TMGetObjectByHash_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; + /** + * Calls the JtLedgerReq-dispatched processor synchronously, so the reply is + * visible through `sent()`. + */ + class GetObjectPeer : public CapturePeer + { + public: + using CapturePeer::CapturePeer; + + void + runProcessGetObjectByHash(std::shared_ptr const& m) + { + processGetObjectByHash(m); + } + }; static std::shared_ptr createRequest(size_t const numObjects, Env& env) @@ -90,15 +94,13 @@ class TMGetObjectByHash_test : public beast::unit_test::Suite testcase("Reply Object Count"); Env env(*this); - PeerTest::resetId(); - - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = makeCapturePeer(env); auto request = createRequest(numObjects, env); peer->runProcessGetObjectByHash(request); // Verify that a reply was sent - auto sentMessage = peer->getLastSentMessage(); + auto sentMessage = peer->lastSent(); BEAST_EXPECT(sentMessage != nullptr); // Parse the reply message diff --git a/src/test/overlay/TMTransaction_test.cpp b/src/test/overlay/TMTransaction_test.cpp index b208b3d81b..5a23e25005 100644 --- a/src/test/overlay/TMTransaction_test.cpp +++ b/src/test/overlay/TMTransaction_test.cpp @@ -1,21 +1,10 @@ #include #include -#include +#include -#include -#include -#include - -#include #include #include -#include -#include -#include -#include -#include - #include #include @@ -26,18 +15,14 @@ using namespace jtx; class TMTransaction_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; - void testFailureDeserializingTransactionIsCharged() { testcase("Undeserializable Transaction Is Charged"); Env env{*this, envconfig()}; - PeerTest::resetId(); - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = makeCapturePeer(env); auto tx = std::make_shared(); tx->set_status(protocol::tsNEW); @@ -45,7 +30,7 @@ class TMTransaction_test : public beast::unit_test::Suite tx->set_rawtransaction("\x01\x02\x03", 3); peer->onMessage(tx); - BEAST_EXPECT(peer->getCurrentFeeCharge() == resource::kFeeInvalidData); + BEAST_EXPECT(peer->feeCharge() == resource::kFeeInvalidData); } void diff --git a/src/test/overlay/TMTransactions_test.cpp b/src/test/overlay/TMTransactions_test.cpp index 67d36cc04b..87c09498f2 100644 --- a/src/test/overlay/TMTransactions_test.cpp +++ b/src/test/overlay/TMTransactions_test.cpp @@ -1,28 +1,21 @@ #include #include #include -#include +#include #include -#include -#include -#include -#include +#include -#include #include #include - -#include -#include -#include -#include -#include +#include #include #include #include +#include +#include namespace xrpl::test { @@ -30,9 +23,6 @@ using namespace jtx; class TMTransactions_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; - static std::shared_ptr createRequest(std::size_t const numTransactions) { @@ -55,13 +45,17 @@ class TMTransactions_test : public beast::unit_test::Suite *this, envconfig(), std::make_unique(kLimitExceededMessage, &foundExpectedLog)}; - PeerTest::resetId(); - auto peer = makePeerTest(env, context_, protocolVersion_); - peer->txReduceRelayEnabled(true); + // `PeerImp` decides `txReduceRelayEnabled()` in its constructor, from + // the config and the handshake header, so set this first. + env.app().config().txReduceRelayEnable = true; + http_request_type request; + request.insert("X-Protocol-Ctl", makeFeaturesRequestHeader(false, false, true, false)); + + auto peer = makeCapturePeer(env, std::nullopt, std::move(request)); peer->onMessage(createRequest(numTransactions)); - auto fee = peer->getCurrentFeeCharge(); + auto fee = peer->feeCharge(); if (expectRejected) { BEAST_EXPECT(fee == resource::kFeeMalformedRequest); diff --git a/src/test/overlay/reduce_relay_test.cpp b/src/test/overlay/reduce_relay_test.cpp index 4091efa0ca..ff3eb51a4d 100644 --- a/src/test/overlay/reduce_relay_test.cpp +++ b/src/test/overlay/reduce_relay_test.cpp @@ -1,4 +1,5 @@ #include +#include #include #include @@ -12,10 +13,8 @@ #include #include #include -#include #include #include -#include #include #include #include @@ -27,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -67,16 +65,16 @@ static constexpr std::uint32_t kMaxMessages = 200000; /** * Simulate two entities - peer directly connected to the server * (via squelch in PeerSim) and PeerImp (via Overlay) + * + * `PeerStub` supplies the rest of the `Peer` interface as no-ops. */ -class PeerPartial : public Peer +class PeerPartial : public PeerStub { public: - PeerPartial() : nodePublicKey(derivePublicKey(KeyType::Ed25519, randomSecretKey())) - { - } + using PeerStub::PeerStub; + // Keep the base overload visible; the one below would otherwise hide it. + using PeerStub::send; - PublicKey nodePublicKey; - ~PeerPartial() override = default; virtual void onMessage(MessageSPtr const& m, SquelchCB f) = 0; virtual void @@ -86,111 +84,6 @@ public: { onMessage(squelch); } - - // dummy implementation - void - send(std::shared_ptr const& m) override - { - } - [[nodiscard]] beast::ip::Endpoint - getRemoteAddress() const override - { - return {}; - } - void - charge(resource::Charge const& fee, std::string const& context = {}) override - { - } - [[nodiscard]] bool - cluster() const override - { - return false; - } - [[nodiscard]] bool - isHighLatency() const override - { - return false; - } - [[nodiscard]] int - getScore(bool) const override - { - return 0; - } - [[nodiscard]] PublicKey const& - getNodePublic() const override - { - return nodePublicKey; - } - json::Value - json() override - { - return {}; - } - [[nodiscard]] bool - supportsFeature(ProtocolFeature f) const override - { - return false; - } - [[nodiscard]] std::optional - publisherListSequence(PublicKey const&) const override - { - return {}; - } - void - setPublisherListSequence(PublicKey const&, std::size_t const) override - { - } - [[nodiscard]] uint256 - getClosedLedgerHash() const override - { - static uint256 const kHash{}; - return kHash; - } - [[nodiscard]] bool - hasLedger(uint256 const& hash, std::uint32_t seq) const override - { - return false; - } - void - ledgerRange(std::uint32_t& minSeq, std::uint32_t& maxSeq) const override - { - } - [[nodiscard]] bool - hasTxSet(uint256 const& hash) const override - { - return false; - } - void - cycleStatus() override - { - } - bool - hasRange(std::uint32_t uMin, std::uint32_t uMax) override - { - return false; - } - [[nodiscard]] bool - compressionEnabled() const override - { - return false; - } - [[nodiscard]] bool - txReduceRelayEnabled() const override - { - return false; - } - void - sendTxQueue() override - { - } - void - addTxQueue(uint256 const&) override - { - } - void - removeTxQueue(uint256 const&) override - { - } }; /** @@ -466,24 +359,13 @@ class PeerSim : public PeerPartial, public std::enable_shared_from_this { public: using id_t = Peer::id_t; - PeerSim(Overlay& overlay, beast::Journal journal) : overlay_(overlay), squelch_(journal) + PeerSim(Overlay& overlay, beast::Journal journal) + : PeerPartial(sid++), overlay_(overlay), squelch_(journal) { } ~PeerSim() override = default; - id_t - id() const override - { - return id_; - } - - std::string const& - fingerprint() const override - { - return fingerprint_; - } - static void resetId() { @@ -525,8 +407,6 @@ public: private: inline static id_t sid = 0; - std::string fingerprint_; - id_t id_{sid++}; Overlay& overlay_; reduce_relay::Squelch squelch_; }; diff --git a/src/test/overlay/tx_reduce_relay_test.cpp b/src/test/overlay/tx_reduce_relay_test.cpp index 8626d3e19c..e97fba88e5 100644 --- a/src/test/overlay/tx_reduce_relay_test.cpp +++ b/src/test/overlay/tx_reduce_relay_test.cpp @@ -1,38 +1,24 @@ #include #include +#include #include #include -#include #include #include #include #include -#include #include -#include -#include #include -#include #include #include #include #include -#include #include -#include -#include -#include -#include -#include -#include -#include - #include -#include #include #include #include @@ -47,13 +33,6 @@ namespace xrpl::test { class tx_reduce_relay_test : public beast::unit_test::Suite { -public: - using socket_type = boost::asio::ip::tcp::socket; - using middle_type = boost::beast::tcp_stream; - using stream_type = boost::beast::ssl_stream; - using shared_context = std::shared_ptr; - -private: void doTest(std::string const& msg, bool log, std::function f) { @@ -116,107 +95,83 @@ private: }); } - class PeerTest : public PeerImp + /** + * Counts queued transaction hashes. Relayed messages are counted through + * the inherited `sent()`. + */ + class TxReducePeer : public CapturePeer { public: - PeerTest( - Application& app, - std::shared_ptr const& slot, - http_request_type&& request, - PublicKey const& publicKey, - ProtocolVersion protocol, - resource::Consumer consumer, - std::unique_ptr&& streamPtr, - OverlayImpl& overlay) - : PeerImp( - app, - sid, - slot, - std::move(request), - publicKey, - protocol, - consumer, - std::move(streamPtr), - overlay) - { - sid++; - } - ~PeerTest() override = default; + using CapturePeer::CapturePeer; void - run() override + addTxQueue(uint256 const&) override { + ++queued_; } - void - send(std::shared_ptr const&) override + + /** + * @return The number of transaction hashes queued for this peer. + */ + std::size_t + queued() const { - sendTx++; + return queued_; } - void - addTxQueue(uint256 const& hash) override - { - queueTx++; - } - static void - init() - { - queueTx = 0; - sendTx = 0; - sid = 0; - } - inline static std::size_t sid = 0; - inline static std::uint16_t queueTx = 0; - inline static std::uint16_t sendTx = 0; + + private: + std::size_t queued_{0}; }; - std::uint16_t lid_{0}; - std::uint16_t rid_{1}; - shared_context context_; - ProtocolVersion protocolVersion_; - boost::beast::multi_buffer readBuf_; - -public: - tx_reduce_relay_test() : context_(makeSslContext("")), protocolVersion_{1, 7} - { - } - -private: + /** + * Build one peer and register it with the overlay. + * + * The first `nDisabled` peers get no `X-Protocol-Ctl` header, which leaves + * tx reduce-relay disabled on them. Built first, they sit at the front of + * `peers`, where `testRelay`'s skip set expects them. + * + * @param env The environment owning the overlay. + * @param peers Receives the peer; the overlay holds only a weak + * pointer, so the caller keeps it alive. + * @param nDisabled How many more peers to leave disabled; decremented + * per peer built. + */ void - addPeer(jtx::Env& env, std::vector>& peers, std::uint16_t& nDisabled) + addPeer( + jtx::Env& env, + std::vector>& peers, + std::uint16_t& nDisabled) { auto& overlay = dynamic_cast(env.app().getOverlay()); - boost::beast::http::request request; - (nDisabled == 0) - ? request.insert("X-Protocol-Ctl", makeFeaturesRequestHeader(false, false, true, false)) - : (void)nDisabled--; - auto streamPtr = std::make_unique( - socket_type(std::forward(env.app().getIOContext())), - *context_); - beast::ip::Endpoint const local( - boost::asio::ip::make_address("172.1.1." + std::to_string(lid_))); - beast::ip::Endpoint const remote( - boost::asio::ip::make_address("172.1.1." + std::to_string(rid_))); PublicKey const key(std::get<0>(randomKeyPair(KeyType::Ed25519))); - auto consumer = overlay.resourceManager().newInboundEndpoint(remote); - auto [slot, _] = overlay.peerFinder().newInboundSlot(local, remote); - auto const peer = std::make_shared( - env.app(), - slot, - std::move(request), - key, - protocolVersion_, - consumer, - std::move(streamPtr), - overlay); + + bool const disabled = nDisabled > 0; + if (disabled) + --nDisabled; + + http_request_type request; + if (!disabled) + request.insert("X-Protocol-Ctl", makeFeaturesRequestHeader(false, false, true, false)); + BEAST_EXPECT(overlay.findPeerByPublicKey(key) == std::shared_ptr{}); - overlay.addActive(peer); + auto const peer = makeCapturePeer(env, key, std::move(request)); BEAST_EXPECT(overlay.findPeerByPublicKey(key) == peer); - peers.emplace_back(peer); // overlay stores week ptr to PeerImp - lid_ += 2; - rid_ += 2; - assert(lid_ <= 254); + peers.emplace_back(peer); } + /** + * Relay one transaction to `nPeers` peers and check the split. + * + * @param test The testcase name. + * @param txRREnabled The `tx_enable` config value. + * @param nPeers How many peers to attach to the overlay. + * @param nDisabled How many of those peers have reduce-relay disabled. + * @param minPeers The `tx_min_peers` config value. + * @param relayPercentage The `tx_relay_percentage` config value. + * @param expectRelay The expected number of peers relayed to. + * @param expectQueue The expected number of peers queued for. + * @param nSkip How many of the first-built peers to skip. + */ void testRelay( std::string const& test, @@ -227,20 +182,30 @@ private: std::uint16_t relayPercentage, std::uint16_t expectRelay, std::uint16_t expectQueue, - std::set const& toSkip = {}) + std::size_t nSkip = 0) { testcase(test); jtx::Env env(*this); - std::vector> peers; + std::vector> peers; + // `PeerImp` decides `txReduceRelayEnabled()` in its constructor, from + // the config and the handshake header, so set these first. env.app().config().txReduceRelayEnable = txRREnabled; env.app().config().txReduceRelayMinPeers = minPeers; env.app().config().txRelayPercentage = relayPercentage; - PeerTest::init(); - lid_ = 0; - rid_ = 0; for (int i = 0; i < nPeers; i++) addPeer(env, peers, nDisabled); + // An under-filled skip set would also fail the relay counts below, for + // a reason that looks unrelated. + if (!BEAST_EXPECT(nSkip <= peers.size())) + return; + + // Skip the peers built first, so the skip set overlaps the disabled + // peers as the expected counts assume. + std::set toSkip; + for (std::size_t i = 0; i < nSkip; ++i) + toSkip.insert(peers[i]->id()); + auto const jtx = env.jt(noop(env.master)); if (BEAST_EXPECT(jtx.stx)) { @@ -251,7 +216,15 @@ private: m.set_deferred(false); m.set_status(protocol::TransactionStatus::tsNEW); env.app().getOverlay().relay(uint256{0}, m, toSkip); - BEAST_EXPECT(PeerTest::sendTx == expectRelay && PeerTest::queueTx == expectQueue); + + std::size_t sendTx = 0; + std::size_t queueTx = 0; + for (auto const& peer : peers) + { + sendTx += peer->sent().size(); + queueTx += peer->queued(); + } + BEAST_EXPECT(sendTx == expectRelay && queueTx == expectQueue); } } @@ -259,12 +232,11 @@ private: run() override { bool const log = false; - std::set skip = {0, 1, 2, 3, 4}; testConfig(log); // relay to all peers, no hash queue testRelay("feature disabled", false, 10, 0, 10, 25, 10, 0); // relay to nPeers - skip (10-5=5) - testRelay("feature disabled & skip", false, 10, 0, 10, 25, 5, 0, skip); + testRelay("feature disabled & skip", false, 10, 0, 10, 25, 5, 0, 5); // relay to all peers because min is greater than nPeers testRelay("relay all 1", true, 10, 0, 20, 25, 10, 0); // relay to all peers because min + disabled is greater thant nPeers @@ -275,24 +247,22 @@ private: // relay to minPeers + 25% of (nPeers - nPeers) - skip // (20+0.25*(60-20)-5=25), queue the rest, skip counts towards relayed // (60-25-5=30) - testRelay("skip", true, 60, 0, 20, 25, 25, 30, skip); + testRelay("skip", true, 60, 0, 20, 25, 25, 30, 5); // relay to minPeers + disabled + 25% of (nPeers - minPeers - disabled) // (20+10+0.25*(70-20-10)=40), queue the rest (30) testRelay("disabled", true, 70, 10, 20, 25, 40, 30); // relay to minPeers + disabled-not-in-skip + 25% of (nPeers - minPeers // - disabled) (20+5+0.25*(70-20-10)=35), queue the rest, skip counts // towards relayed (70-35-5=30)) - testRelay("disabled & skip", true, 70, 10, 20, 25, 35, 30, skip); + testRelay("disabled & skip", true, 70, 10, 20, 25, 35, 30, 5); // relay to minPeers + disabled + 25% of (nPeers - minPeers - disabled) // - skip (10+5+0.25*(15-10-5)-10=5), queue the rest, skip counts // towards relayed (15-5-10=0) - skip = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - testRelay("disabled & skip, no queue", true, 15, 5, 10, 25, 5, 0, skip); + testRelay("disabled & skip, no queue", true, 15, 5, 10, 25, 5, 0, 10); // relay to minPeers + disabled + 25% of (nPeers - minPeers - disabled) // - skip (10+2+0.25*(20-10-2)-14=0), queue the rest, skip counts // towards relayed (20-14=6) - skip = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}; - testRelay("disabled & skip, no relay", true, 20, 2, 10, 25, 0, 6, skip); + testRelay("disabled & skip, no relay", true, 20, 2, 10, 25, 0, 6, 14); } }; diff --git a/src/xrpld/overlay/detail/ProtocolVersion.cpp b/src/xrpld/overlay/detail/ProtocolVersion.cpp index 74dad61828..d24928a4ec 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.cpp +++ b/src/xrpld/overlay/detail/ProtocolVersion.cpp @@ -151,4 +151,12 @@ isProtocolSupported(ProtocolVersion const& v) return std::end(kSupportedProtocolList) != std::ranges::find(kSupportedProtocolList, v); } +ProtocolVersion +newestSupportedProtocolVersion() +{ + // Scans rather than reading the sorted list's last entry, so it does not + // depend on an invariant kept elsewhere. + return *std::ranges::max_element(kSupportedProtocolList); +} + } // namespace xrpl diff --git a/src/xrpld/overlay/detail/ProtocolVersion.h b/src/xrpld/overlay/detail/ProtocolVersion.h index 5c05f63e2a..a4342c2453 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.h +++ b/src/xrpld/overlay/detail/ProtocolVersion.h @@ -68,4 +68,13 @@ supportedProtocolVersions(); bool isProtocolSupported(ProtocolVersion const& v); +/** + * The version negotiated with a peer that speaks everything we speak, so also + * the one that enables every version-gated feature. + * + * @return The largest version in the list of supported protocol versions. + */ +ProtocolVersion +newestSupportedProtocolVersion(); + } // namespace xrpl