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 <brok@incorrekt.com>
XRPLF · Sep 19, 2024 · b6391fe · b6391fe
1 parent 9a6af9c
commit b6391fe
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 6 deletions.
diff --git 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
@@ -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 <test/jtx.h>
+
+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
@@ -20,6 +20,15 @@
 #ifndef RIPPLE_RPC_BOOKCHANGES_H_INCLUDED
 #define RIPPLE_RPC_BOOKCHANGES_H_INCLUDED
 
+#include <xrpl/json/json_value.h>
+#include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STObject.h>
+#include <xrpl/protocol/TxFormats.h>
+#include <xrpl/protocol/jss.h>
+
+#include <memory>
+
 namespace Json {
 class Value;
 }
@@ -171,6 +180,9 @@ computeBookChanges(std::shared_ptr<L const> 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
@@ -204,13 +204,13 @@ doBookOffers(RPC::JsonContext& context)
 Json::Value
 doBookChanges(RPC::JsonContext& context)
 {
-    auto res = RPC::getLedgerByContext(context);
+    std::shared_ptr<ReadView const> ledger;
 
-    if (std::holds_alternative<Json::Value>(res))
-        return std::get<Json::Value>(res);
+    Json::Value result = RPC::lookupLedger(ledger, context);
+    if (ledger == nullptr)
+        return result;
 
-    return RPC::computeBookChanges(
-        std::get<std::shared_ptr<Ledger const>>(res));
+    return RPC::computeBookChanges(ledger);
 }
 
 }  // namespace ripple