diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index a3c06399cf5..b3c9b18d2f8 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -134,6 +134,13 @@ Currently (prior to the release of 2.0), it is available as a "beta" version, me Since `api_version` 2 is in "beta", breaking changes to V2 can currently be made at any time. +#### Removed methods + +In API version 2, the following methods are no longer available: + +- `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 account_info response in V2 - `signer_lists` is returned in the root of the response. Previously, in API version 1, it was nested under `account_data`. (https://github.com/XRPLF/rippled/pull/3770) @@ -158,6 +165,15 @@ Since `api_version` 2 is in "beta", breaking changes to V2 can currently be made - 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) +##### In progress + +- In `Payment` transaction type, JSON RPC field `Amount` is renamed to `DeliverMax`. To enable smooth client transition, `Amount` is still handled, as described below: + - 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. + - On JSON RPC output (e.g. `subscribe`, `account_tx` etc. methods), `DeliverMax` is present in both API version 1 and version 2. + - On JSON RPC output, `Amount` is only present in API version 1 and _not_ in version 2. + + # Unit tests for API changes The following information is useful to developers contributing to this project: diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index ef6f925ec6d..269c107ca9e 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -217,6 +217,7 @@ install ( install ( FILES src/ripple/json/JsonPropertyStream.h + src/ripple/json/MultivarJson.h src/ripple/json/Object.h src/ripple/json/Output.h src/ripple/json/Writer.h @@ -467,6 +468,7 @@ target_sources (rippled PRIVATE src/ripple/app/misc/detail/impl/WorkSSL.cpp src/ripple/app/misc/impl/AccountTxPaging.cpp src/ripple/app/misc/impl/AmendmentTable.cpp + src/ripple/app/misc/impl/DeliverMax.cpp src/ripple/app/misc/impl/LoadFeeTrack.cpp src/ripple/app/misc/impl/Manifest.cpp src/ripple/app/misc/impl/Transaction.cpp @@ -918,6 +920,7 @@ if (tests) src/test/json/Output_test.cpp src/test/json/Writer_test.cpp src/test/json/json_value_test.cpp + src/test/json/MultivarJson_test.cpp #[===============================[ test sources: subdir: jtx @@ -1059,6 +1062,7 @@ if (tests) src/test/rpc/KeyGeneration_test.cpp src/test/rpc/LedgerClosed_test.cpp src/test/rpc/LedgerData_test.cpp + src/test/rpc/LedgerHeader_test.cpp src/test/rpc/LedgerRPC_test.cpp src/test/rpc/LedgerRequestRPC_test.cpp src/test/rpc/ManifestRPC_test.cpp @@ -1082,6 +1086,7 @@ if (tests) src/test/rpc/ValidatorInfo_test.cpp src/test/rpc/ValidatorRPC_test.cpp src/test/rpc/Version_test.cpp + src/test/rpc/Handler_test.cpp #[===============================[ test sources: subdir: server diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt index 832b548e5de..5f50dc66118 100644 --- a/Builds/levelization/results/ordering.txt +++ b/Builds/levelization/results/ordering.txt @@ -135,6 +135,7 @@ test.csf > ripple.protocol test.csf > test.jtx test.json > ripple.beast test.json > ripple.json +test.json > ripple.rpc test.json > test.jtx test.jtx > ripple.app test.jtx > ripple.basics diff --git a/src/ripple/app/ledger/BookListeners.cpp b/src/ripple/app/ledger/BookListeners.cpp index 885ca4d322a..bbc0058bc76 100644 --- a/src/ripple/app/ledger/BookListeners.cpp +++ b/src/ripple/app/ledger/BookListeners.cpp @@ -39,7 +39,7 @@ BookListeners::removeSubscriber(std::uint64_t seq) void BookListeners::publish( - Json::Value const& jvObj, + MultiApiJson const& jvObj, hash_set& havePublished) { std::lock_guard sl(mLock); @@ -54,7 +54,8 @@ BookListeners::publish( // Only publish jvObj if this is the first occurence if (havePublished.emplace(p->getSeq()).second) { - p->send(jvObj, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); } ++it; } diff --git a/src/ripple/app/ledger/BookListeners.h b/src/ripple/app/ledger/BookListeners.h index 2f668f34042..748378a12b1 100644 --- a/src/ripple/app/ledger/BookListeners.h +++ b/src/ripple/app/ledger/BookListeners.h @@ -20,7 +20,9 @@ #ifndef RIPPLE_APP_LEDGER_BOOKLISTENERS_H_INCLUDED #define RIPPLE_APP_LEDGER_BOOKLISTENERS_H_INCLUDED +#include #include + #include #include @@ -58,7 +60,7 @@ class BookListeners */ void - publish(Json::Value const& jvObj, hash_set& havePublished); + publish(MultiApiJson const& jvObj, hash_set& havePublished); private: std::recursive_mutex mLock; diff --git a/src/ripple/app/ledger/OrderBookDB.cpp b/src/ripple/app/ledger/OrderBookDB.cpp index 45262c4d8a9..faa957203a8 100644 --- a/src/ripple/app/ledger/OrderBookDB.cpp +++ b/src/ripple/app/ledger/OrderBookDB.cpp @@ -250,7 +250,7 @@ void OrderBookDB::processTxn( std::shared_ptr const& ledger, const AcceptedLedgerTx& alTx, - Json::Value const& jvObj) + MultiApiJson const& jvObj) { std::lock_guard sl(mLock); diff --git a/src/ripple/app/ledger/OrderBookDB.h b/src/ripple/app/ledger/OrderBookDB.h index ea7c60c5f5b..b072bafb0c3 100644 --- a/src/ripple/app/ledger/OrderBookDB.h +++ b/src/ripple/app/ledger/OrderBookDB.h @@ -23,6 +23,8 @@ #include #include #include +#include + #include namespace ripple { @@ -63,7 +65,7 @@ class OrderBookDB processTxn( std::shared_ptr const& ledger, const AcceptedLedgerTx& alTx, - Json::Value const& jvObj); + MultiApiJson const& jvObj); private: Application& app_; diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 8234ba16f9e..55123ba2362 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -123,6 +124,7 @@ fillJsonTx( else { copyFrom(txJson, txn->getJson(JsonOptions::none)); + RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion); if (stMeta) { txJson[jss::metaData] = stMeta->getJson(JsonOptions::none); diff --git a/src/ripple/app/misc/DeliverMax.h b/src/ripple/app/misc/DeliverMax.h new file mode 100644 index 00000000000..ddc20bdd7b4 --- /dev/null +++ b/src/ripple/app/misc/DeliverMax.h @@ -0,0 +1,53 @@ +//------------------------------------------------------------------------------ +/* + 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_APP_MISC_DELIVERMAX_H_INCLUDED +#define RIPPLE_APP_MISC_DELIVERMAX_H_INCLUDED + +#include + +#include +#include + +namespace Json { +class Value; +} + +namespace ripple { + +namespace RPC { + +/** + Copy `Amount` field to `DeliverMax` field in transaction output JSON. + This only applies to Payment transaction type, all others are ignored. + + When apiVersion > 1 will also remove `Amount` field, forcing users + to access this value using new `DeliverMax` field only. + @{ + */ + +void +insertDeliverMax(Json::Value& tx_json, TxType txnType, unsigned int apiVersion); + +/** @} */ + +} // namespace RPC +} // namespace ripple + +#endif diff --git a/src/ripple/app/misc/FeeEscalation.md b/src/ripple/app/misc/FeeEscalation.md index 30f4dc2784e..b86f8dab945 100644 --- a/src/ripple/app/misc/FeeEscalation.md +++ b/src/ripple/app/misc/FeeEscalation.md @@ -190,11 +190,27 @@ lower) fee to get into the same position as a reference transaction. ### Consensus Health -For consensus to be considered healthy, the consensus process must take -less than 5 seconds. This time limit was chosen based on observed past -behavior of the network. Note that this is not necessarily the time between -ledger closings, as consensus usually starts some amount of time after -a ledger opens. +For consensus to be considered healthy, the peers on the network +should largely remain in sync with one another. It is particularly +important for the validators to remain in sync, because that is required +for participation in consensus. However, the network tolerates some +validators being out of sync. Fundamentally, network health is a +function of validators reaching consensus on sets of recently submitted +transactions. + +Another factor to consider is +the duration of the consensus process itself. This generally takes +under 5 seconds on the main network under low volume. This is based on +historical observations. However factors such as transaction volume +can increase consensus duration. This is because rippled performs +more work as transaction volume increases. Under sufficient load this +tends to increase consensus duration. It's possible that relatively high +consensus duration indicates a problem, but it is not appropriate to +conclude so without investigation. The upper limit for consensus +duration should be roughly 20 seconds. That is far above the normal. +If the network takes this long to close ledgers, then it is almost +certain that there is a problem with the network. This circumstance +often coincides with new ledgers with zero transactions. ### Other Constants diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 541f4f16fe0..a431b5562d3 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,7 @@ #include #include #include +#include #include #include #include @@ -597,12 +599,13 @@ class NetworkOPsImp final : public NetworkOPs void processClusterTimer(); - Json::Value + MultiApiJson transJson( - const STTx& transaction, + std::shared_ptr const& transaction, TER result, bool validated, - std::shared_ptr const& ledger); + std::shared_ptr const& ledger, + std::optional> meta); void pubValidatedTransaction( @@ -2744,7 +2747,8 @@ NetworkOPsImp::pubProposedTransaction( std::shared_ptr const& transaction, TER result) { - Json::Value jvObj = transJson(*transaction, result, false, ledger); + MultiApiJson jvObj = + transJson(transaction, result, false, ledger, std::nullopt); { std::lock_guard sl(mSubLock); @@ -2756,7 +2760,8 @@ NetworkOPsImp::pubProposedTransaction( if (p) { - p->send(jvObj, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3081,12 +3086,13 @@ NetworkOPsImp::getLocalTxCount() // This routine should only be used to publish accepted or validated // transactions. -Json::Value +MultiApiJson NetworkOPsImp::transJson( - const STTx& transaction, + std::shared_ptr const& transaction, TER result, bool validated, - std::shared_ptr const& ledger) + std::shared_ptr const& ledger, + std::optional> meta) { Json::Value jvObj(Json::objectValue); std::string sToken; @@ -3095,7 +3101,14 @@ NetworkOPsImp::transJson( transResultInfo(result, sToken, sHuman); jvObj[jss::type] = "transaction"; - jvObj[jss::transaction] = transaction.getJson(JsonOptions::none); + jvObj[jss::transaction] = transaction->getJson(JsonOptions::none); + + if (meta) + { + jvObj[jss::meta] = meta->get().getJson(JsonOptions::none); + RPC::insertDeliveredAmount( + jvObj[jss::meta], *ledger, transaction, meta->get()); + } if (validated) { @@ -3118,10 +3131,10 @@ NetworkOPsImp::transJson( jvObj[jss::engine_result_code] = result; jvObj[jss::engine_result_message] = sHuman; - if (transaction.getTxnType() == ttOFFER_CREATE) + if (transaction->getTxnType() == ttOFFER_CREATE) { - auto const account = transaction.getAccountID(sfAccount); - auto const amount = transaction.getFieldAmount(sfTakerGets); + auto const account = transaction->getAccountID(sfAccount); + auto const amount = transaction->getFieldAmount(sfTakerGets); // If the offer create is not self funded then add the owner balance if (account != amount.issue().account) @@ -3136,7 +3149,31 @@ NetworkOPsImp::transJson( } } - return jvObj; + MultiApiJson multiObj({jvObj, jvObj}); + // Minimum supported API version must match index 0 in MultiApiJson + static_assert(apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0); + // Beta API version must match last index in MultiApiJson + static_assert( + apiVersionSelector(RPC::apiBetaVersion)() + 1 // + == MultiApiJson::size); + for (unsigned apiVersion = RPC::apiMinimumSupportedVersion, + lastIndex = MultiApiJson::size; + apiVersion <= RPC::apiBetaVersion; + ++apiVersion) + { + unsigned const index = apiVersionSelector(apiVersion)(); + assert(index < MultiApiJson::size); + if (index != lastIndex) + { + RPC::insertDeliverMax( + multiObj.val[index][jss::transaction], + transaction->getTxnType(), + apiVersion); + lastIndex = index; + } + } + + return multiObj; } void @@ -3147,14 +3184,10 @@ NetworkOPsImp::pubValidatedTransaction( { auto const& stTxn = transaction.getTxn(); - Json::Value jvObj = - transJson(*stTxn, transaction.getResult(), true, ledger); - - { - auto const& meta = transaction.getMeta(); - jvObj[jss::meta] = meta.getJson(JsonOptions::none); - RPC::insertDeliveredAmount(jvObj[jss::meta], *ledger, stTxn, meta); - } + // Create two different Json objects, for different API versions + auto const metaRef = std::ref(transaction.getMeta()); + auto const trResult = transaction.getResult(); + MultiApiJson jvObj = transJson(stTxn, trResult, true, ledger, metaRef); { std::lock_guard sl(mSubLock); @@ -3166,7 +3199,8 @@ NetworkOPsImp::pubValidatedTransaction( if (p) { - p->send(jvObj, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3181,7 +3215,8 @@ NetworkOPsImp::pubValidatedTransaction( if (p) { - p->send(jvObj, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3294,30 +3329,35 @@ NetworkOPsImp::pubAccountTransaction( { auto const& stTxn = transaction.getTxn(); - Json::Value jvObj = - transJson(*stTxn, transaction.getResult(), true, ledger); + // Create two different Json objects, for different API versions + auto const metaRef = std::ref(transaction.getMeta()); + auto const trResult = transaction.getResult(); + MultiApiJson jvObj = transJson(stTxn, trResult, true, ledger, metaRef); + for (InfoSub::ref isrListener : notify) { - auto const& meta = transaction.getMeta(); - - jvObj[jss::meta] = meta.getJson(JsonOptions::none); - RPC::insertDeliveredAmount(jvObj[jss::meta], *ledger, stTxn, meta); + isrListener->send( + jvObj.select(apiVersionSelector(isrListener->getApiVersion())), + true); } - for (InfoSub::ref isrListener : notify) - isrListener->send(jvObj, true); - if (last) - jvObj[jss::account_history_boundary] = true; + jvObj.set(jss::account_history_boundary, true); - assert(!jvObj.isMember(jss::account_history_tx_stream)); + assert( + jvObj.isMember(jss::account_history_tx_stream) == + MultiApiJson::none); for (auto& info : accountHistoryNotify) { auto& index = info.index_; if (index->forwardTxIndex_ == 0 && !index->haveHistorical_) - jvObj[jss::account_history_tx_first] = true; - jvObj[jss::account_history_tx_index] = index->forwardTxIndex_++; - info.sink_->send(jvObj, true); + jvObj.set(jss::account_history_tx_first, true); + + jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_++); + + info.sink_->send( + jvObj.select(apiVersionSelector(info.sink_->getApiVersion())), + true); } } } @@ -3371,19 +3411,26 @@ NetworkOPsImp::pubProposedAccountTransaction( if (!notify.empty() || !accountHistoryNotify.empty()) { - Json::Value jvObj = transJson(*tx, result, false, ledger); + // Create two different Json objects, for different API versions + MultiApiJson jvObj = transJson(tx, result, false, ledger, std::nullopt); for (InfoSub::ref isrListener : notify) - isrListener->send(jvObj, true); + isrListener->send( + jvObj.select(apiVersionSelector(isrListener->getApiVersion())), + true); - assert(!jvObj.isMember(jss::account_history_tx_stream)); + assert( + jvObj.isMember(jss::account_history_tx_stream) == + MultiApiJson::none); for (auto& info : accountHistoryNotify) { auto& index = info.index_; if (index->forwardTxIndex_ == 0 && !index->haveHistorical_) - jvObj[jss::account_history_tx_first] = true; - jvObj[jss::account_history_tx_index] = index->forwardTxIndex_++; - info.sink_->send(jvObj, true); + jvObj.set(jss::account_history_tx_first, true); + jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_++); + info.sink_->send( + jvObj.select(apiVersionSelector(info.sink_->getApiVersion())), + true); } } } @@ -3574,7 +3621,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) auto send = [&](Json::Value const& jvObj, bool unsubscribe) -> bool { - if (auto sptr = subInfo.sinkWptr_.lock(); sptr) + if (auto sptr = subInfo.sinkWptr_.lock()) { sptr->send(jvObj, true); if (unsubscribe) @@ -3585,6 +3632,22 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) return false; }; + auto sendMultiApiJson = [&](MultiApiJson const& jvObj, + bool unsubscribe) -> bool { + if (auto sptr = subInfo.sinkWptr_.lock()) + { + sptr->send( + jvObj.select(apiVersionSelector(sptr->getApiVersion())), + true); + + if (unsubscribe) + unsubAccountHistory(sptr, accountId, false); + return true; + } + + return false; + }; + auto getMoreTxns = [&](std::uint32_t minLedger, std::uint32_t maxLedger, @@ -3743,21 +3806,22 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) send(rpcError(rpcINTERNAL), true); return; } - Json::Value jvTx = transJson( - *stTxn, meta->getResultTER(), true, curTxLedger); - jvTx[jss::meta] = meta->getJson(JsonOptions::none); - jvTx[jss::account_history_tx_index] = txHistoryIndex--; + auto const mRef = std::ref(*meta); + auto const trR = meta->getResultTER(); + MultiApiJson jvTx = + transJson(stTxn, trR, true, curTxLedger, mRef); + + jvTx.set( + jss::account_history_tx_index, txHistoryIndex--); if (i + 1 == num_txns || txns[i + 1].first->getLedger() != tx->getLedger()) - jvTx[jss::account_history_boundary] = true; + jvTx.set(jss::account_history_boundary, true); - RPC::insertDeliveredAmount( - jvTx[jss::meta], *curTxLedger, stTxn, *meta); if (isFirstTx(tx, meta)) { - jvTx[jss::account_history_tx_first] = true; - send(jvTx, false); + jvTx.set(jss::account_history_tx_first, true); + sendMultiApiJson(jvTx, false); JLOG(m_journal.trace()) << "AccountHistory job for account " @@ -3767,7 +3831,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) } else { - send(jvTx, false); + sendMultiApiJson(jvTx, false); } } diff --git a/src/ripple/app/misc/impl/DeliverMax.cpp b/src/ripple/app/misc/impl/DeliverMax.cpp new file mode 100644 index 00000000000..810b750a355 --- /dev/null +++ b/src/ripple/app/misc/impl/DeliverMax.cpp @@ -0,0 +1,42 @@ +//------------------------------------------------------------------------------ +/* + 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. +*/ +//============================================================================== + +#include + +#include + +namespace ripple { +namespace RPC { + +void +insertDeliverMax(Json::Value& tx_json, TxType txnType, unsigned int apiVersion) +{ + if (tx_json.isMember(jss::Amount)) + { + if (txnType == ttPAYMENT) + { + tx_json[jss::DeliverMax] = tx_json[jss::Amount]; + if (apiVersion > 1) + tx_json.removeMember(jss::Amount); + } + } +} + +} // namespace RPC +} // namespace ripple diff --git a/src/ripple/json/MultivarJson.h b/src/ripple/json/MultivarJson.h new file mode 100644 index 00000000000..94d0090edc4 --- /dev/null +++ b/src/ripple/json/MultivarJson.h @@ -0,0 +1,112 @@ +//------------------------------------------------------------------------------ +/* + 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_JSON_MULTIVARJSON_H_INCLUDED +#define RIPPLE_JSON_MULTIVARJSON_H_INCLUDED + +#include + +#include +#include +#include +#include + +namespace ripple { +template +struct MultivarJson +{ + std::array val; + constexpr static std::size_t size = Size; + + Json::Value const& + select(auto&& selector) const + requires std::same_as + { + auto const index = selector(); + assert(index < size); + return val[index]; + } + + void + set(const char* key, + auto const& + v) requires std::constructible_from + { + for (auto& a : this->val) + a[key] = v; + } + + // Intentionally not using class enum here, MultivarJson is scope enough + enum IsMemberResult : int { none = 0, some, all }; + + [[nodiscard]] IsMemberResult + isMember(const char* key) const + { + int count = 0; + for (auto& a : this->val) + if (a.isMember(key)) + count += 1; + + return (count == 0 ? none : (count < size ? some : all)); + } +}; + +// Wrapper for Json for all supported API versions. +using MultiApiJson = MultivarJson<2>; + +/* + +NOTE: + +If a future API version change adds another possible format, change the size of +`MultiApiJson`, and update `apiVersionSelector()` to return the appropriate +selection value for the new `apiVersion` and higher. + +e.g. There are 2 formats now, the first, for version one, the second for +versions > 1. Hypothetically, if API version 4 adds a new format, `MultiApiJson` +would be MultivarJson<3>, and `apiVersionSelector` would return +`static_cast(apiVersion < 2 ? 0u : (apiVersion < 4 ? 1u : 2u))` + +NOTE: + +The more different JSON formats we support, the more CPU cycles we need to +pre-build JSON for different API versions e.g. when publishing streams to +`subscribe` clients. Hence it is desirable to keep MultiApiJson small and +instead fully deprecate and remove support for old API versions. For example, if +we removed support for API version 1 and added a different format for API +version 3, the `apiVersionSelector` would change to +`static_cast(apiVersion > 2)` + +*/ + +// Helper to create appropriate selector for indexing MultiApiJson by apiVersion +constexpr auto +apiVersionSelector(unsigned int apiVersion) noexcept +{ + return [apiVersion]() constexpr + { + // apiVersion <= 1 returns 0 + // apiVersion > 1 returns 1 + return static_cast(apiVersion > 1); + }; +} + +} // namespace ripple + +#endif diff --git a/src/ripple/net/InfoSub.h b/src/ripple/net/InfoSub.h index fb44e23b720..092ba0c0035 100644 --- a/src/ripple/net/InfoSub.h +++ b/src/ripple/net/InfoSub.h @@ -229,6 +229,12 @@ class InfoSub : public CountedObject std::shared_ptr const& getRequest(); + void + setApiVersion(unsigned int apiVersion); + + unsigned int + getApiVersion() const noexcept; + protected: std::mutex mLock; @@ -240,6 +246,7 @@ class InfoSub : public CountedObject std::shared_ptr request_; std::uint64_t mSeq; hash_set accountHistorySubscriptions_; + unsigned int apiVersion_ = 0; static int assign_id() diff --git a/src/ripple/net/impl/InfoSub.cpp b/src/ripple/net/impl/InfoSub.cpp index 9ea5962fa96..5b37cf0759b 100644 --- a/src/ripple/net/impl/InfoSub.cpp +++ b/src/ripple/net/impl/InfoSub.cpp @@ -136,4 +136,17 @@ InfoSub::getRequest() return request_; } +void +InfoSub::setApiVersion(unsigned int apiVersion) +{ + apiVersion_ = apiVersion; +} + +unsigned int +InfoSub::getApiVersion() const noexcept +{ + assert(apiVersion_ > 0); + return apiVersion_; +} + } // namespace ripple diff --git a/src/ripple/perflog/impl/PerfLogImp.cpp b/src/ripple/perflog/impl/PerfLogImp.cpp index db5a188fc3e..3d07d0ed625 100644 --- a/src/ripple/perflog/impl/PerfLogImp.cpp +++ b/src/ripple/perflog/impl/PerfLogImp.cpp @@ -43,7 +43,7 @@ namespace ripple { namespace perf { PerfLogImp::Counters::Counters( - std::vector const& labels, + std::set const& labels, JobTypes const& jobTypes) { { diff --git a/src/ripple/perflog/impl/PerfLogImp.h b/src/ripple/perflog/impl/PerfLogImp.h index 493c1dc1a18..4904126d95f 100644 --- a/src/ripple/perflog/impl/PerfLogImp.h +++ b/src/ripple/perflog/impl/PerfLogImp.h @@ -113,9 +113,7 @@ class PerfLogImp : public PerfLog std::unordered_map methods_; mutable std::mutex methodsMutex_; - Counters( - std::vector const& labels, - JobTypes const& jobTypes); + Counters(std::set const& labels, JobTypes const& jobTypes); Json::Value countersJson() const; Json::Value diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 85659579de6..e6b7b16affa 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-b3" +char const* const versionString = "2.0.0-b4" // clang-format on #if defined(DEBUG) || defined(SANITIZER) diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 4a8f80b8c7b..d4b213bcb1b 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -72,6 +72,7 @@ JSS(ClearFlag); // field. JSS(DID); // ledger type. JSS(DIDDelete); // transaction type. JSS(DIDSet); // transaction type. +JSS(DeliverMax); // out: alias to Amount JSS(DeliverMin); // in: TransactionSign JSS(DepositPreauth); // transaction and ledger type. JSS(Destination); // in: TransactionSign; field. diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index cee8a629c75..bd939a92f1c 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -326,6 +327,9 @@ populateJsonResponse( Json::Value& jvObj = jvTxns.append(Json::objectValue); jvObj[jss::tx] = txn->getJson(JsonOptions::include_date); + auto const& sttx = txn->getSTransaction(); + RPC::insertDeliverMax( + jvObj[jss::tx], sttx->getTxnType(), context.apiVersion); if (txnMeta) { jvObj[jss::meta] = @@ -333,8 +337,7 @@ populateJsonResponse( jvObj[jss::validated] = true; insertDeliveredAmount( jvObj[jss::meta], context, txn, *txnMeta); - insertNFTSyntheticInJson( - jvObj, txn->getSTransaction(), *txnMeta); + insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); } } } diff --git a/src/ripple/rpc/handlers/LedgerHandler.h b/src/ripple/rpc/handlers/LedgerHandler.h index 77b361d3466..b0bca8e6635 100644 --- a/src/ripple/rpc/handlers/LedgerHandler.h +++ b/src/ripple/rpc/handlers/LedgerHandler.h @@ -30,6 +30,7 @@ #include #include #include +#include namespace Json { class Object; @@ -58,23 +59,15 @@ class LedgerHandler void writeResult(Object&); - static char const* - name() - { - return "ledger"; - } + static constexpr char name[] = "ledger"; - static Role - role() - { - return Role::USER; - } + static constexpr unsigned minApiVer = RPC::apiMinimumSupportedVersion; - static Condition - condition() - { - return NO_CONDITION; - } + static constexpr unsigned maxApiVer = RPC::apiMaximumValidVersion; + + static constexpr Role role = Role::USER; + + static constexpr Condition condition = NO_CONDITION; private: JsonContext& context_; diff --git a/src/ripple/rpc/handlers/PathFind.cpp b/src/ripple/rpc/handlers/PathFind.cpp index 9d6e0cff1ac..6c3a27302ac 100644 --- a/src/ripple/rpc/handlers/PathFind.cpp +++ b/src/ripple/rpc/handlers/PathFind.cpp @@ -46,6 +46,8 @@ doPathFind(RPC::JsonContext& context) if (!context.infoSub) return rpcError(rpcNO_EVENTS); + context.infoSub->setApiVersion(context.apiVersion); + auto sSubCommand = context.params[jss::subcommand].asString(); if (sSubCommand == "create") diff --git a/src/ripple/rpc/handlers/Subscribe.cpp b/src/ripple/rpc/handlers/Subscribe.cpp index f17aa62b626..e48cfe5049a 100644 --- a/src/ripple/rpc/handlers/Subscribe.cpp +++ b/src/ripple/rpc/handlers/Subscribe.cpp @@ -110,6 +110,7 @@ doSubscribe(RPC::JsonContext& context) { ispSub = context.infoSub; } + ispSub->setApiVersion(context.apiVersion); if (context.params.isMember(jss::streams)) { diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index 83c4d18a236..677581db6a3 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include #include @@ -71,6 +72,8 @@ doTransactionEntry(RPC::JsonContext& context) else { jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none); + RPC::insertDeliverMax( + jvResult[jss::tx_json], sttx->getTxnType(), context.apiVersion); if (stobj) jvResult[jss::metadata] = stobj->getJson(JsonOptions::none); // 'accounts' diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 9416c441209..92d0e4dd673 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -311,6 +312,10 @@ populateJsonResponse( else if (result.txn) { response = result.txn->getJson(JsonOptions::include_date, args.binary); + auto const& sttx = result.txn->getSTransaction(); + if (!args.binary) + RPC::insertDeliverMax( + response, sttx->getTxnType(), context.apiVersion); // populate binary metadata if (auto blob = std::get_if(&result.meta)) @@ -327,8 +332,7 @@ populateJsonResponse( response[jss::meta] = meta->getJson(JsonOptions::none); insertDeliveredAmount( response[jss::meta], context, result.txn, *meta); - insertNFTSyntheticInJson( - response, result.txn->getSTransaction(), *meta); + insertNFTSyntheticInJson(response, sttx, *meta); } } response[jss::validated] = result.validated; diff --git a/src/ripple/rpc/handlers/TxHistory.cpp b/src/ripple/rpc/handlers/TxHistory.cpp index 4c76bfac026..0f3e353fcbc 100644 --- a/src/ripple/rpc/handlers/TxHistory.cpp +++ b/src/ripple/rpc/handlers/TxHistory.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -63,7 +64,12 @@ doTxHistory(RPC::JsonContext& context) obj["used_postgres"] = true; for (auto const& t : trans) - txs.append(t->getJson(JsonOptions::none)); + { + Json::Value tx_json = t->getJson(JsonOptions::none); + RPC::insertDeliverMax( + tx_json, t->getSTransaction()->getTxnType(), context.apiVersion); + txs.append(tx_json); + } return obj; } diff --git a/src/ripple/rpc/handlers/Version.h b/src/ripple/rpc/handlers/Version.h index a9f42b94993..8f33b62f1cf 100644 --- a/src/ripple/rpc/handlers/Version.h +++ b/src/ripple/rpc/handlers/Version.h @@ -46,23 +46,15 @@ class VersionHandler setVersion(obj, apiVersion_, betaEnabled_); } - static char const* - name() - { - return "version"; - } + static constexpr char const* name = "version"; - static Role - role() - { - return Role::USER; - } + static constexpr unsigned minApiVer = RPC::apiMinimumSupportedVersion; - static Condition - condition() - { - return NO_CONDITION; - } + static constexpr unsigned maxApiVer = RPC::apiMaximumValidVersion; + + static constexpr Role role = Role::USER; + + static constexpr Condition condition = NO_CONDITION; private: unsigned int apiVersion_; diff --git a/src/ripple/rpc/impl/Handler.cpp b/src/ripple/rpc/impl/Handler.cpp index b69d2608b0e..d05c3279800 100644 --- a/src/ripple/rpc/impl/Handler.cpp +++ b/src/ripple/rpc/impl/Handler.cpp @@ -17,11 +17,14 @@ */ //============================================================================== +#include #include #include #include #include +#include + namespace ripple { namespace RPC { namespace { @@ -47,6 +50,9 @@ template Status handle(JsonContext& context, Object& object) { + assert( + context.apiVersion >= HandlerImpl::minApiVer && + context.apiVersion <= HandlerImpl::maxApiVer); HandlerImpl handler(context); auto status = handler.check(); @@ -55,7 +61,20 @@ handle(JsonContext& context, Object& object) else handler.writeResult(object); return status; -}; +} + +template +Handler +handlerFrom() +{ + return { + HandlerImpl::name, + &handle, + HandlerImpl::role, + HandlerImpl::condition, + HandlerImpl::minApiVer, + HandlerImpl::maxApiVer}; +} Handler const handlerArray[]{ // Some handlers not specified here are added to the table via addHandler() @@ -110,7 +129,7 @@ Handler const handlerArray[]{ NEEDS_CURRENT_LEDGER}, {"ledger_data", byRef(&doLedgerData), Role::USER, NO_CONDITION}, {"ledger_entry", byRef(&doLedgerEntry), Role::USER, NO_CONDITION}, - {"ledger_header", byRef(&doLedgerHeader), Role::USER, NO_CONDITION}, + {"ledger_header", byRef(&doLedgerHeader), Role::USER, NO_CONDITION, 1, 1}, {"ledger_request", byRef(&doLedgerRequest), Role::ADMIN, NO_CONDITION}, {"log_level", byRef(&doLogLevel), Role::ADMIN, NO_CONDITION}, {"logrotate", byRef(&doLogRotate), Role::ADMIN, NO_CONDITION}, @@ -156,7 +175,7 @@ Handler const handlerArray[]{ NEEDS_CURRENT_LEDGER}, {"transaction_entry", byRef(&doTransactionEntry), Role::USER, NO_CONDITION}, {"tx", byRef(&doTxJson), Role::USER, NEEDS_NETWORK_CONNECTION}, - {"tx_history", byRef(&doTxHistory), Role::USER, NO_CONDITION}, + {"tx_history", byRef(&doTxHistory), Role::USER, NO_CONDITION, 1, 1}, {"tx_reduce_relay", byRef(&doTxReduceRelay), Role::USER, NO_CONDITION}, {"unl_list", byRef(&doUnlList), Role::ADMIN, NO_CONDITION}, {"validation_create", @@ -178,14 +197,42 @@ Handler const handlerArray[]{ class HandlerTable { private: + using handler_table_t = std::multimap; + + // Use with equal_range to enforce that API range of a newly added handler + // does not overlap with API range of an existing handler with same name + [[nodiscard]] bool + overlappingApiVersion( + std::pair range, + unsigned minVer, + unsigned maxVer) + { + assert(minVer <= maxVer); + assert(maxVer <= RPC::apiMaximumValidVersion); + + return std::any_of( + range.first, + range.second, // + [minVer, maxVer](auto const& item) { + return item.second.minApiVer_ <= maxVer && + item.second.maxApiVer_ >= minVer; + }); + } + template explicit HandlerTable(const Handler (&entries)[N]) { - for (std::size_t i = 0; i < N; ++i) + for (auto const& entry : entries) { - auto const& entry = entries[i]; - assert(table_.find(entry.name_) == table_.end()); - table_[entry.name_] = entry; + if (overlappingApiVersion( + table_.equal_range(entry.name_), + entry.minApiVer_, + entry.maxApiVer_)) + LogicError( + std::string("Handler for ") + entry.name_ + + " overlaps with an existing handler"); + + table_.insert({entry.name_, entry}); } // This is where the new-style handlers are added. @@ -201,7 +248,7 @@ class HandlerTable return handlerTable; } - Handler const* + [[nodiscard]] Handler const* getHandler(unsigned version, bool betaEnabled, std::string const& name) const { @@ -209,36 +256,48 @@ class HandlerTable version > (betaEnabled ? RPC::apiBetaVersion : RPC::apiMaximumSupportedVersion)) return nullptr; - auto i = table_.find(name); - return i == table_.end() ? nullptr : &i->second; + + auto const range = table_.equal_range(name); + auto const i = std::find_if( + range.first, range.second, [version](auto const& entry) { + return entry.second.minApiVer_ <= version && + version <= entry.second.maxApiVer_; + }); + + return i == range.second ? nullptr : &i->second; } - std::vector + [[nodiscard]] std::set getHandlerNames() const { - std::vector ret; - ret.reserve(table_.size()); + std::set ret; for (auto const& i : table_) - ret.push_back(i.second.name_); + ret.insert(i.second.name_); + return ret; } private: - std::map table_; + handler_table_t table_; template void addHandler() { - assert(table_.find(HandlerImpl::name()) == table_.end()); + static_assert(HandlerImpl::minApiVer <= HandlerImpl::maxApiVer); + static_assert(HandlerImpl::maxApiVer <= RPC::apiMaximumValidVersion); + static_assert( + RPC::apiMinimumSupportedVersion <= HandlerImpl::minApiVer); - Handler h; - h.name_ = HandlerImpl::name(); - h.valueMethod_ = &handle; - h.role_ = HandlerImpl::role(); - h.condition_ = HandlerImpl::condition(); + if (overlappingApiVersion( + table_.equal_range(HandlerImpl::name), + HandlerImpl::minApiVer, + HandlerImpl::maxApiVer)) + LogicError( + std::string("Handler for ") + HandlerImpl::name + + " overlaps with an existing handler"); - table_[HandlerImpl::name()] = h; + table_.insert({HandlerImpl::name, handlerFrom()}); } }; @@ -250,11 +309,11 @@ getHandler(unsigned version, bool betaEnabled, std::string const& name) return HandlerTable::instance().getHandler(version, betaEnabled, name); } -std::vector +std::set getHandlerNames() { return HandlerTable::instance().getHandlerNames(); -}; +} } // namespace RPC } // namespace ripple diff --git a/src/ripple/rpc/impl/Handler.h b/src/ripple/rpc/impl/Handler.h index e2188ef51e7..28d1ee60c85 100644 --- a/src/ripple/rpc/impl/Handler.h +++ b/src/ripple/rpc/impl/Handler.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -52,6 +53,9 @@ struct Handler Method valueMethod_; Role role_; RPC::Condition condition_; + + unsigned minApiVer_ = apiMinimumSupportedVersion; + unsigned maxApiVer_ = apiMaximumValidVersion; }; Handler const* @@ -70,7 +74,7 @@ makeObjectValue( } /** Return names of all methods. */ -std::vector +std::set getHandlerNames(); template diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index eb02e6ea37a..516f66fc620 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -242,10 +242,12 @@ constexpr unsigned int apiVersionIfUnspecified = 1; constexpr unsigned int apiMinimumSupportedVersion = 1; constexpr unsigned int apiMaximumSupportedVersion = 1; constexpr unsigned int apiBetaVersion = 2; +constexpr unsigned int apiMaximumValidVersion = apiBetaVersion; static_assert(apiMinimumSupportedVersion >= apiVersionIfUnspecified); static_assert(apiMaximumSupportedVersion >= apiMinimumSupportedVersion); static_assert(apiBetaVersion >= apiMaximumSupportedVersion); +static_assert(apiMaximumValidVersion >= apiMaximumSupportedVersion); template void diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 5cd0c9edee3..5dbfa49aef9 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -167,6 +167,22 @@ checkPayment( if (tx_json[jss::TransactionType].asString() != jss::Payment) return Json::Value(); + // DeliverMax is an alias to Amount and we use Amount internally + if (tx_json.isMember(jss::DeliverMax)) + { + if (tx_json.isMember(jss::Amount)) + { + if (tx_json[jss::DeliverMax] != tx_json[jss::Amount]) + return RPC::make_error( + rpcINVALID_PARAMS, + "Cannot specify differing 'Amount' and 'DeliverMax'"); + } + else + tx_json[jss::Amount] = tx_json[jss::DeliverMax]; + + tx_json.removeMember(jss::DeliverMax); + } + if (!tx_json.isMember(jss::Amount)) return RPC::missing_field_error("tx_json.Amount"); diff --git a/src/test/basics/PerfLog_test.cpp b/src/test/basics/PerfLog_test.cpp index 79944e0ed71..f0a6645195b 100644 --- a/src/test/basics/PerfLog_test.cpp +++ b/src/test/basics/PerfLog_test.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -309,7 +310,8 @@ class PerfLog_test : public beast::unit_test::suite // Get the all the labels we can use for RPC interfaces without // causing an assert. - std::vector labels{ripple::RPC::getHandlerNames()}; + std::vector labels = + test::jtx::make_vector(ripple::RPC::getHandlerNames()); std::shuffle(labels.begin(), labels.end(), default_prng()); // Get two IDs to associate with each label. Errors tend to happen at diff --git a/src/test/json/MultivarJson_test.cpp b/src/test/json/MultivarJson_test.cpp new file mode 100644 index 00000000000..8cb3a49aff2 --- /dev/null +++ b/src/test/json/MultivarJson_test.cpp @@ -0,0 +1,293 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/XRPLF/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. +*/ +//============================================================================== + +#include +#include + +#include +#include "ripple/beast/unit_test/suite.hpp" +#include "ripple/json/json_value.h" +#include +#include +#include +#include + +namespace ripple { +namespace test { + +struct MultivarJson_test : beast::unit_test::suite +{ + void + run() override + { + constexpr static Json::StaticString string1("string1"); + static Json::Value const str1{string1}; + + static Json::Value const obj1{[]() { + Json::Value obj1(Json::objectValue); + obj1["one"] = 1; + return obj1; + }()}; + + static Json::Value const jsonNull{}; + + MultivarJson<3> const subject({str1, obj1}); + static_assert(sizeof(subject) == sizeof(subject.val)); + static_assert(subject.size == subject.val.size()); + static_assert( + std::is_same_v>); + + BEAST_EXPECT(subject.val.size() == 3); + BEAST_EXPECT( + (subject.val == std::array{str1, obj1, jsonNull})); + BEAST_EXPECT( + (MultivarJson<3>({obj1, str1}).val == + std::array{obj1, str1, jsonNull})); + BEAST_EXPECT( + (MultivarJson<3>({jsonNull, obj1, str1}).val == + std::array{jsonNull, obj1, str1})); + + { + testcase("default copy construction / assignment"); + + MultivarJson<3> x{subject}; + + BEAST_EXPECT(x.val.size() == subject.val.size()); + BEAST_EXPECT(x.val[0] == subject.val[0]); + BEAST_EXPECT(x.val[1] == subject.val[1]); + BEAST_EXPECT(x.val[2] == subject.val[2]); + BEAST_EXPECT(x.val == subject.val); + BEAST_EXPECT(&x.val[0] != &subject.val[0]); + BEAST_EXPECT(&x.val[1] != &subject.val[1]); + BEAST_EXPECT(&x.val[2] != &subject.val[2]); + + MultivarJson<3> y; + BEAST_EXPECT((y.val == std::array{})); + y = subject; + BEAST_EXPECT(y.val == subject.val); + BEAST_EXPECT(&y.val[0] != &subject.val[0]); + BEAST_EXPECT(&y.val[1] != &subject.val[1]); + BEAST_EXPECT(&y.val[2] != &subject.val[2]); + + y = std::move(x); + BEAST_EXPECT(y.val == subject.val); + BEAST_EXPECT(&y.val[0] != &subject.val[0]); + BEAST_EXPECT(&y.val[1] != &subject.val[1]); + BEAST_EXPECT(&y.val[2] != &subject.val[2]); + } + + { + testcase("select"); + + BEAST_EXPECT( + subject.select([]() -> std::size_t { return 0; }) == str1); + BEAST_EXPECT( + subject.select([]() -> std::size_t { return 1; }) == obj1); + BEAST_EXPECT( + subject.select([]() -> std::size_t { return 2; }) == jsonNull); + + // Tests of requires clause - these are expected to match + static_assert([](auto&& v) { + return requires + { + v.select([]() -> std::size_t { return 0; }); + }; + }(subject)); + static_assert([](auto&& v) { + return requires + { + v.select([]() constexpr->std::size_t { return 0; }); + }; + }(subject)); + static_assert([](auto&& v) { + return requires + { + v.select([]() mutable -> std::size_t { return 0; }); + }; + }(subject)); + + // Tests of requires clause - these are expected NOT to match + static_assert([](auto&& a) { + return !requires + { + subject.select([]() -> int { return 0; }); + }; + }(subject)); + static_assert([](auto&& v) { + return !requires + { + v.select([]() -> void {}); + }; + }(subject)); + static_assert([](auto&& v) { + return !requires + { + v.select([]() -> bool { return false; }); + }; + }(subject)); + } + + { + struct foo_t final + { + }; + testcase("set"); + + auto x = MultivarJson<2>{{Json::objectValue, Json::objectValue}}; + x.set("name1", 42); + BEAST_EXPECT(x.val[0].isMember("name1")); + BEAST_EXPECT(x.val[1].isMember("name1")); + BEAST_EXPECT(x.val[0]["name1"].isInt()); + BEAST_EXPECT(x.val[1]["name1"].isInt()); + BEAST_EXPECT(x.val[0]["name1"].asInt() == 42); + BEAST_EXPECT(x.val[1]["name1"].asInt() == 42); + + x.set("name2", "bar"); + BEAST_EXPECT(x.val[0].isMember("name2")); + BEAST_EXPECT(x.val[1].isMember("name2")); + BEAST_EXPECT(x.val[0]["name2"].isString()); + BEAST_EXPECT(x.val[1]["name2"].isString()); + BEAST_EXPECT(x.val[0]["name2"].asString() == "bar"); + BEAST_EXPECT(x.val[1]["name2"].asString() == "bar"); + + // Tests of requires clause - these are expected to match + static_assert([](auto&& v) { + return requires + { + v.set("name", Json::nullValue); + }; + }(x)); + static_assert([](auto&& v) { + return requires + { + v.set("name", "value"); + }; + }(x)); + static_assert([](auto&& v) { + return requires + { + v.set("name", true); + }; + }(x)); + static_assert([](auto&& v) { + return requires + { + v.set("name", 42); + }; + }(x)); + + // Tests of requires clause - these are expected NOT to match + static_assert([](auto&& v) { + return !requires + { + v.set("name", foo_t{}); + }; + }(x)); + static_assert([](auto&& v) { + return !requires + { + v.set("name", std::nullopt); + }; + }(x)); + } + + { + testcase("isMember"); + + // Well defined behaviour even if we have different types of members + BEAST_EXPECT(subject.isMember("foo") == decltype(subject)::none); + + auto const makeJson = [](const char* key, int val) { + Json::Value obj1(Json::objectValue); + obj1[key] = val; + return obj1; + }; + + { + // All variants have element "One", none have element "Two" + MultivarJson<2> const s1{ + {makeJson("One", 12), makeJson("One", 42)}}; + BEAST_EXPECT(s1.isMember("One") == decltype(s1)::all); + BEAST_EXPECT(s1.isMember("Two") == decltype(s1)::none); + } + + { + // Some variants have element "One" and some have "Two" + MultivarJson<2> const s2{ + {makeJson("One", 12), makeJson("Two", 42)}}; + BEAST_EXPECT(s2.isMember("One") == decltype(s2)::some); + BEAST_EXPECT(s2.isMember("Two") == decltype(s2)::some); + } + + { + // Not all variants have element "One", because last one is null + MultivarJson<3> const s3{ + {makeJson("One", 12), makeJson("One", 42), {}}}; + BEAST_EXPECT(s3.isMember("One") == decltype(s3)::some); + BEAST_EXPECT(s3.isMember("Two") == decltype(s3)::none); + } + } + + { + // NOTE It's fine to change this test when we change API versions + testcase("apiVersionSelector"); + + static_assert(MultiApiJson::size == 2); + static MultiApiJson x{{obj1, str1}}; + + static_assert( + std::is_same_v); + static_assert([](auto&& v) { + return requires + { + v.select(apiVersionSelector(1)); + }; + }(x)); + + BEAST_EXPECT(x.select(apiVersionSelector(0)) == obj1); + BEAST_EXPECT(x.select(apiVersionSelector(2)) == str1); + + static_assert(apiVersionSelector(0)() == 0); + static_assert(apiVersionSelector(1)() == 0); + static_assert(apiVersionSelector(2)() == 1); + static_assert(apiVersionSelector(3)() == 1); + static_assert( + apiVersionSelector( + std::numeric_limits::max())() == 1); + } + + { + // There should be no reson to change this test + testcase("apiVersionSelector invariants"); + + static_assert( + apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0); + static_assert( + apiVersionSelector(RPC::apiBetaVersion)() + 1 // + == MultiApiJson::size); + + BEAST_EXPECT(MultiApiJson::size >= 1); + } + } +}; + +BEAST_DEFINE_TESTSUITE(MultivarJson, ripple_basics, ripple); + +} // namespace test +} // namespace ripple diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index ff681ffa50b..2bee47a6411 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -27,10 +27,19 @@ #include #include +#include + namespace ripple { namespace test { namespace jtx { +// Helper to make vector from iterable +auto +make_vector(auto const& input) requires std::ranges::range +{ + return std::vector(std::ranges::begin(input), std::ranges::end(input)); +} + // Functions used in debugging Json::Value getAccountOffers(Env& env, AccountID const& acct, bool current = false); diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 24f6737917b..8c583ee1254 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -119,14 +119,23 @@ class AccountTx_test : public beast::unit_test::suite // Ledger 3 has the two txs associated with funding the account // All other ledgers have no txs - auto hasTxs = [](Json::Value const& j) { + auto hasTxs = [apiVersion](Json::Value const& j) { return j.isMember(jss::result) && (j[jss::result][jss::status] == "success") && (j[jss::result][jss::transactions].size() == 2) && (j[jss::result][jss::transactions][0u][jss::tx] [jss::TransactionType] == jss::AccountSet) && (j[jss::result][jss::transactions][1u][jss::tx] - [jss::TransactionType] == jss::Payment); + [jss::TransactionType] == jss::Payment) && + (j[jss::result][jss::transactions][1u][jss::tx] + [jss::DeliverMax] == "10000000010") && + ((apiVersion > 1 && + !j[jss::result][jss::transactions][1u][jss::tx].isMember( + jss::Amount)) || + (apiVersion <= 1 && + j[jss::result][jss::transactions][1u][jss::tx][jss::Amount] == + j[jss::result][jss::transactions][1u][jss::tx] + [jss::DeliverMax])); }; auto noTxs = [](Json::Value const& j) { diff --git a/src/test/rpc/Handler_test.cpp b/src/test/rpc/Handler_test.cpp new file mode 100644 index 00000000000..5160a68aac2 --- /dev/null +++ b/src/test/rpc/Handler_test.cpp @@ -0,0 +1,132 @@ +//------------------------------------------------------------------------------ +/* + 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. +*/ +//============================================================================== + +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace ripple::test { + +// NOTE: there should be no need for this function; +// `std::cout << some_duration` should just work if built with a compliant +// C++20 compiler. Sadly, we are not using one, as of today +// TODO: remove this operator<< overload when we bump compiler version +std::ostream& +operator<<(std::ostream& os, std::chrono::nanoseconds ns) +{ + return (os << ns.count() << "ns"); +} + +// NOTE This is a rather naive effort at a microbenchmark. Ideally we want +// Google Benchmark, or something similar. Also, this actually does not belong +// to unit tests, as it makes little sense to run it in conditions very +// dissimilar to how rippled will normally work. +// TODO as https://github.com/XRPLF/rippled/issues/4765 + +class Handler_test : public beast::unit_test::suite +{ + auto + time(std::size_t n, auto f, auto prng) -> auto + { + using clock = std::chrono::steady_clock; + assert(n > 0); + double sum = 0; + double sum_squared = 0; + std::size_t j = 0; + while (j < n) + { + // Generate 100 inputs upfront, separated from the inner loop + std::array inputs = {}; + for (auto& i : inputs) + { + i = prng(); + } + + // Take 100 samples, then sort and throw away 35 from each end, + // using only middle 30. This helps to reduce measurement noise. + std::array samples = {}; + for (std::size_t k = 0; k < 100; ++k) + { + auto start = std::chrono::steady_clock::now(); + f(inputs[k]); + samples[k] = (std::chrono::steady_clock::now() - start).count(); + } + + std::sort(samples.begin(), samples.end()); + for (std::size_t k = 35; k < 65; ++k) + { + j += 1; + sum += samples[k]; + sum_squared += (samples[k] * samples[k]); + } + } + + const double mean_squared = (sum * sum) / (j * j); + return std::make_tuple( + clock::duration{static_cast(sum / j)}, + clock::duration{ + static_cast(std::sqrt((sum_squared / j) - mean_squared))}, + j); + } + + void + reportLookupPerformance() + { + testcase("Handler lookup performance"); + + std::random_device dev; + std::ranlux48 prng(dev()); + + std::vector names = + test::jtx::make_vector(ripple::RPC::getHandlerNames()); + + std::uniform_int_distribution distr{0, names.size() - 1}; + + std::size_t dummy = 0; + auto const [mean, stdev, n] = time( + 1'000'000, + [&](std::size_t i) { + auto const d = RPC::getHandler(1, false, names[i]); + dummy = dummy + i + (int)d->role_; + }, + [&]() -> std::size_t { return distr(prng); }); + + std::cout << "mean=" << mean << " stdev=" << stdev << " N=" << n + << '\n'; + + BEAST_EXPECT(dummy != 0); + } + +public: + void + run() override + { + reportLookupPerformance(); + } +}; + +BEAST_DEFINE_TESTSUITE_MANUAL(Handler, test, ripple); + +} // namespace ripple::test diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index b6e54967c40..5d4c09ef8d1 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -1035,6 +1035,26 @@ static constexpr TxnTestData txnTestArray[] = { "Missing field 'tx_json.Destination'.", "Missing field 'tx_json.Destination'."}}}, + {"Missing 'Destination' in sign_for, use DeliverMax", + __LINE__, + R"({ + "command": "doesnt_matter", + "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "secret": "masterpassphrase", + "tx_json": { + "Account": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", + "DeliverMax": "1000000000", + "Fee": 50, + "Sequence": 0, + "SigningPubKey": "", + "TransactionType": "Payment" + } +})", + {{"Missing field 'tx_json.Destination'.", + "Missing field 'tx_json.Destination'.", + "Missing field 'tx_json.Destination'.", + "Missing field 'tx_json.Destination'."}}}, + {"Missing 'Fee' in sign_for.", __LINE__, R"({ @@ -1692,6 +1712,34 @@ static constexpr TxnTestData txnTestArray[] = { "Missing field 'account'.", "Invalid field 'tx_json.Amount'."}}}, + {"Invalid DeliverMax in submit_multisigned Payment.", + __LINE__, + R"({ + "command": "submit_multisigned", + "tx_json": { + "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "DeliverMax": "NotANumber", + "Destination": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", + "Fee": 50, + "Sequence": 0, + "Signers": [ + { + "Signer": { + "Account": "rPcNzota6B8YBokhYtcTNqQVCngtbnWfux", + "TxnSignature": "3045022100F9ED357606932697A4FAB2BE7F222C21DD93CA4CFDD90357AADD07465E8457D6022038173193E3DFFFB5D78DD738CC0905395F885DA65B98FDB9793901FE3FD26ECE", + "SigningPubKey": "02FE36A690D6973D55F88553F5D2C4202DE75F2CF8A6D0E17C70AC223F044501F8" + } + } + ], + "SigningPubKey": "", + "TransactionType": "Payment" + } +})", + {{"Missing field 'secret'.", + "Missing field 'secret'.", + "Missing field 'account'.", + "Invalid field 'tx_json.Amount'."}}}, + {"No build_path in submit_multisigned.", __LINE__, R"({ @@ -1905,6 +1953,72 @@ static constexpr TxnTestData txnTestArray[] = { "A Signer may not be the transaction's Account " "(rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh)."}}}, + {"Empty Signers array in submit_multisigned, use DeliverMax", + __LINE__, + R"({ + "command": "submit_multisigned", + "tx_json": { + "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "DeliverMax": "10000000", + "Destination": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", + "Fee": 50, + "Sequence": 0, + "Signers": [ + ], + "SigningPubKey": "", + "TransactionType": "Payment" + } +})", + {{"Missing field 'secret'.", + "Missing field 'secret'.", + "Missing field 'account'.", + "tx_json.Signers array may not be empty."}}}, + + {"Empty Signers array in submit_multisigned, use DeliverMax and Amount", + __LINE__, + R"({ + "command": "submit_multisigned", + "tx_json": { + "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "Amount": "10000000", + "DeliverMax": "10000000", + "Destination": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", + "Fee": 50, + "Sequence": 0, + "Signers": [ + ], + "SigningPubKey": "", + "TransactionType": "Payment" + } +})", + {{"Missing field 'secret'.", + "Missing field 'secret'.", + "Missing field 'account'.", + "tx_json.Signers array may not be empty."}}}, + + {"Payment cannot specify different DeliverMax and Amount.", + __LINE__, + R"({ + "command": "doesnt_matter", + "account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "secret": "masterpassphrase", + "debug_signing": 0, + "tx_json": { + "Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "Amount": "1000000000", + "DeliverMax": "1000000020", + "Destination": "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA", + "Fee": 50, + "Sequence": 0, + "SigningPubKey": "", + "TransactionType": "Payment" + } +})", + {{"Cannot specify differing 'Amount' and 'DeliverMax'", + "Cannot specify differing 'Amount' and 'DeliverMax'", + "Cannot specify differing 'Amount' and 'DeliverMax'", + "Cannot specify differing 'Amount' and 'DeliverMax'"}}}, + }; class JSONRPC_test : public beast::unit_test::suite diff --git a/src/test/rpc/LedgerHeader_test.cpp b/src/test/rpc/LedgerHeader_test.cpp new file mode 100644 index 00000000000..d6c0652d5a2 --- /dev/null +++ b/src/test/rpc/LedgerHeader_test.cpp @@ -0,0 +1,91 @@ +//------------------------------------------------------------------------------ +/* + 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. +*/ +//============================================================================== + +#include +#include +#include + +namespace ripple { + +class LedgerHeader_test : public beast::unit_test::suite +{ + void + testSimpleCurrent() + { + testcase("Current ledger"); + using namespace test::jtx; + Env env{*this, envconfig(no_admin)}; + + Json::Value params{Json::objectValue}; + params[jss::api_version] = 1; + params[jss::ledger_index] = "current"; + auto const result = + env.client().invoke("ledger_header", params)[jss::result]; + BEAST_EXPECT(result[jss::status] == "success"); + BEAST_EXPECT(result.isMember("ledger")); + BEAST_EXPECT(result[jss::ledger][jss::closed] == false); + BEAST_EXPECT(result[jss::validated] == false); + } + + void + testSimpleValidated() + { + testcase("Validated ledger"); + using namespace test::jtx; + Env env{*this, envconfig(no_admin)}; + + Json::Value params{Json::objectValue}; + params[jss::api_version] = 1; + params[jss::ledger_index] = "validated"; + auto const result = + env.client().invoke("ledger_header", params)[jss::result]; + BEAST_EXPECT(result[jss::status] == "success"); + BEAST_EXPECT(result.isMember("ledger")); + BEAST_EXPECT(result[jss::ledger][jss::closed] == true); + BEAST_EXPECT(result[jss::validated] == true); + } + + void + testCommandRetired() + { + testcase("Command retired from API v2"); + using namespace test::jtx; + Env env{*this, envconfig(no_admin)}; + + Json::Value params{Json::objectValue}; + params[jss::api_version] = 2; + auto const result = + env.client().invoke("ledger_header", params)[jss::result]; + BEAST_EXPECT(result[jss::error] == "unknownCmd"); + BEAST_EXPECT(result[jss::status] == "error"); + } + +public: + void + run() override + { + testSimpleCurrent(); + testSimpleValidated(); + testCommandRetired(); + } +}; + +BEAST_DEFINE_TESTSUITE(LedgerHeader, rpc, ripple); + +} // namespace ripple diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index 5322319907f..7725390f6b6 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -194,8 +194,16 @@ class Subscribe_test : public beast::unit_test::suite // Check stream update for payment transaction BEAST_EXPECT(wsc->findMsg(5s, [&](auto const& jv) { return jv[jss::meta]["AffectedNodes"][1u]["CreatedNode"] - ["NewFields"][jss::Account] == - Account("alice").human(); + ["NewFields"][jss::Account] // + == Account("alice").human() && + jv[jss::transaction][jss::TransactionType] // + == jss::Payment && + jv[jss::transaction][jss::DeliverMax] // + == "10000000010" && + jv[jss::transaction][jss::Fee] // + == "10" && + jv[jss::transaction][jss::Sequence] // + == 1; })); // Check stream update for accountset transaction @@ -211,7 +219,16 @@ class Subscribe_test : public beast::unit_test::suite // Check stream update for payment transaction BEAST_EXPECT(wsc->findMsg(5s, [&](auto const& jv) { return jv[jss::meta]["AffectedNodes"][1u]["CreatedNode"] - ["NewFields"][jss::Account] == Account("bob").human(); + ["NewFields"][jss::Account] // + == Account("bob").human() && + jv[jss::transaction][jss::TransactionType] // + == jss::Payment && + jv[jss::transaction][jss::DeliverMax] // + == "10000000010" && + jv[jss::transaction][jss::Fee] // + == "10" && + jv[jss::transaction][jss::Sequence] // + == 2; })); // Check stream update for accountset transaction diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index a477a624859..60225f4621d 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -17,6 +17,8 @@ */ //============================================================================== +#include +#include #include #include #include @@ -150,7 +152,7 @@ class TransactionEntry_test : public beast::unit_test::suite auto check_tx = [this, &env]( int index, std::string const txhash, - std::string const type = "") { + std::string const expected_json = "") { // first request using ledger_index to lookup Json::Value const resIndex{[&env, index, &txhash]() { Json::Value params{Json::objectValue}; @@ -164,13 +166,30 @@ class TransactionEntry_test : public beast::unit_test::suite return; BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); - if (!type.empty()) + if (!expected_json.empty()) { - BEAST_EXPECTS( - resIndex[jss::tx_json][jss::TransactionType] == type, - txhash + " is " + - resIndex[jss::tx_json][jss::TransactionType] - .asString()); + Json::Value expected; + Json::Reader().parse(expected_json, expected); + if (RPC::contains_error(expected)) + Throw( + "Internal JSONRPC_test error. Bad test JSON."); + + for (auto memberIt = expected.begin(); + memberIt != expected.end(); + memberIt++) + { + auto const name = memberIt.memberName(); + if (BEAST_EXPECT(resIndex[jss::tx_json].isMember(name))) + { + auto const received = resIndex[jss::tx_json][name]; + BEAST_EXPECTS( + received == *memberIt, + txhash + " contains \n\"" + name + "\": " // + + to_string(received) // + + " but expected " // + + to_string(expected)); + } + } } // second request using ledger_hash to lookup and verify @@ -216,8 +235,30 @@ class TransactionEntry_test : public beast::unit_test::suite // these are actually AccountSet txs because fund does two txs and // env.tx only reports the last one - check_tx(env.closed()->seq(), fund_1_tx); - check_tx(env.closed()->seq(), fund_2_tx); + check_tx(env.closed()->seq(), fund_1_tx, R"( +{ + "Account" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Sequence" : 3, + "SetFlag" : 8, + "SigningPubKey" : "0324CAAFA2212D2AEAB9D42D481535614AED486293E1FB1380FF070C3DD7FB4264", + "TransactionType" : "AccountSet", + "TxnSignature" : "3044022007B35E3B99460534FF6BC3A66FBBA03591C355CC38E38588968E87CCD01BE229022071A443026DE45041B55ABB1CC76812A87EA701E475BBB7E165513B4B242D3474", + "hash" : "F4E9DF90D829A9E8B423FF68C34413E240D8D8BB0EFD080DF08114ED398E2506" +} +)"); + check_tx(env.closed()->seq(), fund_2_tx, R"( +{ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "Fee" : "10", + "Sequence" : 3, + "SetFlag" : 8, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TransactionType" : "AccountSet", + "TxnSignature" : "3045022100C8857FC0759A2AC0D2F320684691A66EAD252EAED9EF88C79791BC58BFCC9D860220421722286487DD0ED6BBA626CE6FCBDD14289F7F4726870C3465A4054C2702D7", + "hash" : "6853CD8226A05068C951CB1F54889FF4E40C5B440DC1C5BA38F114C4E0B1E705" +} +)"); env.trust(A2["USD"](1000), A1); // the trust tx is actually a payment since the trust method @@ -231,16 +272,70 @@ class TransactionEntry_test : public beast::unit_test::suite boost::lexical_cast(env.tx()->getTransactionID()); env.close(); - check_tx(env.closed()->seq(), trust_tx); - check_tx(env.closed()->seq(), pay_tx, jss::Payment.c_str()); + check_tx(env.closed()->seq(), trust_tx, R"( +{ + "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "DeliverMax" : "10", + "Destination" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Flags" : 2147483648, + "Sequence" : 3, + "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020", + "TransactionType" : "Payment", + "TxnSignature" : "3044022033D9EBF7F02950AF2F6B13C07AEE641C8FEBDD540A338FCB9027A965A4AED35B02206E4E227DCC226A3456C0FEF953449D21645A24EB63CA0BB7C5B62470147FD1D1", + "hash" : "C992D97D88FF444A1AB0C06B27557EC54B7F7DA28254778E60238BEA88E0C101" +} +)"); + + check_tx( + env.closed()->seq(), + pay_tx, + R"( +{ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "DeliverMax" : + { + "currency" : "USD", + "issuer" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "value" : "5" + }, + "Destination" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Flags" : 2147483648, + "Sequence" : 4, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TransactionType" : "Payment", + "TxnSignature" : "30450221008A722B7F16EDB2348886E88ED4EC682AE9973CC1EE0FF37C93BB2CEC821D3EDF022059E464472031BA5E0D88A93E944B6A8B8DB3E1D5E5D1399A805F615789DB0BED", + "hash" : "988046D484ACE9F5F6A8C792D89C6EA2DB307B5DDA9864AEBA88E6782ABD0865" +} +)"); env(offer(A2, XRP(100), A2["USD"](1))); auto offer_tx = boost::lexical_cast(env.tx()->getTransactionID()); env.close(); - - check_tx(env.closed()->seq(), offer_tx, jss::OfferCreate.c_str()); + check_tx( + env.closed()->seq(), + offer_tx, + R"( +{ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "Fee" : "10", + "Sequence" : 5, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TakerGets" : + { + "currency" : "USD", + "issuer" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "value" : "1" + }, + "TakerPays" : "100000000", + "TransactionType" : "OfferCreate", + "TxnSignature" : "304502210093FC93ACB77B4E3DE3315441BD010096734859080C1797AB735EB47EBD541BD102205020BB1A7C3B4141279EE4C287C13671E2450EA78914EFD0C6DB2A18344CD4F2", + "hash" : "5FCC1A27A7664F82A0CC4BE5766FBBB7C560D52B93AA7B550CD33B27AEC7EFFB" +} +)"); } public: diff --git a/src/test/rpc/TransactionHistory_test.cpp b/src/test/rpc/TransactionHistory_test.cpp index 3f9b5792744..862eaaee507 100644 --- a/src/test/rpc/TransactionHistory_test.cpp +++ b/src/test/rpc/TransactionHistory_test.cpp @@ -54,6 +54,21 @@ class TransactionHistory_test : public beast::unit_test::suite } } + void + testCommandRetired() + { + testcase("Command retired from API v2"); + using namespace test::jtx; + Env env{*this, envconfig(no_admin)}; + + Json::Value params{Json::objectValue}; + params[jss::api_version] = 2; + auto const result = + env.client().invoke("tx_history", params)[jss::result]; + BEAST_EXPECT(result[jss::error] == "unknownCmd"); + BEAST_EXPECT(result[jss::status] == "error"); + } + void testRequest() { @@ -148,6 +163,7 @@ class TransactionHistory_test : public beast::unit_test::suite { testBadInput(); testRequest(); + testCommandRetired(); } }; diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index cc3f70a1516..c16d7bbd004 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -692,6 +693,60 @@ class Transaction_test : public beast::unit_test::suite } } + void + testRequest(FeatureBitset features) + { + testcase("Test Request"); + + using namespace test::jtx; + using std::to_string; + + const char* COMMAND = jss::tx.c_str(); + + Env env{*this}; + Account const alice{"alice"}; + Account const alie{"alie"}; + Account const gw{"gw"}; + auto const USD{gw["USD"]}; + + env.fund(XRP(1000000), alice, gw); + env.close(); + + // AccountSet + env(noop(alice)); + + // Payment + env(pay(alice, gw, XRP(100))); + + std::shared_ptr txn = env.tx(); + env.close(); + std::shared_ptr meta = + env.closed()->txRead(env.tx()->getTransactionID()).second; + + Json::Value expected = txn->getJson(JsonOptions::none); + expected[jss::DeliverMax] = expected[jss::Amount]; + + auto const result = + env.rpc(COMMAND, to_string(txn->getTransactionID())); + BEAST_EXPECT(result[jss::result][jss::status] == jss::success); + + for (auto memberIt = expected.begin(); memberIt != expected.end(); + memberIt++) + { + std::string const name = memberIt.memberName(); + if (BEAST_EXPECT(result[jss::result].isMember(name))) + { + auto const received = result[jss::result][name]; + BEAST_EXPECTS( + received == *memberIt, + "Transaction contains \n\"" + name + "\": " // + + to_string(received) // + + " but expected " // + + to_string(expected)); + } + } + } + public: void run() override @@ -708,6 +763,7 @@ class Transaction_test : public beast::unit_test::suite testRangeCTIDRequest(features); testCTIDValidation(features); testCTIDRPC(features); + testRequest(features); } };