From dd4b060f0998f8efef11e1f7601a985d01c503d8 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] 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 | 129 ++++++++++ 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 | 239 ++++++++++++------ 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, 460 insertions(+), 164 deletions(-) diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt index 32ab5546e..6505b7018 100644 --- a/Builds/levelization/results/ordering.txt +++ b/Builds/levelization/results/ordering.txt @@ -85,6 +85,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 f11ce4048..e5813e041 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -7,6 +7,135 @@ 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. + + +## Action Required + +If you run an XRP Ledger server, upgrade to version 2.3.0 as soon as possible to ensure service continuity. + +Additionally, new amendments are now open for voting according to the XRP Ledger's [amendment process](https://xrpl.org/amendments.html), which enables protocol changes following two weeks of >80% support from trusted validators. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. + +## Full Changelog + +### Amendments + +The following amendments are open for voting with this release: + +- **XLS-70 Credentials** - Users can issue Credentials on the ledger and use Credentials to pre-approve incoming payments when using Deposit Authorization instead of individually approving payers. ([#5103](https://github.com/XRPLF/rippled/pull/5103)) + - related fix: #5189 (https://github.com/XRPLF/rippled/pull/5189) +- **XLS-33 Multi-Purpose Tokens** - A new type of fungible token optimized for institutional DeFi including stablecoins. ([#5143](https://github.com/XRPLF/rippled/pull/5143)) +- **XLS-37 AMM Clawback** - Allows clawback-enabled tokens to be used in AMMs, with appropriate guardrails. ([#5142](https://github.com/XRPLF/rippled/pull/5142)) +- **XLS-52 NFTokenMintOffer** - Allows creating an NFT sell offer as part of minting a new NFT. ([#4845](https://github.com/XRPLF/rippled/pull/4845)) +- **fixAMMv1_2** - Fixes two bugs in Automated Market Maker (AMM) transaction processing. ([#5176](https://github.com/XRPLF/rippled/pull/5176)) +- **fixNFTokenPageLinks** - Fixes a bug that can cause NFT directories to have missing links, and introduces a transaction to repair corrupted ledger state. ([#4945](https://github.com/XRPLF/rippled/pull/4945)) +- **fixEnforceNFTokenTrustline** - Fixes two bugs in the interaction between NFT offers and trust lines. ([#4946](https://github.com/XRPLF/rippled/pull/4946)) +- **fixInnerObjTemplate2** - Standardizes the way inner objects are enforced across all transaction and ledger data. ([#5047](https://github.com/XRPLF/rippled/pull/5047)) + +The following amendment is partially implemented but not open for voting: + +- **InvariantsV1_1** - Adds new invariants to ensure transactions process as intended, starting with an invariant to ensure that ledger entries owned by an account are deleted when the account is deleted. ([#4663](https://github.com/XRPLF/rippled/pull/4663)) + +### New Features + +- Allow configuration of SQLite database page size. ([#5135](https://github.com/XRPLF/rippled/pull/5135), [#5140](https://github.com/XRPLF/rippled/pull/5140)) +- In the `libxrpl` C++ library, provide a list of known amendments. ([#5026](https://github.com/XRPLF/rippled/pull/5026)) + +### Deprecations + +- History Shards are removed. ([#5066](https://github.com/XRPLF/rippled/pull/5066)) +- Reporting mode is removed. ([#5092](https://github.com/XRPLF/rippled/pull/5092)) + +For users wanting to store more ledger history, it is recommended to run a Clio server instead. + +### Bug fixes + +- Fix a crash in debug builds when amm_info request contains an invalid AMM account ID. ([#5188](https://github.com/XRPLF/rippled/pull/5188)) +- Fix a crash caused by a race condition in peer-to-peer code. ([#5071](https://github.com/XRPLF/rippled/pull/5071)) +- Fix a crash in certain situations +- Fix several bugs in the book_changes API method. ([#5096](https://github.com/XRPLF/rippled/pull/5096)) +- Fix bug triggered by providing an invalid marker to the account_nfts API method. ([#5045](https://github.com/XRPLF/rippled/pull/5045)) +- Accept lower-case hexadecimal in compact transaction identifier (CTID) parameters in API methods. ([#5049](https://github.com/XRPLF/rippled/pull/5049)) +- Disallow filtering by types that an account can't own in the account_objects API method. ([#5056](https://github.com/XRPLF/rippled/pull/5056)) +- Fix error code returned by the feature API method when providing an invalid parameter. ([#5063](https://github.com/XRPLF/rippled/pull/5063)) +- (API v3) Fix error code returned by amm_info when providing invalid parameters. ([#4924](https://github.com/XRPLF/rippled/pull/4924)) + +### Other Improvements + +- Adds a new default hub, hubs.xrpkuwait.com, to the config file and bootstrapping code. ([#5169](https://github.com/XRPLF/rippled/pull/5169)) +- Improve error message when commandline interface fails with `rpcInternal` because there was no response from the server. ([#4959](https://github.com/XRPLF/rippled/pull/4959)) +- Add tools for debugging specific transactions via replay. ([#5027](https://github.com/XRPLF/rippled/pull/5027), [#5087](https://github.com/XRPLF/rippled/pull/5087)) +- Major reorganization of source code files. ([#4997](https://github.com/XRPLF/rippled/pull/4997)) +- Add new unit tests. ([#4886](https://github.com/XRPLF/rippled/pull/4886)) +- Various improvements to build tools and contributor documentation. ([#5001](https://github.com/XRPLF/rippled/pull/5001), [#5028](https://github.com/XRPLF/rippled/pull/5028), [#5052](https://github.com/XRPLF/rippled/pull/5052), [#5091](https://github.com/XRPLF/rippled/pull/5091), [#5084](https://github.com/XRPLF/rippled/pull/5084), [#5120](https://github.com/XRPLF/rippled/pull/5120), [#5010](https://github.com/XRPLF/rippled/pull/5010). [#5055](https://github.com/XRPLF/rippled/pull/5055), [#5067](https://github.com/XRPLF/rippled/pull/5067), [#5061](https://github.com/XRPLF/rippled/pull/5061), [#5072](https://github.com/XRPLF/rippled/pull/5072), [#5044](https://github.com/XRPLF/rippled/pull/5044) ) +- Various code cleanup and refactoring. ([#4509](https://github.com/XRPLF/rippled/pull/4509), [#4521](https://github.com/XRPLF/rippled/pull/4521), [#4856](https://github.com/XRPLF/rippled/pull/4856), [#5190](https://github.com/XRPLF/rippled/pull/5190), [#5081](https://github.com/XRPLF/rippled/pull/5081), [#5053](https://github.com/XRPLF/rippled/pull/5053), [#5058](https://github.com/XRPLF/rippled/pull/5058), [#5122](https://github.com/XRPLF/rippled/pull/5122), [#5059](https://github.com/XRPLF/rippled/pull/5059), [#5041](https://github.com/XRPLF/rippled/pull/5041)) + + +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.2.3 Version 2.2.3 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release fixes a problem that can cause full-history servers to run out of space in their SQLite databases, depending on configuration. There are no new amendments in this release. diff --git a/include/xrpl/resource/Charge.h b/include/xrpl/resource/Charge.h index 000417db4..e5799710b 100644 --- a/include/xrpl/resource/Charge.h +++ b/include/xrpl/resource/Charge.h @@ -53,8 +53,9 @@ public: 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 00e4ad2c5..00d8a7b58 100644 --- a/include/xrpl/resource/Consumer.h +++ b/include/xrpl/resource/Consumer.h @@ -67,7 +67,7 @@ public: /** 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 1eb1a9bd7..f85e7c8cd 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 a57e529e0..651778e1b 100644 --- a/include/xrpl/resource/detail/Logic.h +++ b/include/xrpl/resource/detail/Logic.h @@ -442,12 +442,34 @@ public: } 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 b57d57348..62234c356 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 deec6b34e..66b5049db 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 b86525468..f938f2acc 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 13ae38bc6..17fb47539 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 009ae4d73..00de72195 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -227,7 +227,8 @@ public: 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 e0b0d006a..e0edae548 100644 --- a/src/test/overlay/reduce_relay_test.cpp +++ b/src/test/overlay/reduce_relay_test.cpp @@ -88,7 +88,8 @@ public: 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 3065bcbb6..3d6c2a4d1 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 @@ private: 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 b652e0bbc..5c566c78d 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -1057,7 +1057,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; } @@ -1072,7 +1073,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; } @@ -1095,7 +1098,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; } @@ -1115,7 +1119,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; } @@ -1127,7 +1131,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 83b074f31..b8f327ff8 100644 --- a/src/xrpld/app/ledger/detail/InboundTransactions.cpp +++ b/src/xrpld/app/ledger/detail/InboundTransactions.cpp @@ -146,7 +146,7 @@ public: if (ta == nullptr) { - peer->charge(Resource::feeUnwantedData); + peer->charge(Resource::feeUselessData, "ledger_data"); return; } @@ -157,7 +157,7 @@ public: { if (!node.has_nodeid() || !node.has_nodedata()) { - peer->charge(Resource::feeInvalidRequest); + peer->charge(Resource::feeMalformedRequest, "ledger_data"); return; } @@ -165,7 +165,7 @@ public: if (!id) { - peer->charge(Resource::feeBadData); + peer->charge(Resource::feeInvalidData, "ledger_data"); return; } @@ -173,7 +173,7 @@ public: } 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 5d21df8d8..37c7c2c18 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -2187,7 +2187,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; } @@ -2195,14 +2195,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; } @@ -2213,7 +2213,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 b9550ba2e..9aff15606 100644 --- a/src/xrpld/overlay/Overlay.h +++ b/src/xrpld/overlay/Overlay.h @@ -183,7 +183,7 @@ public: 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 82ed2c248..2646b24a3 100644 --- a/src/xrpld/overlay/Peer.h +++ b/src/xrpld/overlay/Peer.h @@ -77,7 +77,7 @@ public: /** 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 f73ddcb94..f7e3fee04 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 @@ -1183,17 +1184,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) { @@ -1208,7 +1231,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 a50dfc5e9..9ff6bb2a4 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -238,7 +238,7 @@ public: 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 510c2924c..285556785 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -61,6 +61,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, @@ -94,7 +97,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_) @@ -338,9 +341,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 @@ -996,9 +999,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; @@ -1028,7 +1031,7 @@ PeerImp::onMessageEnd( std::shared_ptr<::google::protobuf::Message> const&) { load_event_.reset(); - charge(fee_); + charge(fee_.fee, fee_.context); } void @@ -1038,12 +1041,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]() { @@ -1057,7 +1060,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; @@ -1094,7 +1097,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; } @@ -1172,7 +1175,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; } @@ -1187,7 +1190,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; } @@ -1210,14 +1213,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; @@ -1242,7 +1250,7 @@ PeerImp::handleTransaction( { JLOG(p_journal_.warn()) << "Ignoring Network relayed Tx containing " "sfEmitDetails (handleTransaction)."; - fee_ = Resource::feeHighBurdenPeer; + fee_.update(Resource::feeHeavyBurdenPeer, "invalid sfEmitDetails"); return; } @@ -1254,7 +1262,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; } @@ -1308,9 +1316,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); }); } } @@ -1326,7 +1336,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()}; @@ -1417,11 +1427,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]() { @@ -1432,9 +1443,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 { @@ -1450,13 +1464,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"); } } @@ -1466,11 +1480,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]() { @@ -1481,9 +1495,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 { @@ -1499,13 +1517,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"); } } @@ -1513,7 +1531,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; }; @@ -1613,7 +1631,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; } @@ -1621,7 +1641,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; } @@ -1926,7 +1946,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!stringIsUint256Sized(m->hash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "bad hash"); return; } @@ -1939,7 +1959,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; } @@ -1961,7 +1981,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; } @@ -1978,7 +1998,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; } @@ -2057,27 +2077,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); @@ -2154,7 +2177,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( @@ -2167,7 +2190,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()); } } @@ -2183,7 +2207,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) @@ -2193,7 +2217,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( @@ -2206,7 +2230,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()); } } @@ -2216,7 +2241,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; } @@ -2244,7 +2269,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; } @@ -2317,7 +2342,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()); } } @@ -2350,7 +2376,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; } @@ -2363,7 +2389,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; @@ -2378,7 +2406,7 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (!stringIsUint256Sized(packet.ledgerhash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "ledger hash"); return; } @@ -2482,7 +2510,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; } @@ -2511,7 +2539,7 @@ PeerImp::handleHaveTransactions( { JLOG(p_journal_.error()) << "TMHaveTransactions with invalid hash size"; - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "hash size"); return; } @@ -2551,7 +2579,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; } @@ -2564,7 +2592,8 @@ PeerImp::onMessage(std::shared_ptr const& m) handleTransaction( std::shared_ptr( m->mutable_transactions(i), [](protocol::TMTransaction*) {}), - false); + false, + true); } void @@ -2580,14 +2609,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); @@ -2595,7 +2624,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; @@ -2615,7 +2644,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; @@ -2656,11 +2685,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()}; @@ -2685,7 +2714,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; } @@ -2695,7 +2724,7 @@ PeerImp::doTransactions( if (!stringIsUint256Sized(obj.hash())) { - fee_ = Resource::feeInvalidRequest; + fee_.update(Resource::feeMalformedRequest, "hash size"); return; } @@ -2707,7 +2736,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; } @@ -2732,7 +2761,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 @@ -2742,7 +2772,7 @@ PeerImp::checkTransaction( { JLOG(p_journal_.warn()) << "Ignoring Network relayed Tx containing " "sfEmitDetails (checkSignature)."; - charge(Resource::feeHighBurdenPeer); + charge(Resource::feeHeavyBurdenPeer, "invalid sfEmitDetails"); return; } @@ -2752,10 +2782,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. @@ -2774,7 +2839,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; } } @@ -2795,7 +2862,7 @@ PeerImp::checkTransaction( << "Exception checking transaction: " << reason; } app_.getHashRouter().setFlags(stx->getTransactionID(), SF_BAD); - charge(Resource::feeInvalidSignature); + charge(Resource::feeInvalidSignature, "tx (impossible)"); return; } @@ -2808,7 +2875,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()); } } @@ -2826,8 +2894,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; } @@ -2863,8 +2932,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; } @@ -2894,7 +2964,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()); } } @@ -3063,7 +3134,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()) @@ -3125,7 +3197,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; @@ -3235,6 +3308,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()); @@ -3289,6 +3365,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 9c76ddb4d..9f52c1c7a 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -145,10 +145,28 @@ private: // // 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 @@ public: } void - charge(Resource::Charge const& fee) override; + charge(Resource::Charge const& fee, std::string const& context) override; // // Identity @@ -482,11 +500,15 @@ private: 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 @@ private: 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 c560d9d0c..7c89a2b61 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -1174,7 +1174,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 f771be274..28184709d 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -639,7 +639,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; } @@ -656,7 +656,7 @@ ServerHandler::processSession( is->user()); if (Role::FORBID == role) { - loadType = Resource::feeInvalidRPC; + loadType = Resource::feeMalformedRPC; jr[jss::result] = rpcError(rpcFORBIDDEN); } else @@ -924,7 +924,7 @@ ServerHandler::processRequest( if (role == Role::FORBID) { - usage.charge(Resource::feeInvalidRPC); + usage.charge(Resource::feeMalformedRPC); if (!batch) { HTTPReply(403, "Forbidden", output, rpcJ); @@ -938,7 +938,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); @@ -953,7 +953,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); @@ -969,7 +969,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); @@ -997,7 +997,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; } @@ -1006,7 +1006,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; } @@ -1022,7 +1022,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 8fd13d472..26f338cb4 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 0ec676330..fd6476aec 100644 --- a/src/xrpld/rpc/handlers/LedgerHandler.cpp +++ b/src/xrpld/rpc/handlers/LedgerHandler.cpp @@ -87,7 +87,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 cab14f9b5..6c9021ce9 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 5e18a1210..80c336004 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 da8ff869f..80f5594e1 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 ed634161f..2547ef8ea 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 9949d3e32..5535e3768 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);