Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write proper forAllApiVersions used in NetworkOPs.cpp #4833

Merged
merged 25 commits into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
20a0580
Simplify setting ledger_index
Bronek Nov 30, 2023
bf3d84f
Refactor MultivarJson, implement forApiVersions
Bronek Nov 22, 2023
18ddd69
Add test for ApiVersions
Bronek Nov 29, 2023
661830c
Add constraint to visit() with tests
Bronek Nov 30, 2023
c5f9d73
Add Json-only overloads to MultivarJson visitor
Bronek Dec 1, 2023
91d3209
Allow binding rvalue MultivarJson to visitor
Bronek Dec 1, 2023
e09201d
Add RPC::apiVersion
Bronek Dec 1, 2023
c05401b
Fix comment
Bronek Dec 1, 2023
1e91f79
Add if constexpr for version dispatch
Bronek Jan 10, 2024
54e0abe
Improve MultivarJson::visitor_t overloading
Bronek Jan 10, 2024
2675f11
Removed empty requires declaration clause
Bronek Jan 18, 2024
f47a462
Merge branch 'develop' into feature/for_all_versions
Bronek Jan 23, 2024
12e266f
Merge branch 'develop' into feature/for_all_versions
Bronek Jan 31, 2024
bd9770a
Merge branch 'develop' into feature/for_all_versions
Bronek Jan 31, 2024
392f9b3
Minor improvements
Bronek Feb 1, 2024
8231028
Amend API version invariant
Bronek Feb 1, 2024
25e4627
Minor formatting
Bronek Feb 1, 2024
8f1b369
Move MultivarJson to protocol
Bronek Feb 1, 2024
e1c89d0
Split MultivarJson Policy into component parameters
Bronek Feb 1, 2024
f967972
Rename MultivarJson to MultiApiJson
Bronek Feb 2, 2024
0afad05
Rename unit tests to MultiApiJson. Add tests
Bronek Feb 2, 2024
3e4f549
Merge branch 'develop' into feature/for_all_versions
Bronek Mar 13, 2024
ab22ace
Merge branch 'develop' into feature/for_all_versions
Bronek Mar 14, 2024
712dc19
Merge branch 'develop' into feature/for_all_versions
Bronek Mar 21, 2024
6c78c73
Merge branch 'develop' into feature/for_all_versions
seelabs Mar 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ install (
src/ripple/protocol/AccountID.h
src/ripple/protocol/AMMCore.h
src/ripple/protocol/AmountConversions.h
src/ripple/protocol/ApiVersion.h
src/ripple/protocol/Book.h
src/ripple/protocol/BuildInfo.h
src/ripple/protocol/ErrorCodes.h
Expand Down Expand Up @@ -1015,6 +1016,7 @@ if (tests)
test sources:
subdir: protocol
#]===============================]
src/test/protocol/ApiVersion_test.cpp
src/test/protocol/BuildInfo_test.cpp
src/test/protocol/InnerObjectFormats_test.cpp
src/test/protocol/Issue_test.cpp
Expand Down
1 change: 0 additions & 1 deletion Builds/levelization/results/ordering.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ test.csf > ripple.json
test.csf > ripple.protocol
test.json > ripple.beast
test.json > ripple.json
test.json > ripple.rpc
test.json > test.jtx
test.jtx > ripple.app
test.jtx > ripple.basics
Expand Down
5 changes: 3 additions & 2 deletions src/ripple/app/ledger/BookListeners.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ BookListeners::publish(
// Only publish jvObj if this is the first occurence
if (havePublished.emplace(p->getSeq()).second)
{
p->send(
jvObj.select(apiVersionSelector(p->getApiVersion())), true);
jvObj.visit(
p->getApiVersion(), //
[&](Json::Value const& jv) { p->send(jv, true); });
}
++it;
}
Expand Down
1 change: 1 addition & 0 deletions src/ripple/app/ledger/BookListeners.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include <ripple/json/MultivarJson.h>
#include <ripple/net/InfoSub.h>
#include <ripple/protocol/ApiVersion.h>

#include <memory>
#include <mutex>
Expand Down
1 change: 1 addition & 0 deletions src/ripple/app/ledger/OrderBookDB.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <ripple/app/ledger/BookListeners.h>
#include <ripple/app/main/Application.h>
#include <ripple/json/MultivarJson.h>
#include <ripple/protocol/ApiVersion.h>

#include <mutex>

Expand Down
4 changes: 1 addition & 3 deletions src/ripple/app/ledger/impl/LedgerToJson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,7 @@ fillJsonTx(
if (validated)
{
auto const seq = fill.ledger.seq();
txJson[jss::ledger_index] = (fill.context->apiVersion > 1)
? Json::Value(seq)
: Json::Value(std::to_string(seq));
txJson[jss::ledger_index] = seq;
if (fill.closeTime)
txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime);
}
Expand Down
72 changes: 38 additions & 34 deletions src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include <ripple/overlay/Cluster.h>
#include <ripple/overlay/Overlay.h>
#include <ripple/overlay/predicates.h>
#include <ripple/protocol/ApiVersion.h>
#include <ripple/protocol/BuildInfo.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/STParsedJSON.h>
Expand All @@ -68,7 +69,6 @@
#include <ripple/rpc/BookChanges.h>
#include <ripple/rpc/DeliveredAmount.h>
#include <ripple/rpc/ServerHandler.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <boost/asio/ip/host_name.hpp>
#include <boost/asio/steady_timer.hpp>

Expand Down Expand Up @@ -2213,11 +2213,11 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
// NOTE Use MultiApiJson to publish two slightly different JSON objects
// for consumers supporting different API versions
MultiApiJson multiObj{jvObj};
visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
multiObj, //
[](Json::Value& jvTx, unsigned int apiVersion) {
multiObj.visit(
RPC::apiVersion<1>, //
[](Json::Value& jvTx) {
// Type conversion for older API versions to string
if (jvTx.isMember(jss::ledger_index) && apiVersion < 2)
if (jvTx.isMember(jss::ledger_index))
{
jvTx[jss::ledger_index] =
std::to_string(jvTx[jss::ledger_index].asUInt());
Expand All @@ -2229,9 +2229,9 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
{
if (auto p = i->second.lock())
{
p->send(
multiObj.select(apiVersionSelector(p->getApiVersion())),
true);
multiObj.visit(
p->getApiVersion(), //
[&](Json::Value const& jv) { p->send(jv, true); });
++i;
}
else
Expand Down Expand Up @@ -2768,8 +2768,9 @@ NetworkOPsImp::pubProposedTransaction(

if (p)
{
p->send(
jvObj.select(apiVersionSelector(p->getApiVersion())), true);
jvObj.visit(
p->getApiVersion(), //
[&](Json::Value const& jv) { p->send(jv, true); });
++it;
}
else
Expand Down Expand Up @@ -3166,13 +3167,14 @@ NetworkOPsImp::transJson(

std::string const hash = to_string(transaction->getTransactionID());
MultiApiJson multiObj{jvObj};
visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
multiObj, //
[&](Json::Value& jvTx, unsigned int apiVersion) {
forAllApiVersions(
multiObj.visit(), //
[&](Json::Value& jvTx, auto v) {
constexpr unsigned version = decltype(v)::value;
RPC::insertDeliverMax(
jvTx[jss::transaction], transaction->getTxnType(), apiVersion);
jvTx[jss::transaction], transaction->getTxnType(), version);

if (apiVersion > 1)
if constexpr (version > 1)
thejohnfreeman marked this conversation as resolved.
Show resolved Hide resolved
{
jvTx[jss::tx_json] = jvTx.removeMember(jss::transaction);
jvTx[jss::hash] = hash;
Expand Down Expand Up @@ -3209,8 +3211,9 @@ NetworkOPsImp::pubValidatedTransaction(

if (p)
{
p->send(
jvObj.select(apiVersionSelector(p->getApiVersion())), true);
jvObj.visit(
p->getApiVersion(), //
[&](Json::Value const& jv) { p->send(jv, true); });
++it;
}
else
Expand All @@ -3225,8 +3228,9 @@ NetworkOPsImp::pubValidatedTransaction(

if (p)
{
p->send(
jvObj.select(apiVersionSelector(p->getApiVersion())), true);
jvObj.visit(
p->getApiVersion(), //
[&](Json::Value const& jv) { p->send(jv, true); });
++it;
}
else
Expand Down Expand Up @@ -3346,9 +3350,9 @@ NetworkOPsImp::pubAccountTransaction(

for (InfoSub::ref isrListener : notify)
{
isrListener->send(
jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
true);
jvObj.visit(
isrListener->getApiVersion(), //
[&](Json::Value const& jv) { isrListener->send(jv, true); });
}

if (last)
Expand All @@ -3365,9 +3369,9 @@ NetworkOPsImp::pubAccountTransaction(

jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_++);

info.sink_->send(
jvObj.select(apiVersionSelector(info.sink_->getApiVersion())),
true);
jvObj.visit(
info.sink_->getApiVersion(), //
[&](Json::Value const& jv) { info.sink_->send(jv, true); });
}
}
}
Expand Down Expand Up @@ -3425,9 +3429,9 @@ NetworkOPsImp::pubProposedAccountTransaction(
MultiApiJson jvObj = transJson(tx, result, false, ledger, std::nullopt);

for (InfoSub::ref isrListener : notify)
isrListener->send(
jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
true);
jvObj.visit(
isrListener->getApiVersion(), //
[&](Json::Value const& jv) { isrListener->send(jv, true); });

assert(
jvObj.isMember(jss::account_history_tx_stream) ==
Expand All @@ -3438,9 +3442,9 @@ NetworkOPsImp::pubProposedAccountTransaction(
if (index->forwardTxIndex_ == 0 && !index->haveHistorical_)
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);
jvObj.visit(
info.sink_->getApiVersion(), //
[&](Json::Value const& jv) { info.sink_->send(jv, true); });
}
}
}
Expand Down Expand Up @@ -3646,9 +3650,9 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo)
bool unsubscribe) -> bool {
if (auto sptr = subInfo.sinkWptr_.lock())
{
sptr->send(
jvObj.select(apiVersionSelector(sptr->getApiVersion())),
true);
jvObj.visit(
sptr->getApiVersion(), //
[&](Json::Value const& jv) { sptr->send(jv, true); });

if (unsubscribe)
unsubAccountHistory(sptr, accountId, false);
Expand Down
Loading
Loading