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..547fdb8666 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,34 @@ 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())) + // The id is arbitrary but fixed: the replay code only ever compares ids, + // and every task here is served by a single peer. + 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..6f52893e8b --- /dev/null +++ b/src/test/jtx/PeerStub.h @@ -0,0 +1,187 @@ +#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. + * + * `Peer` is a two dozen method pure interface, and a test double normally + * cares about one or two of them. Derive from this and override only those. + * Adding a method to `Peer` then costs one stub here rather than one per + * double. + * + * Identity is the exception to "returns a default": 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..37dbd70a43 --- /dev/null +++ b/src/test/overlay/CapturePeer.h @@ -0,0 +1,249 @@ +#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 everything a test drives through + * `onMessage` runs production code. Derive from this to reach a `protected` + * `PeerImp` member; `CapturePeerBuilder::build` takes the derived type. + */ +class CapturePeer : public PeerImp +{ +public: + using MiddleType = boost::beast::tcp_stream; + using StreamType = boost::beast::ssl_stream; + using SocketType = boost::asio::ip::tcp::socket; + + /** + * Forwards to `PeerImp`, restating its two sinks by value. + * + * `PeerImp` declares `request` and `streamPtr` as rvalue references. Taking + * them by value instead is what lets a derived double write a plain + * `using CapturePeer::CapturePeer;`. An inherited constructor has no body, + * so an inherited rvalue-reference parameter is always reported as never + * moved from. + * + * @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, + // `resource::Consumer` is copy-only, so `std::move` here would + // be a copy anyway. + consumer, + std::move(streamPtr), + overlay) + { + } + + ~CapturePeer() override = default; + + /** + * Deliberately does nothing, which is what keeps the peer alive. + * + * `OverlayImpl::addActive` calls `run()`, and for an inbound peer that + * reaches `PeerImp::doAccept`, which reads the ssl handshake off a socket + * a test never connected. That fails, and the peer closes and detaches + * itself again. Doing nothing leaves it registered and inert. + */ + void + run() override + { + } + + /** + * Captures rather than writes, which is what makes replies 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(); + } + + /** + * Exposes the charge `PeerImp` has accumulated but not yet applied. + * + * `PeerImp::currentFeeCharge` is `protected` for exactly this reason: a + * test can check which fee a message earned without draining it through + * `charge()`. Reading it needs no production accessor, only a derived + * class, and every suite that reads it wants the same one. + * + * @return The charge accumulated on the peer so far. + */ + resource::Charge + feeCharge() const + { + return currentFeeCharge(); + } + +private: + std::vector> sent_; +}; + +/** + * Builds active `CapturePeer` peers against an environment's overlay. + * + * Holds the SSL context and hands out connection ids and remote addresses, so + * no two peers built by one builder collide. Ids start at 1 and only ever + * increase, as in production. + */ +class CapturePeerBuilder +{ +public: + /** + * Build an active peer 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. Pass one carrying an + * `X-Protocol-Ctl` header to negotiate features; + * `PeerImp` reads it in its constructor. + * @return The peer, already registered with the overlay. Throws rather + * than returning if the peer finder refused a slot. + */ + template + std::shared_ptr + build( + 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()), *context_); + + // Every peer needs its own remote address, not merely its own port. The + // peer finder caps inbound connections per address at `ipLimit`, which + // is at most 2 unless configured, and it refuses the slot once that is + // reached. + beast::ip::Endpoint const local(boost::asio::ip::make_address("172.1.1.1"), kPort); + beast::ip::Endpoint const remote(boost::asio::ip::address_v4(nextRemote_++), kPort); + + auto consumer = overlay.resourceManager().newInboundEndpoint(remote); + auto [slot, _] = overlay.peerFinder().newInboundSlot(local, remote); + + // The slot is unseated when the endpoint is already connected or its + // address is at the peer finder's per-address limit, and `PeerImp` + // dereferences the slot in its constructor. Fail here, where the cause + // is visible, rather than there with a segfault. + if (!slot) + { + Throw( + "CapturePeerBuilder::build: no slot for " + to_string(remote)); + } + + if (!key) + key = PublicKey(std::get<0>(randomKeyPair(KeyType::Ed25519))); + + auto peer = std::make_shared( + env.app(), + nextId_++, + slot, + std::move(request), + *key, + // A peer claiming an unsupported version would silently fail every + // test `PeerImp::supportsFeature` makes against the version, and so + // would only ever reach the legacy branch of a version-gated reply. + newestSupportedProtocolVersion(), + consumer, + std::move(streamPtr), + overlay); + + overlay.addActive(peer); + return peer; + } + +private: + static constexpr std::uint16_t kPort = 51235; + // Remote addresses are handed out from 172.2.0.1 upward, so ~900k fit + // before the counter reaches 172.16/12 and the peer finder starts treating + // them as private. Keeping them public is what makes a test peer look like + // a real inbound connection. + static constexpr std::uint32_t kFirstRemote = 0xAC020001; + + std::shared_ptr context_{makeSslContext("")}; + Peer::id_t nextId_{1}; + std::uint32_t nextRemote_{kFirstRemote}; +}; + +} // 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..b89ca11f34 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,26 @@ using namespace jtx; class TMGetLedger_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; + /** + * Adds a synchronous entry point to the JobQueue-dispatched processor. + * + * The production path runs this on JtLedgerReq; tests need to call it + * directly so the reply can be inspected through `lastSent()`. + * `PeerImp::processLedgerRequest` is `protected` for that purpose. + */ + 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 +71,17 @@ class TMGetLedger_test : public beast::unit_test::Suite testcase("Node ID Count Accepted"); Env env{*this}; - PeerTest::resetId(); + CapturePeerBuilder builder; - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = builder.build(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 +91,9 @@ class TMGetLedger_test : public beast::unit_test::Suite Env env{*this}; env.close(); - PeerTest::resetId(); + CapturePeerBuilder builder; - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = builder.build(env); // Ask for the account-state root node of the closed ledger. auto request = createRequest(numNodeIds); @@ -96,7 +103,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..f58e975340 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,24 @@ using namespace jtx; */ class TMGetObjectByHash_test : public beast::unit_test::Suite { - PeerTest::SharedContext context_{makeSslContext("")}; - ProtocolVersion protocolVersion_{1, 7}; + /** + * Adds a synchronous entry point to the JobQueue-dispatched processor. + * + * The production path runs this on JtLedgerReq; tests need to call it + * directly so the reply can be inspected through `sent()`. + * `PeerImp::processGetObjectByHash` is `protected` for that purpose. + */ + 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 +97,14 @@ class TMGetObjectByHash_test : public beast::unit_test::Suite testcase("Reply Object Count"); Env env(*this); - PeerTest::resetId(); - - auto peer = makePeerTest(env, context_, protocolVersion_); + CapturePeerBuilder builder; + auto peer = builder.build(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..6886a5cd3b 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,15 @@ 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(); + CapturePeerBuilder builder; - auto peer = makePeerTest(env, context_, protocolVersion_); + auto peer = builder.build(env); auto tx = std::make_shared(); tx->set_status(protocol::tsNEW); @@ -45,7 +31,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..23c52baf5f 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,19 @@ class TMTransactions_test : public beast::unit_test::Suite *this, envconfig(), std::make_unique(kLimitExceededMessage, &foundExpectedLog)}; - PeerTest::resetId(); + CapturePeerBuilder builder; - auto peer = makePeerTest(env, context_, protocolVersion_); - peer->txReduceRelayEnabled(true); + // Set before building the peer: `PeerImp` decides + // `txReduceRelayEnabled()` in its constructor, from the config and the + // handshake header together. + env.app().config().txReduceRelayEnable = true; + http_request_type request; + request.insert("X-Protocol-Ctl", makeFeaturesRequestHeader(false, false, true, false)); + + auto peer = builder.build(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..3325b51959 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,87 @@ private: }); } - class PeerTest : public PeerImp + /** + * Counts the transaction hashes queued for this peer. + * + * `send` is inherited, so relayed messages are counted through `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 are built without an `X-Protocol-Ctl` + * header, which is what leaves tx reduce-relay disabled on them. Because + * they are built first, they occupy the lowest connection ids, which is + * what makes them overlap the skipped peers in `testRelay`. + * + * @param env The environment owning the overlay. + * @param builder Supplies the connection id and remote address. + * @param peers Receives the peer; the overlay only holds a weak + * pointer, so the caller has to keep it alive. + * @param nDisabled How many more peers to leave reduce-relay disabled; + * decremented for each one built. + */ void - addPeer(jtx::Env& env, std::vector>& peers, std::uint16_t& nDisabled) + addPeer( + jtx::Env& env, + CapturePeerBuilder& builder, + 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 = builder.build(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,19 +186,31 @@ 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; + CapturePeerBuilder builder; + std::vector> peers; + // Set before building any peer: `PeerImp` decides + // `txReduceRelayEnabled()` in its constructor, from the config and the + // handshake header together. 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); + addPeer(env, builder, peers, nDisabled); + + // Bail out rather than fall through: an under-filled skip set would + // fail the relay counts below too, for a reason that looks unrelated. + if (!BEAST_EXPECT(nSkip <= peers.size())) + return; + + // Skip the peers built first, so the skipped set overlaps the + // reduce-relay-disabled peers the way 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 +222,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 +238,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 +253,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..a6414cf751 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.cpp +++ b/src/xrpld/overlay/detail/ProtocolVersion.cpp @@ -151,4 +151,13 @@ isProtocolSupported(ProtocolVersion const& v) return std::end(kSupportedProtocolList) != std::ranges::find(kSupportedProtocolList, v); } +ProtocolVersion +newestSupportedProtocolVersion() +{ + // The list above is sorted, so this could read its last entry instead. It + // scans for the maximum so that it stays correct on its own, rather than on + // an invariant a separate static_assert keeps. The list holds two entries. + 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..3d74cbcbb7 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.h +++ b/src/xrpld/overlay/detail/ProtocolVersion.h @@ -68,4 +68,16 @@ supportedProtocolVersions(); bool isProtocolSupported(ProtocolVersion const& v); +/** + * The newest protocol version we support. + * + * This is the version we negotiate with any peer that speaks everything we + * speak, so it is also the version a caller wants when it needs one that + * enables every version-gated feature. + * + * @return The largest version in the list of supported protocol versions. + */ +ProtocolVersion +newestSupportedProtocolVersion(); + } // namespace xrpl