From b14c24960bbe904cdeb3a4915b45ea4f7e42dd3d Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Mon, 13 Jan 2025 21:08:38 +0000 Subject: [PATCH 1/4] Reduce the peer charges for well-behaved peers: - 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 charge 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. - Improve logging: include the reason for fees charged to a peer. Co-authored-by: Ed Hennis --- Builds/levelization/results/ordering.txt | 1 + RELEASENOTES.md | 56 +++++ include/xrpl/resource/Charge.h | 5 +- include/xrpl/resource/Consumer.h | 2 +- include/xrpl/resource/Fees.h | 16 +- include/xrpl/resource/detail/Logic.h | 26 +- include/xrpl/resource/detail/Tuning.h | 2 +- src/libxrpl/resource/Charge.cpp | 6 +- src/libxrpl/resource/Consumer.cpp | 4 +- src/libxrpl/resource/Fees.cpp | 24 +- src/test/app/LedgerReplay_test.cpp | 3 +- src/test/overlay/reduce_relay_test.cpp | 3 +- src/test/overlay/tx_reduce_relay_test.cpp | 24 +- src/xrpld/app/ledger/detail/InboundLedger.cpp | 15 +- .../app/ledger/detail/InboundTransactions.cpp | 8 +- src/xrpld/app/ledger/detail/LedgerMaster.cpp | 9 +- src/xrpld/overlay/Overlay.h | 2 +- src/xrpld/overlay/Peer.h | 2 +- src/xrpld/overlay/detail/OverlayImpl.cpp | 35 ++- src/xrpld/overlay/detail/OverlayImpl.h | 2 +- src/xrpld/overlay/detail/PeerImp.cpp | 235 ++++++++++++------ src/xrpld/overlay/detail/PeerImp.h | 33 ++- src/xrpld/rpc/detail/RPCHelpers.cpp | 2 +- src/xrpld/rpc/detail/ServerHandler.cpp | 18 +- src/xrpld/rpc/handlers/GatewayBalances.cpp | 2 +- src/xrpld/rpc/handlers/LedgerHandler.cpp | 2 +- src/xrpld/rpc/handlers/PathFind.cpp | 2 +- src/xrpld/rpc/handlers/RipplePathFind.cpp | 2 +- src/xrpld/rpc/handlers/SignFor.cpp | 2 +- src/xrpld/rpc/handlers/SignHandler.cpp | 2 +- src/xrpld/rpc/handlers/SubmitMultiSigned.cpp | 2 +- 31 files changed, 385 insertions(+), 162 deletions(-) diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt index 396c99cb711..fd5fc272cb8 100644 --- 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/RELEASENOTES.md b/RELEASENOTES.md index c6e8266e348..5d4d14d503c 100644 --- a/RELEASENOTES.md +++ b/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) + + + +## 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 . + +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 +JoelKatz +Sophia Xie <106177003+sophiax851@users.noreply.github.com> +Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> + + +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: + # 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. diff --git a/include/xrpl/resource/Charge.h b/include/xrpl/resource/Charge.h index 000417db4d0..e5799710b2c 100644 --- 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 index 00e4ad2c5f5..00d8a7b58b4 100644 --- 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 index 1eb1a9bd725..f85e7c8cd20 100644 --- 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 index a57e529e0a2..651778e1bee 100644 --- 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 index b57d57348b9..62234c35693 100644 --- 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 index deec6b34eb6..66b5049dbd3 100644 --- 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 index b8652546841..f938f2acce6 100644 --- 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 index 13ae38bc6ad..17fb475394c 100644 --- 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 index 72db7443110..3ebf0591782 100644 --- 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 index e0b0d006a2f..e0edae54897 100644 --- 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 index 3065bcbb685..3d6c2a4d1ef 100644 --- 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 #include #include #include @@ -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 index 16b15c2fce7..b3111dc3fee 100644 --- 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 index 83b074f317a..b8f327ff8a3 100644 --- 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,7 +157,7 @@ class InboundTransactionsImp : public InboundTransactions { if (!node.has_nodeid() || !node.has_nodedata()) { - peer->charge(Resource::feeInvalidRequest); + peer->charge(Resource::feeMalformedRequest, "ledger_data"); return; } @@ -165,7 +165,7 @@ class InboundTransactionsImp : public InboundTransactions if (!id) { - peer->charge(Resource::feeBadData); + peer->charge(Resource::feeInvalidData, "ledger_data"); return; } @@ -173,7 +173,7 @@ class InboundTransactionsImp : public InboundTransactions } 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 index 53edef17d33..57f9dd03604 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -2116,7 +2116,7 @@ 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; } @@ -2124,14 +2124,14 @@ LedgerMaster::makeFetchPack( { 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 index b9550ba2ef4..9aff1560682 100644 --- 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> m, std::set const& toSkip) = 0; /** Visit every active peer. diff --git a/src/xrpld/overlay/Peer.h b/src/xrpld/overlay/Peer.h index 82ed2c2481a..2646b24a3ed 100644 --- 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 diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index e41a08a43d1..72d2a32201d 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -1199,17 +1200,39 @@ OverlayImpl::getManifestsMessage() void OverlayImpl::relay( uint256 const& hash, - protocol::TMTransaction& m, + std::optional> tx, std::set const& toSkip) { - auto const sm = std::make_shared(m, protocol::mtTRANSACTION); + bool relay = tx.has_value(); + if (relay) + { + auto& txn = tx->get(); + SerialIter sit(makeSlice(txn.rawtransaction())); + relay = !isPseudoTx(STTx{sit}); + } + + Overlay::PeerSequence peers = {}; std::size_t total = 0; std::size_t disabled = 0; std::size_t enabledInSkip = 0; - // total peers excluding peers in toSkip - auto peers = getActivePeers(toSkip, total, disabled, enabledInSkip); - auto minRelay = app_.config().TX_REDUCE_RELAY_MIN_PEERS + disabled; + if (!relay) + { + if (!app_.config().TX_REDUCE_RELAY_ENABLE) + return; + + peers = getActivePeers(toSkip, total, disabled, enabledInSkip); + JLOG(journal_.trace()) + << "not relaying tx, total peers " << peers.size(); + for (auto const& p : peers) + p->addTxQueue(hash); + return; + } + + auto& txn = tx->get(); + auto const sm = std::make_shared(txn, protocol::mtTRANSACTION); + peers = getActivePeers(toSkip, total, disabled, enabledInSkip); + auto const minRelay = app_.config().TX_REDUCE_RELAY_MIN_PEERS + disabled; if (!app_.config().TX_REDUCE_RELAY_ENABLE || total <= minRelay) { @@ -1224,7 +1247,7 @@ OverlayImpl::relay( // We have more peers than the minimum (disabled + minimum enabled), // relay to all disabled and some randomly selected enabled that // do not have the transaction. - auto enabledTarget = app_.config().TX_REDUCE_RELAY_MIN_PEERS + + auto const enabledTarget = app_.config().TX_REDUCE_RELAY_MIN_PEERS + (total - minRelay) * app_.config().TX_RELAY_PERCENTAGE / 100; txMetrics_.addMetrics(enabledTarget, toSkip.size(), disabled); diff --git a/src/xrpld/overlay/detail/OverlayImpl.h b/src/xrpld/overlay/detail/OverlayImpl.h index a50dfc5e905..9ff6bb2a45f 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -238,7 +238,7 @@ class OverlayImpl : public Overlay, public reduce_relay::SquelchHandler void relay( uint256 const&, - protocol::TMTransaction& m, + std::optional> m, std::set const& skip) override; std::shared_ptr diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 4f5f1470f8e..d36add97ba1 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -62,6 +62,9 @@ std::chrono::milliseconds constexpr peerHighLatency{300}; std::chrono::seconds constexpr peerTimerInterval{60}; } // namespace +// TODO: Remove this exclusion once unit tests are added after the hotfix +// release. + PeerImp::PeerImp( Application& app, id_t id, @@ -95,7 +98,7 @@ PeerImp::PeerImp( , creationTime_(clock_type::now()) , squelch_(app_.journal("Squelch")) , usage_(consumer) - , fee_(Resource::feeLightPeer) + , fee_{Resource::feeTrivialPeer, ""} , slot_(slot) , request_(std::move(request)) , headers_(request_) @@ -339,9 +342,9 @@ PeerImp::removeTxQueue(uint256 const& hash) } void -PeerImp::charge(Resource::Charge const& fee) +PeerImp::charge(Resource::Charge const& fee, std::string const& context) { - if ((usage_.charge(fee) == Resource::drop) && + if ((usage_.charge(fee, context) == Resource::drop) && usage_.disconnect(p_journal_) && strand_.running_in_this_thread()) { // Sever the connection @@ -997,9 +1000,9 @@ PeerImp::onMessageBegin( std::size_t uncompressed_size, bool isCompressed) { - load_event_ = - app_.getJobQueue().makeLoadEvent(jtPEER, protocolMessageName(type)); - fee_ = Resource::feeLightPeer; + auto const name = protocolMessageName(type); + load_event_ = app_.getJobQueue().makeLoadEvent(jtPEER, name); + fee_ = {Resource::feeTrivialPeer, name}; auto const category = TrafficCount::categorize(*m, type, true); overlay_.reportTraffic(category, true, static_cast(size)); using namespace protocol; @@ -1029,7 +1032,7 @@ PeerImp::onMessageEnd( std::shared_ptr<::google::protobuf::Message> const&) { load_event_.reset(); - charge(fee_); + charge(fee_.fee, fee_.context); } void @@ -1039,12 +1042,12 @@ PeerImp::onMessage(std::shared_ptr const& m) if (s == 0) { - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "empty"); return; } if (s > 100) - fee_ = Resource::feeMediumBurdenPeer; + fee_.update(Resource::feeModerateBurdenPeer, "oversize"); app_.getJobQueue().addJob( jtMANIFEST, "receiveManifests", [this, that = shared_from_this(), m]() { @@ -1058,7 +1061,7 @@ PeerImp::onMessage(std::shared_ptr const& m) if (m->type() == protocol::TMPing::ptPING) { // We have received a ping request, reply with a pong - fee_ = Resource::feeMediumBurdenPeer; + fee_.update(Resource::feeModerateBurdenPeer, "ping request"); m->set_type(protocol::TMPing::ptPONG); send(std::make_shared(*m, protocol::mtPING)); return; @@ -1095,7 +1098,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // VFALCO NOTE I think we should drop the peer immediately if (!cluster()) { - fee_ = Resource::feeUnwantedData; + fee_.fee = Resource::feeUselessData; return; } @@ -1173,7 +1176,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // implication for the protocol. if (m->endpoints_v2().size() >= 1024) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "endpoints too large"); return; } @@ -1188,7 +1191,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.error()) << "failed to parse incoming endpoint: {" << tm.endpoint() << "}"; - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "endpoints malformed"); continue; } @@ -1211,14 +1214,19 @@ PeerImp::onMessage(std::shared_ptr const& m) void PeerImp::onMessage(std::shared_ptr const& m) { - handleTransaction(m, true); + handleTransaction(m, true, false); } void PeerImp::handleTransaction( std::shared_ptr const& m, - bool eraseTxQueue) + bool eraseTxQueue, + bool batch) { + assert( + (eraseTxQueue != batch) && + // Include a message to make this easier to convert to XRPL_ASSERT + ("ripple::PeerImp::handleTransaction correct function params")); if (tracking_.load() == Tracking::diverged) return; @@ -1246,7 +1254,7 @@ PeerImp::handleTransaction( // we have seen this transaction recently if (flags & SF_BAD) { - fee_ = Resource::feeInvalidSignature; + fee_.update(Resource::feeUselessData, "known bad"); JLOG(p_journal_.debug()) << "Ignoring known bad tx " << txID; } @@ -1300,9 +1308,11 @@ PeerImp::handleTransaction( [weak = std::weak_ptr(shared_from_this()), flags, checkSignature, + batch, stx]() { if (auto peer = weak.lock()) - peer->checkTransaction(flags, checkSignature, stx); + peer->checkTransaction( + flags, checkSignature, stx, batch); }); } } @@ -1318,7 +1328,7 @@ void PeerImp::onMessage(std::shared_ptr const& m) { auto badData = [&](std::string const& msg) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "get_ledger " + msg); JLOG(p_journal_.warn()) << "TMGetLedger: " << msg; }; auto const itype{m->itype()}; @@ -1409,11 +1419,12 @@ PeerImp::onMessage(std::shared_ptr const& m) JLOG(p_journal_.trace()) << "onMessage, TMProofPathRequest"; if (!ledgerReplayEnabled_) { - charge(Resource::feeInvalidRequest); + charge(Resource::feeMalformedRequest, "proof_path_request disabled"); return; } - fee_ = Resource::feeMediumBurdenPeer; + fee_.update( + Resource::feeModerateBurdenPeer, "received a proof path request"); std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( jtREPLAY_REQ, "recvProofPathRequest", [weak, m]() { @@ -1424,9 +1435,12 @@ PeerImp::onMessage(std::shared_ptr const& m) if (reply.has_error()) { if (reply.error() == protocol::TMReplyError::reBAD_REQUEST) - peer->charge(Resource::feeInvalidRequest); + peer->charge( + Resource::feeMalformedRequest, + "proof_path_request"); else - peer->charge(Resource::feeRequestNoReply); + peer->charge( + Resource::feeRequestNoReply, "proof_path_request"); } else { @@ -1442,13 +1456,13 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!ledgerReplayEnabled_) { - charge(Resource::feeInvalidRequest); + charge(Resource::feeMalformedRequest, "proof_path_response disabled"); return; } if (!ledgerReplayMsgHandler_.processProofPathResponse(m)) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "proof_path_response"); } } @@ -1458,11 +1472,11 @@ PeerImp::onMessage(std::shared_ptr const& m) JLOG(p_journal_.trace()) << "onMessage, TMReplayDeltaRequest"; if (!ledgerReplayEnabled_) { - charge(Resource::feeInvalidRequest); + charge(Resource::feeMalformedRequest, "replay_delta_request disabled"); return; } - fee_ = Resource::feeMediumBurdenPeer; + fee_.fee = Resource::feeModerateBurdenPeer; std::weak_ptr weak = shared_from_this(); app_.getJobQueue().addJob( jtREPLAY_REQ, "recvReplayDeltaRequest", [weak, m]() { @@ -1473,9 +1487,13 @@ PeerImp::onMessage(std::shared_ptr const& m) if (reply.has_error()) { if (reply.error() == protocol::TMReplyError::reBAD_REQUEST) - peer->charge(Resource::feeInvalidRequest); + peer->charge( + Resource::feeMalformedRequest, + "replay_delta_request"); else - peer->charge(Resource::feeRequestNoReply); + peer->charge( + Resource::feeRequestNoReply, + "replay_delta_request"); } else { @@ -1491,13 +1509,13 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!ledgerReplayEnabled_) { - charge(Resource::feeInvalidRequest); + charge(Resource::feeMalformedRequest, "replay_delta_response disabled"); return; } if (!ledgerReplayMsgHandler_.processReplayDeltaResponse(m)) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "replay_delta_response"); } } @@ -1505,7 +1523,7 @@ void PeerImp::onMessage(std::shared_ptr const& m) { auto badData = [&](std::string const& msg) { - fee_ = Resource::feeBadData; + fee_.update(Resource::feeInvalidData, msg); JLOG(p_journal_.warn()) << "TMLedgerData: " << msg; }; @@ -1605,7 +1623,9 @@ PeerImp::onMessage(std::shared_ptr const& m) (publicKeyType(makeSlice(set.nodepubkey())) != KeyType::secp256k1)) { JLOG(p_journal_.warn()) << "Proposal: malformed"; - fee_ = Resource::feeInvalidSignature; + fee_.update( + Resource::feeInvalidSignature, + " signature can't be longer than 72 bytes"); return; } @@ -1613,7 +1633,7 @@ PeerImp::onMessage(std::shared_ptr const& m) !stringIsUint256Sized(set.previousledger())) { JLOG(p_journal_.warn()) << "Proposal: malformed"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "bad hashes"); return; } @@ -1918,7 +1938,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!stringIsUint256Sized(m->hash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "bad hash"); return; } @@ -1931,7 +1951,7 @@ PeerImp::onMessage(std::shared_ptr const& m) if (std::find(recentTxSets_.begin(), recentTxSets_.end(), hash) != recentTxSets_.end()) { - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "duplicate (tsHAVE)"); return; } @@ -1953,7 +1973,7 @@ PeerImp::onValidatorListMessage( JLOG(p_journal_.warn()) << "Ignored malformed " << messageType << " from peer " << remote_address_; // This shouldn't ever happen with a well-behaved peer - fee_ = Resource::feeHighBurdenPeer; + fee_.update(Resource::feeHeavyBurdenPeer, "no blobs"); return; } @@ -1970,7 +1990,7 @@ PeerImp::onValidatorListMessage( // Charging this fee here won't hurt the peer in the normal // course of operation (ie. refresh every 5 minutes), but // will add up if the peer is misbehaving. - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "duplicate"); return; } @@ -2049,27 +2069,30 @@ PeerImp::onValidatorListMessage( // Charging this fee here won't hurt the peer in the normal // course of operation (ie. refresh every 5 minutes), but // will add up if the peer is misbehaving. - fee_ = Resource::feeUnwantedData; + fee_.update( + Resource::feeUselessData, + " duplicate (same_sequence or known_sequence)"); break; case ListDisposition::stale: // There are very few good reasons for a peer to send an // old list, particularly more than once. - fee_ = Resource::feeBadData; + fee_.update(Resource::feeInvalidData, "expired"); break; case ListDisposition::untrusted: // Charging this fee here won't hurt the peer in the normal // course of operation (ie. refresh every 5 minutes), but // will add up if the peer is misbehaving. - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "untrusted"); break; case ListDisposition::invalid: // This shouldn't ever happen with a well-behaved peer - fee_ = Resource::feeInvalidSignature; + fee_.update( + Resource::feeInvalidSignature, "invalid list disposition"); break; case ListDisposition::unsupported_version: // During a version transition, this may be legitimate. // If it happens frequently, that's probably bad. - fee_ = Resource::feeBadData; + fee_.update(Resource::feeInvalidData, "version"); break; default: assert(false); @@ -2146,7 +2169,7 @@ PeerImp::onMessage(std::shared_ptr const& m) << "ValidatorList: received validator list from peer using " << "protocol version " << to_string(protocol_) << " which shouldn't support this feature."; - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "unsupported peer"); return; } onValidatorListMessage( @@ -2159,7 +2182,8 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.warn()) << "ValidatorList: Exception, " << e.what() << " from peer " << remote_address_; - fee_ = Resource::feeBadData; + using namespace std::string_literals; + fee_.update(Resource::feeInvalidData, e.what()); } } @@ -2175,7 +2199,7 @@ PeerImp::onMessage( << "ValidatorListCollection: received validator list from peer " << "using protocol version " << to_string(protocol_) << " which shouldn't support this feature."; - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "unsupported peer"); return; } else if (m->version() < 2) @@ -2185,7 +2209,7 @@ PeerImp::onMessage( "version " << m->version() << " from peer using protocol version " << to_string(protocol_); - fee_ = Resource::feeBadData; + fee_.update(Resource::feeInvalidData, "wrong version"); return; } onValidatorListMessage( @@ -2198,7 +2222,8 @@ PeerImp::onMessage( { JLOG(p_journal_.warn()) << "ValidatorListCollection: Exception, " << e.what() << " from peer " << remote_address_; - fee_ = Resource::feeBadData; + using namespace std::string_literals; + fee_.update(Resource::feeInvalidData, e.what()); } } @@ -2208,7 +2233,7 @@ PeerImp::onMessage(std::shared_ptr const& m) if (m->validation().size() < 50) { JLOG(p_journal_.warn()) << "Validation: Too small"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "too small"); return; } @@ -2236,7 +2261,7 @@ PeerImp::onMessage(std::shared_ptr const& m) val->getSeenTime())) { JLOG(p_journal_.trace()) << "Validation: Not current"; - fee_ = Resource::feeUnwantedData; + fee_.update(Resource::feeUselessData, "not current"); return; } @@ -2309,7 +2334,8 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.warn()) << "Exception processing validation: " << e.what(); - fee_ = Resource::feeInvalidRequest; + using namespace std::string_literals; + fee_.update(Resource::feeMalformedRequest, e.what()); } } @@ -2342,7 +2368,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.error()) << "TMGetObjectByHash: tx reduce-relay is disabled"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "disabled"); return; } @@ -2355,7 +2381,9 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - fee_ = Resource::feeMediumBurdenPeer; + fee_.update( + Resource::feeModerateBurdenPeer, + " received a get object by hash request"); protocol::TMGetObjectByHash reply; @@ -2370,7 +2398,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!stringIsUint256Sized(packet.ledgerhash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "ledger hash"); return; } @@ -2474,7 +2502,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.error()) << "TMHaveTransactions: tx reduce-relay is disabled"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "disabled"); return; } @@ -2503,7 +2531,7 @@ PeerImp::handleHaveTransactions( { JLOG(p_journal_.error()) << "TMHaveTransactions with invalid hash size"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "hash size"); return; } @@ -2543,7 +2571,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { JLOG(p_journal_.error()) << "TMTransactions: tx reduce-relay is disabled"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "disabled"); return; } @@ -2556,7 +2584,8 @@ PeerImp::onMessage(std::shared_ptr const& m) handleTransaction( std::shared_ptr( m->mutable_transactions(i), [](protocol::TMTransaction*) {}), - false); + false, + true); } void @@ -2572,14 +2601,14 @@ PeerImp::onMessage(std::shared_ptr const& m) if (!m->has_validatorpubkey()) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "squelch no pubkey"); return; } auto validator = m->validatorpubkey(); auto const slice{makeSlice(validator)}; if (!publicKeyType(slice)) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "squelch bad pubkey"); return; } PublicKey key(slice); @@ -2587,7 +2616,7 @@ PeerImp::onMessage(std::shared_ptr const& m) // Ignore non-validator squelch if (!app_.validators().listed(key)) { - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "squelch non-validator"); JLOG(p_journal_.debug()) << "onMessage: TMSquelch discarding non-validator squelch " << slice; @@ -2607,7 +2636,7 @@ PeerImp::onMessage(std::shared_ptr const& m) if (!m->squelch()) squelch_.removeSquelch(key); else if (!squelch_.addSquelch(key, std::chrono::seconds{duration})) - charge(Resource::feeBadData); + charge(Resource::feeInvalidData, "squelch duration"); JLOG(p_journal_.debug()) << "onMessage: TMSquelch " << slice << " " << id() << " " << duration; @@ -2648,11 +2677,11 @@ PeerImp::doFetchPack(const std::shared_ptr& packet) if (!stringIsUint256Sized(packet->ledgerhash())) { JLOG(p_journal_.warn()) << "FetchPack hash size malformed"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "hash size"); return; } - fee_ = Resource::feeHighBurdenPeer; + fee_.fee = Resource::feeHeavyBurdenPeer; uint256 const hash{packet->ledgerhash()}; @@ -2677,7 +2706,7 @@ PeerImp::doTransactions( if (packet->objects_size() > reduce_relay::MAX_TX_QUEUE_SIZE) { JLOG(p_journal_.error()) << "doTransactions, invalid number of hashes"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "too big"); return; } @@ -2687,7 +2716,7 @@ PeerImp::doTransactions( if (!stringIsUint256Sized(obj.hash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "hash size"); return; } @@ -2699,7 +2728,7 @@ PeerImp::doTransactions( { JLOG(p_journal_.error()) << "doTransactions, transaction not found " << Slice(hash.data(), hash.size()); - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "tx not found"); return; } @@ -2724,7 +2753,8 @@ void PeerImp::checkTransaction( int flags, bool checkSignature, - std::shared_ptr const& stx) + std::shared_ptr const& stx, + bool batch) { // VFALCO TODO Rewrite to not use exceptions try @@ -2735,10 +2765,45 @@ PeerImp::checkTransaction( app_.getLedgerMaster().getValidLedgerIndex())) { app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); - charge(Resource::feeUnwantedData); + charge(Resource::feeUselessData, "expired tx"); return; } + if (isPseudoTx(*stx)) + { + // Don't do anything with pseudo transactions except put them in the + // TransactionMaster cache + std::string reason; + auto tx = std::make_shared(stx, reason, app_); + assert(tx->getStatus() == NEW); + if (tx->getStatus() == NEW) + { + JLOG(p_journal_.debug()) + << "Processing " << (batch ? "batch" : "unsolicited") + << " pseudo-transaction tx " << tx->getID(); + + app_.getMasterTransaction().canonicalize(&tx); + // Tell the overlay about it, but don't relay it. + auto const toSkip = + app_.getHashRouter().shouldRelay(tx->getID()); + if (toSkip) + { + JLOG(p_journal_.debug()) + << "Passing skipped pseudo pseudo-transaction tx " + << tx->getID(); + app_.overlay().relay(tx->getID(), {}, *toSkip); + } + if (!batch) + { + JLOG(p_journal_.debug()) + << "Charging for pseudo-transaction tx " << tx->getID(); + charge(Resource::feeUselessData, "pseudo tx"); + } + + return; + } + } + if (checkSignature) { // Check the signature before handing off to the job queue. @@ -2757,7 +2822,9 @@ PeerImp::checkTransaction( // Probably not necessary to set SF_BAD, but doesn't hurt. app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); - charge(Resource::feeInvalidSignature); + charge( + Resource::feeInvalidSignature, + "check transaction signature failure"); return; } } @@ -2778,7 +2845,7 @@ PeerImp::checkTransaction( << "Exception checking transaction: " << reason; } app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); - charge(Resource::feeInvalidSignature); + charge(Resource::feeInvalidSignature, "tx (impossible)"); return; } @@ -2791,7 +2858,8 @@ PeerImp::checkTransaction( JLOG(p_journal_.warn()) << "Exception in " << __func__ << ": " << ex.what(); app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); - charge(Resource::feeBadData); + using namespace std::string_literals; + charge(Resource::feeInvalidData, "tx "s + ex.what()); } } @@ -2809,8 +2877,9 @@ PeerImp::checkPropose( if (!cluster() && !peerPos.checkSign()) { - JLOG(p_journal_.warn()) << "Proposal fails sig check"; - charge(Resource::feeInvalidSignature); + std::string desc{"Proposal fails sig check"}; + JLOG(p_journal_.warn()) << desc; + charge(Resource::feeInvalidSignature, desc); return; } @@ -2846,8 +2915,9 @@ PeerImp::checkValidation( { if (!val->isValid()) { - JLOG(p_journal_.debug()) << "Validation forwarded by peer is invalid"; - charge(Resource::feeInvalidSignature); + std::string desc{"Validation forwarded by peer is invalid"}; + JLOG(p_journal_.debug()) << desc; + charge(Resource::feeInvalidSignature, desc); return; } @@ -2877,7 +2947,8 @@ PeerImp::checkValidation( { JLOG(p_journal_.trace()) << "Exception processing validation: " << ex.what(); - charge(Resource::feeInvalidRequest); + using namespace std::string_literals; + charge(Resource::feeMalformedRequest, "validation "s + ex.what()); } } @@ -3046,7 +3117,8 @@ PeerImp::getLedger(std::shared_ptr const& m) { // Do not resource charge a peer responding to a relay if (!m->has_requestcookie()) - charge(Resource::feeInvalidRequest); + charge( + Resource::feeMalformedRequest, "get_ledger ledgerSeq"); ledger.reset(); JLOG(p_journal_.warn()) @@ -3108,7 +3180,8 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) { // Do not resource charge a peer responding to a relay if (!m->has_requestcookie()) - charge(Resource::feeMediumBurdenPeer); + charge( + Resource::feeModerateBurdenPeer, "received a get ledger request"); std::shared_ptr ledger; std::shared_ptr sharedMap; @@ -3218,6 +3291,9 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) for (auto const& d : data) { + if (ledgerData.nodes_size() >= + Tuning::hardMaxReplyNodes) + break; protocol::TMLedgerNode* node{ledgerData.add_nodes()}; node->set_nodeid(d.first.getRawString()); node->set_nodedata(d.second.data(), d.second.size()); @@ -3272,6 +3348,9 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) << ledgerData.nodes_size() << " nodes"; } + if (ledgerData.nodes_size() == 0) + return; + send(std::make_shared(ledgerData, protocol::mtLEDGER_DATA)); } diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index 9c76ddb4db8..9f52c1c7ade 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -145,10 +145,28 @@ class PeerImp : public Peer, // // June 2019 + struct ChargeWithContext + { + Resource::Charge fee = Resource::feeTrivialPeer; + std::string context = {}; + + void + update(Resource::Charge f, std::string const& add) + { + assert(f >= fee); + fee = f; + if (!context.empty()) + { + context += " "; + } + context += add; + } + }; + std::mutex mutable recentLock_; protocol::TMStatusChange last_status_; Resource::Consumer usage_; - Resource::Charge fee_; + ChargeWithContext fee_; std::shared_ptr const slot_; boost::beast::multi_buffer read_buffer_; http_request_type request_; @@ -304,7 +322,7 @@ class PeerImp : public Peer, } void - charge(Resource::Charge const& fee) override; + charge(Resource::Charge const& fee, std::string const& context) override; // // Identity @@ -482,11 +500,15 @@ class PeerImp : public Peer, the queue when called from onMessage(TMTransactions) because this message is a response to the missing transactions request and the queue would not have any of these transactions. + @param batch is false when called from onMessage(TMTransaction) + and is true when called from onMessage(TMTransactions). If true, then the + transaction is part of a batch, and should not be charged an extra fee. */ void handleTransaction( std::shared_ptr const& m, - bool eraseTxQueue); + bool eraseTxQueue, + bool batch); /** Handle protocol message with hashes of transactions that have not been relayed by an upstream node down to its peers - request @@ -598,7 +620,8 @@ class PeerImp : public Peer, checkTransaction( int flags, bool checkSignature, - std::shared_ptr const& stx); + std::shared_ptr const& stx, + bool batch); void checkPropose( @@ -664,7 +687,7 @@ PeerImp::PeerImp( , creationTime_(clock_type::now()) , squelch_(app_.journal("Squelch")) , usage_(usage) - , fee_(Resource::feeLightPeer) + , fee_{Resource::feeTrivialPeer} , slot_(std::move(slot)) , response_(std::move(response)) , headers_(response_) diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index af204eaedf7..ce6639d21f1 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -1043,7 +1043,7 @@ getLedgerByContext(RPC::JsonContext& context) "Exactly one of ledger_hash and ledger_index can be set."); } - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; if (hasHash) { diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index ccf0c12b5ad..082cb6b74cf 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -444,7 +444,7 @@ ServerHandler::processSession( if (jv.isMember(jss::api_version)) jr[jss::api_version] = jv[jss::api_version]; - is->getConsumer().charge(Resource::feeInvalidRPC); + is->getConsumer().charge(Resource::feeMalformedRPC); return jr; } @@ -461,7 +461,7 @@ ServerHandler::processSession( is->user()); if (Role::FORBID == role) { - loadType = Resource::feeInvalidRPC; + loadType = Resource::feeMalformedRPC; jr[jss::result] = rpcError(rpcFORBIDDEN); } else @@ -729,7 +729,7 @@ ServerHandler::processRequest( if (role == Role::FORBID) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(403, "Forbidden", output, rpcJ); @@ -743,7 +743,7 @@ ServerHandler::processRequest( if (!jsonRPC.isMember(jss::method) || jsonRPC[jss::method].isNull()) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(400, "Null method", output, rpcJ); @@ -758,7 +758,7 @@ ServerHandler::processRequest( Json::Value const& method = jsonRPC[jss::method]; if (!method.isString()) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(400, "method is not string", output, rpcJ); @@ -774,7 +774,7 @@ ServerHandler::processRequest( std::string strMethod = method.asString(); if (strMethod.empty()) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(400, "method is empty", output, rpcJ); @@ -802,7 +802,7 @@ ServerHandler::processRequest( else if (!params.isArray() || params.size() != 1) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); HTTPReply(400, "params unparseable", output, rpcJ); return; } @@ -811,7 +811,7 @@ ServerHandler::processRequest( params = std::move(params[0u]); if (!params.isObjectOrNull()) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); HTTPReply(400, "params unparseable", output, rpcJ); return; } @@ -827,7 +827,7 @@ ServerHandler::processRequest( { if (!params[jss::ripplerpc].isString()) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(400, "ripplerpc is not a string", output, rpcJ); diff --git a/src/xrpld/rpc/handlers/GatewayBalances.cpp b/src/xrpld/rpc/handlers/GatewayBalances.cpp index 8fd13d472cc..26f338cb4f9 100644 --- a/src/xrpld/rpc/handlers/GatewayBalances.cpp +++ b/src/xrpld/rpc/handlers/GatewayBalances.cpp @@ -74,7 +74,7 @@ doGatewayBalances(RPC::JsonContext& context) if (!id) return rpcError(rpcACT_MALFORMED); auto const accountID{std::move(id.value())}; - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; result[jss::account] = toBase58(accountID); diff --git a/src/xrpld/rpc/handlers/LedgerHandler.cpp b/src/xrpld/rpc/handlers/LedgerHandler.cpp index 2bf4fb09f94..ae48b94c5c6 100644 --- a/src/xrpld/rpc/handlers/LedgerHandler.cpp +++ b/src/xrpld/rpc/handlers/LedgerHandler.cpp @@ -80,7 +80,7 @@ LedgerHandler::check() return rpcTOO_BUSY; } context_.loadType = - binary ? Resource::feeMediumBurdenRPC : Resource::feeHighBurdenRPC; + binary ? Resource::feeMediumBurdenRPC : Resource::feeHeavyBurdenRPC; } if (queue) { diff --git a/src/xrpld/rpc/handlers/PathFind.cpp b/src/xrpld/rpc/handlers/PathFind.cpp index cab14f9b52e..6c9021ce97d 100644 --- a/src/xrpld/rpc/handlers/PathFind.cpp +++ b/src/xrpld/rpc/handlers/PathFind.cpp @@ -52,7 +52,7 @@ doPathFind(RPC::JsonContext& context) if (sSubCommand == "create") { - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; context.infoSub->clearRequest(); return context.app.getPathRequests().makePathRequest( context.infoSub, lpLedger, context.params); diff --git a/src/xrpld/rpc/handlers/RipplePathFind.cpp b/src/xrpld/rpc/handlers/RipplePathFind.cpp index 5e18a1210e9..80c336004b4 100644 --- a/src/xrpld/rpc/handlers/RipplePathFind.cpp +++ b/src/xrpld/rpc/handlers/RipplePathFind.cpp @@ -34,7 +34,7 @@ doRipplePathFind(RPC::JsonContext& context) if (context.app.config().PATH_SEARCH_MAX == 0) return rpcError(rpcNOT_SUPPORTED); - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; std::shared_ptr lpLedger; Json::Value jvResult; diff --git a/src/xrpld/rpc/handlers/SignFor.cpp b/src/xrpld/rpc/handlers/SignFor.cpp index da8ff869fba..80f5594e1ea 100644 --- a/src/xrpld/rpc/handlers/SignFor.cpp +++ b/src/xrpld/rpc/handlers/SignFor.cpp @@ -40,7 +40,7 @@ doSignFor(RPC::JsonContext& context) rpcNOT_SUPPORTED, "Signing is not supported by this server."); } - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard(failHard); diff --git a/src/xrpld/rpc/handlers/SignHandler.cpp b/src/xrpld/rpc/handlers/SignHandler.cpp index ed634161f74..2547ef8ead7 100644 --- a/src/xrpld/rpc/handlers/SignHandler.cpp +++ b/src/xrpld/rpc/handlers/SignHandler.cpp @@ -38,7 +38,7 @@ doSign(RPC::JsonContext& context) rpcNOT_SUPPORTED, "Signing is not supported by this server."); } - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; NetworkOPs::FailHard const failType = NetworkOPs::doFailHard( context.params.isMember(jss::fail_hard) && context.params[jss::fail_hard].asBool()); diff --git a/src/xrpld/rpc/handlers/SubmitMultiSigned.cpp b/src/xrpld/rpc/handlers/SubmitMultiSigned.cpp index 9949d3e3212..5535e376818 100644 --- a/src/xrpld/rpc/handlers/SubmitMultiSigned.cpp +++ b/src/xrpld/rpc/handlers/SubmitMultiSigned.cpp @@ -33,7 +33,7 @@ namespace ripple { Json::Value doSubmitMultiSigned(RPC::JsonContext& context) { - context.loadType = Resource::feeHighBurdenRPC; + context.loadType = Resource::feeHeavyBurdenRPC; auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard(failHard); From f3e201f983169ae3f10c94b526aa1e066c5fd07e Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Mon, 27 Jan 2025 21:00:53 +0000 Subject: [PATCH 2/4] Set version to 2.3.1-rc1 --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index 995df262885..af5bf4ef33b 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/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) From cb0ddbf863a66aba1dcf3d6afd05159d7ecf217f Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 28 Jan 2025 19:42:30 -0500 Subject: [PATCH 3/4] Update conan in the "nix" CI jobs --- .github/workflows/nix.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 6b8261c5d69..03936535853 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -64,6 +64,9 @@ jobs: env: build_dir: .build steps: + - name: upgrade conan + run: | + pip install --upgrade "conan<2" - name: checkout uses: actions/checkout@v4 - name: check environment @@ -122,6 +125,9 @@ jobs: env: build_dir: .build steps: + - name: upgrade conan + run: | + pip install --upgrade "conan<2" - name: download cache uses: actions/download-artifact@v3 with: @@ -171,6 +177,9 @@ jobs: env: build_dir: .build steps: + - name: upgrade conan + run: | + pip install --upgrade "conan<2" - name: download cache uses: actions/download-artifact@v3 with: @@ -241,6 +250,9 @@ jobs: build_dir: .build configuration: Release steps: + - name: upgrade conan + run: | + pip install --upgrade "conan<2" - name: download cache uses: actions/download-artifact@v3 with: From 8458233a31c98cbb13cfae212189ce3d8a1095d4 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 29 Jan 2025 09:26:27 -0500 Subject: [PATCH 4/4] Set version to 2.3.1 --- src/libxrpl/protocol/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxrpl/protocol/BuildInfo.cpp b/src/libxrpl/protocol/BuildInfo.cpp index af5bf4ef33b..a15417134d7 100644 --- a/src/libxrpl/protocol/BuildInfo.cpp +++ b/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.1-rc1" +char const* const versionString = "2.3.1" // clang-format on #if defined(DEBUG) || defined(SANITIZER)