Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Unify JSON serialization format of transactions #4775

Merged
merged 37 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
65346dc
Remove include <ranges>
Bronek Oct 25, 2023
ee53c5d
Formatting fix
Bronek Oct 25, 2023
aa6de60
Output for subscriptions
Bronek Oct 23, 2023
76eee5a
Output from sign, submit etc.
Bronek Oct 18, 2023
92635aa
Output from ledger
Bronek Oct 18, 2023
e4ce8b1
Output from account_tx
Bronek Oct 18, 2023
0fb0517
Output from transaction_entry
Bronek Oct 18, 2023
910f125
Output from tx
Bronek Oct 18, 2023
4d517e2
Store close_time_iso in API v2 output
Bronek Oct 23, 2023
2b397c9
Add small APIv2 unit test for subscribe
Bronek Oct 24, 2023
1a96800
Add unit test for transaction_entry
Bronek Oct 24, 2023
c2a3b52
Add unit test for tx
Bronek Oct 24, 2023
bdf90e5
Remove inLedger from API version 2
Bronek Oct 24, 2023
2035bbc
Set ledger_hash and ledger_index
Bronek Oct 24, 2023
1115cef
Move isValidated from RPCHelpers to LedgerMaster
Bronek Oct 26, 2023
4e92ee4
Store closeTime in LedgerFill
Bronek Oct 26, 2023
9f1d544
Time formatting fix
Bronek Oct 30, 2023
e386093
additional tests for Subscribe unit tests
ckeshava Oct 27, 2023
c477eb4
Improved comments
Bronek Oct 30, 2023
c81bece
Rename mInLedger to mLedgerIndex
Bronek Oct 30, 2023
ef1276c
Minor fixes
Bronek Oct 30, 2023
54f1d60
Set ledger_hash on closed ledger, even if not validated
Bronek Oct 30, 2023
625c812
Merge branch 'develop' into feature/unify_transaction_json
Bronek Oct 30, 2023
0d8673c
Update API-CHANGELOG.md
Bronek Oct 31, 2023
cdb3384
Add ledger_hash, ledger_index to transaction_entry
Bronek Oct 31, 2023
5973390
Fix validated and close_time_iso in account_tx
Bronek Nov 1, 2023
17e6588
Fix typos
Bronek Nov 2, 2023
95b6055
Improve getJson for Transaction and STTx
Bronek Nov 2, 2023
b2a8a2d
Minor improvements
Bronek Nov 2, 2023
94c16d8
Replace class enum JsonOptions with struct
Bronek Nov 2, 2023
7770bc5
simplify the extraction of transactionID from Transaction object
ckeshava Nov 2, 2023
b5b1118
Remove obsolete comments
Bronek Nov 2, 2023
942e209
Unconditionally set validated in account_tx output
Bronek Nov 2, 2023
c1f2ea4
Merge branch 'develop' into feature/unify_transaction_json
Bronek Nov 2, 2023
53e384f
Minor improvements
Bronek Nov 3, 2023
d0060f8
Minor fixes
Bronek Nov 6, 2023
311952f
Merge branch 'develop' into feature/unify_transaction_json
Bronek Nov 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions API-CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.

Bronek marked this conversation as resolved.
Show resolved Hide resolved
#### 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)
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/app/ledger/LedgerMaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/ripple/app/ledger/LedgerToJson.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
#define RIPPLE_APP_LEDGER_LEDGERTOJSON_H_INCLUDED

#include <ripple/app/ledger/Ledger.h>
#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/misc/TxQ.h>
#include <ripple/basics/StringUtilities.h>
#include <ripple/basics/chrono.h>
#include <ripple/json/Object.h>
#include <ripple/protocol/STTx.h>
#include <ripple/protocol/jss.h>
Expand All @@ -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 {
Expand All @@ -58,6 +62,7 @@ struct LedgerFill
std::vector<TxQ::TxDetails> txQueue;
LedgerEntryType type;
RPC::Context* context;
std::optional<NetClock::time_point> closeTime;
};

/** Given a Ledger and options, fill a Json::Object or Json::Value with a
Expand Down
49 changes: 49 additions & 0 deletions src/ripple/app/ledger/impl/LedgerMaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,55 @@ LedgerMaster::clearLedger(std::uint32_t seq)
mCompleteLedgers.erase(seq);
}

bool
LedgerMaster::isValidated(ReadView const& ledger)
{
if (app_.config().reporting())
return true; // Reporting mode only supports validated ledger

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);
ximinez marked this conversation as resolved.
Show resolved Hide resolved

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 ?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel app_.journal("LedgerMaster::IsValidated") is a descriptive name

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LedgerMaster has an m_journal member. May as well use that. In which case, you won't really need the stream local. Just fall back to the usual pattern of JLOG(m_journal.warn()).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know how these names are rendered, but it appears we almost never use them with :: in the middle ...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, i see

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(
Expand Down
79 changes: 65 additions & 14 deletions src/ripple/app/ledger/impl/LedgerToJson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
*/
//==============================================================================

#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/ledger/LedgerToJson.h>
#include <ripple/app/main/Application.h>
#include <ripple/app/misc/DeliverMax.h>
#include <ripple/app/misc/TxQ.h>
#include <ripple/basics/base_uint.h>
#include <ripple/core/Pg.h>
#include <ripple/protocol/jss.h>
#include <ripple/rpc/Context.h>
#include <ripple/rpc/DeliveredAmount.h>

Expand Down Expand Up @@ -83,6 +85,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);
}
}

Expand Down Expand Up @@ -118,24 +121,68 @@ 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)
{
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 (!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);
}
}
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});
}
}
}

Expand Down Expand Up @@ -254,7 +301,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);
}
}

Expand Down
32 changes: 26 additions & 6 deletions src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#include <ripple/protocol/BuildInfo.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/STParsedJSON.h>
#include <ripple/protocol/jss.h>
#include <ripple/resource/Fees.h>
#include <ripple/resource/ResourceManager.h>
#include <ripple/rpc/BookChanges.h>
Expand All @@ -74,6 +75,7 @@

#include <algorithm>
#include <mutex>
#include <optional>
#include <string>
#include <tuple>
#include <unordered_map>
Expand Down Expand Up @@ -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
ximinez marked this conversation as resolved.
Show resolved Hide resolved
// 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);

if (meta)
{
Expand All @@ -3110,13 +3117,16 @@ 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;
jvObj[jss::close_time_iso] = to_string_iso(ledger->info().closeTime);

// WRITEME: Put the account next seq here
}
Expand Down Expand Up @@ -3165,11 +3175,21 @@ NetworkOPsImp::transJson(
assert(index < MultiApiJson::size);
if (index != lastIndex)
{
RPC::insertDeliverMax(
multiObj.val[index][jss::transaction],
transaction->getTxnType(),
apiVersion);
lastIndex = index;

Json::Value& jvTx = multiObj.val[index];
RPC::insertDeliverMax(
jvTx[jss::transaction], transaction->getTxnType(), apiVersion);

if (apiVersion > 1)
{
jvTx[jss::tx_json] = jvTx.removeMember(jss::transaction);
jvTx[jss::hash] = hash;
}
else
{
jvTx[jss::transaction][jss::hash] = hash;
}
}
}

Expand Down
12 changes: 7 additions & 5 deletions src/ripple/app/misc/Transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@
#include <ripple/beast/utility/Journal.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/protocol/Protocol.h>
#include <ripple/protocol/STBase.h>
Bronek marked this conversation as resolved.
Show resolved Hide resolved
#include <ripple/protocol/STTx.h>
#include <ripple/protocol/TER.h>
#include <ripple/protocol/TxMeta.h>
#include <boost/optional.hpp>

#include <optional>
#include <variant>

Expand Down Expand Up @@ -99,13 +100,13 @@ class Transaction : public std::enable_shared_from_this<Transaction>,
LedgerIndex
getLedger() const
{
return mInLedger;
return mLedgerIndex;
}

bool
isValidated() const
{
return mInLedger != 0;
return mLedgerIndex != 0;
}

TransStatus
Expand Down Expand Up @@ -138,7 +139,7 @@ class Transaction : public std::enable_shared_from_this<Transaction>,
void
setLedger(LedgerIndex ledger)
{
mInLedger = ledger;
mLedgerIndex = ledger;
}

/**
Expand Down Expand Up @@ -305,6 +306,7 @@ class Transaction : public std::enable_shared_from_this<Transaction>,
validatedLedger, fee, accountSeq, availableSeq);
}

/// Same as similar overload for STTx::getJson
Json::Value
getJson(JsonOptions options, bool binary = false) const;

Expand Down Expand Up @@ -386,7 +388,7 @@ class Transaction : public std::enable_shared_from_this<Transaction>,

uint256 mTransactionID;

LedgerIndex mInLedger = 0;
LedgerIndex mLedgerIndex = 0;
TransStatus mStatus = INVALID;
TER mResult = temUNCERTAIN;
bool mApplying = false;
Expand Down
Loading
Loading