Merge branch 'develop' into mvadari/perf-relay-skip-reparse

The overload of Overlay::relay that takes a parsed STTx no longer
takes a separate hash; it uses tx.getTransactionID(). Its message
parameter is now const.

src/test/overlay/tx_reduce_relay_test.cpp

@@ -13,8 +13,11 @@
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/KeyType.h>
 #include <xrpl/protocol/PublicKey.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/SecretKey.h>
 #include <xrpl/protocol/Serializer.h>
+#include <xrpl/protocol/TxFormats.h>
 #include <xrpl/server/Handoff.h>
 
 #include <xrpl.pb.h>
@@ -273,6 +276,74 @@ class tx_reduce_relay_test : public beast::unit_test::Suite
         BEAST_EXPECT(sendTx == 0 && queueTx == expectQueue);
     }
 
+    /**
+     * Relay through the overload that takes the parsed STTx, and check that a
+     * pseudo-transaction is queued, not sent, on both overloads.
+     *
+     * All peers negotiated the feature and the minimum exceeds the peer
+     * count, so an ordinary transaction goes to every peer.
+     *
+     * @param test Name of the case.
+     * @param pseudo Whether to relay a pseudo-transaction.
+     * @param parsed Whether to use the overload that takes the parsed STTx.
+     */
+    void
+    testParsed(std::string const& test, bool pseudo, bool parsed)
+    {
+        testcase(test);
+        jtx::Env env(*this);
+        std::vector<std::shared_ptr<TxReducePeer>> peers;
+        env.app().config().txReduceRelayEnable = true;
+        env.app().config().txReduceRelayMinPeers = 20;
+        env.app().config().txRelayPercentage = 25;
+        constexpr std::uint16_t kPeers = 10;
+        std::uint16_t nDisabled = 0;
+        for (int i = 0; i < kPeers; i++)
+            addPeer(env, peers, nDisabled);
+
+        auto const stx = pseudo ? std::make_shared<STTx const>(
+                                      ttAMENDMENT,
+                                      [](auto& obj) {
+                                          obj.setAccountID(sfAccount, AccountID());
+                                          obj.setFieldH256(sfAmendment, UInt256{1});
+                                          obj.setFieldU32(sfLedgerSequence, 1);
+                                      })
+                                : env.jt(noop(env.master)).stx;
+        if (!BEAST_EXPECT(stx))
+            return;
+
+        protocol::TMTransaction m;
+        Serializer s;
+        stx->add(s);
+        m.set_rawtransaction(s.data(), s.size());
+        m.set_deferred(false);
+        m.set_status(protocol::TransactionStatus::tsNEW);
+        if (parsed)
+        {
+            env.app().getOverlay().relay(m, {}, *stx);
+        }
+        else
+        {
+            env.app().getOverlay().relay(UInt256{0}, m, {});
+        }
+
+        std::size_t sendTx = 0;
+        std::size_t queueTx = 0;
+        for (auto const& peer : peers)
+        {
+            sendTx += peer->sent().size();
+            queueTx += peer->queued();
+        }
+        if (pseudo)
+        {
+            BEAST_EXPECT(sendTx == 0 && queueTx == kPeers);
+        }
+        else
+        {
+            BEAST_EXPECT(sendTx == kPeers && queueTx == 0);
+        }
+    }
+
     void
     run() override
     {
@@ -316,6 +387,9 @@ class tx_reduce_relay_test : public beast::unit_test::Suite
         testQueueOnly("queue only, some disabled", true, 10, 4, 6);
         // queue nobody, since no peer would understand the announcement
         testQueueOnly("queue only, all disabled", true, 10, 10, 0);
+        testParsed("parsed tx", false, true);
+        testParsed("parsed pseudo-tx", true, true);
+        testParsed("unparsed pseudo-tx", true, false);
     }
 };
 

src/xrpld/app/ledger/detail/OpenLedger.cpp

@@ -150,7 +150,7 @@ OpenLedger::accept(
             msg.set_rawtransaction(s.data(), s.size());
             msg.set_status(protocol::tsNEW);
             msg.set_receivetimestamp(app.getTimeKeeper().now().time_since_epoch().count());
-            app.getOverlay().relay(txId, msg, *toSkip);
+            app.getOverlay().relay(msg, *toSkip, *tx);
         }
     }
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1898,7 +1898,7 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
                         registry_.get().getTimeKeeper().now().time_since_epoch().count());
                     tx.set_deferred(e.result == terQUEUED);
                     // FIXME: This should be when we received it
-                    registry_.get().getOverlay().relay(e.transaction->getID(), tx, *toSkip);
+                    registry_.get().getOverlay().relay(tx, *toSkip, sttx);
                     e.transaction->setBroadcast();
                 }
             }

src/xrpld/overlay/Overlay.h

@@ -31,6 +31,8 @@ class context;
 
 namespace xrpl {
 
+class STTx;
+
 /**
  * Manages the set of connected peers.
  */
@@ -187,6 +189,15 @@ public:
         std::optional<std::reference_wrapper<protocol::TMTransaction>> m,
         std::set<Peer::ID> const& toSkip) = 0;
 
+    /**
+     * Relay a transaction that the caller has already parsed, so that it is
+     * not parsed again. Otherwise the same as the overload above, with the
+     * hash taken from `tx`.
+     * @param tx the parsed form of `m`
+     */
+    virtual void
+    relay(protocol::TMTransaction const& m, std::set<Peer::ID> const& toSkip, STTx const& tx) = 0;
+
     /**
      * Visit every active peer.
      *

src/xrpld/overlay/detail/OverlayImpl.cpp

@@ -1319,14 +1319,14 @@ OverlayImpl::relay(
     std::optional<std::reference_wrapper<protocol::TMTransaction>> tx,
     std::set<Peer::ID> const& toSkip)
 {
-    bool relay = tx.has_value();
-    if (relay)
+    if (tx)
     {
         auto const& txn = tx->get();
         SerialIter sit(makeSlice(txn.rawtransaction()));
         try
         {
-            relay = !isPseudoTx(STTx{sit});
+            if (isPseudoTx(STTx{sit}))
+                tx.reset();
         }
         catch (std::exception const&)
         {
@@ -1336,6 +1336,33 @@ OverlayImpl::relay(
         }
     }
 
+    relayChecked(hash, tx, toSkip);
+}
+
+void
+OverlayImpl::relay(
+    protocol::TMTransaction const& m,
+    std::set<Peer::ID> const& toSkip,
+    STTx const& tx)
+{
+    if (isPseudoTx(tx))
+    {
+        relayChecked(tx.getTransactionID(), std::nullopt, toSkip);
+    }
+    else
+    {
+        relayChecked(tx.getTransactionID(), m, toSkip);
+    }
+}
+
+void
+OverlayImpl::relayChecked(
+    UInt256 const& hash,
+    std::optional<std::reference_wrapper<protocol::TMTransaction const>> tx,
+    std::set<Peer::ID> const& toSkip)
+{
+    bool const relay = tx.has_value();
+
     Overlay::PeerSequence peers = {};
     std::size_t total = 0;
     std::size_t disabled = 0;

src/xrpld/overlay/detail/OverlayImpl.h

@@ -248,6 +248,10 @@ public:
         std::optional<std::reference_wrapper<protocol::TMTransaction>> m,
         std::set<Peer::ID> const& skip) override;
 
+    void
+    relay(protocol::TMTransaction const& m, std::set<Peer::ID> const& toSkip, STTx const& tx)
+        override;
+
     std::shared_ptr<Message>
     getManifestsMessage();
 
@@ -451,6 +455,16 @@ public:
     }
 
 private:
+    /**
+     * Relay `tx` if present, else only queue its hash for peers that have
+     * tx reduce-relay enabled. The caller has already excluded pseudo-txs.
+     */
+    void
+    relayChecked(
+        UInt256 const& hash,
+        std::optional<std::reference_wrapper<protocol::TMTransaction const>> tx,
+        std::set<Peer::ID> const& toSkip);
+
     void
     squelch(PublicKey const& validator, Peer::ID const id, std::uint32_t squelchDuration)
         const override;