From 53e384fdcf82e988620f04739c1c40c75d1c5756 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Fri, 3 Nov 2023 13:37:29 +0000 Subject: [PATCH] Minor improvements --- src/ripple/app/ledger/impl/LedgerMaster.cpp | 5 +- src/ripple/app/ledger/impl/LedgerToJson.cpp | 92 ++++++++++----------- src/ripple/app/misc/NetworkOPs.cpp | 2 +- src/ripple/protocol/STBase.h | 12 ++- src/ripple/protocol/STTx.h | 1 - src/ripple/protocol/impl/STTx.cpp | 34 ++++---- 6 files changed, 71 insertions(+), 75 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 0e20da46305..857c0efcc28 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -621,7 +621,7 @@ LedgerMaster::isValidated(ReadView const& ledger) // Use the skip list in the last validated ledger to see if ledger // comes before the last validated ledger (and thus has been // validated). - auto hash = walkHashBySeq(seq, InboundLedger::Reason::GENERIC); + auto const hash = walkHashBySeq(seq, InboundLedger::Reason::GENERIC); if (!hash || ledger.info().hash != *hash) { @@ -642,8 +642,7 @@ LedgerMaster::isValidated(ReadView const& ledger) } catch (SHAMapMissingNode const& mn) { - auto stream = app_.journal("IsValidated").warn(); // TODO Better name ? - JLOG(stream) << "Ledger #" << seq << ": " << mn.what(); + JLOG(m_journal.warn()) << "Ledger #" << seq << ": " << mn.what(); return false; } diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 2ba452446b1..d22cc7cd487 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -129,60 +129,56 @@ fillJsonTx( if (stMeta) txJson[json_meta] = serializeHex(*stMeta); } - else + else if (fill.context->apiVersion > 1) { - if (fill.context->apiVersion > 1) - { - copyFrom( - txJson[jss::tx_json], - txn->getJson(JsonOptions::disable_API_prior_V2, false)); - txJson[jss::hash] = to_string(txn->getTransactionID()); - RPC::insertDeliverMax( - txJson[jss::tx_json], txnType, fill.context->apiVersion); + copyFrom( + txJson[jss::tx_json], + txn->getJson(JsonOptions::disable_API_prior_V2, false)); + txJson[jss::hash] = to_string(txn->getTransactionID()); + RPC::insertDeliverMax( + txJson[jss::tx_json], txnType, fill.context->apiVersion); - if (stMeta) - { - txJson[jss::meta] = stMeta->getJson(JsonOptions::none); - - // If applicable, insert delivered amount - if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) - RPC::insertDeliveredAmount( - txJson[jss::meta], - fill.ledger, - txn, - {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); - } + if (stMeta) + { + txJson[jss::meta] = stMeta->getJson(JsonOptions::none); + + // If applicable, insert delivered amount + if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) + RPC::insertDeliveredAmount( + txJson[jss::meta], + fill.ledger, + txn, + {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); + } - if (!fill.ledger.open()) - txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash); + if (!fill.ledger.open()) + txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash); - const bool validated = - fill.context->ledgerMaster.isValidated(fill.ledger); - txJson[jss::validated] = validated; - if (validated) - { - txJson[jss::ledger_index] = to_string(fill.ledger.seq()); - if (fill.closeTime) - txJson[jss::close_time_iso] = - to_string_iso(*fill.closeTime); - } + const bool validated = + fill.context->ledgerMaster.isValidated(fill.ledger); + txJson[jss::validated] = validated; + if (validated) + { + txJson[jss::ledger_index] = to_string(fill.ledger.seq()); + if (fill.closeTime) + txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime); } - else + } + else + { + copyFrom(txJson, txn->getJson(JsonOptions::none)); + RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion); + if (stMeta) { - copyFrom(txJson, txn->getJson(JsonOptions::none)); - RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion); - if (stMeta) - { - txJson[jss::metaData] = stMeta->getJson(JsonOptions::none); - - // If applicable, insert delivered amount - if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) - RPC::insertDeliveredAmount( - txJson[jss::metaData], - fill.ledger, - txn, - {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); - } + txJson[jss::metaData] = stMeta->getJson(JsonOptions::none); + + // If applicable, insert delivered amount + if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) + RPC::insertDeliveredAmount( + txJson[jss::metaData], + fill.ledger, + txn, + {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } } diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index b35a56b5ff9..2a4968481bc 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3106,7 +3106,6 @@ NetworkOPsImp::transJson( // NOTE jvObj which is not a finished object for either API version. After // it's populated, we need to finish it for a specific API version. This is // done in a loop, near the end of this function. - std::string const hash = to_string(transaction->getTransactionID()); jvObj[jss::transaction] = transaction->getJson(JsonOptions::disable_API_prior_V2, false); @@ -3159,6 +3158,7 @@ NetworkOPsImp::transJson( } } + std::string const hash = to_string(transaction->getTransactionID()); MultiApiJson multiObj({jvObj, jvObj}); // Minimum supported API version must match index 0 in MultiApiJson static_assert(apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0); diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h index ceb7fe3e7bc..ec8c34a9ddd 100644 --- a/src/ripple/protocol/STBase.h +++ b/src/ripple/protocol/STBase.h @@ -38,10 +38,14 @@ struct JsonOptions underlying_t value; enum values : underlying_t { - none = 0u, - include_date = 1u, - disable_API_prior_V2 = 2u, - _all = 3u // NOTE see `operator~` and bump as needed when adding enums + // clang-format off + none = 0b0000'0000, + include_date = 0b0000'0001, + disable_API_prior_V2 = 0b0000'0010, + + // IMPORTANT `_all` must be union of all of the above; see also operator~ + _all = 0b0000'0011 + // clang-format on }; constexpr JsonOptions(underlying_t v) noexcept : value(v) diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index 84524760d6d..e166eb20dd4 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 9ee93a05b30..8106e997f3a 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -237,31 +237,29 @@ Json::Value STTx::getJson(JsonOptions) const Json::Value STTx::getJson(JsonOptions options, bool binary) const { - if (!(options & JsonOptions::disable_API_prior_V2)) + bool const V1 = !(options & JsonOptions::disable_API_prior_V2); + + if (binary) { - // Behaviour before API version 2 - if (binary) + Serializer s = STObject::getSerializer(); + std::string const dataBin = strHex(s.peekData()); + + if (V1) { - Json::Value ret; - Serializer s = STObject::getSerializer(); - ret[jss::tx] = strHex(s.peekData()); + Json::Value ret(Json::objectValue); + ret[jss::tx] = dataBin; ret[jss::hash] = to_string(getTransactionID()); return ret; } - return getJson(options); + else + return Json::Value{dataBin}; } - // API version 2 behaviour - if (binary) - { - Serializer s = STObject::getSerializer(); - Json::Value ret = strHex(s.peekData()); - return ret; - } - // We want virtualy the same output as `getJson(JsonOptions)` overload - // above, but without the hash element. Since `getJson(JsonOptions)` - // is using STObject::getJson(JsonOptions::none), we use it here as well - return STObject::getJson(JsonOptions::none); + Json::Value ret = STObject::getJson(JsonOptions::none); + if (V1) + ret[jss::hash] = to_string(getTransactionID()); + + return ret; } std::string const&