fixes for pseudo tx relaying and fees

parent f64cf91 author Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> 1736802518 +0000 committer Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> 1738008707 +0000 gpgsig -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEfrrtWa+LJEJ1XMcuyguytSYMMyoFAmeX6IMACgkQyguytSYM MyoL0RAAw9BSki6NO1DMxTv/VKT1TzzbvPmGhcRCmXJzbMobJHiJBtXO6wKcMY8t +pZcj5+56N2JimSIWJfvoZgrze3cwUgdMXuAGhy0+bbGCgPRp5WEc8hVerk8o62S 9x2kwqRHCDx2JlNpaqyVv+QdgOVCOqug2nCBbW7VYp/ElAX/yeJM+uuH0r2BnsaS rixujqrlp1noPZhGKJUyM1VaiyI61xFFBpD/oyps9jk4l9frd72Fa41Q8/h75ixH ij04wqNE4GEfi0HKKAbu8fNYIdTy9hHeN8Hzblf7J7wZwFDooXIWftR5HkeQbgrD F8mMRlHXFmOLjFrnv+i8LpNFBlhe4DDILzTMXBM7u9TZRSepwrJxZb5TKJuz9q72 aDeXQ2FITNry4s7pVm/X0ZBdH9qdFn8wuJWGIsPlYhmvzT9d6qLCnSZ/NAYhXIxC oBDXfldzVx+1zFz1Ab9KFRl4Dx2drNC18CUXcZdXJVGFZEug26xE5795Tf1SJ1N8 u5JWrfGQZ2SItJOofYbNKm5jbqpEQQWq0V36uYRZ/hWROocMtKWl3E3cU2RDFVjT MILVHGIGBPoOa0ZT5ufHeZCvW2fdfPFlOFXlS6ixTNu3IRzNhY7Ri2Fxfk5EKd8P uDlceyhBeDgiZUOtyoOHm462jQrJ44JxA+QoJv34DikjI/42ZQg= =0wkh -----END PGP SIGNATURE----- fix fee from feeInvalidSignature to feeUnwantedData update release notes feeMediumBurdenPeer and feeInvalidSignature context details Improve pseudo-transaction peer message handling: * Before: Pseudo-transactions received from a peer will fail the signature check, even if they were requested (using TMGetObjectByHash), because they have no signature. This causes the peer to be charge for an invalid signature. * After: Pseudo-transactions, are put into the global cache (TransactionMaster) only. If the transaction is part of a TMTransactions batch, the peer is charged the equivalent of one trivial request per transaction. If not, the peer is charged an unwanted data fee. These fees will not be a problem in the normal course of operations, but should dissuade peers from behaving badly by sending a bunch of junk. Tell the overlay about pseudo transactions, but don't relay them * If reduce relay is enabled, it will queue up the tx id for peers that also have it enabled so they can ask for it later if they need it. Log higher resource charges with higher log levels Include context on (some) resource charges Add context to all Peer charges stop relaying pseudo-tx increased dropThreshold and feeHighBurdenPeer increased the fees fixing formatting fixing unit test missing header fix fee init - missing ctor code refactoring addressing review comment update levelization fixing formatting Limit the size of outgoing mtLEDGER_DATA / TMLedgerData messages Update include/xrpl/resource/detail/Logic.h Co-authored-by: Ed Hennis <ed@ripple.com> Update src/xrpld/overlay/detail/OverlayImpl.cpp Co-authored-by: Ed Hennis <ed@ripple.com> code refactoring assert on update removing assert renaming removing spaces adding spaceship operator unit test update revert fee logging release notes and typo fix Update src/xrpld/overlay/detail/PeerImp.cpp Co-authored-by: Ed Hennis <ed@ripple.com> Update src/xrpld/overlay/detail/OverlayImpl.cpp Co-authored-by: Ed Hennis <ed@ripple.com> finished refactoring relay adding extra debug logs remove dynamic fees Update src/xrpld/overlay/detail/OverlayImpl.cpp Co-authored-by: Ed Hennis <ed@ripple.com> Update src/xrpld/overlay/detail/OverlayImpl.cpp Co-authored-by: Ed Hennis <ed@ripple.com> Update src/xrpld/overlay/detail/OverlayImpl.cpp Co-authored-by: Ed Hennis <ed@ripple.com> var def static getStream temp removal charge update cleanup 1
vlntb · Jan 27, 2025 · 00edaea · 00edaea
1 parent f64cf91
commit 00edaea
Show file tree

Hide file tree

Showing 30 changed files with 329 additions and 162 deletions.
diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt
@@ -82,6 +82,7 @@ test.nodestore > xrpld.core
 test.nodestore > xrpld.nodestore
 test.nodestore > xrpld.unity
 test.overlay > test.jtx
+test.overlay > test.toplevel
 test.overlay > test.unit_test
 test.overlay > xrpl.basics
 test.overlay > xrpld.app

diff --git a/include/xrpl/resource/Charge.h b/include/xrpl/resource/Charge.h
@@ -53,8 +53,9 @@ class Charge
 
     bool
     operator==(Charge const&) const;
-    bool
-    operator!=(Charge const&) const;
+
+    std::strong_ordering
+    operator<=>(Charge const&) const;
 
 private:
     value_type m_cost;

diff --git a/include/xrpl/resource/Consumer.h b/include/xrpl/resource/Consumer.h
@@ -67,7 +67,7 @@ class Consumer
 
     /** Apply a load charge to the consumer. */
     Disposition
-    charge(Charge const& fee);
+    charge(Charge const& fee, std::string const& context = {});
 
     /** Returns `true` if the consumer should be warned.
         This consumes the warning.

diff --git a/include/xrpl/resource/Fees.h b/include/xrpl/resource/Fees.h
@@ -28,28 +28,28 @@ namespace Resource {
 // clang-format off
 /** Schedule of fees charged for imposing load on the server. */
 /** @{ */
-extern Charge const feeInvalidRequest;    // A request that we can immediately
+extern Charge const feeMalformedRequest;    // A request that we can immediately
                                           //   tell is invalid
 extern Charge const feeRequestNoReply;    // A request that we cannot satisfy
 extern Charge const feeInvalidSignature;  // An object whose signature we had
                                           //   to check and it failed
-extern Charge const feeUnwantedData;      // Data we have no use for
-extern Charge const feeBadData;           // Data we have to verify before
+extern Charge const feeUselessData;      // Data we have no use for
+extern Charge const feeInvalidData;           // Data we have to verify before
                                           //   rejecting
 
 // RPC loads
-extern Charge const feeInvalidRPC;        // An RPC request that we can
+extern Charge const feeMalformedRPC;        // An RPC request that we can
                                           //   immediately tell is invalid.
 extern Charge const feeReferenceRPC;      // A default "reference" unspecified
                                           //   load
 extern Charge const feeExceptionRPC;      // RPC load that causes an exception
 extern Charge const feeMediumBurdenRPC;   // A somewhat burdensome RPC load
-extern Charge const feeHighBurdenRPC;     // A very burdensome RPC load
+extern Charge const feeHeavyBurdenRPC;     // A very burdensome RPC load
 
 // Peer loads
-extern Charge const feeLightPeer;         // Requires no reply
-extern Charge const feeMediumBurdenPeer;  // Requires some work
-extern Charge const feeHighBurdenPeer;    // Extensive work
+extern Charge const feeTrivialPeer;         // Requires no reply
+extern Charge const feeModerateBurdenPeer;  // Requires some work
+extern Charge const feeHeavyBurdenPeer;    // Extensive work
 
 // Administrative
 extern Charge const feeWarning;           // The cost of receiving a warning

diff --git a/include/xrpl/resource/detail/Logic.h b/include/xrpl/resource/detail/Logic.h
@@ -442,12 +442,34 @@ class Logic
     }
 
     Disposition
-    charge(Entry& entry, Charge const& fee)
+    charge(Entry& entry, Charge const& fee, std::string context = {})
     {
+        static constexpr Charge::value_type feeLogAsWarn = 3000;
+        static constexpr Charge::value_type feeLogAsInfo = 1000;
+        static constexpr Charge::value_type feeLogAsDebug = 100;
+        static_assert(
+            feeLogAsWarn > feeLogAsInfo && feeLogAsInfo > feeLogAsDebug &&
+            feeLogAsDebug > 10);
+
+        static auto getStream = [](Resource::Charge::value_type cost,
+                                   beast::Journal& journal) {
+            if (cost >= feeLogAsWarn)
+                return journal.warn();
+            if (cost >= feeLogAsInfo)
+                return journal.info();
+            if (cost >= feeLogAsDebug)
+                return journal.debug();
+            return journal.trace();
+        };
+
+        if (!context.empty())
+            context = " (" + context + ")";
+
         std::lock_guard _(lock_);
         clock_type::time_point const now(m_clock.now());
         int const balance(entry.add(fee.cost(), now));
-        JLOG(m_journal.trace()) << "Charging " << entry << " for " << fee;
+        JLOG(getStream(fee.cost(), m_journal))
+            << "Charging " << entry << " for " << fee << context;
         return disposition(balance);
     }
 

diff --git a/include/xrpl/resource/detail/Tuning.h b/include/xrpl/resource/detail/Tuning.h
@@ -32,7 +32,7 @@ enum {
 
     // Balance at which the consumer is disconnected
     ,
-    dropThreshold = 15000
+    dropThreshold = 25000
 
     // The number of seconds in the exponential decay window
     // (This should be a power of two)

diff --git a/src/libxrpl/resource/Charge.cpp b/src/libxrpl/resource/Charge.cpp
@@ -61,10 +61,10 @@ Charge::operator==(Charge const& c) const
     return c.m_cost == m_cost;
 }
 
-bool
-Charge::operator!=(Charge const& c) const
+std::strong_ordering
+Charge::operator<=>(Charge const& c) const
 {
-    return c.m_cost != m_cost;
+    return m_cost <=> c.m_cost;
 }
 
 }  // namespace Resource

diff --git a/src/libxrpl/resource/Consumer.cpp b/src/libxrpl/resource/Consumer.cpp
@@ -96,12 +96,12 @@ Consumer::disposition() const
 }
 
 Disposition
-Consumer::charge(Charge const& what)
+Consumer::charge(Charge const& what, std::string const& context)
 {
     Disposition d = ok;
 
     if (m_logic && m_entry && !m_entry->isUnlimited())
-        d = m_logic->charge(*m_entry, what);
+        d = m_logic->charge(*m_entry, what, context);
 
     return d;
 }

diff --git a/src/libxrpl/resource/Fees.cpp b/src/libxrpl/resource/Fees.cpp
@@ -22,24 +22,26 @@
 namespace ripple {
 namespace Resource {
 
-Charge const feeInvalidRequest(100, "malformed request");
+Charge const feeMalformedRequest(200, "malformed request");
 Charge const feeRequestNoReply(10, "unsatisfiable request");
-Charge const feeInvalidSignature(1000, "invalid signature");
-Charge const feeUnwantedData(150, "useless data");
-Charge const feeBadData(200, "invalid data");
+Charge const feeInvalidSignature(2000, "invalid signature");
+Charge const feeUselessData(150, "useless data");
+Charge const feeInvalidData(400, "invalid data");
 
-Charge const feeInvalidRPC(100, "malformed RPC");
+Charge const feeMalformedRPC(100, "malformed RPC");
 Charge const feeReferenceRPC(20, "reference RPC");
 Charge const feeExceptionRPC(100, "exceptioned RPC");
 Charge const feeMediumBurdenRPC(400, "medium RPC");
-Charge const feeHighBurdenRPC(3000, "heavy RPC");
+Charge const feeHeavyBurdenRPC(3000, "heavy RPC");
 
-Charge const feeLightPeer(1, "trivial peer request");
-Charge const feeMediumBurdenPeer(250, "moderate peer request");
-Charge const feeHighBurdenPeer(2000, "heavy peer request");
+Charge const feeTrivialPeer(1, "trivial peer request");
+Charge const feeModerateBurdenPeer(250, "moderate peer request");
+Charge const feeHeavyBurdenPeer(2000, "heavy peer request");
 
-Charge const feeWarning(2000, "received warning");
-Charge const feeDrop(3000, "dropped");
+Charge const feeWarning(4000, "received warning");
+Charge const feeDrop(6000, "dropped");
+
+// See also Resource::Logic::charge for log level cutoff values
 
 }  // namespace Resource
 }  // namespace ripple
diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp
@@ -221,7 +221,8 @@ class TestPeer : public Peer
         return {};
     }
     void
-    charge(Resource::Charge const& fee) override
+    charge(Resource::Charge const& fee, std::string const& context = {})
+        override
     {
     }
     id_t

diff --git a/src/test/overlay/reduce_relay_test.cpp b/src/test/overlay/reduce_relay_test.cpp
@@ -88,7 +88,8 @@ class PeerPartial : public Peer
         return {};
     }
     void
-    charge(Resource::Charge const& fee) override
+    charge(Resource::Charge const& fee, std::string const& context = {})
+        override
     {
     }
     bool

diff --git a/src/test/overlay/tx_reduce_relay_test.cpp b/src/test/overlay/tx_reduce_relay_test.cpp
@@ -16,6 +16,7 @@
     OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */
 //==============================================================================
+#include <test/jtx.h>
 #include <test/jtx/Env.h>
 #include <xrpld/overlay/detail/OverlayImpl.h>
 #include <xrpld/overlay/detail/PeerImp.h>
@@ -222,14 +223,21 @@ class tx_reduce_relay_test : public beast::unit_test::suite
         rid_ = 0;
         for (int i = 0; i < nPeers; i++)
             addPeer(env, peers, nDisabled);
-        protocol::TMTransaction m;
-        m.set_rawtransaction("transaction");
-        m.set_deferred(false);
-        m.set_status(protocol::TransactionStatus::tsNEW);
-        env.app().overlay().relay(uint256{0}, m, toSkip);
-        BEAST_EXPECT(
-            PeerTest::sendTx_ == expectRelay &&
-            PeerTest::queueTx_ == expectQueue);
+
+        auto const jtx = env.jt(noop(env.master));
+        if (BEAST_EXPECT(jtx.stx))
+        {
+            protocol::TMTransaction m;
+            Serializer s;
+            jtx.stx->add(s);
+            m.set_rawtransaction(s.data(), s.size());
+            m.set_deferred(false);
+            m.set_status(protocol::TransactionStatus::tsNEW);
+            env.app().overlay().relay(uint256{0}, m, toSkip);
+            BEAST_EXPECT(
+                PeerTest::sendTx_ == expectRelay &&
+                PeerTest::queueTx_ == expectQueue);
+        }
     }
 
     void

diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp
@@ -1063,7 +1063,8 @@ InboundLedger::processData(
         if (packet.nodes().empty())
         {
             JLOG(journal_.warn()) << peer->id() << ": empty header data";
-            peer->charge(Resource::feeInvalidRequest);
+            peer->charge(
+                Resource::feeMalformedRequest, "ledger_data empty header");
             return -1;
         }
 
@@ -1078,7 +1079,9 @@ InboundLedger::processData(
                 if (!takeHeader(packet.nodes(0).nodedata()))
                 {
                     JLOG(journal_.warn()) << "Got invalid header data";
-                    peer->charge(Resource::feeInvalidRequest);
+                    peer->charge(
+                        Resource::feeMalformedRequest,
+                        "ledger_data invalid header");
                     return -1;
                 }
 
@@ -1101,7 +1104,8 @@ InboundLedger::processData(
         {
             JLOG(journal_.warn())
                 << "Included AS/TX root invalid: " << ex.what();
-            peer->charge(Resource::feeBadData);
+            using namespace std::string_literals;
+            peer->charge(Resource::feeInvalidData, "ledger_data "s + ex.what());
             return -1;
         }
 
@@ -1121,7 +1125,7 @@ InboundLedger::processData(
         if (packet.nodes().empty())
         {
             JLOG(journal_.info()) << peer->id() << ": response with no nodes";
-            peer->charge(Resource::feeInvalidRequest);
+            peer->charge(Resource::feeMalformedRequest, "ledger_data no nodes");
             return -1;
         }
 
@@ -1133,7 +1137,8 @@ InboundLedger::processData(
             if (!node.has_nodeid() || !node.has_nodedata())
             {
                 JLOG(journal_.warn()) << "Got bad node";
-                peer->charge(Resource::feeInvalidRequest);
+                peer->charge(
+                    Resource::feeMalformedRequest, "ledger_data bad node");
                 return -1;
             }
         }

diff --git a/src/xrpld/app/ledger/detail/InboundTransactions.cpp b/src/xrpld/app/ledger/detail/InboundTransactions.cpp
@@ -146,7 +146,7 @@ class InboundTransactionsImp : public InboundTransactions
 
         if (ta == nullptr)
         {
-            peer->charge(Resource::feeUnwantedData);
+            peer->charge(Resource::feeUselessData, "ledger_data");
             return;
         }
 
@@ -157,23 +157,23 @@ class InboundTransactionsImp : public InboundTransactions
         {
             if (!node.has_nodeid() || !node.has_nodedata())
             {
-                peer->charge(Resource::feeInvalidRequest);
+                peer->charge(Resource::feeMalformedRequest, "ledger_data");
                 return;
             }
 
             auto const id = deserializeSHAMapNodeID(node.nodeid());
 
             if (!id)
             {
-                peer->charge(Resource::feeBadData);
+                peer->charge(Resource::feeInvalidData, "ledger_data");
                 return;
             }
 
             data.emplace_back(std::make_pair(*id, makeSlice(node.nodedata())));
         }
 
         if (!ta->takeNodes(data, peer).isUseful())
-            peer->charge(Resource::feeUnwantedData);
+            peer->charge(Resource::feeUselessData, "ledger_data not useful");
     }
 
     void

diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp
@@ -2116,22 +2116,22 @@ LedgerMaster::makeFetchPack(
     {
         JLOG(m_journal.info())
             << "Peer requests fetch pack for ledger we don't have: " << have;
-        peer->charge(Resource::feeRequestNoReply);
+        peer->charge(Resource::feeRequestNoReply, "get_object ledger");
         return;
     }
 
     if (have->open())
     {
         JLOG(m_journal.warn())
             << "Peer requests fetch pack from open ledger: " << have;
-        peer->charge(Resource::feeInvalidRequest);
+        peer->charge(Resource::feeMalformedRequest, "get_object ledger open");
         return;
     }
 
     if (have->info().seq < getEarliestFetch())
     {
         JLOG(m_journal.debug()) << "Peer requests fetch pack that is too early";
-        peer->charge(Resource::feeInvalidRequest);
+        peer->charge(Resource::feeMalformedRequest, "get_object ledger early");
         return;
     }
 
@@ -2142,7 +2142,8 @@ LedgerMaster::makeFetchPack(
         JLOG(m_journal.info())
             << "Peer requests fetch pack for ledger whose predecessor we "
             << "don't have: " << have;
-        peer->charge(Resource::feeRequestNoReply);
+        peer->charge(
+            Resource::feeRequestNoReply, "get_object ledger no parent");
         return;
     }
 

diff --git a/src/xrpld/overlay/Overlay.h b/src/xrpld/overlay/Overlay.h
@@ -183,7 +183,7 @@ class Overlay : public beast::PropertyStream::Source
     virtual void
     relay(
         uint256 const& hash,
-        protocol::TMTransaction& m,
+        std::optional<std::reference_wrapper<protocol::TMTransaction>> m,
         std::set<Peer::id_t> const& toSkip) = 0;
 
     /** Visit every active peer.

diff --git a/src/xrpld/overlay/Peer.h b/src/xrpld/overlay/Peer.h
@@ -77,7 +77,7 @@ class Peer
 
     /** Adjust this peer's load balance based on the type of load imposed. */
     virtual void
-    charge(Resource::Charge const& fee) = 0;
+    charge(Resource::Charge const& fee, std::string const& context) = 0;
 
     //
     // Identity