Proposed 2.3.1-rc1: Reduce the peer charges for well-behaved peers

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

RELEASENOTES.md

@@ -6,6 +6,62 @@ This document contains the release notes for `rippled`, the reference server imp
 
 Have new ideas? Need help with setting up your node? [Please open an issue here](https://github.com/xrplf/rippled/issues/new/choose).
 
+# Version 2.3.1
+
+Version 2.3.1 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. 
+This is a hotfix release that includes the following updates:
+- Fix an erroneous high fee penalty that peers could incur for sending older transactions. 
+- Update to the fees charged for imposing a load on the server. 
+- Prevent the relaying of internal pseudo-transactions. 
+    - 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 charged for an invalid signature. 
+    - After: Pseudo-transactions, are put into the global cache (TransactionMaster) only. If the transaction is not part of a TMTransactions batch, 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.
+- Improved logging now specifies the reason for the fee charged to the peer.
+
+[Sign Up for Future Release Announcements](https://groups.google.com/g/ripple-server)
+
+<!-- BREAK -->
+
+## Action Required
+
+If you run an XRP Ledger validator, upgrade to version 2.3.1 as soon as possible to ensure stable and uninterrupted network behavior.
+
+## Changelog
+
+### Amendments and New Features
+
+- None
+
+### Bug Fixes and Performance Improvements
+
+- Change the charged fee for sending older transactions from feeInvalidSignature to feeUnwantedData. [#5243](https://github.com/XRPLF/rippled/pull/5243)
+
+### Docs and Build System
+
+- None
+
+### GitHub
+
+The public source code repository for `rippled` is hosted on GitHub at <https://github.com/XRPLF/rippled>.
+
+We welcome all contributions and invite everyone to join the community of XRP Ledger developers to help build the Internet of Value.
+
+
+## Credits
+
+The following people contributed directly to this release:
+
+Ed Hennis <[email protected]>
+JoelKatz <[email protected]>
+Sophia Xie <[email protected]>
+Valentin Balaschenko <[email protected]>
+
+
+Bug Bounties and Responsible Disclosures:
+
+We welcome reviews of the `rippled` code and urge researchers to responsibly disclose any issues they may find.
+
+To report a bug, please send a detailed report to: <[email protected]>
+
 # Version 2.3.0
 
 Version 2.3.0 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release includes 8 new amendments, including Multi-Purpose Tokens, Credentials, Clawback support for AMMs, and the ability to make offers as part of minting NFTs. Additionally, this release includes important fixes for stability, so server operators are encouraged to upgrade as soon as possible.

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;

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.

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

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);
     }
 

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)

src/libxrpl/protocol/BuildInfo.cpp

@@ -33,7 +33,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "2.3.0"
+char const* const versionString = "2.3.1-rc1"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/libxrpl/resource/Charge.cpp

@@ -61,10 +61,10 @@
     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

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;
 }

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

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

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

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

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;
             }
         }

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