From b6391fe0110f8a2d5a406ab458b3f2e0453adcc6 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Thu, 19 Sep 2024 08:39:10 -0700 Subject: [PATCH] fix(book_changes): add "validated" field and reduce RPC latency (#5096) Update book_changes RPC to reduce latency, add "validated" field, and accept shortcut strings (current, closed, validated) for ledger_index. `"validated": true` indicates that the transaction has been included in a validated ledger so the result of the transaction is immutable. Fix #5033 Fix #5034 Fix #5035 Fix #5036 --------- Co-authored-by: Bronek Kozicki --- API-CHANGELOG.md | 10 ++- src/test/rpc/BookChanges_test.cpp | 100 ++++++++++++++++++++++++++ src/xrpld/rpc/BookChanges.h | 12 ++++ src/xrpld/rpc/handlers/BookOffers.cpp | 10 +-- 4 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 src/test/rpc/BookChanges_test.cpp diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 58ceed0cde1..967e1ce6052 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -26,7 +26,15 @@ In `api_version: 2`, the `signer_lists` field [will be moved](#modifications-to- The `network_id` field was added in the `server_info` response in version 1.5.0 (2019), but it is not returned in [reporting mode](https://xrpl.org/rippled-server-modes.html#reporting-mode). -## XRP Ledger server version 2.0.0 +### Breaking change in 2.3 + +- `book_changes`: If the requested ledger version is not available on this node, a `ledgerNotFound` error is returned and the node does not attempt to acquire the ledger from the p2p network (as with other non-admin RPCs). + +Admins can still attempt to retrieve old ledgers with the `ledger_request` RPC. + +### Addition in 2.3 + +- `book_changes`: Returns a `validated` field in its response, which was missing in prior versions. ### Additions in 2.2 diff --git a/src/test/rpc/BookChanges_test.cpp b/src/test/rpc/BookChanges_test.cpp new file mode 100644 index 00000000000..95997538d79 --- /dev/null +++ b/src/test/rpc/BookChanges_test.cpp @@ -0,0 +1,100 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include + +namespace ripple { +namespace test { + +class BookChanges_test : public beast::unit_test::suite +{ +public: + void + testConventionalLedgerInputStrings() + { + testcase("Specify well-known strings as ledger input"); + jtx::Env env(*this); + Json::Value params, resp; + + // As per convention in XRPL, ledgers can be specified with strings + // "closed", "validated" or "current" + params["ledger"] = "validated"; + resp = env.rpc("json", "book_changes", to_string(params)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(resp[jss::result][jss::status] == "success"); + BEAST_EXPECT(resp[jss::result][jss::validated] == true); + + params["ledger"] = "current"; + resp = env.rpc("json", "book_changes", to_string(params)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(resp[jss::result][jss::status] == "success"); + BEAST_EXPECT(resp[jss::result][jss::validated] == false); + + params["ledger"] = "closed"; + resp = env.rpc("json", "book_changes", to_string(params)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(resp[jss::result][jss::status] == "success"); + + // In the unit-test framework, requesting for "closed" ledgers appears + // to yield "validated" ledgers. This is not new behavior. It is also + // observed in the unit tests for the LedgerHeader class. + BEAST_EXPECT(resp[jss::result][jss::validated] == true); + + // non-conventional ledger input should throw an error + params["ledger"] = "non_conventional_ledger_input"; + resp = env.rpc("json", "book_changes", to_string(params)); + BEAST_EXPECT(resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(resp[jss::result][jss::status] != "success"); + } + + void + testLedgerInputDefaultBehavior() + { + testcase( + "If ledger_hash or ledger_index is not specified, the behavior " + "must default to the `current` ledger"); + jtx::Env env(*this); + + // As per convention in XRPL, ledgers can be specified with strings + // "closed", "validated" or "current" + Json::Value const resp = + env.rpc("json", "book_changes", to_string(Json::Value{})); + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(resp[jss::result][jss::status] == "success"); + + // I dislike asserting the below statement, because its dependent on the + // unit-test framework BEAST_EXPECT(resp[jss::result][jss::ledger_index] + // == 3); + } + + void + run() override + { + testConventionalLedgerInputStrings(); + testLedgerInputDefaultBehavior(); + + // Note: Other aspects of the book_changes rpc are fertile grounds for + // unit-testing purposes. It can be included in future work + } +}; + +BEAST_DEFINE_TESTSUITE(BookChanges, app, ripple); + +} // namespace test +} // namespace ripple diff --git a/src/xrpld/rpc/BookChanges.h b/src/xrpld/rpc/BookChanges.h index 7d7978d3fe2..c87fa0ccf4e 100644 --- a/src/xrpld/rpc/BookChanges.h +++ b/src/xrpld/rpc/BookChanges.h @@ -20,6 +20,15 @@ #ifndef RIPPLE_RPC_BOOKCHANGES_H_INCLUDED #define RIPPLE_RPC_BOOKCHANGES_H_INCLUDED +#include +#include +#include +#include +#include +#include + +#include + namespace Json { class Value; } @@ -171,6 +180,9 @@ computeBookChanges(std::shared_ptr const& lpAccepted) Json::Value jvObj(Json::objectValue); jvObj[jss::type] = "bookChanges"; + + // retrieve validated information from LedgerHeader class + jvObj[jss::validated] = lpAccepted->info().validated; jvObj[jss::ledger_index] = lpAccepted->info().seq; jvObj[jss::ledger_hash] = to_string(lpAccepted->info().hash); jvObj[jss::ledger_time] = Json::Value::UInt( diff --git a/src/xrpld/rpc/handlers/BookOffers.cpp b/src/xrpld/rpc/handlers/BookOffers.cpp index 6126913a5b6..dccc03de5be 100644 --- a/src/xrpld/rpc/handlers/BookOffers.cpp +++ b/src/xrpld/rpc/handlers/BookOffers.cpp @@ -204,13 +204,13 @@ doBookOffers(RPC::JsonContext& context) Json::Value doBookChanges(RPC::JsonContext& context) { - auto res = RPC::getLedgerByContext(context); + std::shared_ptr ledger; - if (std::holds_alternative(res)) - return std::get(res); + Json::Value result = RPC::lookupLedger(ledger, context); + if (ledger == nullptr) + return result; - return RPC::computeBookChanges( - std::get>(res)); + return RPC::computeBookChanges(ledger); } } // namespace ripple