diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index a3c06399cf5..968a03c0784 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -158,6 +158,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..4758d0791e8 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 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/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/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/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/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/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/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/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/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/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); } };