From cc0cc5ee7bed09a2db50035d0711f0a02ada59bb Mon Sep 17 00:00:00 2001 From: Bronek Kozicki <823856+Bronek@users.noreply.github.com> Date: Thu, 28 Sep 2023 20:21:17 +0100 Subject: [PATCH] Factor out MultivarJson for transaction streams --- src/ripple/app/ledger/BookListeners.cpp | 9 +- src/ripple/app/ledger/BookListeners.h | 4 +- src/ripple/app/ledger/OrderBookDB.cpp | 5 +- src/ripple/app/ledger/OrderBookDB.h | 5 +- src/ripple/app/misc/NetworkOPs.cpp | 188 ++++++++++-------------- src/ripple/basics/MultivarJson.h | 88 +++++++++++ 6 files changed, 176 insertions(+), 123 deletions(-) create mode 100644 src/ripple/basics/MultivarJson.h diff --git a/src/ripple/app/ledger/BookListeners.cpp b/src/ripple/app/ledger/BookListeners.cpp index 379e2e32adb..bbc0058bc76 100644 --- a/src/ripple/app/ledger/BookListeners.cpp +++ b/src/ripple/app/ledger/BookListeners.cpp @@ -39,8 +39,7 @@ BookListeners::removeSubscriber(std::uint64_t seq) void BookListeners::publish( - Json::Value const& jvObj, - Json::Value const& jvObjApiVer2, + MultiApiJson const& jvObj, hash_set& havePublished) { std::lock_guard sl(mLock); @@ -55,10 +54,8 @@ BookListeners::publish( // Only publish jvObj if this is the first occurence if (havePublished.emplace(p->getSeq()).second) { - if (p->getApiVersion() == 1) - p->send(jvObj, true); - else - p->send(jvObjApiVer2, 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 6e0fed48c07..080e15716a6 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, Json::Value const& jvObjApiVer2, 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 163321f8ed9..faa957203a8 100644 --- a/src/ripple/app/ledger/OrderBookDB.cpp +++ b/src/ripple/app/ledger/OrderBookDB.cpp @@ -250,8 +250,7 @@ void OrderBookDB::processTxn( std::shared_ptr const& ledger, const AcceptedLedgerTx& alTx, - Json::Value const& jvObj, - Json::Value const& jvObjApiVer2) + MultiApiJson const& jvObj) { std::lock_guard sl(mLock); @@ -278,7 +277,7 @@ OrderBookDB::processTxn( {data->getFieldAmount(sfTakerGets).issue(), data->getFieldAmount(sfTakerPays).issue()}); if (listeners) - listeners->publish(jvObj, jvObjApiVer2, havePublished); + listeners->publish(jvObj, havePublished); } }; diff --git a/src/ripple/app/ledger/OrderBookDB.h b/src/ripple/app/ledger/OrderBookDB.h index 8fd5945072b..9b41109e5c6 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,8 +65,7 @@ class OrderBookDB processTxn( std::shared_ptr const& ledger, const AcceptedLedgerTx& alTx, - Json::Value const& jvObj, - Json::Value const& jvObjApiVer2); + MultiApiJson const& jvObj); private: Application& app_; diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index b70b2bd00e9..25021858019 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -2745,12 +2746,12 @@ NetworkOPsImp::pubProposedTransaction( std::shared_ptr const& transaction, TER result) { - Json::Value jvObj = transJson(*transaction, result, false, ledger); - Json::Value jvObjApi2 = transJson(*transaction, result, false, ledger); + Json::Value jvSeed = transJson(*transaction, result, false, ledger); + MultiApiJson jvObj = MultiApiJson::fill(jvSeed); RPC::insertDeliverMax( - jvObj[jss::transaction], transaction->getTxnType(), 1); + jvObj[0][jss::transaction], transaction->getTxnType(), 1); RPC::insertDeliverMax( - jvObjApi2[1][jss::transaction], transaction->getTxnType(), 2); + jvObj[1][jss::transaction], transaction->getTxnType(), 2); { std::lock_guard sl(mSubLock); @@ -2762,10 +2763,8 @@ NetworkOPsImp::pubProposedTransaction( if (p) { - if (p->getApiVersion() == 1) - p->send(jvObj, true); - else - p->send(jvObjApi2, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3156,23 +3155,20 @@ NetworkOPsImp::pubValidatedTransaction( { auto const& stTxn = transaction.getTxn(); - Json::Value jvObj = - transJson(*stTxn, transaction.getResult(), true, ledger); - Json::Value jvObjApi2 = + Json::Value jvSeed = 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); - RPC::insertDeliverMax(jvObj[jss::transaction], stTxn->getTxnType(), 1); - - jvObjApi2[jss::meta] = meta.getJson(JsonOptions::none); - RPC::insertDeliveredAmount(jvObjApi2[jss::meta], *ledger, stTxn, meta); - RPC::insertDeliverMax( - jvObjApi2[jss::transaction], stTxn->getTxnType(), 2); + jvSeed[jss::meta] = meta.getJson(JsonOptions::none); + RPC::insertDeliveredAmount(jvSeed[jss::meta], *ledger, stTxn, meta); } + // Create two different Json objects, for different API versions + MultiApiJson jvObj = MultiApiJson::fill(jvSeed); + RPC::insertDeliverMax(jvObj[0][jss::transaction], stTxn->getTxnType(), 1); + RPC::insertDeliverMax(jvObj[1][jss::transaction], stTxn->getTxnType(), 2); + { std::lock_guard sl(mSubLock); @@ -3183,10 +3179,8 @@ NetworkOPsImp::pubValidatedTransaction( if (p) { - if (p->getApiVersion() == 1) - p->send(jvObj, true); - else - p->send(jvObjApi2, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3201,10 +3195,8 @@ NetworkOPsImp::pubValidatedTransaction( if (p) { - if (p->getApiVersion() == 1) - p->send(jvObj, true); - else - p->send(jvObjApi2, true); + p->send( + jvObj.select(apiVersionSelector(p->getApiVersion())), true); ++it; } else @@ -3213,7 +3205,7 @@ NetworkOPsImp::pubValidatedTransaction( } if (transaction.getResult() == tesSUCCESS) - app_.getOrderBookDB().processTxn(ledger, transaction, jvObj, jvObjApi2); + app_.getOrderBookDB().processTxn(ledger, transaction, jvObj); pubAccountTransaction(ledger, transaction, last); } @@ -3317,57 +3309,46 @@ NetworkOPsImp::pubAccountTransaction( { auto const& stTxn = transaction.getTxn(); - Json::Value jvObj = - transJson(*stTxn, transaction.getResult(), true, ledger); - Json::Value jvObjApi2 = + Json::Value jvSeed = 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); - RPC::insertDeliverMax( - jvObj[jss::transaction], stTxn->getTxnType(), 1); - - jvObjApi2[jss::meta] = meta.getJson(JsonOptions::none); - RPC::insertDeliveredAmount( - jvObjApi2[jss::meta], *ledger, stTxn, meta); - RPC::insertDeliverMax( - jvObjApi2[jss::transaction], stTxn->getTxnType(), 2); + jvSeed[jss::meta] = meta.getJson(JsonOptions::none); + RPC::insertDeliveredAmount(jvSeed[jss::meta], *ledger, stTxn, meta); } + // Create two different Json objects, for different API versions + MultiApiJson jvObj = MultiApiJson::fill(jvSeed); + RPC::insertDeliverMax( + jvObj[0][jss::transaction], stTxn->getTxnType(), 1); + RPC::insertDeliverMax( + jvObj[1][jss::transaction], stTxn->getTxnType(), 2); + for (InfoSub::ref isrListener : notify) { - if (isrListener->getApiVersion() == 1) - isrListener->send(jvObj, true); - else - isrListener->send(jvObjApi2, true); + isrListener->send( + jvObj.select(apiVersionSelector(isrListener->getApiVersion())), + true); } if (last) - { - jvObj[jss::account_history_boundary] = true; - jvObjApi2[jss::account_history_boundary] = true; - } + jvObj.set(jss::account_history_boundary, true); - assert(!jvObj.isMember(jss::account_history_tx_stream)); + assert(!jvObj[0].isMember(jss::account_history_tx_stream)); for (auto& info : accountHistoryNotify) { auto& index = info.index_; if (index->forwardTxIndex_ == 0 && !index->haveHistorical_) - { - jvObj[jss::account_history_tx_first] = true; - jvObjApi2[jss::account_history_tx_first] = true; - } - jvObj[jss::account_history_tx_index] = index->forwardTxIndex_; - jvObjApi2[jss::account_history_tx_index] = index->forwardTxIndex_; + jvObj.set(jss::account_history_tx_first, true); + + jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_); index->forwardTxIndex_ += 1; - if (info.sink_->getApiVersion() == 1) - info.sink_->send(jvObj, true); - else - info.sink_->send(jvObjApi2, true); + info.sink_->send( + jvObj.select(apiVersionSelector(info.sink_->getApiVersion())), + true); } } } @@ -3421,35 +3402,28 @@ NetworkOPsImp::pubProposedAccountTransaction( if (!notify.empty() || !accountHistoryNotify.empty()) { - Json::Value jvObj = transJson(*tx, result, false, ledger); - Json::Value jvObjApi2 = transJson(*tx, result, false, ledger); - RPC::insertDeliverMax(jvObj[jss::transaction], tx->getTxnType(), 1); - RPC::insertDeliverMax(jvObjApi2[jss::transaction], tx->getTxnType(), 2); + Json::Value jvSeed = transJson(*tx, result, false, ledger); + + // Create two different Json objects, for different API versions + MultiApiJson jvObj = MultiApiJson::fill(jvSeed); + RPC::insertDeliverMax(jvObj[0][jss::transaction], tx->getTxnType(), 1); + RPC::insertDeliverMax(jvObj[1][jss::transaction], tx->getTxnType(), 2); for (InfoSub::ref isrListener : notify) - { - if (isrListener->getApiVersion() == 1) - isrListener->send(jvObj, true); - else - isrListener->send(jvObjApi2, true); - } + isrListener->send( + jvObj.select(apiVersionSelector(isrListener->getApiVersion())), + true); assert(!jvObj.isMember(jss::account_history_tx_stream)); for (auto& info : accountHistoryNotify) { auto& index = info.index_; if (index->forwardTxIndex_ == 0 && !index->haveHistorical_) - { - jvObj[jss::account_history_tx_first] = true; - jvObjApi2[jss::account_history_tx_first] = true; - } - jvObj[jss::account_history_tx_index] = index->forwardTxIndex_; - jvObjApi2[jss::account_history_tx_index] = index->forwardTxIndex_; + jvObj.set(jss::account_history_tx_first, true); + jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_); index->forwardTxIndex_ += 1; - - if (info.sink_->getApiVersion() == 1) - info.sink_->send(jvObj, true); - else - info.sink_->send(jvObjApi2, true); + info.sink_->send( + jvObj.select(apiVersionSelector(info.sink_->getApiVersion())), + true); } } } @@ -3651,15 +3625,13 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) return false; }; - auto send2 = [&](Json::Value const& jvObj, - Json::Value const& jvObjApi2, - bool unsubscribe) -> bool { + auto sendMultiApiJson = [&](MultiApiJson const& jvObj, + bool unsubscribe) -> bool { if (auto sptr = subInfo.sinkWptr_.lock(); sptr) { - if (sptr->getApiVersion() == 1) - sptr->send(jvObj, true); - else - sptr->send(jvObjApi2, true); + sptr->send( + jvObj.select(apiVersionSelector(sptr->getApiVersion())), + true); if (unsubscribe) unsubAccountHistory(sptr, accountId, false); @@ -3828,40 +3800,34 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) return; } - Json::Value jvTx = transJson( + Json::Value jvSeed = transJson( *stTxn, meta->getResultTER(), true, curTxLedger); - jvTx[jss::meta] = meta->getJson(JsonOptions::none); - jvTx[jss::account_history_tx_index] = txHistoryIndex; - - Json::Value jvTxApi2 = transJson( - *stTxn, meta->getResultTER(), true, curTxLedger); - jvTxApi2[jss::meta] = meta->getJson(JsonOptions::none); - jvTxApi2[jss::account_history_tx_index] = - txHistoryIndex; + jvSeed[jss::meta] = meta->getJson(JsonOptions::none); + jvSeed[jss::account_history_tx_index] = txHistoryIndex; txHistoryIndex -= 1; if (i + 1 == num_txns || txns[i + 1].first->getLedger() != tx->getLedger()) - { - jvTx[jss::account_history_boundary] = true; - jvTxApi2[jss::account_history_boundary] = true; - } + jvSeed[jss::account_history_boundary] = true; RPC::insertDeliveredAmount( - jvTx[jss::meta], *curTxLedger, stTxn, *meta); - RPC::insertDeliverMax( - jvTx[jss::transaction], stTxn->getTxnType(), 1); + jvSeed[jss::meta], *curTxLedger, stTxn, *meta); - RPC::insertDeliveredAmount( - jvTxApi2[jss::meta], *curTxLedger, stTxn, *meta); + // Create two Json objects, for different API versions + MultiApiJson jvTx = MultiApiJson::fill(jvSeed); + RPC::insertDeliverMax( + jvTx.val[0][jss::transaction], + stTxn->getTxnType(), + 1); RPC::insertDeliverMax( - jvTxApi2[jss::transaction], stTxn->getTxnType(), 2); + jvTx.val[1][jss::transaction], + stTxn->getTxnType(), + 2); if (isFirstTx(tx, meta)) { - jvTx[jss::account_history_tx_first] = true; - jvTxApi2[jss::account_history_tx_first] = true; - send2(jvTx, jvTxApi2, false); + jvTx.set(jss::account_history_tx_first, true); + sendMultiApiJson(jvTx, false); JLOG(m_journal.trace()) << "AccountHistory job for account " @@ -3871,7 +3837,7 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo) } else { - send2(jvTx, jvTxApi2, false); + sendMultiApiJson(jvTx, false); } } diff --git a/src/ripple/basics/MultivarJson.h b/src/ripple/basics/MultivarJson.h new file mode 100644 index 00000000000..f328e2bc78f --- /dev/null +++ b/src/ripple/basics/MultivarJson.h @@ -0,0 +1,88 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2023 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_BASICS_MULTIVARJSON_H_INCLUDED +#define RIPPLE_BASICS_MULTIVARJSON_H_INCLUDED + +#include + +#include +#include +#include + +namespace ripple { +template +struct MultivarJson +{ + std::array val; + + Json::Value& + operator[](std::size_t i) + { + return val[i]; + } + + Json::Value const& + operator[](std::size_t i) const + { + return val[i]; + } + + Json::Value const& + select(auto selector) const + { + static_assert(std::is_same_v); + return val[selector()]; + } + + static MultivarJson + fill(Json::Value const& v) + { + MultivarJson ret; + for (auto& a : ret.val) + a = v; + return ret; + } + + void + set(const char* key, auto const& v) + { + for (auto& a : this->val) + a[key] = v; + } +}; + +// Wrapper for Json for all supported API versions. +using MultiApiJson = MultivarJson<2>; + +// Helper to create appropriate selector for indexing MultiApiJson by apiVersion +constexpr auto +apiVersionSelector(int apiVersion) noexcept +{ + return [apiVersion]() constexpr + { + // apiVersion <= 1 returns 0 + // apiVersion > 1 returns 1 + return static_cast(apiVersion > 1); + }; +} + +} // namespace ripple + +#endif