From 656948cd0f6d5067c79e3c84e3394dc90b206035 Mon Sep 17 00:00:00 2001 From: Hussein Badakhchani Date: Wed, 6 Dec 2023 22:26:18 +0000 Subject: [PATCH 1/5] fix typo: 'of' instead of 'on' (#4821) Co-authored-by: Hussein Badakhchani --- src/ripple/app/tx/impl/AMMCreate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/AMMCreate.h b/src/ripple/app/tx/impl/AMMCreate.h index 81917c4114a..f521f5870b4 100644 --- a/src/ripple/app/tx/impl/AMMCreate.h +++ b/src/ripple/app/tx/impl/AMMCreate.h @@ -27,7 +27,7 @@ namespace ripple { /** AMMCreate implements Automatic Market Maker(AMM) creation Transactor. * It creates a new AMM instance with two tokens. Any trader, or Liquidity * Provider (LP), can create the AMM instance and receive in return shares - * of the AMM pool in the form on LPTokens. The number of tokens that LP gets + * of the AMM pool in the form of LPTokens. The number of tokens that LP gets * are determined by LPTokens = sqrt(A * B), where A and B is the current * composition of the AMM pool. LP can add (AMMDeposit) or withdraw * (AMMWithdraw) tokens from AMM and From 3b191a3097d66b1af59c79d60da1577163a0662c Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Wed, 6 Dec 2023 14:27:33 -0800 Subject: [PATCH 2/5] docs(API-CHANGELOG): clarify changes for V2 (#4773) --- API-CHANGELOG.md | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index fc3d5e01ded..db158cf001c 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -24,18 +24,20 @@ In `api_version: 2`, the `signer_lists` field [will be moved](#modifications-to- #### server_info - network_id -The `network_id` field was added in the `server_info` response in version 1.5.0 (2019), but it was not returned in [reporting mode](https://xrpl.org/rippled-server-modes.html#reporting-mode). +The `network_id` field was added in the `server_info` response in version 1.5.0 (2019), but it is not returned in [reporting mode](https://xrpl.org/rippled-server-modes.html#reporting-mode). -## Unreleased (expected in XRP Ledger version 2.0) +## XRP Ledger server version 2.0.0 -### Additions +### Additions in 2.0 Additions are intended to be non-breaking (because they are purely additive). - `server_definitions`: A new RPC that generates a `definitions.json`-like output that can be used in XRPL libraries. -- In `Payment` transactions, `DeliverMax` has been added. This is a replacement for the `Amount` field, which should be no longer used - instead, use `delivered_amount` in transaction metadata. To ease the transition, `DeliverMax` is present regardless of API version, since adding a field is non-breaking. The field `Amount` is no longer present in `Payment`s in API version 2. +- In `Payment` transactions, `DeliverMax` has been added. This is a replacement for the `Amount` field, which should not be used. Typically, the `delivered_amount` (in transaction metadata) should be used. To ease the transition, `DeliverMax` is present regardless of API version, since adding a field is non-breaking. +- API version 2 has been moved from beta to supported, meaning that it is generally available (regardless of the `beta_rpc_api` setting). -## XRP Ledger version 1.12.0 + +## XRP Ledger server version 1.12.0 [Version 1.12.0](https://github.com/XRPLF/rippled/releases/tag/1.12.0) was released on Sep 6, 2023. @@ -72,12 +74,12 @@ Additions are intended to be non-breaking (because they are purely additive). - tecAMM_BALANCE: AMM has invalid balance. Calculated balances greater than the current pool balances. - tecAMM_FAILED: AMM transaction failed. Fails due to a processing failure. - tecAMM_INVALID_TOKENS: AMM invalid LP tokens. Invalid input values, format, or calculated values. - - tecAMM_EMPTY: AMM is in empty state. Transaction expects AMM in non-empty state (LP tokens > 0). - - tecAMM_NOT_EMPTY: AMM is not in empty state. Transaction expects AMM in empty state (LP tokens == 0). + - tecAMM_EMPTY: AMM is in empty state. Transaction requires AMM in non-empty state (LP tokens > 0). + - tecAMM_NOT_EMPTY: AMM is not in empty state. Transaction requires AMM in empty state (LP tokens == 0). - tecAMM_ACCOUNT: AMM account. Clawback of AMM account. - tecINCOMPLETE: Some work was completed, but more submissions required to finish. AMMDelete partially deletes the trustlines. -## XRP Ledger version 1.11.0 +## XRP Ledger server version 1.11.0 [Version 1.11.0](https://github.com/XRPLF/rippled/releases/tag/1.11.0) was released on Jun 20, 2023. @@ -107,7 +109,7 @@ Additions are intended to be non-breaking (because they are purely additive). - Added `NFTokenPages` to the `account_objects` RPC. (https://github.com/XRPLF/rippled/pull/4352) - Fixed: `marker` returned from the `account_lines` command would not work on subsequent commands. (https://github.com/XRPLF/rippled/pull/4361) -## XRP Ledger version 1.10.0 +## XRP Ledger server version 1.10.0 [Version 1.10.0](https://github.com/XRPLF/rippled/releases/tag/1.10.0) was released on Mar 14, 2023. @@ -124,14 +126,15 @@ API version 2 is introduced in `rippled` version 2.0. Users can request it expli #### Removed methods -In API version 2, the following methods are no longer available: (https://github.com/XRPLF/rippled/pull/4759) +In API version 2, the following deprecated methods are no longer available: (https://github.com/XRPLF/rippled/pull/4759) - `tx_history` - Instead, use other methods such as `account_tx` or `ledger` with the `transactions` field set to `true`. - `ledger_header` - Instead, use the `ledger` method. #### Modifications to JSON transaction element in V2 -In API version 2, JSON elements for transaction output have been changed and made consistent for all methods which output transactions: (https://github.com/XRPLF/rippled/pull/4775) +In API version 2, JSON elements for transaction output have been changed and made consistent for all methods which output transactions. (https://github.com/XRPLF/rippled/pull/4775) +This helps to unify the JSON serialization format of transactions. (https://github.com/XRPLF/clio/issues/722, https://github.com/XRPLF/rippled/issues/4727) - JSON transaction element is named `tx_json` - Binary transaction element is named `tx_blob` @@ -156,6 +159,8 @@ This change affects the following methods: #### Modification to `Payment` transaction JSON schema +When reading Payments, the `Amount` field should generally **not** be used. Instead, use [delivered_amount](https://xrpl.org/partial-payments.html#the-delivered_amount-field) to see the amount that the Payment delivered. To clarify its meaning, the `Amount` field is being renamed to `DeliverMax`. (https://github.com/XRPLF/rippled/pull/4733) + - In `Payment` transaction type, JSON RPC field `Amount` is renamed to `DeliverMax`. To enable smooth client transition, `Amount` is still handled, as described below: (https://github.com/XRPLF/rippled/pull/4733) - On JSON RPC input (e.g. `submit_multisigned` etc. methods), `Amount` is recognized as an alias to `DeliverMax` for both API version 1 and version 2 clients. - On JSON RPC input, submitting both `Amount` and `DeliverMax` fields is allowed _only_ if they are identical; otherwise such input is rejected with `rpcINVALID_PARAMS` error. @@ -166,21 +171,19 @@ This change affects the following methods: - `signer_lists` is returned in the root of the response. In API version 1, it was nested under `account_data`. (https://github.com/XRPLF/rippled/pull/3770) - When using an invalid `signer_lists` value, the API now returns an "invalidParams" error. (https://github.com/XRPLF/rippled/pull/4585) - - (`signer_lists` must be a boolean. In API version 1, strings are accepted and may return a normal response - as if `signer_lists` were `true`.) + - (`signer_lists` must be a boolean. In API version 1, strings were accepted and may return a normal response - i.e. as if `signer_lists` were `true`.) #### Modifications to [account_tx](https://xrpl.org/account_tx.html#account_tx) response - Using `ledger_index_min`, `ledger_index_max`, and `ledger_index` returns `invalidParams` because if you use `ledger_index_min` or `ledger_index_max`, then it does not make sense to also specify `ledger_index`. In API version 1, no error was returned. (https://github.com/XRPLF/rippled/pull/4571) - The same applies for `ledger_index_min`, `ledger_index_max`, and `ledger_hash`. (https://github.com/XRPLF/rippled/issues/4545#issuecomment-1565065579) - Using a `ledger_index_min` or `ledger_index_max` beyond the range of ledgers that the server has: - - returns `lgrIdxMalformed` in API version 2. (https://github.com/XRPLF/rippled/issues/4288) - - In API version 1, no error was returned. - -- Attempting to use a non-boolean value (such as a string) for the `binary` or `forward` parameters returns `invalidParams` (`rpcINVALID_PARAMS`). In API version 1, no error was returned. () + - returns `lgrIdxMalformed` in API version 2. Previously, in API version 1, no error was returned. (https://github.com/XRPLF/rippled/issues/4288) +- Attempting to use a non-boolean value (such as a string) for the `binary` or `forward` parameters returns `invalidParams` (`rpcINVALID_PARAMS`). Previously, in API version 1, no error was returned. (https://github.com/XRPLF/rippled/pull/4620) #### Modifications to [noripple_check](https://xrpl.org/noripple_check.html#noripple_check) response -- Attempting to use a non-boolean value (such as a string) for the `transactions` parameter returns `invalidParams` (`rpcINVALID_PARAMS`). In API version 1, no error was returned. () +- Attempting to use a non-boolean value (such as a string) for the `transactions` parameter returns `invalidParams` (`rpcINVALID_PARAMS`). Previously, in API version 1, no error was returned. (https://github.com/XRPLF/rippled/pull/4620) # Unit tests for API changes From ffb53f2085b70662058f4091b5e3cdac5e6d63da Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 19 Dec 2023 20:52:25 +0000 Subject: [PATCH 3/5] Revert "Add ProtocolStart and GracefulClose P2P protocol messages (#3839)" (#4850) This reverts commit 8f89694faeff882bd02dd91d1ca243e35073dce7. --- Builds/CMake/RippledCore.cmake | 1 - src/ripple/overlay/Message.h | 17 +- src/ripple/overlay/Peer.h | 1 - src/ripple/overlay/README.md | 44 ---- src/ripple/overlay/impl/InboundHandoff.cpp | 185 ------------- src/ripple/overlay/impl/InboundHandoff.h | 102 ------- src/ripple/overlay/impl/Message.cpp | 7 +- src/ripple/overlay/impl/OverlayImpl.cpp | 15 +- src/ripple/overlay/impl/PeerImp.cpp | 277 ++++++++------------ src/ripple/overlay/impl/PeerImp.h | 16 +- src/ripple/overlay/impl/ProtocolMessage.h | 12 - src/ripple/overlay/impl/ProtocolVersion.cpp | 3 +- src/ripple/proto/ripple.proto | 23 -- src/test/overlay/ProtocolVersion_test.cpp | 2 +- 14 files changed, 140 insertions(+), 565 deletions(-) delete mode 100644 src/ripple/overlay/impl/InboundHandoff.cpp delete mode 100644 src/ripple/overlay/impl/InboundHandoff.h diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 269c107ca9e..ab5083aa208 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -637,7 +637,6 @@ target_sources (rippled PRIVATE src/ripple/overlay/impl/Cluster.cpp src/ripple/overlay/impl/ConnectAttempt.cpp src/ripple/overlay/impl/Handshake.cpp - src/ripple/overlay/impl/InboundHandoff.cpp src/ripple/overlay/impl/Message.cpp src/ripple/overlay/impl/OverlayImpl.cpp src/ripple/overlay/impl/PeerImp.cpp diff --git a/src/ripple/overlay/Message.h b/src/ripple/overlay/Message.h index 6cb6900c639..0d6479366e8 100644 --- a/src/ripple/overlay/Message.h +++ b/src/ripple/overlay/Message.h @@ -100,14 +100,6 @@ class Message : public std::enable_shared_from_this return validatorKey_; } - /** Get the message type from the payload header. - * First four bytes are the compression/algorithm flag and the payload size. - * Next two bytes are the message type - * @return Message type - */ - int - getType() const; - private: std::vector buffer_; std::vector bufferCompressed_; @@ -137,6 +129,15 @@ class Message : public std::enable_shared_from_this */ void compress(); + + /** Get the message type from the payload header. + * First four bytes are the compression/algorithm flag and the payload size. + * Next two bytes are the message type + * @param in Payload header pointer + * @return Message type + */ + int + getType(std::uint8_t const* in) const; }; } // namespace ripple diff --git a/src/ripple/overlay/Peer.h b/src/ripple/overlay/Peer.h index dbe5416e590..ba415974151 100644 --- a/src/ripple/overlay/Peer.h +++ b/src/ripple/overlay/Peer.h @@ -39,7 +39,6 @@ enum class ProtocolFeature { ValidatorListPropagation, ValidatorList2Propagation, LedgerReplay, - StartProtocol }; /** Represents a peer connection in the overlay. */ diff --git a/src/ripple/overlay/README.md b/src/ripple/overlay/README.md index bfead075135..6525e5edf86 100644 --- a/src/ripple/overlay/README.md +++ b/src/ripple/overlay/README.md @@ -365,50 +365,6 @@ transferred between A and B and will not be able to intelligently tamper with th message stream between Alice and Bob, although she may be still be able to inject delays or terminate the link. -## Peer Connection Sequence - -The _PeerImp_ object can be constructed as either an outbound or an inbound peer. -The outbound peer is constructed by the _ConnectAttempt_ - the client side of -the connection. The inbound peer is constructed by the _InboundHandoff_ - -the server side of the connection. This differentiation of the peers matters only -in terms of the object construction. Once constructed, both inbound and outbound -peer play the same role. - -### Outbound Peer - -An outbound connection is initiated once a second by -the _OverlayImpl::Timer::on_timer()_ method. This method calls -_OverlayImpl::autoConnect()_, which in turn calls _OverlayImpl::connect()_ for -every outbound endpoint generated by _PeerFinder::autoconnect()_. _connect()_ -method constructs _ConnectAttempt_ object. _ConnectAttempt_ attempts to connect -to the provided endpoint and on a successful connection executes the client side -of the handshake protocol described above. If the handshake is successful then -the outbound _PeerImp_ object is constructed and passed to the overlay manager -_OverlayImpl_, which adds the object to the list of peers and children. The latter -maintains a list of objects which might be executing an asynchronous operation -and therefore have to be stopped on shutdown. The outbound _PeerImp_ sends -_TMStartProtocol_ message on start to instruct the connected inbound peer that -the outbound peer is ready to receive the protocol messages. - -### Inbound Peer - -Construction of the inbound peer is more involved. A multi protocol-server, -_ServerImpl_ located in _src/ripple/server_ module, maintains multiple configured -listening ports. Each listening port allows for multiple protocols including HTTP, -HTTP/S, WebSocket, Secure WebSocket, and the Peer protocol. For simplicity this -sequence describes only the Peer protocol. _ServerImpl_ constructs -_Door_ object for each configured protocol. Each instance of the _Door_ object -accepts connections on the configured port. On a successful connection the _Door_ -constructs _SSLHTTPPeer_ object since the Peer protocol always uses SSL -connection. _SSLHTTPPeer_ executes the SSL handshake. If the handshake is successful -then a server handler, _ServerHandlerImpl_ located in _src/ripple/src/impl_, hands off -the connection to the _OverlayImpl::onHandoff()_ method. _onHandoff()_ method -validates the client's HTTP handshake request described above. If the request is -valid then the _InboundHandoff_ object is constructed. _InboundHandoff_ sends -HTTP response to the connected client, constructs the inbound _PeerImp_ object, -and passes it to the overlay manager _OverlayImpl_, which adds the object to -the list of peers and children. Once the inbound _PeerImp_ receives -_TMStartProtocol_ message, it starts sending the protocol messages. # Ripple Clustering # diff --git a/src/ripple/overlay/impl/InboundHandoff.cpp b/src/ripple/overlay/impl/InboundHandoff.cpp deleted file mode 100644 index 1f45e1d37a7..00000000000 --- a/src/ripple/overlay/impl/InboundHandoff.cpp +++ /dev/null @@ -1,185 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012-2021 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include - -#include - -namespace ripple { - -InboundHandoff::InboundHandoff( - Application& app, - id_t id, - std::shared_ptr const& slot, - http_request_type&& request, - PublicKey const& publicKey, - ProtocolVersion protocol, - Resource::Consumer consumer, - std::unique_ptr&& stream_ptr, - OverlayImpl& overlay) - : OverlayImpl::Child(overlay) - , app_(app) - , id_(id) - , sink_( - app_.journal("Peer"), - [id]() { - std::stringstream ss; - ss << "[" << std::setfill('0') << std::setw(3) << id << "] "; - return ss.str(); - }()) - , journal_(sink_) - , stream_ptr_(std::move(stream_ptr)) - , strand_(stream_ptr_->next_layer().socket().get_executor()) - , remote_address_(slot->remote_endpoint()) - , protocol_(protocol) - , publicKey_(publicKey) - , usage_(consumer) - , slot_(slot) - , request_(std::move(request)) -{ -} - -void -InboundHandoff::run() -{ - if (!strand_.running_in_this_thread()) - return post( - strand_, std::bind(&InboundHandoff::run, shared_from_this())); - sendResponse(); -} - -void -InboundHandoff::stop() -{ - if (!strand_.running_in_this_thread()) - return post( - strand_, std::bind(&InboundHandoff::stop, shared_from_this())); - if (stream_ptr_->next_layer().socket().is_open()) - { - JLOG(journal_.debug()) << "Stop"; - } - close(); -} - -void -InboundHandoff::sendResponse() -{ - auto const sharedValue = makeSharedValue(*stream_ptr_, journal_); - // This shouldn't fail since we already computed - // the shared value successfully in OverlayImpl - if (!sharedValue) - return fail("makeSharedValue: Unexpected failure"); - - JLOG(journal_.info()) << "Protocol: " << to_string(protocol_); - JLOG(journal_.info()) << "Public Key: " - << toBase58(TokenType::NodePublic, publicKey_); - - auto write_buffer = std::make_shared(); - - boost::beast::ostream(*write_buffer) << makeResponse( - !overlay_.peerFinder().config().peerPrivate, - request_, - overlay_.setup().public_ip, - remote_address_.address(), - *sharedValue, - overlay_.setup().networkID, - protocol_, - app_); - - // Write the whole buffer and only start protocol when that's done. - boost::asio::async_write( - *stream_ptr_, - write_buffer->data(), - boost::asio::transfer_all(), - bind_executor( - strand_, - [this, write_buffer, self = shared_from_this()]( - error_code ec, std::size_t bytes_transferred) { - if (!stream_ptr_->next_layer().socket().is_open()) - return; - if (ec == boost::asio::error::operation_aborted) - return; - if (ec) - return fail("onWriteResponse", ec); - if (write_buffer->size() == bytes_transferred) - return createPeer(); - return fail("Failed to write header"); - })); -} - -void -InboundHandoff::fail(std::string const& name, error_code const& ec) -{ - if (socket().is_open()) - { - JLOG(journal_.warn()) - << name << " from " << toBase58(TokenType::NodePublic, publicKey_) - << " at " << remote_address_.to_string() << ": " << ec.message(); - } - close(); -} - -void -InboundHandoff::fail(std::string const& reason) -{ - if (journal_.active(beast::severities::kWarning) && socket().is_open()) - { - auto const n = app_.cluster().member(publicKey_); - JLOG(journal_.warn()) - << (n ? remote_address_.to_string() : *n) << " failed: " << reason; - } - close(); -} - -void -InboundHandoff::close() -{ - if (socket().is_open()) - { - socket().close(); - JLOG(journal_.debug()) << "Closed"; - } -} - -void -InboundHandoff::createPeer() -{ - auto peer = std::make_shared( - app_, - id_, - slot_, - std::move(request_), - publicKey_, - protocol_, - usage_, - std::move(stream_ptr_), - overlay_); - - overlay_.add_active(peer); -} - -InboundHandoff::socket_type& -InboundHandoff::socket() const -{ - return stream_ptr_->next_layer().socket(); -} - -} // namespace ripple \ No newline at end of file diff --git a/src/ripple/overlay/impl/InboundHandoff.h b/src/ripple/overlay/impl/InboundHandoff.h deleted file mode 100644 index 3f3154c3a8f..00000000000 --- a/src/ripple/overlay/impl/InboundHandoff.h +++ /dev/null @@ -1,102 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012-2021 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#ifndef RIPPLE_OVERLAY_INBOUNDHANDOFF_H_INCLUDED -#define RIPPLE_OVERLAY_INBOUNDHANDOFF_H_INCLUDED - -#include - -namespace ripple { - -/** Sends HTTP response. Instantiates the inbound peer - * once the response is sent. Maintains all data members - * required for the inbound peer instantiation. - */ -class InboundHandoff : public OverlayImpl::Child, - public std::enable_shared_from_this -{ -private: - using error_code = boost::system::error_code; - using socket_type = boost::asio::ip::tcp::socket; - using middle_type = boost::beast::tcp_stream; - using stream_type = boost::beast::ssl_stream; - using id_t = Peer::id_t; - Application& app_; - id_t const id_; - beast::WrappedSink sink_; - beast::Journal const journal_; - std::unique_ptr stream_ptr_; - boost::asio::strand strand_; - beast::IP::Endpoint const remote_address_; - ProtocolVersion protocol_; - PublicKey const publicKey_; - Resource::Consumer usage_; - std::shared_ptr const slot_; - http_request_type request_; - -public: - virtual ~InboundHandoff() override = default; - - InboundHandoff( - Application& app, - id_t id, - std::shared_ptr const& slot, - http_request_type&& request, - PublicKey const& publicKey, - ProtocolVersion protocol, - Resource::Consumer consumer, - std::unique_ptr&& stream_ptr, - OverlayImpl& overlay); - - // This class isn't meant to be copied - InboundHandoff(InboundHandoff const&) = delete; - InboundHandoff& - operator=(InboundHandoff const&) = delete; - - /** Start the handshake */ - void - run(); - /** Stop the child */ - void - stop() override; - -private: - /** Send upgrade response to the client */ - void - sendResponse(); - /** Instantiate and run the overlay peer */ - void - createPeer(); - /** Log and close */ - void - fail(std::string const& name, error_code const& ec); - /** Log and close */ - void - fail(std::string const& reason); - /** Close connection */ - void - close(); - /** Get underlying socket */ - socket_type& - socket() const; -}; - -} // namespace ripple - -#endif // RIPPLE_OVERLAY_INBOUNDHANDOFF_H_INCLUDED diff --git a/src/ripple/overlay/impl/Message.cpp b/src/ripple/overlay/impl/Message.cpp index 1b434225501..b4cb1f192aa 100644 --- a/src/ripple/overlay/impl/Message.cpp +++ b/src/ripple/overlay/impl/Message.cpp @@ -70,7 +70,7 @@ Message::compress() using namespace ripple::compression; auto const messageBytes = buffer_.size() - headerBytes; - auto type = getType(); + auto type = getType(buffer_.data()); bool const compressible = [&] { if (messageBytes <= 70) @@ -221,10 +221,9 @@ Message::getBuffer(Compressed tryCompressed) } int -Message::getType() const +Message::getType(std::uint8_t const* in) const { - int type = - (static_cast(*(buffer_.data() + 4)) << 8) + *(buffer_.data() + 5); + int type = (static_cast(*(in + 4)) << 8) + *(in + 5); return type; } diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index c48ab378cb3..6ed046f0403 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -280,7 +279,7 @@ OverlayImpl::onHandoff( } } - auto const ih = std::make_shared( + auto const peer = std::make_shared( app_, id, slot, @@ -291,10 +290,18 @@ OverlayImpl::onHandoff( std::move(stream_ptr), *this); { + // As we are not on the strand, run() must be called + // while holding the lock, otherwise new I/O can be + // queued after a call to stop(). std::lock_guard lock(mutex_); - list_.emplace(ih.get(), ih); + { + auto const result = m_peers.emplace(peer->slot(), peer); + assert(result.second); + (void)result.second; + } + list_.emplace(peer.get(), peer); - ih->run(); + peer->run(); } handoff.moved = true; return handoff; diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 80eeba11895..3afec605cfa 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -65,30 +65,6 @@ std::chrono::milliseconds constexpr peerHighLatency{300}; std::chrono::seconds constexpr peerTimerInterval{60}; } // namespace -std::string -closeReasonToString(protocol::TMCloseReason reason) -{ - switch (reason) - { - case protocol::TMCloseReason::crCHARGE_RESOURCES: - return "Charge: Resources"; - case protocol::TMCloseReason::crMALFORMED_HANDSHAKE1: - return "Malformed handshake data (1)"; - case protocol::TMCloseReason::crMALFORMED_HANDSHAKE2: - return "Malformed handshake data (2)"; - case protocol::TMCloseReason::crMALFORMED_HANDSHAKE3: - return "Malformed handshake data (3)"; - case protocol::TMCloseReason::crLARGE_SENDQUEUE: - return "Large send queue"; - case protocol::TMCloseReason::crNOT_USEFUL: - return "Not useful"; - case protocol::TMCloseReason::crPING_TIMEOUT: - return "Ping timeout"; - default: - return "Unknown reason"; - } -} - PeerImp::PeerImp( Application& app, id_t id, @@ -155,11 +131,6 @@ PeerImp::PeerImp( << " tx reduce-relay enabled " << txReduceRelayEnabled_ << " on " << remote_address_ << " " << id_; - if (auto member = app_.cluster().member(publicKey_)) - { - name_ = *member; - JLOG(journal_.info()) << "Cluster name: " << *member; - } } PeerImp::~PeerImp() @@ -210,7 +181,7 @@ PeerImp::run() closed = parseLedgerHash(iter->value()); if (!closed) - fail(protocol::TMCloseReason::crMALFORMED_HANDSHAKE1); + fail("Malformed handshake data (1)"); } if (auto const iter = headers_.find("Previous-Ledger"); @@ -219,11 +190,11 @@ PeerImp::run() previous = parseLedgerHash(iter->value()); if (!previous) - fail(protocol::TMCloseReason::crMALFORMED_HANDSHAKE2); + fail("Malformed handshake data (2)"); } if (previous && !closed) - fail(protocol::TMCloseReason::crMALFORMED_HANDSHAKE3); + fail("Malformed handshake data (3)"); { std::lock_guard sl(recentLock_); @@ -233,7 +204,10 @@ PeerImp::run() previousLedgerHash_ = *previous; } - doProtocolStart(); + if (inbound_) + doAccept(); + else + doProtocolStart(); // Anything else that needs to be done with the connection should be // done in doProtocolStart @@ -375,7 +349,7 @@ PeerImp::charge(Resource::Charge const& fee) { // Sever the connection overlay_.incPeerDisconnectCharges(); - fail(protocol::TMCloseReason::crCHARGE_RESOURCES); + fail("charge: Resources"); } } @@ -533,8 +507,6 @@ PeerImp::supportsFeature(ProtocolFeature f) const return protocol_ >= make_protocol(2, 2); case ProtocolFeature::LedgerReplay: return ledgerReplayEnabled_; - case ProtocolFeature::StartProtocol: - return protocol_ >= make_protocol(2, 3); } return false; } @@ -627,34 +599,22 @@ PeerImp::close() } void -PeerImp::fail(protocol::TMCloseReason reason) +PeerImp::fail(std::string const& reason) { if (!strand_.running_in_this_thread()) return post( strand_, std::bind( - (void (Peer::*)(protocol::TMCloseReason)) & PeerImp::fail, + (void (Peer::*)(std::string const&)) & PeerImp::fail, shared_from_this(), reason)); if (journal_.active(beast::severities::kWarning) && socket_.is_open()) { std::string const n = name(); JLOG(journal_.warn()) << (n.empty() ? remote_address_.to_string() : n) - << " failed: " << closeReasonToString(reason); - } - - // erase all outstanding messages except for the one - // currently being executed - if (send_queue_.size() > 1) - { - decltype(send_queue_) q({send_queue_.front()}); - send_queue_.swap(q); + << " failed: " << reason; } - - closeOnWriteComplete_ = true; - protocol::TMGracefulClose tmGC; - tmGC.set_reason(reason); - send(std::make_shared(tmGC, protocol::mtGRACEFUL_CLOSE)); + close(); } void @@ -746,7 +706,7 @@ PeerImp::onTimer(error_code const& ec) if (large_sendq_++ >= Tuning::sendqIntervals) { - fail(protocol::TMCloseReason::crLARGE_SENDQUEUE); + fail("Large send queue"); return; } @@ -765,7 +725,7 @@ PeerImp::onTimer(error_code const& ec) (duration > app_.config().MAX_UNKNOWN_TIME))) { overlay_.peerFinder().on_failure(slot_); - fail(protocol::TMCloseReason::crLARGE_SENDQUEUE); + fail("Not useful"); return; } } @@ -773,7 +733,7 @@ PeerImp::onTimer(error_code const& ec) // Already waiting for PONG if (lastPingSeq_) { - fail(protocol::TMCloseReason::crPING_TIMEOUT); + fail("Ping Timeout"); return; } @@ -805,6 +765,71 @@ PeerImp::onShutdown(error_code ec) } //------------------------------------------------------------------------------ +void +PeerImp::doAccept() +{ + assert(read_buffer_.size() == 0); + + JLOG(journal_.debug()) << "doAccept: " << remote_address_; + + auto const sharedValue = makeSharedValue(*stream_ptr_, journal_); + + // This shouldn't fail since we already computed + // the shared value successfully in OverlayImpl + if (!sharedValue) + return fail("makeSharedValue: Unexpected failure"); + + JLOG(journal_.info()) << "Protocol: " << to_string(protocol_); + JLOG(journal_.info()) << "Public Key: " + << toBase58(TokenType::NodePublic, publicKey_); + + if (auto member = app_.cluster().member(publicKey_)) + { + { + std::unique_lock lock{nameMutex_}; + name_ = *member; + } + JLOG(journal_.info()) << "Cluster name: " << *member; + } + + overlay_.activate(shared_from_this()); + + // XXX Set timer: connection is in grace period to be useful. + // XXX Set timer: connection idle (idle may vary depending on connection + // type.) + + auto write_buffer = std::make_shared(); + + boost::beast::ostream(*write_buffer) << makeResponse( + !overlay_.peerFinder().config().peerPrivate, + request_, + overlay_.setup().public_ip, + remote_address_.address(), + *sharedValue, + overlay_.setup().networkID, + protocol_, + app_); + + // Write the whole buffer and only start protocol when that's done. + boost::asio::async_write( + stream_, + write_buffer->data(), + boost::asio::transfer_all(), + bind_executor( + strand_, + [this, write_buffer, self = shared_from_this()]( + error_code ec, std::size_t bytes_transferred) { + if (!socket_.is_open()) + return; + if (ec == boost::asio::error::operation_aborted) + return; + if (ec) + return fail("onWriteResponse", ec); + if (write_buffer->size() == bytes_transferred) + return doProtocolStart(); + return fail("Failed to write header"); + })); +} std::string PeerImp::name() const @@ -828,50 +853,40 @@ PeerImp::doProtocolStart() { onReadMessage(error_code(), 0); - bool supportedProtocol = supportsFeature(ProtocolFeature::StartProtocol); - - if (!inbound_) + // Send all the validator lists that have been loaded + if (inbound_ && supportsFeature(ProtocolFeature::ValidatorListPropagation)) { - // Instruct connected inbound peer to start sending - // protocol messages - if (supportedProtocol) - { - JLOG(journal_.debug()) - << "doProtocolStart(): outbound sending mtSTART_PROTOCOL to " - << remote_address_; - protocol::TMStartProtocol tmPS; - tmPS.set_starttime(std::chrono::duration_cast( - clock_type::now().time_since_epoch()) - .count()); - send(std::make_shared(tmPS, protocol::mtSTART_PROTOCOL)); - } - else - { - JLOG(journal_.debug()) << "doProtocolStart(): outbound connected " - "to an older protocol on " - << remote_address_ << " " << protocol_.first - << " " << protocol_.second; - } - - if (auto m = overlay_.getManifestsMessage()) - send(m); + app_.validators().for_each_available( + [&](std::string const& manifest, + std::uint32_t version, + std::map const& blobInfos, + PublicKey const& pubKey, + std::size_t maxSequence, + uint256 const& hash) { + ValidatorList::sendValidatorList( + *this, + 0, + pubKey, + maxSequence, + version, + manifest, + blobInfos, + app_.getHashRouter(), + p_journal_); - // Request shard info from peer - protocol::TMGetPeerShardInfoV2 tmGPS; - tmGPS.set_relays(0); - send(std::make_shared( - tmGPS, protocol::mtGET_PEER_SHARD_INFO_V2)); - } - // Backward compatibility with the older protocols - else if (!supportedProtocol) - { - JLOG(journal_.debug()) - << "doProtocolStart(): inbound handling of an older protocol on " - << remote_address_ << " " << protocol_.first << " " - << protocol_.second; - onStartProtocol(); + // Don't send it next time. + app_.getHashRouter().addSuppressionPeer(hash, id_); + }); } + if (auto m = overlay_.getManifestsMessage()) + send(m); + + // Request shard info from peer + protocol::TMGetPeerShardInfoV2 tmGPS; + tmGPS.set_relays(0); + send(std::make_shared(tmGPS, protocol::mtGET_PEER_SHARD_INFO_V2)); + setTimer(); } @@ -938,11 +953,7 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) if (!socket_.is_open()) return; if (ec == boost::asio::error::operation_aborted) - { - if (closeOnWriteComplete_) - close(); return; - } if (ec) return fail("onWriteMessage", ec); if (auto stream = journal_.trace()) @@ -956,11 +967,6 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) metrics_.sent.add_message(bytes_transferred); assert(!send_queue_.empty()); - if (send_queue_.front()->getType() == protocol::mtGRACEFUL_CLOSE) - { - close(); - return; - } send_queue_.pop(); if (!send_queue_.empty()) { @@ -2934,69 +2940,6 @@ PeerImp::onMessage(std::shared_ptr const& m) << "onMessage: TMSquelch " << slice << " " << id() << " " << duration; } -void -PeerImp::onStartProtocol() -{ - JLOG(journal_.debug()) << "onStartProtocol(): " << remote_address_; - // Send all the validator lists that have been loaded - if (supportsFeature(ProtocolFeature::ValidatorListPropagation)) - { - app_.validators().for_each_available( - [&](std::string const& manifest, - std::uint32_t version, - std::map const& blobInfos, - PublicKey const& pubKey, - std::size_t maxSequence, - uint256 const& hash) { - ValidatorList::sendValidatorList( - *this, - 0, - pubKey, - maxSequence, - version, - manifest, - blobInfos, - app_.getHashRouter(), - p_journal_); - - // Don't send it next time. - app_.getHashRouter().addSuppressionPeer(hash, id_); - }); - } - - if (auto m = overlay_.getManifestsMessage()) - send(m); - - // Request shard info from peer - protocol::TMGetPeerShardInfoV2 tmGPS; - tmGPS.set_relays(0); - send(std::make_shared(tmGPS, protocol::mtGET_PEER_SHARD_INFO_V2)); -} - -void -PeerImp::onMessage(std::shared_ptr const& m) -{ - JLOG(journal_.debug()) << "onMessage(TMStartProtocol): " << remote_address_; - onStartProtocol(); -} - -void -PeerImp::onMessage(const std::shared_ptr& m) -{ - using on_message_fn = - void (PeerImp::*)(std::shared_ptr const&); - if (!strand_.running_in_this_thread()) - return post( - strand_, - std::bind( - (on_message_fn)&PeerImp::onMessage, shared_from_this(), m)); - - JLOG(journal_.info()) << "got graceful close from: " << remote_address_ - << " reason: " << closeReasonToString(m->reason()); - - close(); -} - //-------------------------------------------------------------------------- void diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index d922e757946..710ab4d74d6 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -180,8 +180,6 @@ class PeerImp : public Peer, bool vpReduceRelayEnabled_ = false; bool ledgerReplayEnabled_ = false; LedgerReplayMsgHandler ledgerReplayMsgHandler_; - // close connection when async write is complete - bool closeOnWriteComplete_ = false; friend class OverlayImpl; @@ -237,7 +235,7 @@ class PeerImp : public Peer, /** Create outgoing, handshaked peer. */ // VFALCO legacyPublicKey should be implied by the Slot - template + template PeerImp( Application& app, std::unique_ptr&& stream_ptr, @@ -415,7 +413,7 @@ class PeerImp : public Peer, isHighLatency() const override; void - fail(protocol::TMCloseReason reason); + fail(std::string const& reason); // Return any known shard info from this peer and its sub peers [[nodiscard]] hash_map const @@ -460,6 +458,9 @@ class PeerImp : public Peer, void onShutdown(error_code ec); + void + doAccept(); + std::string name() const; @@ -583,10 +584,6 @@ class PeerImp : public Peer, onMessage(std::shared_ptr const& m); void onMessage(std::shared_ptr const& m); - void - onMessage(std::shared_ptr const& m); - void - onMessage(std::shared_ptr const& m); private: //-------------------------------------------------------------------------- @@ -645,9 +642,6 @@ class PeerImp : public Peer, void processLedgerRequest(std::shared_ptr const& m); - - void - onStartProtocol(); }; //------------------------------------------------------------------------------ diff --git a/src/ripple/overlay/impl/ProtocolMessage.h b/src/ripple/overlay/impl/ProtocolMessage.h index 6071a621db5..d6fb14bc78c 100644 --- a/src/ripple/overlay/impl/ProtocolMessage.h +++ b/src/ripple/overlay/impl/ProtocolMessage.h @@ -112,10 +112,6 @@ protocolMessageName(int type) return "get_peer_shard_info_v2"; case protocol::mtPEER_SHARD_INFO_V2: return "peer_shard_info_v2"; - case protocol::mtSTART_PROTOCOL: - return "start_protocol"; - case protocol::mtGRACEFUL_CLOSE: - return "graceful_close"; default: break; } @@ -496,14 +492,6 @@ invokeProtocolMessage( success = detail::invoke( *header, buffers, handler); break; - case protocol::mtSTART_PROTOCOL: - success = detail::invoke( - *header, buffers, handler); - break; - case protocol::mtGRACEFUL_CLOSE: - success = detail::invoke( - *header, buffers, handler); - break; default: handler.onMessageUnknown(header->message_type); success = true; diff --git a/src/ripple/overlay/impl/ProtocolVersion.cpp b/src/ripple/overlay/impl/ProtocolVersion.cpp index 8325f6d32fb..fbd48474420 100644 --- a/src/ripple/overlay/impl/ProtocolVersion.cpp +++ b/src/ripple/overlay/impl/ProtocolVersion.cpp @@ -37,8 +37,7 @@ namespace ripple { constexpr ProtocolVersion const supportedProtocolList[] { {2, 1}, - {2, 2}, - {2, 3} + {2, 2} }; // clang-format on diff --git a/src/ripple/proto/ripple.proto b/src/ripple/proto/ripple.proto index c0cdc3cd467..74cbfe8f6cb 100644 --- a/src/ripple/proto/ripple.proto +++ b/src/ripple/proto/ripple.proto @@ -33,8 +33,6 @@ enum MessageType mtPEER_SHARD_INFO_V2 = 62; mtHAVE_TRANSACTIONS = 63; mtTRANSACTIONS = 64; - mtSTART_PROTOCOL = 65; - mtGRACEFUL_CLOSE = 66; } // token, iterations, target, challenge = issue demand for proof of work @@ -452,24 +450,3 @@ message TMHaveTransactions repeated bytes hashes = 1; } -message TMStartProtocol -{ - required uint64 startTime = 1; -} - -enum TMCloseReason -{ - crMALFORMED_HANDSHAKE1 = 1; - crMALFORMED_HANDSHAKE2 = 2; - crMALFORMED_HANDSHAKE3 = 3; - crCHARGE_RESOURCES = 4; - crLARGE_SENDQUEUE = 5; - crNOT_USEFUL = 6; - crPING_TIMEOUT = 7; -} - -message TMGracefulClose -{ - required TMCloseReason reason = 1; -} - diff --git a/src/test/overlay/ProtocolVersion_test.cpp b/src/test/overlay/ProtocolVersion_test.cpp index 3bfba5099f4..a5a26fe74ec 100644 --- a/src/test/overlay/ProtocolVersion_test.cpp +++ b/src/test/overlay/ProtocolVersion_test.cpp @@ -88,7 +88,7 @@ class ProtocolVersion_test : public beast::unit_test::suite BEAST_EXPECT( negotiateProtocolVersion( "RTXP/1.2, XRPL/2.2, XRPL/2.3, XRPL/999.999") == - make_protocol(2, 3)); + make_protocol(2, 2)); BEAST_EXPECT( negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt); From c53a5e7a72844c80b0176b8c5cbd0b231fc343a4 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Wed, 20 Dec 2023 09:30:12 -0800 Subject: [PATCH 4/5] Revert "Apply transaction batches in periodic intervals (#4504)" (#4852) This reverts commit 002893f280cbd578f0693d79e7b2304e3d9cd1d8. There were two files with conflicts in the automated revert: - src/ripple/rpc/impl/RPCHelpers.h and - src/test/rpc/JSONRPC_test.cpp Those files were manually resolved. --- Builds/CMake/RippledCore.cmake | 1 - cfg/rippled-example.cfg | 2 +- cfg/rippled-reporting.cfg | 2 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 33 ++- src/ripple/app/main/Application.cpp | 1 - src/ripple/app/misc/NetworkOPs.cpp | 219 +++++++++--------- src/ripple/app/misc/NetworkOPs.h | 43 +--- src/ripple/app/tx/impl/apply.cpp | 10 +- src/ripple/basics/SubmitSync.h | 41 ---- src/ripple/core/Config.h | 11 +- src/ripple/overlay/impl/PeerImp.cpp | 10 +- src/ripple/protocol/TER.h | 1 - src/ripple/protocol/impl/TER.cpp | 1 - src/ripple/protocol/jss.h | 1 - src/ripple/rpc/handlers/Submit.cpp | 10 +- src/ripple/rpc/handlers/SubmitMultiSigned.cpp | 9 +- src/ripple/rpc/impl/RPCHelpers.cpp | 21 -- src/ripple/rpc/impl/RPCHelpers.h | 10 - src/ripple/rpc/impl/TransactionSign.cpp | 11 +- src/ripple/rpc/impl/TransactionSign.h | 14 +- src/test/app/Transaction_ordering_test.cpp | 4 - src/test/jtx/Env_test.cpp | 91 -------- src/test/rpc/JSONRPC_test.cpp | 9 +- src/test/rpc/RobustTransaction_test.cpp | 6 +- 24 files changed, 163 insertions(+), 398 deletions(-) delete mode 100644 src/ripple/basics/SubmitSync.h diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index ab5083aa208..f9e43810a05 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -201,7 +201,6 @@ install ( src/ripple/basics/StringUtilities.h src/ripple/basics/TaggedCache.h src/ripple/basics/tagged_integer.h - src/ripple/basics/SubmitSync.h src/ripple/basics/ThreadSafetyAnalysis.h src/ripple/basics/ToString.h src/ripple/basics/UnorderedContainers.h diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index a3bcf0056be..1ae87e11f79 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -482,7 +482,7 @@ # # Configure the maximum number of transactions to have in the job queue # -# Must be a number between 1000 and 100000, defaults to 10000 +# Must be a number between 100 and 1000, defaults to 250 # # # [overlay] diff --git a/cfg/rippled-reporting.cfg b/cfg/rippled-reporting.cfg index 632a8a7800e..6ef10df5bf8 100644 --- a/cfg/rippled-reporting.cfg +++ b/cfg/rippled-reporting.cfg @@ -454,7 +454,7 @@ # # Configure the maximum number of transactions to have in the job queue # -# Must be a number between 1000 and 100000, defaults to 10000 +# Must be a number between 100 and 1000, defaults to 250 # # # [overlay] diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 45ab3e89ee7..0006d6b0dbd 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -549,25 +549,22 @@ void LedgerMaster::applyHeldTransactions() { std::lock_guard sl(m_mutex); - // It can be expensive to modify the open ledger even with no transactions - // to process. Regardless, make sure to reset held transactions with - // the parent. - if (mHeldTransactions.size()) - { - app_.openLedger().modify([&](OpenView& view, beast::Journal j) { - bool any = false; - for (auto const& it : mHeldTransactions) - { - ApplyFlags flags = tapNONE; - auto const result = - app_.getTxQ().apply(app_, view, it.second, flags, j); - if (result.second) - any = true; - } - return any; - }); - } + app_.openLedger().modify([&](OpenView& view, beast::Journal j) { + bool any = false; + for (auto const& it : mHeldTransactions) + { + ApplyFlags flags = tapNONE; + auto const result = + app_.getTxQ().apply(app_, view, it.second, flags, j); + if (result.second) + any = true; + } + return any; + }); + + // VFALCO TODO recreate the CanonicalTxSet object instead of resetting + // it. // VFALCO NOTE The hash for an open ledger is undefined so we use // something that is a reasonable substitute. mHeldTransactions.reset(app_.openLedger().current()->info().parentHash); diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 08ba296b271..871daffec32 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1533,7 +1533,6 @@ ApplicationImp::start(bool withTimers) { setSweepTimer(); setEntropyTimer(); - m_networkOPs->setBatchApplyTimer(); } m_io_latency_sampler.start(); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index bc686fc61d5..785e1c12c8b 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -238,7 +237,6 @@ class NetworkOPsImp final : public NetworkOPs , heartbeatTimer_(io_svc) , clusterTimer_(io_svc) , accountHistoryTxTimer_(io_svc) - , batchApplyTimer_(io_svc) , mConsensus( app, make_FeeVote( @@ -288,12 +286,43 @@ class NetworkOPsImp final : public NetworkOPs processTransaction( std::shared_ptr& transaction, bool bUnlimited, - RPC::SubmitSync sync, bool bLocal, FailHard failType) override; - bool - transactionBatch(bool drain) override; + /** + * For transactions submitted directly by a client, apply batch of + * transactions and wait for this transaction to complete. + * + * @param transaction Transaction object. + * @param bUnliimited Whether a privileged client connection submitted it. + * @param failType fail_hard setting from transaction submission. + */ + void + doTransactionSync( + std::shared_ptr transaction, + bool bUnlimited, + FailHard failType); + + /** + * For transactions not submitted by a locally connected client, fire and + * forget. Add to batch and trigger it to be processed if there's no batch + * currently being applied. + * + * @param transaction Transaction object + * @param bUnlimited Whether a privileged client connection submitted it. + * @param failType fail_hard setting from transaction submission. + */ + void + doTransactionAsync( + std::shared_ptr transaction, + bool bUnlimited, + FailHard failtype); + + /** + * Apply transactions in batches. Continue until none are queued. + */ + void + transactionBatch(); /** * Attempt to apply transactions and post-process based on the results. @@ -567,15 +596,6 @@ class NetworkOPsImp final : public NetworkOPs << "NetworkOPs: accountHistoryTxTimer cancel error: " << ec.message(); } - - ec.clear(); - batchApplyTimer_.cancel(ec); - if (ec) - { - JLOG(m_journal.error()) - << "NetworkOPs: batchApplyTimer cancel error: " - << ec.message(); - } } // Make sure that any waitHandlers pending in our timers are done. using namespace std::chrono_literals; @@ -697,9 +717,6 @@ class NetworkOPsImp final : public NetworkOPs void setAccountHistoryJobTimer(SubAccountHistoryInfoWeak subInfo); - void - setBatchApplyTimer() override; - Application& app_; beast::Journal m_journal; @@ -718,8 +735,6 @@ class NetworkOPsImp final : public NetworkOPs boost::asio::steady_timer heartbeatTimer_; boost::asio::steady_timer clusterTimer_; boost::asio::steady_timer accountHistoryTxTimer_; - //! This timer is for applying transaction batches. - boost::asio::steady_timer batchApplyTimer_; RCLConsensus mConsensus; @@ -993,42 +1008,6 @@ NetworkOPsImp::setAccountHistoryJobTimer(SubAccountHistoryInfoWeak subInfo) [this, subInfo]() { setAccountHistoryJobTimer(subInfo); }); } -void -NetworkOPsImp::setBatchApplyTimer() -{ - using namespace std::chrono_literals; - // 100ms lag between batch intervals provides significant throughput gains - // with little increased latency. Tuning this figure further will - // require further testing. In general, increasing this figure will - // also increase theoretical throughput, but with diminishing returns. - auto constexpr batchInterval = 100ms; - - setTimer( - batchApplyTimer_, - batchInterval, - [this]() { - { - std::lock_guard lock(mMutex); - // Only do the job if there's work to do and it's not currently - // being done. - if (mTransactions.size() && - mDispatchState == DispatchState::none) - { - if (m_job_queue.addJob( - jtBATCH, "transactionBatch", [this]() { - transactionBatch(false); - })) - { - mDispatchState = DispatchState::scheduled; - } - return; - } - } - setBatchApplyTimer(); - }, - [this]() { setBatchApplyTimer(); }); -} - void NetworkOPsImp::processHeartbeatTimer() { @@ -1205,8 +1184,7 @@ NetworkOPsImp::submitTransaction(std::shared_ptr const& iTrans) m_job_queue.addJob(jtTRANSACTION, "submitTxn", [this, tx]() { auto t = tx; - processTransaction( - t, false, RPC::SubmitSync::async, false, FailHard::no); + processTransaction(t, false, false, FailHard::no); }); } @@ -1214,7 +1192,6 @@ void NetworkOPsImp::processTransaction( std::shared_ptr& transaction, bool bUnlimited, - RPC::SubmitSync sync, bool bLocal, FailHard failType) { @@ -1244,7 +1221,7 @@ NetworkOPsImp::processTransaction( // Not concerned with local checks at this point. if (validity == Validity::SigBad) { - JLOG(m_journal.trace()) << "Transaction has bad signature: " << reason; + JLOG(m_journal.info()) << "Transaction has bad signature: " << reason; transaction->setStatus(INVALID); transaction->setResult(temBAD_SIGNATURE); app_.getHashRouter().setFlags(transaction->getID(), SF_BAD); @@ -1254,72 +1231,100 @@ NetworkOPsImp::processTransaction( // canonicalize can change our pointer app_.getMasterTransaction().canonicalize(&transaction); - std::unique_lock lock(mMutex); + if (bLocal) + doTransactionSync(transaction, bUnlimited, failType); + else + doTransactionAsync(transaction, bUnlimited, failType); +} + +void +NetworkOPsImp::doTransactionAsync( + std::shared_ptr transaction, + bool bUnlimited, + FailHard failType) +{ + std::lock_guard lock(mMutex); + + if (transaction->getApplying()) + return; + + mTransactions.push_back( + TransactionStatus(transaction, bUnlimited, false, failType)); + transaction->setApplying(); + + if (mDispatchState == DispatchState::none) + { + if (m_job_queue.addJob( + jtBATCH, "transactionBatch", [this]() { transactionBatch(); })) + { + mDispatchState = DispatchState::scheduled; + } + } +} + +void +NetworkOPsImp::doTransactionSync( + std::shared_ptr transaction, + bool bUnlimited, + FailHard failType) +{ + std::unique_lock lock(mMutex); + if (!transaction->getApplying()) { - transaction->setApplying(); mTransactions.push_back( - TransactionStatus(transaction, bUnlimited, bLocal, failType)); + TransactionStatus(transaction, bUnlimited, true, failType)); + transaction->setApplying(); } - switch (sync) + + do { - using enum RPC::SubmitSync; - case sync: - do + if (mDispatchState == DispatchState::running) + { + // A batch processing job is already running, so wait. + mCond.wait(lock); + } + else + { + apply(lock); + + if (mTransactions.size()) { - // If a batch is being processed, then wait. Otherwise, - // process a batch. - if (mDispatchState == DispatchState::running) - mCond.wait(lock); - else - apply(lock); - } while (transaction->getApplying()); - break; - - case async: - // It's conceivable for the submitted transaction to be - // processed and its result to be modified before being returned - // to the client. Make a copy of the transaction and set its - // status to guarantee that the client gets the terSUBMITTED - // result in all cases. - transaction = std::make_shared(*transaction); - transaction->setResult(terSUBMITTED); - break; - - case wait: - mCond.wait( - lock, [&transaction] { return !transaction->getApplying(); }); - break; - - default: - assert(false); - } + // More transactions need to be applied, but by another job. + if (m_job_queue.addJob(jtBATCH, "transactionBatch", [this]() { + transactionBatch(); + })) + { + mDispatchState = DispatchState::scheduled; + } + } + } + } while (transaction->getApplying()); } -bool -NetworkOPsImp::transactionBatch(bool const drain) +void +NetworkOPsImp::transactionBatch() { - { - std::unique_lock lock(mMutex); - if (mDispatchState == DispatchState::running || mTransactions.empty()) - return false; + std::unique_lock lock(mMutex); - do - apply(lock); - while (drain && mTransactions.size()); + if (mDispatchState == DispatchState::running) + return; + + while (mTransactions.size()) + { + apply(lock); } - setBatchApplyTimer(); - return true; } void NetworkOPsImp::apply(std::unique_lock& batchLock) { - assert(!mTransactions.empty()); - assert(mDispatchState != DispatchState::running); std::vector submit_held; std::vector transactions; mTransactions.swap(transactions); + assert(!transactions.empty()); + + assert(mDispatchState != DispatchState::running); mDispatchState = DispatchState::running; batchLock.unlock(); @@ -1703,9 +1708,7 @@ NetworkOPsImp::checkLastClosedLedger( switchLedgers = false; } else - { networkClosed = closedLedger; - } if (!switchLedgers) return false; diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index 59285311172..d53127ed3b6 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -71,10 +71,6 @@ enum class OperatingMode { FULL = 4 //!< we have the ledger and can even validate }; -namespace RPC { -enum class SubmitSync; -} - /** Provides server functionality for clients. Clients include backend applications, local commands, and connected @@ -127,47 +123,22 @@ class NetworkOPs : public InfoSub::Source virtual void submitTransaction(std::shared_ptr const&) = 0; - /** Process a transaction. - * - * The transaction has been submitted either from the peer network or - * from a client. For client submissions, there are 3 distinct behaviors: - * 1) sync (default): process transactions in a batch immediately, - * and return only once the transaction has been processed. - * 2) async: Put transaction into the batch for the next processing - * interval and return immediately. - * 3) wait: Put transaction into the batch for the next processing - * interval and return only after it is processed. + /** + * Process transactions as they arrive from the network or which are + * submitted by clients. Process local transactions synchronously * - * @param transaction Transaction object. + * @param transaction Transaction object * @param bUnlimited Whether a privileged client connection submitted it. - * @param sync Client submission synchronous behavior type requested. - * @param bLocal Whether submitted by client (local) or peer. - * @param failType Whether to fail hard or not. + * @param bLocal Client submission. + * @param failType fail_hard setting from transaction submission. */ virtual void processTransaction( std::shared_ptr& transaction, bool bUnlimited, - RPC::SubmitSync sync, bool bLocal, FailHard failType) = 0; - /** Apply transactions in batches. - * - * Only a single batch unless drain is set. This is to optimize performance - * because there is significant overhead in applying each batch, whereas - * processing an individual transaction is fast. - * - * Setting the drain parameter is relevant for some transaction - * processing unit tests that expect all submitted transactions to - * be processed synchronously. - * - * @param drain Whether to process batches until none remain. - * @return Whether any transactions were processed. - */ - virtual bool - transactionBatch(bool drain) = 0; - //-------------------------------------------------------------------------- // // Owner functions @@ -216,8 +187,6 @@ class NetworkOPs : public InfoSub::Source setStandAlone() = 0; virtual void setStateTimer() = 0; - virtual void - setBatchApplyTimer() = 0; virtual void setNeedNetworkLedger() = 0; diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index 4881f2a49b7..c0704c5c3ae 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -134,7 +134,7 @@ applyTransaction( if (retryAssured) flags = flags | tapRETRY; - JLOG(j.trace()) << "TXN " << txn.getTransactionID() + JLOG(j.debug()) << "TXN " << txn.getTransactionID() << (retryAssured ? "/retry" : "/final"); try @@ -142,7 +142,7 @@ applyTransaction( auto const result = apply(app, view, txn, flags, j); if (result.second) { - JLOG(j.trace()) + JLOG(j.debug()) << "Transaction applied: " << transHuman(result.first); return ApplyResult::Success; } @@ -151,17 +151,17 @@ applyTransaction( isTelLocal(result.first)) { // failure - JLOG(j.trace()) + JLOG(j.debug()) << "Transaction failure: " << transHuman(result.first); return ApplyResult::Fail; } - JLOG(j.trace()) << "Transaction retry: " << transHuman(result.first); + JLOG(j.debug()) << "Transaction retry: " << transHuman(result.first); return ApplyResult::Retry; } catch (std::exception const& ex) { - JLOG(j.trace()) << "Throws: " << ex.what(); + JLOG(j.warn()) << "Throws: " << ex.what(); return ApplyResult::Fail; } } diff --git a/src/ripple/basics/SubmitSync.h b/src/ripple/basics/SubmitSync.h deleted file mode 100644 index 12311c676e8..00000000000 --- a/src/ripple/basics/SubmitSync.h +++ /dev/null @@ -1,41 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2023 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#ifndef RIPPLE_BASICS_SUBMITSYNC_H_INCLUDED -#define RIPPLE_BASICS_SUBMITSYNC_H_INCLUDED - -namespace ripple { -namespace RPC { - -/** - * Possible values for defining synchronous behavior of the transaction - * submission API. - * 1) sync (default): Process transactions in a batch immediately, - * and return only once the transaction has been processed. - * 2) async: Put transaction into the batch for the next processing - * interval and return immediately. - * 3) wait: Put transaction into the batch for the next processing - * interval and return only after it is processed. - */ -enum class SubmitSync { sync, async, wait }; - -} // namespace RPC -} // namespace ripple - -#endif \ No newline at end of file diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index a9acd4c6b2b..cf41678a16c 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -215,7 +215,7 @@ class Config : public BasicConfig // Node storage configuration std::uint32_t LEDGER_HISTORY = 256; - std::uint32_t FETCH_DEPTH = 1'000'000'000; + std::uint32_t FETCH_DEPTH = 1000000000; // Tunable that adjusts various parameters, typically associated // with hardware parameters (RAM size and CPU cores). The default @@ -232,11 +232,10 @@ class Config : public BasicConfig // Enable the experimental Ledger Replay functionality bool LEDGER_REPLAY = false; - // Work queue limits. 10000 transactions is 2 full seconds of slowdown at - // 5000/s. - int MAX_TRANSACTIONS = 10'000; - static constexpr int MAX_JOB_QUEUE_TX = 100'000; - static constexpr int MIN_JOB_QUEUE_TX = 1'000; + // Work queue limits + int MAX_TRANSACTIONS = 250; + static constexpr int MAX_JOB_QUEUE_TX = 1000; + static constexpr int MIN_JOB_QUEUE_TX = 100; // Amendment majority time std::chrono::seconds AMENDMENT_MAJORITY_TIME = defaultAmendmentMajorityTime; diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 3afec605cfa..0d58a10abac 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -40,14 +39,13 @@ #include #include #include -#include #include +#include #include #include #include -#include #include #include #include @@ -3111,11 +3109,7 @@ PeerImp::checkTransaction( bool const trusted(flags & SF_TRUSTED); app_.getOPs().processTransaction( - tx, - trusted, - RPC::SubmitSync::async, - false, - NetworkOPs::FailHard::no); + tx, trusted, false, NetworkOPs::FailHard::no); } catch (std::exception const& ex) { diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 23d4fb3ef00..61028d60e9d 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -218,7 +218,6 @@ enum TERcodes : TERUnderlyingType { terQUEUED, // Transaction is being held in TxQ until fee drops terPRE_TICKET, // Ticket is not yet in ledger but might be on its way terNO_AMM, // AMM doesn't exist for the asset pair - terSUBMITTED // Has been submitted async. }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 1c2db3feb3b..5f608e806ab 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -211,7 +211,6 @@ transResults() MAKE_ERROR(terQUEUED, "Held until escalated fee drops."), MAKE_ERROR(terPRE_TICKET, "Ticket is not yet in ledger."), MAKE_ERROR(terNO_AMM, "AMM doesn't exist for the asset pair."), - MAKE_ERROR(terSUBMITTED, "Transaction has been submitted."), MAKE_ERROR(tesSUCCESS, "The transaction was applied. Only final in a validated ledger."), }; diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 8a701defad8..fd1c94c67d2 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -636,7 +636,6 @@ JSS(sub_index); // in: LedgerEntry JSS(subcommand); // in: PathFind JSS(success); // rpc JSS(supported); // out: AmendmentTableImpl -JSS(sync_mode); // in: Submit JSS(system_time_offset); // out: NetworkOPs JSS(tag); // out: Peers JSS(taker); // in: Subscribe, BookOffers diff --git a/src/ripple/rpc/handlers/Submit.cpp b/src/ripple/rpc/handlers/Submit.cpp index 8e998f1ea6c..b5577ecb576 100644 --- a/src/ripple/rpc/handlers/Submit.cpp +++ b/src/ripple/rpc/handlers/Submit.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -49,10 +48,6 @@ doSubmit(RPC::JsonContext& context) { context.loadType = Resource::feeMediumBurdenRPC; - auto const sync = RPC::getSubmitSyncMode(context.params); - if (!sync) - return sync.error(); - if (!context.params.isMember(jss::tx_blob)) { auto const failType = getFailHard(context); @@ -68,8 +63,7 @@ doSubmit(RPC::JsonContext& context) context.role, context.ledgerMaster.getValidatedLedgerAge(), context.app, - RPC::getProcessTxnFn(context.netOps), - *sync); + RPC::getProcessTxnFn(context.netOps)); ret[jss::deprecated] = "Signing support in the 'submit' command has been " @@ -138,7 +132,7 @@ doSubmit(RPC::JsonContext& context) auto const failType = getFailHard(context); context.netOps.processTransaction( - tpTrans, isUnlimited(context.role), *sync, true, failType); + tpTrans, isUnlimited(context.role), true, failType); } catch (std::exception& e) { diff --git a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp index 82fa52a4623..5b9d5b34ac6 100644 --- a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp +++ b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp @@ -18,12 +18,10 @@ //============================================================================== #include -#include #include #include #include #include -#include #include namespace ripple { @@ -39,10 +37,6 @@ doSubmitMultiSigned(RPC::JsonContext& context) auto const failHard = context.params[jss::fail_hard].asBool(); auto const failType = NetworkOPs::doFailHard(failHard); - auto const sync = RPC::getSubmitSyncMode(context.params); - if (!sync) - return sync.error(); - return RPC::transactionSubmitMultiSigned( context.params, context.apiVersion, @@ -50,8 +44,7 @@ doSubmitMultiSigned(RPC::JsonContext& context) context.role, context.ledgerMaster.getValidatedLedgerAge(), context.app, - RPC::getProcessTxnFn(context.netOps), - *sync); + RPC::getProcessTxnFn(context.netOps)); } } // namespace ripple diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 672095fe950..5e7300adea8 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -1125,26 +1125,5 @@ getLedgerByContext(RPC::JsonContext& context) return RPC::make_error( rpcNOT_READY, "findCreate failed to return an inbound ledger"); } - -ripple::Expected -getSubmitSyncMode(Json::Value const& params) -{ - using enum RPC::SubmitSync; - if (params.isMember(jss::sync_mode)) - { - std::string const syncMode = params[jss::sync_mode].asString(); - if (syncMode == "async") - return async; - else if (syncMode == "wait") - return wait; - else if (syncMode != "sync") - return Unexpected(RPC::make_error( - rpcINVALID_PARAMS, - "sync_mode parameter must be one of \"sync\", \"async\", or " - "\"wait\".")); - } - return sync; -} - } // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 0c2a299d8ab..c28c2d0f244 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -26,8 +26,6 @@ #include #include -#include -#include #include #include #include @@ -294,14 +292,6 @@ keypairForSignature( Json::Value const& params, Json::Value& error, unsigned int apiVersion = apiVersionIfUnspecified); -/** Helper to parse submit_mode parameter to RPC submit. - * - * @param params RPC parameters - * @return Either the mode or an error object. - */ -ripple::Expected -getSubmitSyncMode(Json::Value const& params); - } // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 915764c6eb4..0881881bd1a 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -834,8 +834,7 @@ transactionSubmit( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction, - RPC::SubmitSync sync) + ProcessTransactionFn const& processTransaction) { using namespace detail; @@ -861,7 +860,8 @@ transactionSubmit( // Finally, submit the transaction. try { - processTransaction(txn.second, isUnlimited(role), sync, failType); + // FIXME: For performance, should use asynch interface + processTransaction(txn.second, isUnlimited(role), true, failType); } catch (std::exception&) { @@ -1072,8 +1072,7 @@ transactionSubmitMultiSigned( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction, - RPC::SubmitSync sync) + ProcessTransactionFn const& processTransaction) { auto const& ledger = app.openLedger().current(); auto j = app.journal("RPCHandler"); @@ -1246,7 +1245,7 @@ transactionSubmitMultiSigned( try { // FIXME: For performance, should use asynch interface - processTransaction(txn.second, isUnlimited(role), sync, failType); + processTransaction(txn.second, isUnlimited(role), true, failType); } catch (std::exception&) { diff --git a/src/ripple/rpc/impl/TransactionSign.h b/src/ripple/rpc/impl/TransactionSign.h index 48d2859ccf5..2a38031f50a 100644 --- a/src/ripple/rpc/impl/TransactionSign.h +++ b/src/ripple/rpc/impl/TransactionSign.h @@ -21,7 +21,6 @@ #define RIPPLE_RPC_TRANSACTIONSIGN_H_INCLUDED #include -#include #include #include @@ -76,7 +75,7 @@ checkFee( using ProcessTransactionFn = std::function& transaction, bool bUnlimited, - RPC::SubmitSync sync, + bool bLocal, NetworkOPs::FailHard failType)>; inline ProcessTransactionFn @@ -85,10 +84,9 @@ getProcessTxnFn(NetworkOPs& netOPs) return [&netOPs]( std::shared_ptr& transaction, bool bUnlimited, - RPC::SubmitSync sync, + bool bLocal, NetworkOPs::FailHard failType) { - netOPs.processTransaction( - transaction, bUnlimited, sync, true, failType); + netOPs.processTransaction(transaction, bUnlimited, bLocal, failType); }; } @@ -111,8 +109,7 @@ transactionSubmit( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction, - RPC::SubmitSync sync); + ProcessTransactionFn const& processTransaction); /** Returns a Json::objectValue. */ Json::Value @@ -133,8 +130,7 @@ transactionSubmitMultiSigned( Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction, - RPC::SubmitSync sync); + ProcessTransactionFn const& processTransaction); } // namespace RPC } // namespace ripple diff --git a/src/test/app/Transaction_ordering_test.cpp b/src/test/app/Transaction_ordering_test.cpp index 01f870d0668..0353df90663 100644 --- a/src/test/app/Transaction_ordering_test.cpp +++ b/src/test/app/Transaction_ordering_test.cpp @@ -15,7 +15,6 @@ */ //============================================================================== -#include #include #include #include @@ -92,7 +91,6 @@ struct Transaction_ordering_test : public beast::unit_test::suite env(tx2, ter(terPRE_SEQ)); BEAST_EXPECT(env.seq(alice) == aliceSequence); env(tx1); - BEAST_EXPECT(env.app().getOPs().transactionBatch(false)); env.app().getJobQueue().rendezvous(); BEAST_EXPECT(env.seq(alice) == aliceSequence + 2); @@ -145,8 +143,6 @@ struct Transaction_ordering_test : public beast::unit_test::suite } env(tx[0]); - // Apply until no more deferred/held transactions. - BEAST_EXPECT(env.app().getOPs().transactionBatch(true)); env.app().getJobQueue().rendezvous(); BEAST_EXPECT(env.seq(alice) == aliceSequence + 5); diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 6e26a40e25a..6f09f49ed5d 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -29,7 +29,6 @@ #include #include -#include #include namespace ripple { @@ -901,95 +900,6 @@ class Env_test : public beast::unit_test::suite pass(); } - void - testSyncSubmit() - { - using namespace jtx; - Env env(*this); - - auto const alice = Account{"alice"}; - auto const n = XRP(10000); - env.fund(n, alice); - BEAST_EXPECT(env.balance(alice) == n); - - // submit only - auto applyBlobTxn = [&env](char const* syncMode, auto&&... txnArgs) { - auto jt = env.jt(txnArgs...); - Serializer s; - jt.stx->add(s); - - Json::Value args{Json::objectValue}; - - args[jss::tx_blob] = strHex(s.slice()); - args[jss::fail_hard] = true; - args[jss::sync_mode] = syncMode; - - return env.rpc("json", "submit", args.toStyledString()); - }; - - auto jr = applyBlobTxn("sync", noop(alice)); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); - - jr = applyBlobTxn("async", noop(alice)); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "terSUBMITTED"); - // Make sure it gets processed before submitting and waiting. - env.app().getOPs().transactionBatch(true); - - auto applier = [&env]() { - while (!env.app().getOPs().transactionBatch(false)) - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - }; - auto t = std::thread(applier); - - jr = applyBlobTxn("wait", noop(alice)); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); - t.join(); - - jr = applyBlobTxn("scott", noop(alice)); - BEAST_EXPECT(jr[jss::result][jss::error] == "invalidParams"); - - // sign and submit - auto applyJsonTxn = [&env]( - char const* syncMode, - std::string const secret, - Json::Value const& val) { - Json::Value args{Json::objectValue}; - args[jss::secret] = secret; - args[jss::tx_json] = val; - args[jss::fail_hard] = true; - args[jss::sync_mode] = syncMode; - - return env.rpc("json", "submit", args.toStyledString()); - }; - - Json::Value payment; - auto secret = toBase58(generateSeed("alice")); - payment = noop("alice"); - payment[sfSequence.fieldName] = env.seq("alice"); - payment[sfSetFlag.fieldName] = 0; - jr = applyJsonTxn("sync", secret, payment); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); - - payment[sfSequence.fieldName] = env.seq("alice"); - jr = applyJsonTxn("async", secret, payment); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "terSUBMITTED"); - - env.app().getOPs().transactionBatch(true); - payment[sfSequence.fieldName] = env.seq("alice"); - - auto aSeq = env.seq("alice"); - t = std::thread(applier); - jr = applyJsonTxn("wait", secret, payment); - BEAST_EXPECT(jr[jss::result][jss::engine_result] == "tesSUCCESS"); - t.join(); - // Ensure the last transaction was processed. - BEAST_EXPECT(env.seq("alice") == aSeq + 1); - - payment[sfSequence.fieldName] = env.seq("alice"); - jr = applyJsonTxn("scott", secret, payment); - BEAST_EXPECT(jr[jss::result][jss::error] == "invalidParams"); - } - void run() override { @@ -1015,7 +925,6 @@ class Env_test : public beast::unit_test::suite testSignAndSubmit(); testFeatures(); testExceptionalShutdown(); - testSyncSubmit(); } }; diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index 1e8ce554be3..33a85d1fe36 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -2499,7 +2498,7 @@ class JSONRPC_test : public beast::unit_test::suite fakeProcessTransaction( std::shared_ptr&, bool, - SubmitSync, + bool, NetworkOPs::FailHard) { ; @@ -2549,8 +2548,7 @@ class JSONRPC_test : public beast::unit_test::suite Role role, std::chrono::seconds validatedLedgerAge, Application& app, - ProcessTransactionFn const& processTransaction, - RPC::SubmitSync sync); + ProcessTransactionFn const& processTransaction); using TestStuff = std::tuple; @@ -2605,8 +2603,7 @@ class JSONRPC_test : public beast::unit_test::suite testRole, 1s, env.app(), - processTxn, - RPC::SubmitSync::sync); + processTxn); } std::string errStr; diff --git a/src/test/rpc/RobustTransaction_test.cpp b/src/test/rpc/RobustTransaction_test.cpp index 01ac71e272a..37b16c58d7f 100644 --- a/src/test/rpc/RobustTransaction_test.cpp +++ b/src/test/rpc/RobustTransaction_test.cpp @@ -17,7 +17,6 @@ */ //============================================================================== -#include #include #include #include @@ -89,8 +88,7 @@ class RobustTransaction_test : public beast::unit_test::suite } BEAST_EXPECT(jv[jss::result][jss::engine_result] == "tefPAST_SEQ"); - // Submit future sequence transaction -- this transaction should be - // held until the sequence gap is closed. + // Submit future sequence transaction payment[jss::tx_json][sfSequence.fieldName] = env.seq("alice") + 1; jv = wsc->invoke("submit", payment); if (wsc->version() == 2) @@ -116,8 +114,6 @@ class RobustTransaction_test : public beast::unit_test::suite } BEAST_EXPECT(jv[jss::result][jss::engine_result] == "tesSUCCESS"); - // Apply held transactions. - env.app().getOPs().transactionBatch(true); // Wait for the jobqueue to process everything env.app().getJobQueue().rendezvous(); From ca3198164c2869406e1f04020f3c1a74c7d35485 Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Wed, 20 Dec 2023 09:33:33 -0800 Subject: [PATCH 5/5] Set version to 2.0.0-rc6 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index f5e447306a8..b097fca175a 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/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.0.0-rc5" +char const* const versionString = "2.0.0-rc6" // clang-format on #if defined(DEBUG) || defined(SANITIZER)