From 65346dc54d5326786b3ff055a14eadef2104fb81 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 25 Oct 2023 18:07:18 +0000 Subject: [PATCH 01/34] Remove include --- src/test/jtx/TestHelpers.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index 2bee47a6411..7b4cae51554 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -27,17 +27,25 @@ #include #include -#include +#include namespace ripple { namespace test { namespace jtx { // Helper to make vector from iterable +template +concept iterable = requires(T& v) { + std::begin(v); + std::end(v); + }; + +template auto -make_vector(auto const& input) requires std::ranges::range +make_vector(Input const& input) + requires iterable { - return std::vector(std::ranges::begin(input), std::ranges::end(input)); + return std::vector(std::begin(input), std::end(input)); } // Functions used in debugging From ee53c5ddff12b94b67c41361838ba93002da02a3 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 25 Oct 2023 18:17:10 +0000 Subject: [PATCH 02/34] Formatting fix --- src/test/jtx/TestHelpers.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/jtx/TestHelpers.h b/src/test/jtx/TestHelpers.h index 7b4cae51554..61d0eb87f9f 100644 --- a/src/test/jtx/TestHelpers.h +++ b/src/test/jtx/TestHelpers.h @@ -35,15 +35,15 @@ namespace jtx { // Helper to make vector from iterable template -concept iterable = requires(T& v) { - std::begin(v); - std::end(v); - }; +concept iterable = requires(T& v) +{ + std::begin(v); + std::end(v); +}; template auto -make_vector(Input const& input) - requires iterable +make_vector(Input const& input) requires iterable { return std::vector(std::begin(input), std::end(input)); } From aa6de60cf20a6f7cebe3b9aa9b0adb611219e298 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 23 Oct 2023 18:20:21 +0000 Subject: [PATCH 03/34] Output for subscriptions --- src/ripple/app/misc/NetworkOPs.cpp | 25 +++++++++++++++++++++---- src/ripple/protocol/STTx.h | 10 +++++++++- src/ripple/protocol/impl/STTx.cpp | 26 +++++++++++++++++++++----- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index a431b5562d3..27dba963422 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -63,6 +63,7 @@ #include #include #include +#include #include #include #include @@ -74,6 +75,7 @@ #include #include +#include #include #include #include @@ -3101,7 +3103,12 @@ NetworkOPsImp::transJson( transResultInfo(result, sToken, sHuman); jvObj[jss::type] = "transaction"; - jvObj[jss::transaction] = transaction->getJson(JsonOptions::none); + // 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 hash = {}; + jvObj[jss::transaction] = + transaction->getJson(JsonOptions::none, false, {std::ref(hash)}); if (meta) { @@ -3165,11 +3172,21 @@ NetworkOPsImp::transJson( assert(index < MultiApiJson::size); if (index != lastIndex) { + Json::Value& jvTx = multiObj.val[index]; RPC::insertDeliverMax( - multiObj.val[index][jss::transaction], - transaction->getTxnType(), - apiVersion); + jvTx[jss::transaction], transaction->getTxnType(), apiVersion); lastIndex = index; + + if (apiVersion > 1) + { + jvTx[jss::tx_json] = jvTx.removeMember(jss::transaction); + jvTx[jss::hash] = hash; + // TODO set `jvObj[jss::close_time_iso]` if validated + } + else + { + jvTx[jss::transaction][jss::hash] = hash; + } } } diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index c6a9e053c3d..195f3a1de3f 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -108,8 +108,16 @@ class STTx final : public STObject, public CountedObject Json::Value getJson(JsonOptions options) const override; + + /// If `hash` is set, will store hash inside the provided string. Otherwise + /// hash will be stored as nested jss::hash element inside the returned JSON + /// Additionally, if `hash` is set and `binary` is true, will not create + /// nested jss::tx for binary hex; instead will return it as JSON string Json::Value - getJson(JsonOptions options, bool binary) const; + getJson( + JsonOptions options, + bool binary, + std::optional> hash = {}) const; void sign(PublicKey const& publicKey, SecretKey const& secretKey); diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 1ce4ddb64b7..abcd01049b0 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -234,17 +234,33 @@ Json::Value STTx::getJson(JsonOptions) const } Json::Value -STTx::getJson(JsonOptions options, bool binary) const +STTx::getJson( + JsonOptions options, + bool binary, + std::optional> hash) const { + if (!hash) // Old behaviour - default because `hash = {}` in declaration + { + if (binary) + { + Json::Value ret; + Serializer s = STObject::getSerializer(); + ret[jss::tx] = strHex(s.peekData()); + ret[jss::hash] = to_string(getTransactionID()); + return ret; + } + return getJson(options); + } + + // Since `hash` is set, do not populate `hash` inside JSON output + hash->get() = to_string(getTransactionID()); if (binary) { - Json::Value ret; Serializer s = STObject::getSerializer(); - ret[jss::tx] = strHex(s.peekData()); - ret[jss::hash] = to_string(getTransactionID()); + Json::Value ret = strHex(s.peekData()); return ret; } - return getJson(options); + return STObject::getJson(JsonOptions::none); // Yes, want `none` } std::string const& From 76eee5ab03472589ba72b8795ab631125312c51a Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 18 Oct 2023 17:23:16 +0100 Subject: [PATCH 04/34] Output from sign, submit etc. --- src/ripple/app/misc/Transaction.h | 6 ++++- src/ripple/app/misc/impl/Transaction.cpp | 7 ++++-- src/ripple/rpc/handlers/SignFor.cpp | 1 + src/ripple/rpc/handlers/SignHandler.cpp | 1 + src/ripple/rpc/handlers/Submit.cpp | 1 + src/ripple/rpc/handlers/SubmitMultiSigned.cpp | 1 + src/ripple/rpc/impl/TransactionSign.cpp | 25 ++++++++++++++----- src/ripple/rpc/impl/TransactionSign.h | 4 +++ src/test/rpc/JSONRPC_test.cpp | 8 ++++-- 9 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index 07802becfeb..1da21f96011 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -305,8 +305,12 @@ class Transaction : public std::enable_shared_from_this, validatedLedger, fee, accountSeq, availableSeq); } + /// Same as similar overload for STTx::getJson Json::Value - getJson(JsonOptions options, bool binary = false) const; + getJson( + JsonOptions options, + bool binary = false, + std::optional> hash = {}) const; // Information used to locate a transaction. // Contains a nodestore hash and ledger sequence pair if the transaction was diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index 9adef982d01..a7667b8c886 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -165,9 +165,12 @@ Transaction::load( // options 1 to include the date of the transaction Json::Value -Transaction::getJson(JsonOptions options, bool binary) const +Transaction::getJson( + JsonOptions options, + bool binary, + std::optional> hash) const { - Json::Value ret(mTransaction->getJson(JsonOptions::none, binary)); + Json::Value ret(mTransaction->getJson(JsonOptions::none, binary, hash)); if (mInLedger) { diff --git a/src/ripple/rpc/handlers/SignFor.cpp b/src/ripple/rpc/handlers/SignFor.cpp index ac76fa0d8a1..722cf7da157 100644 --- a/src/ripple/rpc/handlers/SignFor.cpp +++ b/src/ripple/rpc/handlers/SignFor.cpp @@ -46,6 +46,7 @@ doSignFor(RPC::JsonContext& context) auto ret = RPC::transactionSignFor( context.params, + context.apiVersion, failType, context.role, context.ledgerMaster.getValidatedLedgerAge(), diff --git a/src/ripple/rpc/handlers/SignHandler.cpp b/src/ripple/rpc/handlers/SignHandler.cpp index 15d433da49c..4d89cdcb2e0 100644 --- a/src/ripple/rpc/handlers/SignHandler.cpp +++ b/src/ripple/rpc/handlers/SignHandler.cpp @@ -45,6 +45,7 @@ doSign(RPC::JsonContext& context) auto ret = RPC::transactionSign( context.params, + context.apiVersion, failType, context.role, context.ledgerMaster.getValidatedLedgerAge(), diff --git a/src/ripple/rpc/handlers/Submit.cpp b/src/ripple/rpc/handlers/Submit.cpp index 8a702c5bd3e..8e998f1ea6c 100644 --- a/src/ripple/rpc/handlers/Submit.cpp +++ b/src/ripple/rpc/handlers/Submit.cpp @@ -63,6 +63,7 @@ doSubmit(RPC::JsonContext& context) auto ret = RPC::transactionSubmit( context.params, + context.apiVersion, failType, context.role, context.ledgerMaster.getValidatedLedgerAge(), diff --git a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp index 9b455a1961f..82fa52a4623 100644 --- a/src/ripple/rpc/handlers/SubmitMultiSigned.cpp +++ b/src/ripple/rpc/handlers/SubmitMultiSigned.cpp @@ -45,6 +45,7 @@ doSubmitMultiSigned(RPC::JsonContext& context) return RPC::transactionSubmitMultiSigned( context.params, + context.apiVersion, failType, context.role, context.ledgerMaster.getValidatedLedgerAge(), diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 5dbfa49aef9..eb620a03d3e 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -645,12 +645,21 @@ transactionConstructImpl( } static Json::Value -transactionFormatResultImpl(Transaction::pointer tpTrans) +transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) { Json::Value jvResult; try { - jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + if (apiVersion > 1) + { + std::string hash = {}; + jvResult[jss::tx_json] = + tpTrans->getJson(JsonOptions::none, false, {std::ref(hash)}); + jvResult[jss::hash] = hash; + } + else + jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + jvResult[jss::tx_blob] = strHex(tpTrans->getSTransaction()->getSerializer().peekData()); @@ -777,6 +786,7 @@ checkFee( Json::Value transactionSign( Json::Value jvRequest, + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -807,13 +817,14 @@ transactionSign( if (!txn.second) return txn.first; - return transactionFormatResultImpl(txn.second); + return transactionFormatResultImpl(txn.second, apiVersion); } /** Returns a Json::objectValue. */ Json::Value transactionSubmit( Json::Value jvRequest, + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -853,7 +864,7 @@ transactionSubmit( rpcINTERNAL, "Exception occurred during transaction submission."); } - return transactionFormatResultImpl(txn.second); + return transactionFormatResultImpl(txn.second, apiVersion); } namespace detail { @@ -943,6 +954,7 @@ sortAndValidateSigners(STArray& signers, AccountID const& signingForID) Json::Value transactionSignFor( Json::Value jvRequest, + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -1043,13 +1055,14 @@ transactionSignFor( if (!txn.second) return txn.first; - return transactionFormatResultImpl(txn.second); + return transactionFormatResultImpl(txn.second, apiVersion); } /** Returns a Json::objectValue. */ Json::Value transactionSubmitMultiSigned( Json::Value jvRequest, + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -1236,7 +1249,7 @@ transactionSubmitMultiSigned( rpcINTERNAL, "Exception occurred during transaction submission."); } - return transactionFormatResultImpl(txn.second); + return transactionFormatResultImpl(txn.second, apiVersion); } } // namespace RPC diff --git a/src/ripple/rpc/impl/TransactionSign.h b/src/ripple/rpc/impl/TransactionSign.h index a396e65af52..48d2859ccf5 100644 --- a/src/ripple/rpc/impl/TransactionSign.h +++ b/src/ripple/rpc/impl/TransactionSign.h @@ -96,6 +96,7 @@ getProcessTxnFn(NetworkOPs& netOPs) Json::Value transactionSign( Json::Value params, // Passed by value so it can be modified locally. + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -105,6 +106,7 @@ transactionSign( Json::Value transactionSubmit( Json::Value params, // Passed by value so it can be modified locally. + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -116,6 +118,7 @@ transactionSubmit( Json::Value transactionSignFor( Json::Value params, // Passed by value so it can be modified locally. + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, @@ -125,6 +128,7 @@ transactionSignFor( Json::Value transactionSubmitMultiSigned( Json::Value params, // Passed by value so it can be modified locally. + unsigned apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index 5d4c09ef8d1..1e8ce554be3 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -2536,17 +2536,19 @@ class JSONRPC_test : public beast::unit_test::suite // A list of all the functions we want to test. using signFunc = Json::Value (*)( Json::Value params, + unsigned int apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, - Application & app); + Application& app); using submitFunc = Json::Value (*)( Json::Value params, + unsigned int apiVersion, NetworkOPs::FailHard failType, Role role, std::chrono::seconds validatedLedgerAge, - Application & app, + Application& app, ProcessTransactionFn const& processTransaction, RPC::SubmitSync sync); @@ -2586,6 +2588,7 @@ class JSONRPC_test : public beast::unit_test::suite assert(get<1>(testFunc) == nullptr); result = signFn( req, + 1, NetworkOPs::FailHard::yes, testRole, 1s, @@ -2597,6 +2600,7 @@ class JSONRPC_test : public beast::unit_test::suite assert(submitFn != nullptr); result = submitFn( req, + 1, NetworkOPs::FailHard::yes, testRole, 1s, From 92635aa4e9bd482b7f4f5031b7d62ee7150a6567 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 18 Oct 2023 17:23:49 +0100 Subject: [PATCH 05/34] Output from ledger --- src/ripple/app/ledger/impl/LedgerToJson.cpp | 66 ++++++++++++++++----- src/ripple/protocol/jss.h | 1 + 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 55123ba2362..81e84303933 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -118,24 +119,57 @@ fillJsonTx( if (bBinary) { txJson[jss::tx_blob] = serializeHex(*txn); + if (fill.context->apiVersion > 1) + txJson[jss::hash] = to_string(txn->getTransactionID()); + + auto const json_meta = + (fill.context->apiVersion > 1 ? jss::meta_blob : jss::meta); if (stMeta) - txJson[jss::meta] = serializeHex(*stMeta); + txJson[json_meta] = serializeHex(*stMeta); } else { - copyFrom(txJson, txn->getJson(JsonOptions::none)); - RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion); - if (stMeta) + if (fill.context->apiVersion > 1) + { + std::string hash; + copyFrom( + txJson[jss::tx_json], + txn->getJson( + JsonOptions::none, false, {std::optional(std::ref(hash))})); + txJson[jss::hash] = hash; + // TODO set `txJson[jss::close_time_iso]` + 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}); + } + } + else { - 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}); + 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}); + } } } @@ -254,7 +288,11 @@ fillJsonQueue(Object& json, LedgerFill const& fill) if (tx.lastResult) txJson["last_result"] = transToken(*tx.lastResult); - txJson[jss::tx] = fillJsonTx(fill, bBinary, bExpanded, tx.txn, nullptr); + auto&& temp = fillJsonTx(fill, bBinary, bExpanded, tx.txn, nullptr); + if (fill.context->apiVersion > 1) + copyFrom(txJson, temp); + else + copyFrom(txJson[jss::tx], temp); } } diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index d4b213bcb1b..281a410127d 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -460,6 +460,7 @@ JSS(median_fee); // out: TxQ JSS(median_level); // out: TxQ JSS(message); // error. JSS(meta); // out: NetworkOPs, AccountTx*, Tx +JSS(meta_blob); // out: NetworkOPs, AccountTx*, Tx JSS(metaData); JSS(metadata); // out: TransactionEntry JSS(method); // RPC From e4ce8b199024613ee0d87c296c9f0165af614633 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 18 Oct 2023 17:24:08 +0100 Subject: [PATCH 06/34] Output from account_tx --- src/ripple/rpc/handlers/AccountTx.cpp | 21 ++++++++++-- src/test/rpc/AccountTx_test.cpp | 47 ++++++++++++++++++--------- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index bd939a92f1c..2a4ae3648e3 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -326,10 +326,23 @@ populateJsonResponse( { Json::Value& jvObj = jvTxns.append(Json::objectValue); - jvObj[jss::tx] = txn->getJson(JsonOptions::include_date); + auto const json_tx = + (context.apiVersion > 1 ? jss::tx_json : jss::tx); + if (context.apiVersion > 1) + { + std::string hash; + jvObj[json_tx] = txn->getJson( + JsonOptions::include_date, false, {std::ref(hash)}); + jvObj[jss::hash] = hash; + // TODO set `jvObj[jss::close_time_iso]` + } + else + jvObj[json_tx] = + txn->getJson(JsonOptions::include_date); + auto const& sttx = txn->getSTransaction(); RPC::insertDeliverMax( - jvObj[jss::tx], sttx->getTxnType(), context.apiVersion); + jvObj[json_tx], sttx->getTxnType(), context.apiVersion); if (txnMeta) { jvObj[jss::meta] = @@ -352,7 +365,9 @@ populateJsonResponse( Json::Value& jvObj = jvTxns.append(Json::objectValue); jvObj[jss::tx_blob] = strHex(std::get<0>(binaryData)); - jvObj[jss::meta] = strHex(std::get<1>(binaryData)); + auto const json_meta = + (context.apiVersion > 1 ? jss::meta_blob : jss::meta); + jvObj[json_meta] = strHex(std::get<1>(binaryData)); jvObj[jss::ledger_index] = std::get<2>(binaryData); jvObj[jss::validated] = true; } diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 8c583ee1254..3cfcda75847 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -120,22 +120,37 @@ class AccountTx_test : public beast::unit_test::suite // All other ledgers have no txs 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) && - (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])); + switch (apiVersion) + { + case 1: + 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) && + (j[jss::result][jss::transactions][1u][jss::tx] + [jss::DeliverMax] == "10000000010") && + (j[jss::result][jss::transactions][1u][jss::tx] + [jss::Amount] == + j[jss::result][jss::transactions][1u][jss::tx] + [jss::DeliverMax]); + case 2: + 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_json] + [jss::TransactionType] == jss::AccountSet) && + (j[jss::result][jss::transactions][1u][jss::tx_json] + [jss::TransactionType] == jss::Payment) && + (j[jss::result][jss::transactions][1u][jss::tx_json] + [jss::DeliverMax] == "10000000010") && + (!j[jss::result][jss::transactions][1u][jss::tx_json] + .isMember(jss::Amount)); + default: + return false; + } }; auto noTxs = [](Json::Value const& j) { From 0fb0517a4e78390a84a87e482b57f954c7659eb7 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 18 Oct 2023 17:24:26 +0100 Subject: [PATCH 07/34] Output from transaction_entry --- src/ripple/rpc/handlers/TransactionEntry.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index 677581db6a3..b7956168848 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -71,11 +71,24 @@ doTransactionEntry(RPC::JsonContext& context) } else { - jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none); + if (context.apiVersion > 1) + { + std::string hash; + jvResult[jss::tx_json] = + sttx->getJson(JsonOptions::none, false, {std::ref(hash)}); + jvResult[jss::hash] = hash; + // TODO set `jvResult[jss::close_time_iso]` + } + else + jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none); + RPC::insertDeliverMax( jvResult[jss::tx_json], sttx->getTxnType(), context.apiVersion); + + auto const json_meta = + (context.apiVersion > 1 ? jss::meta : jss::metadata); if (stobj) - jvResult[jss::metadata] = stobj->getJson(JsonOptions::none); + jvResult[json_meta] = stobj->getJson(JsonOptions::none); // 'accounts' // 'engine_...' // 'ledger_...' From 910f1259cc88196e6037362308a6bf3a0bdc6209 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 18 Oct 2023 17:24:42 +0100 Subject: [PATCH 08/34] Output from tx --- src/ripple/rpc/handlers/Tx.cpp | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 92d0e4dd673..3769f38a05d 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -311,17 +311,41 @@ populateJsonResponse( // no errors 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); + if (context.apiVersion > 1) + { + std::string hash; + if (args.binary) + response[jss::tx_blob] = result.txn->getJson( + JsonOptions::include_date, true, {std::ref(hash)}); + else + { + response[jss::tx_json] = result.txn->getJson( + JsonOptions::include_date, false, {std::ref(hash)}); + RPC::insertDeliverMax( + response[jss::tx_json], + sttx->getTxnType(), + context.apiVersion); + } + response[jss::hash] = hash; + // TODO set `response[jss::close_time_iso]` + } + else + { + response = + result.txn->getJson(JsonOptions::include_date, args.binary); + if (!args.binary) + RPC::insertDeliverMax( + response, sttx->getTxnType(), context.apiVersion); + } // populate binary metadata if (auto blob = std::get_if(&result.meta)) { assert(args.binary); - response[jss::meta] = strHex(makeSlice(*blob)); + auto json_meta = + (context.apiVersion > 1 ? jss::meta_blob : jss::meta); + response[json_meta] = strHex(makeSlice(*blob)); } // populate meta data else if (auto m = std::get_if>(&result.meta)) From 4d517e2a8d8c5c7a4b53a409eab7076214d46d11 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 23 Oct 2023 15:27:35 +0000 Subject: [PATCH 09/34] Store close_time_iso in API v2 output --- src/ripple/app/ledger/impl/LedgerToJson.cpp | 14 ++++++++- src/ripple/app/misc/NetworkOPs.cpp | 2 ++ src/ripple/basics/chrono.h | 33 ++++++++++++++++++-- src/ripple/core/TimeKeeper.h | 3 +- src/ripple/protocol/jss.h | 2 ++ src/ripple/rpc/handlers/AMMInfo.cpp | 2 +- src/ripple/rpc/handlers/AccountTx.cpp | 6 +++- src/ripple/rpc/handlers/TransactionEntry.cpp | 14 ++++++++- src/ripple/rpc/handlers/Tx.cpp | 15 ++++++++- 9 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 81e84303933..26f1ca2aefd 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -17,6 +17,7 @@ */ //============================================================================== +#include #include #include #include @@ -26,6 +27,7 @@ #include #include #include +#include namespace ripple { @@ -137,7 +139,6 @@ fillJsonTx( txn->getJson( JsonOptions::none, false, {std::optional(std::ref(hash))})); txJson[jss::hash] = hash; - // TODO set `txJson[jss::close_time_iso]` RPC::insertDeliverMax( txJson[jss::tx_json], txnType, fill.context->apiVersion); @@ -153,6 +154,17 @@ fillJsonTx( txn, {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } + + const bool validated = RPC::isValidated( + fill.context->ledgerMaster, fill.ledger, fill.context->app); + txJson[jss::validated] = validated; + if (validated) + { + if (auto close_time = + fill.context->ledgerMaster.getCloseTimeBySeq( + fill.ledger.seq())) + txJson[jss::close_time_iso] = to_string_iso(*close_time); + } } else { diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 27dba963422..7c79c1b68e1 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3124,6 +3124,8 @@ NetworkOPsImp::transJson( jvObj[jss::transaction][jss::date] = ledger->info().closeTime.time_since_epoch().count(); jvObj[jss::validated] = true; + if (auto close_time = m_ledgerMaster.getCloseTimeBySeq(ledger->seq())) + jvObj[jss::close_time_iso] = to_string_iso(*close_time); // WRITEME: Put the account next seq here } diff --git a/src/ripple/basics/chrono.h b/src/ripple/basics/chrono.h index f50d765d58f..21353e0873a 100644 --- a/src/ripple/basics/chrono.h +++ b/src/ripple/basics/chrono.h @@ -25,6 +25,7 @@ #include #include #include + #include #include #include @@ -43,8 +44,19 @@ using weeks = std::chrono:: /** Clock for measuring the network time. The epoch is January 1, 2000 - epoch_offset = days(10957); // 2000-01-01 + + epoch_offset + = date(2000-01-01) - date(1970-0-01) + = days(10957) + = seconds(946684800) */ + +constexpr static std::chrono::seconds epoch_offset = + date::sys_days{date::year{2000} / 1 / 1} - + date::sys_days{date::year{1970} / 1 / 1}; + +static_assert(epoch_offset.count() == 946684800); + class NetClock { public: @@ -71,7 +83,24 @@ to_string(NetClock::time_point tp) // 2000-01-01 00:00:00 UTC is 946684800s from 1970-01-01 00:00:00 UTC using namespace std::chrono; return to_string( - system_clock::time_point{tp.time_since_epoch() + 946684800s}); + system_clock::time_point{tp.time_since_epoch() + epoch_offset}); +} + +template +std::string +to_string_iso(date::sys_time tp) +{ + using namespace std::chrono; + return date::format("%FT%H:%M:%OSZ", tp); +} + +inline std::string +to_string_iso(NetClock::time_point tp) +{ + // 2000-01-01 00:00:00 UTC is 946684800s from 1970-01-01 00:00:00 UTC + using namespace std::chrono; + return to_string_iso( + system_clock::time_point{tp.time_since_epoch() + epoch_offset}); } /** A clock for measuring elapsed time. diff --git a/src/ripple/core/TimeKeeper.h b/src/ripple/core/TimeKeeper.h index 55970ec8227..e239a2f7565 100644 --- a/src/ripple/core/TimeKeeper.h +++ b/src/ripple/core/TimeKeeper.h @@ -22,6 +22,7 @@ #include #include + #include namespace ripple { @@ -37,7 +38,7 @@ class TimeKeeper : public beast::abstract_clock adjust(std::chrono::system_clock::time_point when) { return time_point(std::chrono::duration_cast( - when.time_since_epoch() - days(10957))); + when.time_since_epoch() - epoch_offset)); } public: diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 281a410127d..8a701defad8 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -230,6 +230,8 @@ JSS(close); // out: BookChanges JSS(close_flags); // out: LedgerToJson JSS(close_time); // in: Application, out: NetworkOPs, // RCLCxPeerPos, LedgerToJson +JSS(close_time_iso); // out: Tx, NetworkOPs, TransactionEntry + // AccountTx, LedgerToJson JSS(close_time_estimated); // in: Application, out: LedgerToJson JSS(close_time_human); // out: LedgerToJson JSS(close_time_offset); // out: NetworkOPs diff --git a/src/ripple/rpc/handlers/AMMInfo.cpp b/src/ripple/rpc/handlers/AMMInfo.cpp index 11e124afb44..47d3e117e8e 100644 --- a/src/ripple/rpc/handlers/AMMInfo.cpp +++ b/src/ripple/rpc/handlers/AMMInfo.cpp @@ -66,7 +66,7 @@ to_iso8601(NetClock::time_point tp) return date::format( "%Y-%Om-%dT%H:%M:%OS%z", date::sys_time( - system_clock::time_point{tp.time_since_epoch() + 946684800s})); + system_clock::time_point{tp.time_since_epoch() + epoch_offset})); } Json::Value diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 2a4ae3648e3..8f130daadf5 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -334,7 +334,6 @@ populateJsonResponse( jvObj[json_tx] = txn->getJson( JsonOptions::include_date, false, {std::ref(hash)}); jvObj[jss::hash] = hash; - // TODO set `jvObj[jss::close_time_iso]` } else jvObj[json_tx] = @@ -351,6 +350,11 @@ populateJsonResponse( insertDeliveredAmount( jvObj[jss::meta], context, txn, *txnMeta); insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); + if (auto closeTime = + context.ledgerMaster.getCloseTimeBySeq( + txnMeta->getIndex())) + jvObj[jss::close_time_iso] = + to_string_iso(*closeTime); } } } diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index b7956168848..db826f57d37 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -17,6 +17,7 @@ */ //============================================================================== +#include #include #include #include @@ -77,7 +78,18 @@ doTransactionEntry(RPC::JsonContext& context) jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none, false, {std::ref(hash)}); jvResult[jss::hash] = hash; - // TODO set `jvResult[jss::close_time_iso]` + + bool const validated = RPC::isValidated( + context.ledgerMaster, *lpLedger, context.app); + + jvResult[jss::validated] = validated; + if (validated) + { + if (auto closeTime = context.ledgerMaster.getCloseTimeBySeq( + lpLedger->seq())) + jvResult[jss::close_time_iso] = + to_string_iso(*closeTime); + } } else jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none); diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 3769f38a05d..16fa7baa8ba 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -55,6 +56,7 @@ struct TxResult std::variant, Blob> meta; bool validated = false; std::optional ctid; + std::optional closeTime; TxSearched searchedAll; }; @@ -140,6 +142,12 @@ doTxPostgres(RPC::Context& context, TxArgs const& args) *(args.hash), res.txn->getLedger(), *meta); } res.validated = true; + + auto const ledgerInfo = + context.app.getRelationalDatabase().getLedgerInfoByIndex( + locator.getLedgerSequence()); + res.closeTime = ledgerInfo->closeTime; + return {res, rpcSUCCESS}; } else @@ -269,6 +277,9 @@ doTxHelp(RPC::Context& context, TxArgs args) } result.validated = isValidated( context.ledgerMaster, ledger->info().seq, ledger->info().hash); + if (result.validated) + result.closeTime = + context.ledgerMaster.getCloseTimeBySeq(txn->getLedger()); // compute outgoing CTID uint32_t lgrSeq = ledger->info().seq; @@ -328,7 +339,9 @@ populateJsonResponse( context.apiVersion); } response[jss::hash] = hash; - // TODO set `response[jss::close_time_iso]` + if (result.closeTime) + response[jss::close_time_iso] = + to_string_iso(*result.closeTime); } else { From 2b397c9217ce8ea1d4342b9e95cce3aff1243ab0 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 24 Oct 2023 15:45:41 +0000 Subject: [PATCH 10/34] Add small APIv2 unit test for subscribe --- src/test/rpc/Subscribe_test.cpp | 76 +++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index 7725390f6b6..bb419cbb4a1 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include "ripple/json/json_value.h" #include #include #include @@ -307,6 +308,80 @@ class Subscribe_test : public beast::unit_test::suite BEAST_EXPECT(jv[jss::status] == "success"); } + void + testTransactions_APIv2() + { + testcase("transactions API version 2"); + + using namespace std::chrono_literals; + using namespace jtx; + Env env(*this); + auto wsc = makeWSClient(env.app().config()); + Json::Value stream{Json::objectValue}; + + { + // RPC subscribe to transactions stream + stream[jss::api_version] = 2; + stream[jss::streams] = Json::arrayValue; + stream[jss::streams].append("transactions"); + auto jv = wsc->invoke("subscribe", stream); + if (wsc->version() == 2) + { + BEAST_EXPECT( + jv.isMember(jss::jsonrpc) && jv[jss::jsonrpc] == "2.0"); + BEAST_EXPECT( + jv.isMember(jss::ripplerpc) && jv[jss::ripplerpc] == "2.0"); + BEAST_EXPECT(jv.isMember(jss::id) && jv[jss::id] == 5); + } + BEAST_EXPECT(jv[jss::status] == "success"); + } + + { + env.fund(XRP(10000), "alice"); + env.close(); + + // 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() && + jv[jss::close_time_iso] // + == "2000-01-01T00:00:10Z" && + jv[jss::validated] == true && // + jv[jss::tx_json][jss::TransactionType] // + == jss::Payment && + jv[jss::tx_json][jss::DeliverMax] // + == "10000000010" && + !jv[jss::tx_json].isMember(jss::Amount) && + jv[jss::tx_json][jss::Fee] // + == "10" && + jv[jss::tx_json][jss::Sequence] // + == 1; + })); + + // Check stream update for accountset transaction + BEAST_EXPECT(wsc->findMsg(5s, [&](auto const& jv) { + return jv[jss::meta]["AffectedNodes"][0u]["ModifiedNode"] + ["FinalFields"][jss::Account] == + Account("alice").human(); + })); + } + + { + // RPC unsubscribe + auto jv = wsc->invoke("unsubscribe", stream); + if (wsc->version() == 2) + { + BEAST_EXPECT( + jv.isMember(jss::jsonrpc) && jv[jss::jsonrpc] == "2.0"); + BEAST_EXPECT( + jv.isMember(jss::ripplerpc) && jv[jss::ripplerpc] == "2.0"); + BEAST_EXPECT(jv.isMember(jss::id) && jv[jss::id] == 5); + } + BEAST_EXPECT(jv[jss::status] == "success"); + } + } + void testManifests() { @@ -1223,6 +1298,7 @@ class Subscribe_test : public beast::unit_test::suite testServer(); testLedger(); testTransactions(); + testTransactions_APIv2(); testManifests(); testValidations(all - xrpFees); testValidations(all); From 1a968007909901f33d22e957f8677f59f7641bdb Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 24 Oct 2023 16:53:31 +0000 Subject: [PATCH 11/34] Add unit test for transaction_entry --- src/test/rpc/TransactionEntry_test.cpp | 68 ++++++++++++++++++-------- 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index 60225f4621d..5b25d67d3a8 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -143,21 +144,23 @@ class TransactionEntry_test : public beast::unit_test::suite } void - testRequest() + testRequest(unsigned apiVersion) { - testcase("Basic request"); + testcase("Basic request API version " + std::to_string(apiVersion)); using namespace test::jtx; Env env{*this}; - auto check_tx = [this, &env]( + auto check_tx = [this, &env, apiVersion]( int index, std::string const txhash, - std::string const expected_json = "") { + std::string const expected_json = "", + std::string const close_time_iso = "") { // first request using ledger_index to lookup - Json::Value const resIndex{[&env, index, &txhash]() { + Json::Value const resIndex{[&env, index, &txhash, apiVersion]() { Json::Value params{Json::objectValue}; params[jss::ledger_index] = index; params[jss::tx_hash] = txhash; + params[jss::api_version] = apiVersion; return env.client().invoke( "transaction_entry", params)[jss::result]; }()}; @@ -165,7 +168,19 @@ class TransactionEntry_test : public beast::unit_test::suite if (!BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash)) return; - BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); + if (apiVersion > 1) + { + BEAST_EXPECT(resIndex[jss::hash] == txhash); + BEAST_EXPECT(resIndex[jss::validated] == true); + BEAST_EXPECT(!resIndex[jss::tx_json].isMember(jss::Amount)); + + if (!close_time_iso.empty()) + BEAST_EXPECT( + resIndex[jss::close_time_iso] == close_time_iso); + } + else + BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); + if (!expected_json.empty()) { Json::Value expected; @@ -198,12 +213,14 @@ class TransactionEntry_test : public beast::unit_test::suite Json::Value params{Json::objectValue}; params[jss::ledger_hash] = resIndex[jss::ledger_hash]; params[jss::tx_hash] = txhash; + params[jss::api_version] = apiVersion; Json::Value const resHash = env.client().invoke( "transaction_entry", params)[jss::result]; BEAST_EXPECT(resHash == resIndex); } // Use the command line form with the index. + if (apiVersion == RPC::apiMaximumSupportedVersion) { Json::Value const clIndex{env.rpc( "transaction_entry", txhash, std::to_string(index))}; @@ -211,6 +228,7 @@ class TransactionEntry_test : public beast::unit_test::suite } // Use the command line form with the ledger_hash. + if (apiVersion == RPC::apiMaximumSupportedVersion) { Json::Value const clHash{env.rpc( "transaction_entry", @@ -235,7 +253,10 @@ 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, R"( + check_tx( + env.closed()->seq(), + fund_1_tx, + R"( { "Account" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", "Fee" : "10", @@ -244,10 +265,13 @@ class TransactionEntry_test : public beast::unit_test::suite "SigningPubKey" : "0324CAAFA2212D2AEAB9D42D481535614AED486293E1FB1380FF070C3DD7FB4264", "TransactionType" : "AccountSet", "TxnSignature" : "3044022007B35E3B99460534FF6BC3A66FBBA03591C355CC38E38588968E87CCD01BE229022071A443026DE45041B55ABB1CC76812A87EA701E475BBB7E165513B4B242D3474", - "hash" : "F4E9DF90D829A9E8B423FF68C34413E240D8D8BB0EFD080DF08114ED398E2506" } -)"); - check_tx(env.closed()->seq(), fund_2_tx, R"( +)", + "2000-01-01T00:00:10Z"); + check_tx( + env.closed()->seq(), + fund_2_tx, + R"( { "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", "Fee" : "10", @@ -256,9 +280,9 @@ class TransactionEntry_test : public beast::unit_test::suite "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", "TransactionType" : "AccountSet", "TxnSignature" : "3045022100C8857FC0759A2AC0D2F320684691A66EAD252EAED9EF88C79791BC58BFCC9D860220421722286487DD0ED6BBA626CE6FCBDD14289F7F4726870C3465A4054C2702D7", - "hash" : "6853CD8226A05068C951CB1F54889FF4E40C5B440DC1C5BA38F114C4E0B1E705" } -)"); +)", + "2000-01-01T00:00:10Z"); env.trust(A2["USD"](1000), A1); // the trust tx is actually a payment since the trust method @@ -272,7 +296,10 @@ class TransactionEntry_test : public beast::unit_test::suite boost::lexical_cast(env.tx()->getTransactionID()); env.close(); - check_tx(env.closed()->seq(), trust_tx, R"( + check_tx( + env.closed()->seq(), + trust_tx, + R"( { "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", "DeliverMax" : "10", @@ -283,9 +310,9 @@ class TransactionEntry_test : public beast::unit_test::suite "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020", "TransactionType" : "Payment", "TxnSignature" : "3044022033D9EBF7F02950AF2F6B13C07AEE641C8FEBDD540A338FCB9027A965A4AED35B02206E4E227DCC226A3456C0FEF953449D21645A24EB63CA0BB7C5B62470147FD1D1", - "hash" : "C992D97D88FF444A1AB0C06B27557EC54B7F7DA28254778E60238BEA88E0C101" } -)"); +)", + "2000-01-01T00:00:20Z"); check_tx( env.closed()->seq(), @@ -306,9 +333,9 @@ class TransactionEntry_test : public beast::unit_test::suite "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", "TransactionType" : "Payment", "TxnSignature" : "30450221008A722B7F16EDB2348886E88ED4EC682AE9973CC1EE0FF37C93BB2CEC821D3EDF022059E464472031BA5E0D88A93E944B6A8B8DB3E1D5E5D1399A805F615789DB0BED", - "hash" : "988046D484ACE9F5F6A8C792D89C6EA2DB307B5DDA9864AEBA88E6782ABD0865" } -)"); +)", + "2000-01-01T00:00:20Z"); env(offer(A2, XRP(100), A2["USD"](1))); auto offer_tx = @@ -333,9 +360,9 @@ class TransactionEntry_test : public beast::unit_test::suite "TakerPays" : "100000000", "TransactionType" : "OfferCreate", "TxnSignature" : "304502210093FC93ACB77B4E3DE3315441BD010096734859080C1797AB735EB47EBD541BD102205020BB1A7C3B4141279EE4C287C13671E2450EA78914EFD0C6DB2A18344CD4F2", - "hash" : "5FCC1A27A7664F82A0CC4BE5766FBBB7C560D52B93AA7B550CD33B27AEC7EFFB" } -)"); +)", + "2000-01-01T00:00:30Z"); } public: @@ -343,7 +370,8 @@ class TransactionEntry_test : public beast::unit_test::suite run() override { testBadInput(); - testRequest(); + testRequest(1); + testRequest(2); } }; From c2a3b52548964e4bb91e4490b59da2ebe5931790 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 24 Oct 2023 17:39:45 +0000 Subject: [PATCH 12/34] Add unit test for tx --- src/test/rpc/Transaction_test.cpp | 41 ++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index c16d7bbd004..22d3ed2e29b 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -694,15 +694,13 @@ class Transaction_test : public beast::unit_test::suite } void - testRequest(FeatureBitset features) + testRequest(FeatureBitset features, unsigned apiVersion) { - testcase("Test Request"); + testcase("Test Request API version " + std::to_string(apiVersion)); 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"}; @@ -725,18 +723,42 @@ class Transaction_test : public beast::unit_test::suite Json::Value expected = txn->getJson(JsonOptions::none); expected[jss::DeliverMax] = expected[jss::Amount]; + if (apiVersion > 1) + { + expected.removeMember(jss::hash); + expected.removeMember(jss::Amount); + } + + Json::Value const result = {[&env, txn, apiVersion]() { + Json::Value params{Json::objectValue}; + params[jss::transaction] = to_string(txn->getTransactionID()); + params[jss::binary] = false; + params[jss::api_version] = apiVersion; + return env.client().invoke("tx", params); + }()}; - auto const result = - env.rpc(COMMAND, to_string(txn->getTransactionID())); BEAST_EXPECT(result[jss::result][jss::status] == jss::success); + if (apiVersion > 1) + { + BEAST_EXPECT( + result[jss::result][jss::close_time_iso] == + "2000-01-01T00:00:20Z"); + BEAST_EXPECT( + result[jss::result][jss::hash] == + to_string(txn->getTransactionID())); + BEAST_EXPECT(result[jss::result][jss::validated] == true); + } 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& result_transaction = + (apiVersion > 1 ? result[jss::result][jss::tx_json] + : result[jss::result]); + if (BEAST_EXPECT(result_transaction.isMember(name))) { - auto const received = result[jss::result][name]; + auto const received = result_transaction[name]; BEAST_EXPECTS( received == *memberIt, "Transaction contains \n\"" + name + "\": " // @@ -763,7 +785,8 @@ class Transaction_test : public beast::unit_test::suite testRangeCTIDRequest(features); testCTIDValidation(features); testCTIDRPC(features); - testRequest(features); + testRequest(features, 1); + testRequest(features, 2); } }; From bdf90e545279becd76d20da36263d67f823d3e61 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 24 Oct 2023 18:24:30 +0000 Subject: [PATCH 13/34] Remove inLedger from API version 2 --- src/ripple/app/misc/Transaction.h | 3 ++- src/ripple/app/misc/impl/Transaction.cpp | 5 ++++- src/ripple/rpc/handlers/AccountTx.cpp | 9 ++++++--- src/ripple/rpc/handlers/Tx.cpp | 8 ++++---- src/ripple/rpc/impl/TransactionSign.cpp | 7 ++++--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index 1da21f96011..24849994161 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -27,7 +27,7 @@ #include #include #include -#include + #include #include @@ -310,6 +310,7 @@ class Transaction : public std::enable_shared_from_this, getJson( JsonOptions options, bool binary = false, + bool showInLedger = false, std::optional> hash = {}) const; // Information used to locate a transaction. diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index a7667b8c886..c6cd01b7096 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -168,13 +168,16 @@ Json::Value Transaction::getJson( JsonOptions options, bool binary, + bool showInLedger, std::optional> hash) const { Json::Value ret(mTransaction->getJson(JsonOptions::none, binary, hash)); if (mInLedger) { - ret[jss::inLedger] = mInLedger; // Deprecated. + if (showInLedger) + ret[jss::inLedger] = mInLedger; // Deprecated. + ret[jss::ledger_index] = mInLedger; if (options == JsonOptions::include_date) diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 8f130daadf5..dc3e1e3936e 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -332,12 +332,15 @@ populateJsonResponse( { std::string hash; jvObj[json_tx] = txn->getJson( - JsonOptions::include_date, false, {std::ref(hash)}); + JsonOptions::include_date, + false, + false, + {std::ref(hash)}); jvObj[jss::hash] = hash; } else - jvObj[json_tx] = - txn->getJson(JsonOptions::include_date); + jvObj[json_tx] = txn->getJson( + JsonOptions::include_date, false, true); auto const& sttx = txn->getSTransaction(); RPC::insertDeliverMax( diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 16fa7baa8ba..2827256bbd0 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -328,11 +328,11 @@ populateJsonResponse( std::string hash; if (args.binary) response[jss::tx_blob] = result.txn->getJson( - JsonOptions::include_date, true, {std::ref(hash)}); + JsonOptions::include_date, true, false, {std::ref(hash)}); else { response[jss::tx_json] = result.txn->getJson( - JsonOptions::include_date, false, {std::ref(hash)}); + JsonOptions::include_date, false, false, {std::ref(hash)}); RPC::insertDeliverMax( response[jss::tx_json], sttx->getTxnType(), @@ -345,8 +345,8 @@ populateJsonResponse( } else { - response = - result.txn->getJson(JsonOptions::include_date, args.binary); + response = result.txn->getJson( + JsonOptions::include_date, args.binary, true); if (!args.binary) RPC::insertDeliverMax( response, sttx->getTxnType(), context.apiVersion); diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index eb620a03d3e..c5ad5d43957 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -653,12 +653,13 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) if (apiVersion > 1) { std::string hash = {}; - jvResult[jss::tx_json] = - tpTrans->getJson(JsonOptions::none, false, {std::ref(hash)}); + jvResult[jss::tx_json] = tpTrans->getJson( + JsonOptions::none, false, false, {std::ref(hash)}); jvResult[jss::hash] = hash; } else - jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + jvResult[jss::tx_json] = + tpTrans->getJson(JsonOptions::none, false, true); jvResult[jss::tx_blob] = strHex(tpTrans->getSTransaction()->getSerializer().peekData()); From 2035bbc2f295f60ef65bc2f600971bccc0f256ec Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 24 Oct 2023 19:15:50 +0000 Subject: [PATCH 14/34] Set ledger_hash and ledger_index --- src/ripple/app/ledger/impl/LedgerToJson.cpp | 3 +++ src/ripple/app/misc/NetworkOPs.cpp | 1 - src/ripple/rpc/handlers/AccountTx.cpp | 4 ++++ src/ripple/rpc/handlers/Tx.cpp | 16 +++++++++++++--- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 26f1ca2aefd..f3721156a0c 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -86,6 +86,7 @@ fillJson(Object& json, bool closed, LedgerInfo const& info, bool bFull) json[jss::close_time_human] = to_string(info.closeTime); if (!getCloseAgree(info)) json[jss::close_time_estimated] = true; + json[jss::close_time_iso] = to_string_iso(info.closeTime); } } @@ -160,6 +161,8 @@ fillJsonTx( txJson[jss::validated] = validated; if (validated) { + txJson[jss::ledger_index] = to_string(fill.ledger.seq()); + txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash); if (auto close_time = fill.context->ledgerMaster.getCloseTimeBySeq( fill.ledger.seq())) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 7c79c1b68e1..e624eef963c 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3183,7 +3183,6 @@ NetworkOPsImp::transJson( { jvTx[jss::tx_json] = jvTx.removeMember(jss::transaction); jvTx[jss::hash] = hash; - // TODO set `jvObj[jss::close_time_iso]` if validated } else { diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index dc3e1e3936e..6ec7efd3d6c 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -337,6 +337,10 @@ populateJsonResponse( false, {std::ref(hash)}); jvObj[jss::hash] = hash; + jvObj[jss::ledger_index] = txn->getLedger(); + jvObj[jss::ledger_hash] = + to_string(context.ledgerMaster.getHashBySeq( + txn->getLedger())); } else jvObj[json_tx] = txn->getJson( diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 2827256bbd0..39f49b50f7a 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -57,6 +57,7 @@ struct TxResult bool validated = false; std::optional ctid; std::optional closeTime; + std::optional ledgerHash; TxSearched searchedAll; }; @@ -147,6 +148,7 @@ doTxPostgres(RPC::Context& context, TxArgs const& args) context.app.getRelationalDatabase().getLedgerInfoByIndex( locator.getLedgerSequence()); res.closeTime = ledgerInfo->closeTime; + res.ledgerHash = ledgerInfo->hash; return {res, rpcSUCCESS}; } @@ -285,6 +287,7 @@ doTxHelp(RPC::Context& context, TxArgs args) uint32_t lgrSeq = ledger->info().seq; uint32_t txnIdx = meta->getAsObject().getFieldU32(sfTransactionIndex); uint32_t netID = context.app.config().NETWORK_ID; + result.ledgerHash = ledger->info().hash; if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL) result.ctid = @@ -339,9 +342,16 @@ populateJsonResponse( context.apiVersion); } response[jss::hash] = hash; - if (result.closeTime) - response[jss::close_time_iso] = - to_string_iso(*result.closeTime); + if (result.validated) + { + response[jss::ledger_index] = result.txn->getLedger(); + if (result.ledgerHash) + response[jss::ledger_hash] = to_string(*result.ledgerHash); + + if (result.closeTime) + response[jss::close_time_iso] = + to_string_iso(*result.closeTime); + } } else { From 1115cef43b0bf024bb4e5a1d664088e0644a62a9 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 26 Oct 2023 13:22:26 +0100 Subject: [PATCH 15/34] Move isValidated from RPCHelpers to LedgerMaster --- src/ripple/app/ledger/LedgerMaster.h | 2 + src/ripple/app/ledger/impl/LedgerMaster.cpp | 49 +++++++++++++++++ src/ripple/app/ledger/impl/LedgerToJson.cpp | 5 +- src/ripple/rpc/handlers/AMMInfo.cpp | 4 +- src/ripple/rpc/handlers/AccountTx.cpp | 5 +- src/ripple/rpc/handlers/LedgerHandler.cpp | 4 +- src/ripple/rpc/handlers/TransactionEntry.cpp | 4 +- src/ripple/rpc/impl/RPCHelpers.cpp | 56 +------------------- src/ripple/rpc/impl/RPCHelpers.h | 6 --- 9 files changed, 61 insertions(+), 74 deletions(-) diff --git a/src/ripple/app/ledger/LedgerMaster.h b/src/ripple/app/ledger/LedgerMaster.h index 26738844536..e2ca3039935 100644 --- a/src/ripple/app/ledger/LedgerMaster.h +++ b/src/ripple/app/ledger/LedgerMaster.h @@ -215,6 +215,8 @@ class LedgerMaster : public AbstractFetchPackContainer void clearLedger(std::uint32_t seq); bool + isValidated(ReadView const& ledger); + bool getValidatedRange(std::uint32_t& minVal, std::uint32_t& maxVal); bool getFullValidatedRange(std::uint32_t& minVal, std::uint32_t& maxVal); diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 050e2f3ef3d..60897504c91 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -603,6 +603,55 @@ LedgerMaster::clearLedger(std::uint32_t seq) mCompleteLedgers.erase(seq); } +bool +LedgerMaster::isValidated(ReadView const& ledger) +{ + if (app_.config().reporting()) + return haveLedger(ledger.seq()); // TODO Is this correct ? + + if (ledger.open()) + return false; + + if (ledger.info().validated) + return true; + + auto const seq = ledger.info().seq; + try + { + // 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); + + if (!hash || ledger.info().hash != *hash) + { + // This ledger's hash is not the hash of the validated ledger + if (hash) + { + assert(hash->isNonZero()); + uint256 valHash = + app_.getRelationalDatabase().getHashByIndex(seq); + if (valHash == ledger.info().hash) + { + // SQL database doesn't match ledger chain + clearLedger(seq); + } + } + return false; + } + } + catch (SHAMapMissingNode const& mn) + { + auto stream = app_.journal("IsValidated").warn(); // TODO Better name ? + JLOG(stream) << "Ledger #" << seq << ": " << mn.what(); + return false; + } + + // Mark ledger as validated to save time if we see it again. + ledger.info().validated = true; + return true; +} + // returns Ledgers we have all the nodes for bool LedgerMaster::getFullValidatedRange( diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index f3721156a0c..6b3f42d62df 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -27,7 +27,6 @@ #include #include #include -#include namespace ripple { @@ -156,8 +155,8 @@ fillJsonTx( {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } - const bool validated = RPC::isValidated( - fill.context->ledgerMaster, fill.ledger, fill.context->app); + const bool validated = + fill.context->ledgerMaster.isValidated(fill.ledger); txJson[jss::validated] = validated; if (validated) { diff --git a/src/ripple/rpc/handlers/AMMInfo.cpp b/src/ripple/rpc/handlers/AMMInfo.cpp index 47d3e117e8e..a1be636cafd 100644 --- a/src/ripple/rpc/handlers/AMMInfo.cpp +++ b/src/ripple/rpc/handlers/AMMInfo.cpp @@ -16,6 +16,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== +#include #include #include #include @@ -244,8 +245,7 @@ doAMMInfo(RPC::JsonContext& context) if (!result.isMember(jss::ledger_index) && !result.isMember(jss::ledger_hash)) result[jss::ledger_current_index] = ledger->info().seq; - result[jss::validated] = - RPC::isValidated(context.ledgerMaster, *ledger, context.app); + result[jss::validated] = context.ledgerMaster.isValidated(*ledger); return result; } diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 6ec7efd3d6c..052b8ae9edb 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -37,7 +37,6 @@ #include #include #include -#include #include @@ -195,8 +194,8 @@ getLedgerRange( return status; } - bool validated = RPC::isValidated( - context.ledgerMaster, *ledgerView, context.app); + bool validated = + context.ledgerMaster.isValidated(*ledgerView); if (!validated || ledgerView->info().seq > uValidatedMax || ledgerView->info().seq < uValidatedMin) diff --git a/src/ripple/rpc/handlers/LedgerHandler.cpp b/src/ripple/rpc/handlers/LedgerHandler.cpp index 6b4fc77367b..623cb8d75ac 100644 --- a/src/ripple/rpc/handlers/LedgerHandler.cpp +++ b/src/ripple/rpc/handlers/LedgerHandler.cpp @@ -27,7 +27,6 @@ #include #include #include -#include namespace ripple { namespace RPC { @@ -301,8 +300,7 @@ doLedgerGrpc(RPC::GRPCContext& context) response.set_skiplist_included(true); } - response.set_validated( - RPC::isValidated(context.ledgerMaster, *ledger, context.app)); + response.set_validated(context.ledgerMaster.isValidated(*ledger)); auto end = std::chrono::system_clock::now(); auto duration = diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index db826f57d37..c729cc4956c 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -79,8 +79,8 @@ doTransactionEntry(RPC::JsonContext& context) sttx->getJson(JsonOptions::none, false, {std::ref(hash)}); jvResult[jss::hash] = hash; - bool const validated = RPC::isValidated( - context.ledgerMaster, *lpLedger, context.app); + bool const validated = + context.ledgerMaster.isValidated(*lpLedger); jvResult[jss::validated] = validated; if (validated) diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index a9cc0f9fffe..672095fe950 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -600,59 +600,6 @@ getLedger<>( template Status getLedger<>(std::shared_ptr&, uint256 const&, Context&); -bool -isValidated( - LedgerMaster& ledgerMaster, - ReadView const& ledger, - Application& app) -{ - if (app.config().reporting()) - return true; - - if (ledger.open()) - return false; - - if (ledger.info().validated) - return true; - - auto seq = ledger.info().seq; - try - { - // 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 = - ledgerMaster.walkHashBySeq(seq, InboundLedger::Reason::GENERIC); - - if (!hash || ledger.info().hash != *hash) - { - // This ledger's hash is not the hash of the validated ledger - if (hash) - { - assert(hash->isNonZero()); - uint256 valHash = - app.getRelationalDatabase().getHashByIndex(seq); - if (valHash == ledger.info().hash) - { - // SQL database doesn't match ledger chain - ledgerMaster.clearLedger(seq); - } - } - return false; - } - } - catch (SHAMapMissingNode const& mn) - { - auto stream = app.journal("RPCHandler").warn(); - JLOG(stream) << "Ledger #" << seq << ": " << mn.what(); - return false; - } - - // Mark ledger as validated to save time if we see it again. - ledger.info().validated = true; - return true; -} - // The previous version of the lookupLedger command would accept the // "ledger_index" argument as a string and silently treat it as a request to // return the current ledger which, while not strictly wrong, could cause a lot @@ -693,8 +640,7 @@ lookupLedger( result[jss::ledger_current_index] = info.seq; } - result[jss::validated] = - isValidated(context.ledgerMaster, *ledger, context.app); + result[jss::validated] = context.ledgerMaster.isValidated(*ledger); return Status::OK; } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 516f66fc620..97015f1a35d 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -168,12 +168,6 @@ ledgerFromSpecifier( org::xrpl::rpc::v1::LedgerSpecifier const& specifier, Context& context); -bool -isValidated( - LedgerMaster& ledgerMaster, - ReadView const& ledger, - Application& app); - hash_set parseAccountIds(Json::Value const& jvArray); From 4e92ee4f21f8444cdd6b2bf40aeae1b579f67d8c Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 26 Oct 2023 14:04:40 +0000 Subject: [PATCH 16/34] Store closeTime in LedgerFill --- src/ripple/app/ledger/LedgerToJson.h | 5 +++++ src/ripple/app/ledger/impl/LedgerToJson.cpp | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/ledger/LedgerToJson.h b/src/ripple/app/ledger/LedgerToJson.h index f658583885f..78947ca91d1 100644 --- a/src/ripple/app/ledger/LedgerToJson.h +++ b/src/ripple/app/ledger/LedgerToJson.h @@ -21,8 +21,10 @@ #define RIPPLE_APP_LEDGER_LEDGERTOJSON_H_INCLUDED #include +#include #include #include +#include #include #include #include @@ -41,6 +43,8 @@ struct LedgerFill LedgerEntryType t = ltANY) : ledger(l), options(o), txQueue(std::move(q)), type(t), context(ctx) { + if (context) + closeTime = context->ledgerMaster.getCloseTimeBySeq(ledger.seq()); } enum Options { @@ -58,6 +62,7 @@ struct LedgerFill std::vector txQueue; LedgerEntryType type; RPC::Context* context; + std::optional closeTime; }; /** Given a Ledger and options, fill a Json::Object or Json::Value with a diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 6b3f42d62df..c6cfb7cb5a8 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -162,10 +162,9 @@ fillJsonTx( { txJson[jss::ledger_index] = to_string(fill.ledger.seq()); txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash); - if (auto close_time = - fill.context->ledgerMaster.getCloseTimeBySeq( - fill.ledger.seq())) - txJson[jss::close_time_iso] = to_string_iso(*close_time); + if (fill.closeTime) + txJson[jss::close_time_iso] = + to_string_iso(*fill.closeTime); } } else From 9f1d544dcbae542d9d1d11ac43d36c5e168eba9a Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 30 Oct 2023 16:09:17 +0000 Subject: [PATCH 17/34] Time formatting fix --- src/ripple/basics/chrono.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ripple/basics/chrono.h b/src/ripple/basics/chrono.h index 21353e0873a..ea82f928b7e 100644 --- a/src/ripple/basics/chrono.h +++ b/src/ripple/basics/chrono.h @@ -28,7 +28,9 @@ #include #include +#include #include +#include namespace ripple { @@ -91,16 +93,17 @@ std::string to_string_iso(date::sys_time tp) { using namespace std::chrono; - return date::format("%FT%H:%M:%OSZ", tp); + return date::format("%FT%TZ", tp); } inline std::string to_string_iso(NetClock::time_point tp) { // 2000-01-01 00:00:00 UTC is 946684800s from 1970-01-01 00:00:00 UTC - using namespace std::chrono; - return to_string_iso( - system_clock::time_point{tp.time_since_epoch() + epoch_offset}); + // Note, NetClock::duration is seconds, as checked by static_assert + static_assert(std::is_same_v>); + return to_string_iso(date::sys_time{ + tp.time_since_epoch() + epoch_offset}); } /** A clock for measuring elapsed time. From e3860938403f1647359e23144abd2db7de98ec7a Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Thu, 26 Oct 2023 17:51:41 -0700 Subject: [PATCH 18/34] additional tests for Subscribe unit tests --- src/test/rpc/Subscribe_test.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index bb419cbb4a1..ea815eb27ed 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -347,7 +347,12 @@ class Subscribe_test : public beast::unit_test::suite == Account("alice").human() && jv[jss::close_time_iso] // == "2000-01-01T00:00:10Z" && - jv[jss::validated] == true && // + jv[jss::validated] == true && // + jv[jss::ledger_hash] == + "0F1A9E0C109ADEF6DA2BDE19217C12BBEC57174CBDBD212B0EBDC1CEDB" + "853185" && // + !jv[jss::inLedger] && + jv[jss::ledger_index] == 3 && // jv[jss::tx_json][jss::TransactionType] // == jss::Payment && jv[jss::tx_json][jss::DeliverMax] // From c477eb48a43cfe1803369f7d5146901b85790e03 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 30 Oct 2023 17:35:33 +0000 Subject: [PATCH 19/34] Improved comments --- src/ripple/protocol/STTx.h | 10 ++++++---- src/ripple/protocol/impl/STTx.cpp | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index 195f3a1de3f..505b502d07b 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -109,10 +109,12 @@ class STTx final : public STObject, public CountedObject Json::Value getJson(JsonOptions options) const override; - /// If `hash` is set, will store hash inside the provided string. Otherwise - /// hash will be stored as nested jss::hash element inside the returned JSON - /// Additionally, if `hash` is set and `binary` is true, will not create - /// nested jss::tx for binary hex; instead will return it as JSON string + /** + If `hash` is set, will store transaction id inside the provided string. + Otherwise it will be stored as nested jss::hash element inside the returned + JSON. Additionally, if `hash` is set and `binary` is true, will not create + nested jss::tx for binary hex; instead will return it as JSON string + */ Json::Value getJson( JsonOptions options, diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index abcd01049b0..95d859a48e9 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -260,7 +260,10 @@ STTx::getJson( Json::Value ret = strHex(s.peekData()); return ret; } - return STObject::getJson(JsonOptions::none); // Yes, want `none` + // 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); } std::string const& From c81beceacf07fc3ce57793270ce5e303bdcc9bc8 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 30 Oct 2023 17:45:49 +0000 Subject: [PATCH 20/34] Rename mInLedger to mLedgerIndex --- src/ripple/app/misc/Transaction.h | 8 ++++---- src/ripple/app/misc/impl/Transaction.cpp | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index 24849994161..f1195d7f87a 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -99,13 +99,13 @@ class Transaction : public std::enable_shared_from_this, LedgerIndex getLedger() const { - return mInLedger; + return mLedgerIndex; } bool isValidated() const { - return mInLedger != 0; + return mLedgerIndex != 0; } TransStatus @@ -138,7 +138,7 @@ class Transaction : public std::enable_shared_from_this, void setLedger(LedgerIndex ledger) { - mInLedger = ledger; + mLedgerIndex = ledger; } /** @@ -391,7 +391,7 @@ class Transaction : public std::enable_shared_from_this, uint256 mTransactionID; - LedgerIndex mInLedger = 0; + LedgerIndex mLedgerIndex = 0; TransStatus mStatus = INVALID; TER mResult = temUNCERTAIN; bool mApplying = false; diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index c6cd01b7096..1340501a95a 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -62,7 +62,7 @@ void Transaction::setStatus(TransStatus ts, std::uint32_t lseq) { mStatus = ts; - mInLedger = lseq; + mLedgerIndex = lseq; } TransStatus @@ -173,16 +173,16 @@ Transaction::getJson( { Json::Value ret(mTransaction->getJson(JsonOptions::none, binary, hash)); - if (mInLedger) + if (mLedgerIndex) { if (showInLedger) - ret[jss::inLedger] = mInLedger; // Deprecated. + ret[jss::inLedger] = mLedgerIndex; // Deprecated. - ret[jss::ledger_index] = mInLedger; + ret[jss::ledger_index] = mLedgerIndex; if (options == JsonOptions::include_date) { - auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mInLedger); + auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex); if (ct) ret[jss::date] = ct->time_since_epoch().count(); } From ef1276c32bdb42bd2c7b71fc6a968a6477b93269 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 30 Oct 2023 20:59:58 +0000 Subject: [PATCH 21/34] Minor fixes --- src/ripple/app/ledger/impl/LedgerMaster.cpp | 2 +- src/ripple/app/misc/NetworkOPs.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 60897504c91..0e20da46305 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -607,7 +607,7 @@ bool LedgerMaster::isValidated(ReadView const& ledger) { if (app_.config().reporting()) - return haveLedger(ledger.seq()); // TODO Is this correct ? + return true; // Reporting mode only supports validated ledger if (ledger.open()) return false; diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index e624eef963c..7413a4c8648 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3124,8 +3124,7 @@ NetworkOPsImp::transJson( jvObj[jss::transaction][jss::date] = ledger->info().closeTime.time_since_epoch().count(); jvObj[jss::validated] = true; - if (auto close_time = m_ledgerMaster.getCloseTimeBySeq(ledger->seq())) - jvObj[jss::close_time_iso] = to_string_iso(*close_time); + jvObj[jss::close_time_iso] = to_string_iso(ledger->info().closeTime); // WRITEME: Put the account next seq here } @@ -3174,10 +3173,11 @@ NetworkOPsImp::transJson( assert(index < MultiApiJson::size); if (index != lastIndex) { + lastIndex = index; + Json::Value& jvTx = multiObj.val[index]; RPC::insertDeliverMax( jvTx[jss::transaction], transaction->getTxnType(), apiVersion); - lastIndex = index; if (apiVersion > 1) { From 54f1d60573ab1fe6add9df944216ea2869bbd1a0 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 30 Oct 2023 21:19:40 +0000 Subject: [PATCH 22/34] Set ledger_hash on closed ledger, even if not validated --- src/ripple/app/ledger/impl/LedgerToJson.cpp | 4 +++- src/ripple/app/misc/NetworkOPs.cpp | 4 +++- src/ripple/rpc/handlers/Tx.cpp | 11 +++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index c6cfb7cb5a8..796d85779cc 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -155,13 +155,15 @@ fillJsonTx( {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } + 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()); - txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash); if (fill.closeTime) txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 7413a4c8648..b52dce8c22a 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3117,10 +3117,12 @@ NetworkOPsImp::transJson( jvObj[jss::meta], *ledger, transaction, meta->get()); } + if (!ledger->open()) + jvObj[jss::ledger_hash] = to_string(ledger->info().hash); + if (validated) { jvObj[jss::ledger_index] = ledger->info().seq; - jvObj[jss::ledger_hash] = to_string(ledger->info().hash); jvObj[jss::transaction][jss::date] = ledger->info().closeTime.time_since_epoch().count(); jvObj[jss::validated] = true; diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 39f49b50f7a..96c0548a202 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -267,6 +267,9 @@ doTxHelp(RPC::Context& context, TxArgs args) std::shared_ptr ledger = context.ledgerMaster.getLedgerBySeq(txn->getLedger()); + if (ledger && !ledger->open()) + result.ledgerHash = ledger->info().hash; + if (ledger && meta) { if (args.binary) @@ -287,7 +290,6 @@ doTxHelp(RPC::Context& context, TxArgs args) uint32_t lgrSeq = ledger->info().seq; uint32_t txnIdx = meta->getAsObject().getFieldU32(sfTransactionIndex); uint32_t netID = context.app.config().NETWORK_ID; - result.ledgerHash = ledger->info().hash; if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL) result.ctid = @@ -341,13 +343,14 @@ populateJsonResponse( sttx->getTxnType(), context.apiVersion); } + + if (result.ledgerHash) + response[jss::ledger_hash] = to_string(*result.ledgerHash); + response[jss::hash] = hash; if (result.validated) { response[jss::ledger_index] = result.txn->getLedger(); - if (result.ledgerHash) - response[jss::ledger_hash] = to_string(*result.ledgerHash); - if (result.closeTime) response[jss::close_time_iso] = to_string_iso(*result.closeTime); From 0d8673cc544c388e26f03bb9cca2f02115073975 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 31 Oct 2023 17:12:53 +0000 Subject: [PATCH 23/34] Update API-CHANGELOG.md --- API-CHANGELOG.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index b3c9b18d2f8..8c4b7a103d6 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -141,6 +141,31 @@ 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 JSON transaction element in V2 + +In API version 2, JSON elements for transaction output have been changed and made consistent for all methods which output transactions: + +- JSON transaction element is named `tx_json` +- Binary transaction element is named `tx_blob` +- JSON transaction metadata element is named `meta` +- Binary transaction metadata element is named `meta_blob` + +Additionally, these elements are now consistently available next to `tx_json` (i.e. sibling elements), where possible: + +- `hash` - Transaction ID. This data was stored inside transaction output in API version 1, but in API version 2 is a sibling element. +- `ledger_index` - Ledger index (only set on validated ledgers) +- `ledger_hash` - Ledger hash (only set on closed or validated ledgers) +- `close_time_iso` - Ledger close time expressed in ISO 8601 time format (only set on validated ledgers) +- `validated` - Bool element set to `true` if the transaction is in a validated ledger, otherwise `false` + +This change affects the following methods: + +- `tx` - Transaction data moved into element `tx_json` (was inline inside `result`) or, if binary output was requested, moved from `tx` to `tx_blob`. Renamed binary transaction metadata element (if it was requsted) from `meta` to `meta_blob`. Changed location of `hash` and added new elements +- `account_tx` - Renamed transaction element from `tx` to `tx_json`. Renamed binary transaction metadata element (if it was requsted) from `meta` to `meta_blob`. Changed location of `hash` and added new elements +- `transaction_entry` - Renamed transaction metadata element from `metadata` to `meta`. Changed location of `hash` and added new elements +- `subscribe` - Renamed transaction element from `transaction` to `tx_json`. Changed location of `hash` and added new elements +- `sign`, `sign_for`, `submit` and `submit_multisigned` - Changed location of `hash` element. + #### 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) From cdb33845b27722106c3e9287e227b1b6b0739d86 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 31 Oct 2023 17:27:57 +0000 Subject: [PATCH 24/34] Add ledger_hash, ledger_index to transaction_entry --- src/ripple/rpc/handlers/TransactionEntry.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index c729cc4956c..c0cd3fe46c8 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -79,12 +79,17 @@ doTransactionEntry(RPC::JsonContext& context) sttx->getJson(JsonOptions::none, false, {std::ref(hash)}); jvResult[jss::hash] = hash; + if (!lpLedger->open()) + jvResult[jss::ledger_hash] = to_string( + context.ledgerMaster.getHashBySeq(lpLedger->seq())); + bool const validated = context.ledgerMaster.isValidated(*lpLedger); jvResult[jss::validated] = validated; if (validated) { + jvResult[jss::ledger_index] = lpLedger->seq(); if (auto closeTime = context.ledgerMaster.getCloseTimeBySeq( lpLedger->seq())) jvResult[jss::close_time_iso] = From 597339025b6e8c5d4a8e458ed2ed3eadd97ab991 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 1 Nov 2023 15:55:02 +0000 Subject: [PATCH 25/34] Fix validated and close_time_iso in account_tx --- src/ripple/rpc/handlers/AccountTx.cpp | 11 +++++---- src/test/rpc/AccountTx_test.cpp | 35 +++++++++++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 052b8ae9edb..4514f8e2a3b 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -340,6 +340,12 @@ populateJsonResponse( jvObj[jss::ledger_hash] = to_string(context.ledgerMaster.getHashBySeq( txn->getLedger())); + + if (auto closeTime = + context.ledgerMaster.getCloseTimeBySeq( + txn->getLedger())) + jvObj[jss::close_time_iso] = + to_string_iso(*closeTime); } else jvObj[json_tx] = txn->getJson( @@ -356,11 +362,6 @@ populateJsonResponse( insertDeliveredAmount( jvObj[jss::meta], context, txn, *txnMeta); insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); - if (auto closeTime = - context.ledgerMaster.getCloseTimeBySeq( - txnMeta->getIndex())) - jvObj[jss::close_time_iso] = - to_string_iso(*closeTime); } } } diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 3cfcda75847..3834d623dca 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -137,17 +137,36 @@ class AccountTx_test : public beast::unit_test::suite j[jss::result][jss::transactions][1u][jss::tx] [jss::DeliverMax]); case 2: - return j.isMember(jss::result) && + if (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_json] - [jss::TransactionType] == jss::AccountSet) && - (j[jss::result][jss::transactions][1u][jss::tx_json] - [jss::TransactionType] == jss::Payment) && - (j[jss::result][jss::transactions][1u][jss::tx_json] - [jss::DeliverMax] == "10000000010") && - (!j[jss::result][jss::transactions][1u][jss::tx_json] - .isMember(jss::Amount)); + [jss::TransactionType] == jss::AccountSet)) + { + auto const& payment = + j[jss::result][jss::transactions][1u]; + + return (payment.isMember(jss::tx_json)) && + (payment[jss::tx_json][jss::TransactionType] == + jss::Payment) && + (payment[jss::tx_json][jss::DeliverMax] == + "10000000010") && + (!payment[jss::tx_json].isMember(jss::Amount)) && + (!payment[jss::tx_json].isMember(jss::hash)) && + (payment[jss::hash] == + "9F3085D85F472D1CC29627F260DF68EDE59D42D1D0C33E345" + "ECF0D4CE981D0A8") && + (payment[jss::validated] == true) && + (payment[jss::ledger_index] == 3) && + (payment[jss::ledger_hash] == + "5476DCD816EA04CBBA57D47BBF1FC58A5217CC93A5ADD79CB" + "580A5AFDD727E33") && + (payment[jss::close_time_iso] == + "2000-01-01T00:00:10Z"); + } + else + return false; + default: return false; } From 17e6588f56dd04f86a30e98f649fc6519c3720c4 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 12:06:30 +0000 Subject: [PATCH 26/34] Fix typos --- API-CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 8c4b7a103d6..e5d30b3a3d6 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -160,8 +160,8 @@ Additionally, these elements are now consistently available next to `tx_json` (i This change affects the following methods: -- `tx` - Transaction data moved into element `tx_json` (was inline inside `result`) or, if binary output was requested, moved from `tx` to `tx_blob`. Renamed binary transaction metadata element (if it was requsted) from `meta` to `meta_blob`. Changed location of `hash` and added new elements -- `account_tx` - Renamed transaction element from `tx` to `tx_json`. Renamed binary transaction metadata element (if it was requsted) from `meta` to `meta_blob`. Changed location of `hash` and added new elements +- `tx` - Transaction data moved into element `tx_json` (was inline inside `result`) or, if binary output was requested, moved from `tx` to `tx_blob`. Renamed binary transaction metadata element (if it was requested) from `meta` to `meta_blob`. Changed location of `hash` and added new elements +- `account_tx` - Renamed transaction element from `tx` to `tx_json`. Renamed binary transaction metadata element (if it was requested) from `meta` to `meta_blob`. Changed location of `hash` and added new elements - `transaction_entry` - Renamed transaction metadata element from `metadata` to `meta`. Changed location of `hash` and added new elements - `subscribe` - Renamed transaction element from `transaction` to `tx_json`. Changed location of `hash` and added new elements - `sign`, `sign_for`, `submit` and `submit_multisigned` - Changed location of `hash` element. From 95b6055ac487910b3eb2ba5d6194b94c751042b0 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 17:14:53 +0000 Subject: [PATCH 27/34] Improve getJson for Transaction and STTx --- src/ripple/app/ledger/impl/LedgerToJson.cpp | 6 ++-- src/ripple/app/misc/NetworkOPs.cpp | 4 +-- src/ripple/app/misc/Transaction.h | 7 ++--- src/ripple/app/misc/impl/Transaction.cpp | 21 ++++++++------ src/ripple/protocol/STBase.h | 30 +++++++++++++++++++- src/ripple/protocol/STTx.h | 7 ++--- src/ripple/protocol/impl/STTx.cpp | 12 ++++---- src/ripple/rpc/handlers/AccountTx.cpp | 15 +++++----- src/ripple/rpc/handlers/TransactionEntry.cpp | 5 ++-- src/ripple/rpc/handlers/Tx.cpp | 17 +++++------ src/ripple/rpc/impl/TransactionSign.cpp | 11 ++++--- 11 files changed, 78 insertions(+), 57 deletions(-) diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp index 796d85779cc..2ba452446b1 100644 --- a/src/ripple/app/ledger/impl/LedgerToJson.cpp +++ b/src/ripple/app/ledger/impl/LedgerToJson.cpp @@ -133,12 +133,10 @@ fillJsonTx( { if (fill.context->apiVersion > 1) { - std::string hash; copyFrom( txJson[jss::tx_json], - txn->getJson( - JsonOptions::none, false, {std::optional(std::ref(hash))})); - txJson[jss::hash] = hash; + 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); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index b52dce8c22a..b35a56b5ff9 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3106,9 +3106,9 @@ 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 hash = {}; + std::string const hash = to_string(transaction->getTransactionID()); jvObj[jss::transaction] = - transaction->getJson(JsonOptions::none, false, {std::ref(hash)}); + transaction->getJson(JsonOptions::disable_API_prior_V2, false); if (meta) { diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index f1195d7f87a..c3bad22f13d 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -307,11 +308,7 @@ class Transaction : public std::enable_shared_from_this, /// Same as similar overload for STTx::getJson Json::Value - getJson( - JsonOptions options, - bool binary = false, - bool showInLedger = false, - std::optional> hash = {}) const; + getJson(JsonOptions options, bool binary = false) const; // Information used to locate a transaction. // Contains a nodestore hash and ledger sequence pair if the transaction was diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index 1340501a95a..277803c856f 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -165,22 +165,25 @@ Transaction::load( // options 1 to include the date of the transaction Json::Value -Transaction::getJson( - JsonOptions options, - bool binary, - bool showInLedger, - std::optional> hash) const +Transaction::getJson(JsonOptions options, bool binary) const { - Json::Value ret(mTransaction->getJson(JsonOptions::none, binary, hash)); + // Note, we explicitly suppress `include_date` option here + Json::Value ret( + mTransaction->getJson(options % JsonOptions::include_date, binary)); if (mLedgerIndex) { - if (showInLedger) - ret[jss::inLedger] = mLedgerIndex; // Deprecated. + if (!(options & JsonOptions::disable_API_prior_V2)) + { + // Behaviour before API version 2 + ret[jss::inLedger] = mLedgerIndex; + } + // TODO: disable_API_prior_V3 to disable output of both `date` and + // `ledger_index` elements (taking precedence over include_date) ret[jss::ledger_index] = mLedgerIndex; - if (options == JsonOptions::include_date) + if (options & JsonOptions::include_date) { auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex); if (ct) diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h index 914a3e0f60b..09fa3813ed8 100644 --- a/src/ripple/protocol/STBase.h +++ b/src/ripple/protocol/STBase.h @@ -31,7 +31,35 @@ #include namespace ripple { -enum class JsonOptions { none = 0, include_date = 1 }; +/// Note, should be treated as flags that can be | and & +enum class JsonOptions : unsigned int { + none = 0, + include_date = 1, + disable_API_prior_V2 = 2 +}; + +/// Return: JsonOptions union of lh and rh +[[nodiscard]] constexpr JsonOptions +operator|(JsonOptions lh, JsonOptions rh) noexcept +{ + return JsonOptions( + static_cast(lh) | static_cast(rh)); +} + +/// Return: JsonOptions similar to lh, but with rh disabled (i.e. "remainder") +[[nodiscard]] constexpr JsonOptions +operator%(JsonOptions lh, JsonOptions rh) noexcept +{ + return JsonOptions( + static_cast(lh) & ~static_cast(rh)); +} + +/// Return: true if lh contains rh +[[nodiscard]] constexpr bool +operator&(JsonOptions lh, JsonOptions rh) noexcept +{ + return (static_cast(lh) & static_cast(rh)) != 0; +} namespace detail { class STVar; diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index 505b502d07b..c24cc076af3 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -24,11 +24,13 @@ #include #include #include +#include #include #include #include #include #include + #include namespace ripple { @@ -116,10 +118,7 @@ class STTx final : public STObject, public CountedObject nested jss::tx for binary hex; instead will return it as JSON string */ Json::Value - getJson( - JsonOptions options, - bool binary, - std::optional> hash = {}) const; + getJson(JsonOptions options, bool binary) const; void sign(PublicKey const& publicKey, SecretKey const& secretKey); diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 95d859a48e9..9ee93a05b30 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -34,6 +34,7 @@ #include #include #include + #include #include #include @@ -234,13 +235,11 @@ Json::Value STTx::getJson(JsonOptions) const } Json::Value -STTx::getJson( - JsonOptions options, - bool binary, - std::optional> hash) const +STTx::getJson(JsonOptions options, bool binary) const { - if (!hash) // Old behaviour - default because `hash = {}` in declaration + if (!(options & JsonOptions::disable_API_prior_V2)) { + // Behaviour before API version 2 if (binary) { Json::Value ret; @@ -252,8 +251,7 @@ STTx::getJson( return getJson(options); } - // Since `hash` is set, do not populate `hash` inside JSON output - hash->get() = to_string(getTransactionID()); + // API version 2 behaviour if (binary) { Serializer s = STObject::getSerializer(); diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 4514f8e2a3b..85faf02e918 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -329,13 +329,12 @@ populateJsonResponse( (context.apiVersion > 1 ? jss::tx_json : jss::tx); if (context.apiVersion > 1) { - std::string hash; jvObj[json_tx] = txn->getJson( - JsonOptions::include_date, - false, - false, - {std::ref(hash)}); - jvObj[jss::hash] = hash; + JsonOptions::include_date | + JsonOptions::disable_API_prior_V2, + false); + jvObj[jss::hash] = to_string( + txn->getSTransaction()->getTransactionID()); jvObj[jss::ledger_index] = txn->getLedger(); jvObj[jss::ledger_hash] = to_string(context.ledgerMaster.getHashBySeq( @@ -348,8 +347,8 @@ populateJsonResponse( to_string_iso(*closeTime); } else - jvObj[json_tx] = txn->getJson( - JsonOptions::include_date, false, true); + jvObj[json_tx] = + txn->getJson(JsonOptions::include_date); auto const& sttx = txn->getSTransaction(); RPC::insertDeliverMax( diff --git a/src/ripple/rpc/handlers/TransactionEntry.cpp b/src/ripple/rpc/handlers/TransactionEntry.cpp index c0cd3fe46c8..6d157891d1c 100644 --- a/src/ripple/rpc/handlers/TransactionEntry.cpp +++ b/src/ripple/rpc/handlers/TransactionEntry.cpp @@ -74,10 +74,9 @@ doTransactionEntry(RPC::JsonContext& context) { if (context.apiVersion > 1) { - std::string hash; jvResult[jss::tx_json] = - sttx->getJson(JsonOptions::none, false, {std::ref(hash)}); - jvResult[jss::hash] = hash; + sttx->getJson(JsonOptions::disable_API_prior_V2); + jvResult[jss::hash] = to_string(sttx->getTransactionID()); if (!lpLedger->open()) jvResult[jss::ledger_hash] = to_string( diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 96c0548a202..fcdb1372f65 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -33,6 +33,7 @@ #include #include #include + #include #include @@ -330,14 +331,13 @@ populateJsonResponse( auto const& sttx = result.txn->getSTransaction(); if (context.apiVersion > 1) { - std::string hash; + constexpr auto optionsJson = + JsonOptions::include_date | JsonOptions::disable_API_prior_V2; if (args.binary) - response[jss::tx_blob] = result.txn->getJson( - JsonOptions::include_date, true, false, {std::ref(hash)}); + response[jss::tx_blob] = result.txn->getJson(optionsJson, true); else { - response[jss::tx_json] = result.txn->getJson( - JsonOptions::include_date, false, false, {std::ref(hash)}); + response[jss::tx_json] = result.txn->getJson(optionsJson); RPC::insertDeliverMax( response[jss::tx_json], sttx->getTxnType(), @@ -347,7 +347,8 @@ populateJsonResponse( if (result.ledgerHash) response[jss::ledger_hash] = to_string(*result.ledgerHash); - response[jss::hash] = hash; + response[jss::hash] = + to_string(result.txn->getSTransaction()->getTransactionID()); if (result.validated) { response[jss::ledger_index] = result.txn->getLedger(); @@ -358,8 +359,8 @@ populateJsonResponse( } else { - response = result.txn->getJson( - JsonOptions::include_date, args.binary, true); + response = + result.txn->getJson(JsonOptions::include_date, args.binary); if (!args.binary) RPC::insertDeliverMax( response, sttx->getTxnType(), context.apiVersion); diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index c5ad5d43957..7c619290d7b 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -652,14 +652,13 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) { if (apiVersion > 1) { - std::string hash = {}; - jvResult[jss::tx_json] = tpTrans->getJson( - JsonOptions::none, false, false, {std::ref(hash)}); - jvResult[jss::hash] = hash; + jvResult[jss::tx_json] = + tpTrans->getJson(JsonOptions::disable_API_prior_V2); + jvResult[jss::hash] = + to_string(tpTrans->getSTransaction()->getTransactionID()); } else - jvResult[jss::tx_json] = - tpTrans->getJson(JsonOptions::none, false, true); + jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); jvResult[jss::tx_blob] = strHex(tpTrans->getSTransaction()->getSerializer().peekData()); From b2a8a2dc27bfc4cc7de9411915adf8a7f0519a43 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 17:46:54 +0000 Subject: [PATCH 28/34] Minor improvements --- src/ripple/rpc/handlers/Tx.cpp | 2 ++ src/test/rpc/TransactionEntry_test.cpp | 2 +- src/test/rpc/Transaction_test.cpp | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index fcdb1372f65..1f885af2f3d 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -344,6 +344,8 @@ populateJsonResponse( context.apiVersion); } + // Note, result.ledgerHash is only set in a closed or validated + // ledger - as seen in `doTxHelp` and `doTxPostgres` if (result.ledgerHash) response[jss::ledger_hash] = to_string(*result.ledgerHash); diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index 5b25d67d3a8..ae988343a6c 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -174,7 +174,7 @@ class TransactionEntry_test : public beast::unit_test::suite BEAST_EXPECT(resIndex[jss::validated] == true); BEAST_EXPECT(!resIndex[jss::tx_json].isMember(jss::Amount)); - if (!close_time_iso.empty()) + if (BEAST_EXPECT(!close_time_iso.empty())) BEAST_EXPECT( resIndex[jss::close_time_iso] == close_time_iso); } diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index 22d3ed2e29b..36de520187b 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -747,6 +747,11 @@ class Transaction_test : public beast::unit_test::suite result[jss::result][jss::hash] == to_string(txn->getTransactionID())); BEAST_EXPECT(result[jss::result][jss::validated] == true); + BEAST_EXPECT(result[jss::result][jss::ledger_index] == 4); + BEAST_EXPECT( + result[jss::result][jss::ledger_hash] == + "B41882E20F0EC6228417D28B9AE0F33833645D35F6799DFB782AC97FC4BB51" + "D2"); } for (auto memberIt = expected.begin(); memberIt != expected.end(); From 94c16d8539a6a02e9ee6930653996412c39e24ef Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 20:26:09 +0000 Subject: [PATCH 29/34] Replace class enum JsonOptions with struct We may consider turning this into a general-purpose template and using it elsewhere --- src/ripple/app/misc/impl/Transaction.cpp | 2 +- src/ripple/protocol/STBase.h | 77 +++++++++++++++--------- src/ripple/protocol/STObject.h | 1 + 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/ripple/app/misc/impl/Transaction.cpp b/src/ripple/app/misc/impl/Transaction.cpp index 277803c856f..c38a6b7438f 100644 --- a/src/ripple/app/misc/impl/Transaction.cpp +++ b/src/ripple/app/misc/impl/Transaction.cpp @@ -169,7 +169,7 @@ Transaction::getJson(JsonOptions options, bool binary) const { // Note, we explicitly suppress `include_date` option here Json::Value ret( - mTransaction->getJson(options % JsonOptions::include_date, binary)); + mTransaction->getJson(options & ~JsonOptions::include_date, binary)); if (mLedgerIndex) { diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h index 09fa3813ed8..ceb7fe3e7bc 100644 --- a/src/ripple/protocol/STBase.h +++ b/src/ripple/protocol/STBase.h @@ -32,34 +32,57 @@ namespace ripple { /// Note, should be treated as flags that can be | and & -enum class JsonOptions : unsigned int { - none = 0, - include_date = 1, - disable_API_prior_V2 = 2 -}; - -/// Return: JsonOptions union of lh and rh -[[nodiscard]] constexpr JsonOptions -operator|(JsonOptions lh, JsonOptions rh) noexcept -{ - return JsonOptions( - static_cast(lh) | static_cast(rh)); -} - -/// Return: JsonOptions similar to lh, but with rh disabled (i.e. "remainder") -[[nodiscard]] constexpr JsonOptions -operator%(JsonOptions lh, JsonOptions rh) noexcept +struct JsonOptions { - return JsonOptions( - static_cast(lh) & ~static_cast(rh)); -} - -/// Return: true if lh contains rh -[[nodiscard]] constexpr bool -operator&(JsonOptions lh, JsonOptions rh) noexcept -{ - return (static_cast(lh) & static_cast(rh)) != 0; -} + using underlying_t = unsigned int; + 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 + }; + + constexpr JsonOptions(underlying_t v) noexcept : value(v) + { + } + + [[nodiscard]] constexpr explicit operator underlying_t() const noexcept + { + return value; + } + [[nodiscard]] constexpr explicit operator bool() const noexcept + { + return value != 0u; + } + [[nodiscard]] constexpr auto friend + operator==(JsonOptions lh, JsonOptions rh) noexcept -> bool = default; + [[nodiscard]] constexpr auto friend + operator!=(JsonOptions lh, JsonOptions rh) noexcept -> bool = default; + + /// Returns JsonOptions union of lh and rh + [[nodiscard]] constexpr JsonOptions friend + operator|(JsonOptions lh, JsonOptions rh) noexcept + { + return {lh.value | rh.value}; + } + + /// Returns JsonOptions intersection of lh and rh + [[nodiscard]] constexpr JsonOptions friend + operator&(JsonOptions lh, JsonOptions rh) noexcept + { + return {lh.value & rh.value}; + } + + /// Returns JsonOptions binary negation, can be used with & (above) for set + /// difference e.g. `(options & ~JsonOptions::include_date)` + [[nodiscard]] constexpr JsonOptions friend + operator~(JsonOptions v) noexcept + { + return {~v.value & static_cast(_all)}; + } +}; namespace detail { class STVar; diff --git a/src/ripple/protocol/STObject.h b/src/ripple/protocol/STObject.h index 19d4b264734..3e3862bf6c8 100644 --- a/src/ripple/protocol/STObject.h +++ b/src/ripple/protocol/STObject.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include From 7770bc511c8f4b1b68513133ed1e4cf4b5ef7860 Mon Sep 17 00:00:00 2001 From: Chenna Keshava Date: Thu, 2 Nov 2023 13:16:44 -0700 Subject: [PATCH 30/34] simplify the extraction of transactionID from Transaction object --- src/ripple/rpc/handlers/AccountTx.cpp | 3 +-- src/ripple/rpc/handlers/Tx.cpp | 3 +-- src/ripple/rpc/impl/TransactionSign.cpp | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 85faf02e918..005d2053b99 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -333,8 +333,7 @@ populateJsonResponse( JsonOptions::include_date | JsonOptions::disable_API_prior_V2, false); - jvObj[jss::hash] = to_string( - txn->getSTransaction()->getTransactionID()); + jvObj[jss::hash] = to_string(txn->getID()); jvObj[jss::ledger_index] = txn->getLedger(); jvObj[jss::ledger_hash] = to_string(context.ledgerMaster.getHashBySeq( diff --git a/src/ripple/rpc/handlers/Tx.cpp b/src/ripple/rpc/handlers/Tx.cpp index 1f885af2f3d..0237fef22ac 100644 --- a/src/ripple/rpc/handlers/Tx.cpp +++ b/src/ripple/rpc/handlers/Tx.cpp @@ -349,8 +349,7 @@ populateJsonResponse( if (result.ledgerHash) response[jss::ledger_hash] = to_string(*result.ledgerHash); - response[jss::hash] = - to_string(result.txn->getSTransaction()->getTransactionID()); + response[jss::hash] = to_string(result.txn->getID()); if (result.validated) { response[jss::ledger_index] = result.txn->getLedger(); diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 7c619290d7b..48a9c66d81c 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -654,8 +654,7 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) { jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::disable_API_prior_V2); - jvResult[jss::hash] = - to_string(tpTrans->getSTransaction()->getTransactionID()); + jvResult[jss::hash] = to_string(tpTrans->getID()); } else jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); From b5b1118619b59cd8152bd5376f6fe1a62d250a9e Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 21:14:37 +0000 Subject: [PATCH 31/34] Remove obsolete comments --- src/ripple/app/misc/Transaction.h | 1 - src/ripple/protocol/STTx.h | 6 ------ 2 files changed, 7 deletions(-) diff --git a/src/ripple/app/misc/Transaction.h b/src/ripple/app/misc/Transaction.h index c3bad22f13d..36815ba0aa0 100644 --- a/src/ripple/app/misc/Transaction.h +++ b/src/ripple/app/misc/Transaction.h @@ -306,7 +306,6 @@ class Transaction : public std::enable_shared_from_this, validatedLedger, fee, accountSeq, availableSeq); } - /// Same as similar overload for STTx::getJson Json::Value getJson(JsonOptions options, bool binary = false) const; diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h index c24cc076af3..84524760d6d 100644 --- a/src/ripple/protocol/STTx.h +++ b/src/ripple/protocol/STTx.h @@ -111,12 +111,6 @@ class STTx final : public STObject, public CountedObject Json::Value getJson(JsonOptions options) const override; - /** - If `hash` is set, will store transaction id inside the provided string. - Otherwise it will be stored as nested jss::hash element inside the returned - JSON. Additionally, if `hash` is set and `binary` is true, will not create - nested jss::tx for binary hex; instead will return it as JSON string - */ Json::Value getJson(JsonOptions options, bool binary) const; From 942e2096ea6be0044e66d8cf58378cb0895ad2a4 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Thu, 2 Nov 2023 21:26:05 +0000 Subject: [PATCH 32/34] Unconditionally set validated in account_tx output --- src/ripple/rpc/handlers/AccountTx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 005d2053b99..addd9ea0f39 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -324,6 +324,7 @@ populateJsonResponse( if (txn) { Json::Value& jvObj = jvTxns.append(Json::objectValue); + jvObj[jss::validated] = true; auto const json_tx = (context.apiVersion > 1 ? jss::tx_json : jss::tx); @@ -356,7 +357,6 @@ populateJsonResponse( { jvObj[jss::meta] = txnMeta->getJson(JsonOptions::include_date); - jvObj[jss::validated] = true; insertDeliveredAmount( jvObj[jss::meta], context, txn, *txnMeta); insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); From 53e384fdcf82e988620f04739c1c40c75d1c5756 Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Fri, 3 Nov 2023 13:37:29 +0000 Subject: [PATCH 33/34] 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& From d0060f84934827218b5816e9276406d11c0fe00f Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Mon, 6 Nov 2023 12:37:19 +0000 Subject: [PATCH 34/34] Minor fixes --- src/ripple/app/misc/NetworkOPs.cpp | 8 +- src/ripple/protocol/impl/STTx.cpp | 6 +- src/ripple/rpc/handlers/AccountTx.cpp | 3 + src/test/rpc/Subscribe_test.cpp | 6 +- src/test/rpc/TransactionEntry_test.cpp | 168 ++++++++++++++----------- src/test/rpc/Transaction_test.cpp | 4 +- 6 files changed, 110 insertions(+), 85 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 2a4968481bc..abb4acf029e 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -3103,7 +3103,7 @@ NetworkOPsImp::transJson( transResultInfo(result, sToken, sHuman); jvObj[jss::type] = "transaction"; - // NOTE jvObj which is not a finished object for either API version. After + // NOTE jvObj 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. jvObj[jss::transaction] = @@ -3162,13 +3162,13 @@ NetworkOPsImp::transJson( 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 + // Last valid (possibly beta) API ver. must match last index in MultiApiJson static_assert( - apiVersionSelector(RPC::apiBetaVersion)() + 1 // + apiVersionSelector(RPC::apiMaximumValidVersion)() + 1 // == MultiApiJson::size); for (unsigned apiVersion = RPC::apiMinimumSupportedVersion, lastIndex = MultiApiJson::size; - apiVersion <= RPC::apiBetaVersion; + apiVersion <= RPC::apiMaximumValidVersion; ++apiVersion) { unsigned const index = apiVersionSelector(apiVersion)(); diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 8106e997f3a..51fb11ad761 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -227,10 +227,12 @@ STTx::checkSign( return Unexpected("Internal signature check failure."); } -Json::Value STTx::getJson(JsonOptions) const +Json::Value +STTx::getJson(JsonOptions options) const { Json::Value ret = STObject::getJson(JsonOptions::none); - ret[jss::hash] = to_string(getTransactionID()); + if (!(options & JsonOptions::disable_API_prior_V2)) + ret[jss::hash] = to_string(getTransactionID()); return ret; } diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index addd9ea0f39..40395aae32f 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/src/ripple/rpc/handlers/AccountTx.cpp @@ -319,6 +319,7 @@ populateJsonResponse( if (auto txnsData = std::get_if(&result.transactions)) { assert(!args.binary); + for (auto const& [txn, txnMeta] : *txnsData) { if (txn) @@ -361,6 +362,8 @@ populateJsonResponse( jvObj[jss::meta], context, txn, *txnMeta); insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); } + else + assert(false && "Missing transaction medatata"); } } } diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index ea815eb27ed..24ceb54bb94 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -20,9 +20,9 @@ #include #include #include +#include #include #include -#include "ripple/json/json_value.h" #include #include #include @@ -164,7 +164,7 @@ class Subscribe_test : public beast::unit_test::suite } void - testTransactions() + testTransactions_APIv1() { using namespace std::chrono_literals; using namespace jtx; @@ -1302,7 +1302,7 @@ class Subscribe_test : public beast::unit_test::suite testServer(); testLedger(); - testTransactions(); + testTransactions_APIv1(); testTransactions_APIv2(); testManifests(); testValidations(all - xrpFees); diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index ae988343a6c..da1d6de85f8 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -24,6 +24,8 @@ #include #include +#include + namespace ripple { class TransactionEntry_test : public beast::unit_test::suite @@ -154,6 +156,7 @@ class TransactionEntry_test : public beast::unit_test::suite int index, std::string const txhash, std::string const expected_json = "", + std::string const expected_ledger_hash = "", std::string const close_time_iso = "") { // first request using ledger_index to lookup Json::Value const resIndex{[&env, index, &txhash, apiVersion]() { @@ -165,13 +168,16 @@ class TransactionEntry_test : public beast::unit_test::suite "transaction_entry", params)[jss::result]; }()}; - if (!BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash)) + if (!BEAST_EXPECT(resIndex.isMember(jss::tx_json))) return; + BEAST_EXPECT(resIndex[jss::validated] == true); + BEAST_EXPECT(resIndex[jss::ledger_index] == index); + BEAST_EXPECT(resIndex[jss::ledger_hash] == expected_ledger_hash); if (apiVersion > 1) { BEAST_EXPECT(resIndex[jss::hash] == txhash); - BEAST_EXPECT(resIndex[jss::validated] == true); + BEAST_EXPECT(!resIndex[jss::tx_json].isMember(jss::hash)); BEAST_EXPECT(!resIndex[jss::tx_json].isMember(jss::Amount)); if (BEAST_EXPECT(!close_time_iso.empty())) @@ -179,7 +185,11 @@ class TransactionEntry_test : public beast::unit_test::suite resIndex[jss::close_time_iso] == close_time_iso); } else + { BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); + BEAST_EXPECT(!resIndex.isMember(jss::hash)); + BEAST_EXPECT(!resIndex.isMember(jss::close_time_iso)); + } if (!expected_json.empty()) { @@ -244,10 +254,16 @@ class TransactionEntry_test : public beast::unit_test::suite env.fund(XRP(10000), A1); auto fund_1_tx = boost::lexical_cast(env.tx()->getTransactionID()); + BEAST_EXPECT( + fund_1_tx == + "F4E9DF90D829A9E8B423FF68C34413E240D8D8BB0EFD080DF08114ED398E2506"); env.fund(XRP(10000), A2); auto fund_2_tx = boost::lexical_cast(env.tx()->getTransactionID()); + BEAST_EXPECT( + fund_2_tx == + "6853CD8226A05068C951CB1F54889FF4E40C5B440DC1C5BA38F114C4E0B1E705"); env.close(); @@ -256,32 +272,30 @@ class TransactionEntry_test : public beast::unit_test::suite check_tx( env.closed()->seq(), fund_1_tx, - R"( -{ - "Account" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", - "Fee" : "10", - "Sequence" : 3, - "SetFlag" : 8, - "SigningPubKey" : "0324CAAFA2212D2AEAB9D42D481535614AED486293E1FB1380FF070C3DD7FB4264", - "TransactionType" : "AccountSet", - "TxnSignature" : "3044022007B35E3B99460534FF6BC3A66FBBA03591C355CC38E38588968E87CCD01BE229022071A443026DE45041B55ABB1CC76812A87EA701E475BBB7E165513B4B242D3474", -} -)", + R"({ + "Account" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Sequence" : 3, + "SetFlag" : 8, + "SigningPubKey" : "0324CAAFA2212D2AEAB9D42D481535614AED486293E1FB1380FF070C3DD7FB4264", + "TransactionType" : "AccountSet", + "TxnSignature" : "3044022007B35E3B99460534FF6BC3A66FBBA03591C355CC38E38588968E87CCD01BE229022071A443026DE45041B55ABB1CC76812A87EA701E475BBB7E165513B4B242D3474", +})", + "ADB727BCC74B29421BB01B847740B179B8A0ED3248D76A89ED2E39B02C427784", "2000-01-01T00:00:10Z"); check_tx( env.closed()->seq(), fund_2_tx, - R"( -{ - "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", - "Fee" : "10", - "Sequence" : 3, - "SetFlag" : 8, - "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", - "TransactionType" : "AccountSet", - "TxnSignature" : "3045022100C8857FC0759A2AC0D2F320684691A66EAD252EAED9EF88C79791BC58BFCC9D860220421722286487DD0ED6BBA626CE6FCBDD14289F7F4726870C3465A4054C2702D7", -} -)", + R"({ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "Fee" : "10", + "Sequence" : 3, + "SetFlag" : 8, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TransactionType" : "AccountSet", + "TxnSignature" : "3045022100C8857FC0759A2AC0D2F320684691A66EAD252EAED9EF88C79791BC58BFCC9D860220421722286487DD0ED6BBA626CE6FCBDD14289F7F4726870C3465A4054C2702D7", +})", + "ADB727BCC74B29421BB01B847740B179B8A0ED3248D76A89ED2E39B02C427784", "2000-01-01T00:00:10Z"); env.trust(A2["USD"](1000), A1); @@ -290,78 +304,84 @@ class TransactionEntry_test : public beast::unit_test::suite // in the check below auto trust_tx = boost::lexical_cast(env.tx()->getTransactionID()); + BEAST_EXPECT( + trust_tx == + "C992D97D88FF444A1AB0C06B27557EC54B7F7DA28254778E60238BEA88E0C101"); env(pay(A2, A1, A2["USD"](5))); auto pay_tx = boost::lexical_cast(env.tx()->getTransactionID()); env.close(); + BEAST_EXPECT( + pay_tx == + "988046D484ACE9F5F6A8C792D89C6EA2DB307B5DDA9864AEBA88E6782ABD0865"); 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", -} -)", + R"({ + "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", + "DeliverMax" : "10", + "Destination" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Flags" : 2147483648, + "Sequence" : 3, + "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020", + "TransactionType" : "Payment", + "TxnSignature" : "3044022033D9EBF7F02950AF2F6B13C07AEE641C8FEBDD540A338FCB9027A965A4AED35B02206E4E227DCC226A3456C0FEF953449D21645A24EB63CA0BB7C5B62470147FD1D1", +})", + "39AA166131D56622EFD96CB4B2BD58003ACD37091C90977FF6B81419DB451775", "2000-01-01T00:00:20Z"); 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", -} -)", + R"({ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "DeliverMax" : + { + "currency" : "USD", + "issuer" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "value" : "5" + }, + "Destination" : "r4nmQNH4Fhjfh6cHDbvVSsBv7KySbj4cBf", + "Fee" : "10", + "Flags" : 2147483648, + "Sequence" : 4, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TransactionType" : "Payment", + "TxnSignature" : "30450221008A722B7F16EDB2348886E88ED4EC682AE9973CC1EE0FF37C93BB2CEC821D3EDF022059E464472031BA5E0D88A93E944B6A8B8DB3E1D5E5D1399A805F615789DB0BED", +})", + "39AA166131D56622EFD96CB4B2BD58003ACD37091C90977FF6B81419DB451775", "2000-01-01T00:00:20Z"); env(offer(A2, XRP(100), A2["USD"](1))); auto offer_tx = boost::lexical_cast(env.tx()->getTransactionID()); + BEAST_EXPECT( + offer_tx == + "5FCC1A27A7664F82A0CC4BE5766FBBB7C560D52B93AA7B550CD33B27AEC7EFFB"); env.close(); 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", -} -)", + R"({ + "Account" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "Fee" : "10", + "Sequence" : 5, + "SigningPubKey" : "03CFF28E067A2CCE6CC5A598C0B845CBD3F30A7863BE9C0DD55F4960EFABCCF4D0", + "TakerGets" : + { + "currency" : "USD", + "issuer" : "rGpeQzUWFu4fMhJHZ1Via5aqFC3A5twZUD", + "value" : "1" + }, + "TakerPays" : "100000000", + "TransactionType" : "OfferCreate", + "TxnSignature" : "304502210093FC93ACB77B4E3DE3315441BD010096734859080C1797AB735EB47EBD541BD102205020BB1A7C3B4141279EE4C287C13671E2450EA78914EFD0C6DB2A18344CD4F2", +})", + "0589B876DF5AFE335781E8FC12C2EC62A80151DF13BBAFE9EB2DA62E798ED434", "2000-01-01T00:00:30Z"); } @@ -370,8 +390,8 @@ class TransactionEntry_test : public beast::unit_test::suite run() override { testBadInput(); - testRequest(1); - testRequest(2); + test::jtx::forAllApiVersions( + std::bind_front(&TransactionEntry_test::testRequest, this)); } }; diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index 36de520187b..0b5ccf3c9d4 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/src/test/rpc/Transaction_test.cpp @@ -790,8 +790,8 @@ class Transaction_test : public beast::unit_test::suite testRangeCTIDRequest(features); testCTIDValidation(features); testCTIDRPC(features); - testRequest(features, 1); - testRequest(features, 2); + test::jtx::forAllApiVersions( + std::bind_front(&Transaction_test::testRequest, this, features)); } };