Fix http params handling discrepancy (#913)

Fixes #909

CMake/Settings.cmake

@@ -19,13 +19,7 @@ set(COMPILER_FLAGS
   -Wunused
 )
 
-if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-  list(APPEND COMPILER_FLAGS
-    -Wshadow # gcc is to aggressive with shadowing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78147
-  )
-endif ()
-
-if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+if (is_gcc AND NOT lint)
   list(APPEND COMPILER_FLAGS
     -Wduplicated-branches
     -Wduplicated-cond
@@ -34,6 +28,18 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
   )
 endif ()
 
+if (is_clang)
+  list(APPEND COMPILER_FLAGS
+    -Wshadow # gcc is to aggressive with shadowing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78147
+  )
+endif ()
+
+if (is_appleclang)
+  list(APPEND COMPILER_FLAGS
+    -Wreorder-init-list
+  )
+endif ()
+
 # See https://github.com/cpp-best-practices/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang for the flags description
 
 target_compile_options (clio PUBLIC ${COMPILER_FLAGS})

src/etl/impl/LedgerPublisher.h

@@ -171,6 +171,16 @@ public:
 
                 subscriptions_->pubLedger(lgrInfo, *fees, range, transactions.size());
 
+                // order with transaction index
+                std::sort(transactions.begin(), transactions.end(), [](auto const& t1, auto const& t2) {
+                    ripple::SerialIter iter1{t1.metadata.data(), t1.metadata.size()};
+                    ripple::STObject const object1(iter1, ripple::sfMetadata);
+                    ripple::SerialIter iter2{t2.metadata.data(), t2.metadata.size()};
+                    ripple::STObject const object2(iter2, ripple::sfMetadata);
+                    return object1.getFieldU32(ripple::sfTransactionIndex) <
+                        object2.getFieldU32(ripple::sfTransactionIndex);
+                });
+
                 for (auto& txAndMeta : transactions)
                     subscriptions_->pubTransaction(txAndMeta, lgrInfo);
 

src/rpc/RPCEngine.h

@@ -150,12 +150,10 @@ public:
 
             if (v)
                 return v->as_object();
-            else
-            {
+
             notifyErrored(ctx.method);
             return Status{v.error()};
         }
-        }
         catch (data::DatabaseTimeout const& t)
         {
             LOG(log_.error()) << "Database timeout";

src/rpc/RPCHelpers.cpp

@@ -1149,21 +1149,15 @@ parseBook(ripple::Currency pays, ripple::AccountID payIssuer, ripple::Currency g
 {
     if (isXRP(pays) && !isXRP(payIssuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."};
+            RippledError::rpcSRC_ISR_MALFORMED, "Unneeded field 'taker_pays.issuer' for XRP currency specification."};
 
     if (!isXRP(pays) && isXRP(payIssuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
-            "Invalid field 'taker_pays.issuer', expected non-XRP "
-            "issuer."};
+            RippledError::rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', expected non-XRP issuer."};
 
     if (ripple::isXRP(gets) && !ripple::isXRP(getIssuer))
         return Status{
-            RippledError::rpcDST_ISR_MALFORMED,
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."};
+            RippledError::rpcDST_ISR_MALFORMED, "Unneeded field 'taker_gets.issuer' for XRP currency specification."};
 
     if (!ripple::isXRP(gets) && ripple::isXRP(getIssuer))
         return Status{
@@ -1233,15 +1227,11 @@ parseBook(boost::json::object const& request)
 
     if (isXRP(pay_currency) && !isXRP(pay_issuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."};
+            RippledError::rpcSRC_ISR_MALFORMED, "Unneeded field 'taker_pays.issuer' for XRP currency specification."};
 
     if (!isXRP(pay_currency) && isXRP(pay_issuer))
         return Status{
-            RippledError::rpcSRC_ISR_MALFORMED,
-            "Invalid field 'taker_pays.issuer', expected non-XRP "
-            "issuer."};
+            RippledError::rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', expected non-XRP issuer."};
 
     if ((!isXRP(pay_currency)) && (!taker_pays.contains("issuer")))
         return Status{RippledError::rpcSRC_ISR_MALFORMED, "Missing non-XRP issuer."};
@@ -1258,9 +1248,7 @@ parseBook(boost::json::object const& request)
 
         if (get_issuer == ripple::noAccount())
             return Status{
-                RippledError::rpcDST_ISR_MALFORMED,
-                "Invalid field 'taker_gets.issuer', bad issuer account "
-                "one."};
+                RippledError::rpcDST_ISR_MALFORMED, "Invalid field 'taker_gets.issuer', bad issuer account one."};
     }
     else
     {
@@ -1269,9 +1257,7 @@ parseBook(boost::json::object const& request)
 
     if (ripple::isXRP(get_currency) && !ripple::isXRP(get_issuer))
         return Status{
-            RippledError::rpcDST_ISR_MALFORMED,
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."};
+            RippledError::rpcDST_ISR_MALFORMED, "Unneeded field 'taker_gets.issuer' for XRP currency specification."};
 
     if (!ripple::isXRP(get_currency) && ripple::isXRP(get_issuer))
         return Status{

src/rpc/common/Validators.cpp

@@ -146,11 +146,7 @@ CustomValidator IssuerValidator =
 
         if (issuer == ripple::noAccount())
             return Error{Status{
-                RippledError::rpcINVALID_PARAMS,
-                fmt::format(
-                    "Invalid field '{}', bad issuer account "
-                    "one.",
-                    key)}};
+                RippledError::rpcINVALID_PARAMS, fmt::format("Invalid field '{}', bad issuer account one.", key)}};
 
         return MaybeError{};
     }};

src/rpc/common/Validators.h

@@ -417,7 +417,7 @@ public:
 
         auto const res = value_to<Type>(value.as_object().at(key.data()));
         if (std::find(std::begin(options_), std::end(options_), res) == std::end(options_))
-            return Error{Status{RippledError::rpcINVALID_PARAMS}};
+            return Error{Status{RippledError::rpcINVALID_PARAMS, fmt::format("Invalid field '{}'.", key)}};
 
         return {};
     }

src/rpc/handlers/AccountTx.cpp

@@ -107,6 +107,10 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con
         if (ctx.apiVersion > 1u && (input.ledgerIndexMax || input.ledgerIndexMin))
             return Error{Status{RippledError::rpcINVALID_PARAMS, "containsLedgerSpecifierAndRange"}};
 
+        if (!input.ledgerIndexMax && !input.ledgerIndexMin)
+        {
+            // mimic rippled, when both range and index specified, respect the range.
+            // take ledger from ledgerHash or ledgerIndex only when range is not specified
             auto const lgrInfoOrStatus = getLedgerInfoFromHashOrSeq(
                 *sharedPtrBackend_, ctx.yield, input.ledgerHash, input.ledgerIndex, range->maxSequence);
 
@@ -115,6 +119,7 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con
 
             maxIndex = minIndex = std::get<ripple::LedgerHeader>(lgrInfoOrStatus).seq;
         }
+    }
 
     std::optional<data::TransactionsCursor> cursor;
 
@@ -180,8 +185,11 @@ AccountTxHandler::process(AccountTxHandler::Input input, Context const& ctx) con
                     continue;
             }
 
-            obj[JS(tx)].as_object()[JS(ledger_index)] = txnPlusMeta.ledgerSequence;
             obj[JS(tx)].as_object()[JS(date)] = txnPlusMeta.date;
+            obj[JS(tx)].as_object()[JS(ledger_index)] = txnPlusMeta.ledgerSequence;
+
+            if (ctx.apiVersion < 2u)
+                obj[JS(tx)].as_object()[JS(inLedger)] = txnPlusMeta.ledgerSequence;
         }
         else
         {

src/rpc/handlers/BookOffers.h

@@ -95,7 +95,7 @@ public:
             // return INVALID_PARAMS if account format is wrong for "taker"
             {JS(taker),
              meta::WithCustomError{
-                 validation::AccountValidator, Status(RippledError::rpcINVALID_PARAMS, "Invalid field 'taker'")}},
+                 validation::AccountValidator, Status(RippledError::rpcINVALID_PARAMS, "Invalid field 'taker'.")}},
             {JS(limit),
              validation::Type<uint32_t>{},
              validation::Min(1u),

src/rpc/handlers/LedgerData.h

@@ -100,9 +100,7 @@ public:
              meta::WithCustomError{
                  validation::Type<std::string>{},
                  Status{ripple::rpcINVALID_PARAMS, "Invalid field 'type', not string."}},
-             meta::WithCustomError{
-                 validation::OneOf<std::string>(TYPES_KEYS.cbegin(), TYPES_KEYS.cend()),
-                 Status{ripple::rpcINVALID_PARAMS, "Invalid field 'type'."}}},
+             validation::OneOf<std::string>(TYPES_KEYS.cbegin(), TYPES_KEYS.cend())},
 
         };
         return rpcSpec;

src/rpc/handlers/LedgerEntry.cpp

@@ -83,8 +83,8 @@ LedgerEntryHandler::process(LedgerEntryHandler::Input input, Context const& ctx)
     {
         // Must specify 1 of the following fields to indicate what type
         if (ctx.apiVersion == 1)
-            return Error{Status{RippledError::rpcINVALID_PARAMS}};
             return Error{Status{ClioError::rpcUNKNOWN_OPTION}};
+        return Error{Status{RippledError::rpcINVALID_PARAMS}};
     }
 
     // check ledger exists

src/rpc/handlers/Tx.cpp

@@ -36,7 +36,7 @@ TxHandler::process(Input input, Context const& ctx) const
             return Error{Status{RippledError::rpcEXCESSIVE_LGR_RANGE}};
     }
 
-    auto output = TxHandler::Output{};
+    auto output = TxHandler::Output{.apiVersion = ctx.apiVersion};
     auto const dbResponse =
         sharedPtrBackend_->fetchTransaction(ripple::uint256{std::string_view(input.transaction)}, ctx.yield);
 
@@ -55,7 +55,6 @@ TxHandler::process(Input input, Context const& ctx) const
         return Error{Status{RippledError::rpcTXN_NOT_FOUND}};
     }
 
-    // clio does not implement 'inLedger' which is a deprecated field
     if (!input.binary)
     {
         auto const [txn, meta] = toExpandedJson(*dbResponse, NFTokenjson::ENABLE);
@@ -95,6 +94,9 @@ tag_invoke(boost::json::value_from_tag, boost::json::value& jv, TxHandler::Outpu
     obj[JS(date)] = output.date;
     obj[JS(ledger_index)] = output.ledgerIndex;
 
+    if (output.apiVersion < 2u)
+        obj[JS(inLedger)] = output.ledgerIndex;
+
     jv = std::move(obj);
 }
 

src/rpc/handlers/Tx.h

@@ -38,13 +38,14 @@ class TxHandler
 public:
     struct Output
     {
-        uint32_t date;
-        std::string hash;
-        uint32_t ledgerIndex;
-        std::optional<boost::json::object> meta;
-        std::optional<boost::json::object> tx;
-        std::optional<std::string> metaStr;
-        std::optional<std::string> txStr;
+        uint32_t date = 0u;
+        std::string hash{};
+        uint32_t ledgerIndex = 0u;
+        std::optional<boost::json::object> meta{};
+        std::optional<boost::json::object> tx{};
+        std::optional<std::string> metaStr{};
+        std::optional<std::string> txStr{};
+        uint32_t apiVersion = 0u;
         bool validated = true;
     };
 

src/web/RPCServerHandler.h

@@ -89,8 +89,8 @@ public:
             auto req = boost::json::parse(request).as_object();
             LOG(perfLog_.debug()) << connection->tag() << "Adding to work queue";
 
-            if (not connection->upgraded and not req.contains("params"))
-                req["params"] = boost::json::array({boost::json::object{}});
+            if (not connection->upgraded and shouldReplaceParams(req))
+                req[JS(params)] = boost::json::array({boost::json::object{}});
 
             if (!rpcEngine_->post(
                     [this, request = std::move(req), connection](boost::asio::yield_context yield) mutable {
@@ -267,6 +267,27 @@ private:
             return web::detail::ErrorHelper(connection, request).sendInternalError();
         }
     }
+
+    bool
+    shouldReplaceParams(boost::json::object const& req) const
+    {
+        auto const hasParams = req.contains(JS(params));
+        auto const paramsIsArray = hasParams and req.at(JS(params)).is_array();
+        auto const paramsIsEmptyString =
+            hasParams and req.at(JS(params)).is_string() and req.at(JS(params)).as_string().empty();
+        auto const paramsIsEmptyObject =
+            hasParams and req.at(JS(params)).is_object() and req.at(JS(params)).as_object().empty();
+        auto const paramsIsNull = hasParams and req.at(JS(params)).is_null();
+        auto const arrayIsEmpty = paramsIsArray and req.at(JS(params)).as_array().empty();
+        auto const arrayIsNotEmpty = paramsIsArray and not req.at(JS(params)).as_array().empty();
+        auto const firstArgIsNull = arrayIsNotEmpty and req.at(JS(params)).as_array().at(0).is_null();
+        auto const firstArgIsEmptyString = arrayIsNotEmpty and req.at(JS(params)).as_array().at(0).is_string() and
+            req.at(JS(params)).as_array().at(0).as_string().empty();
+
+        // Note: all this compatibility dance is to match `rippled` as close as possible
+        return not hasParams or paramsIsEmptyString or paramsIsNull or paramsIsEmptyObject or arrayIsEmpty or
+            firstArgIsEmptyString or firstArgIsNull;
+    }
 };
 
 }  // namespace web

src/web/impl/HttpBase.h

@@ -180,10 +180,10 @@ public:
 
         if (boost::beast::websocket::is_upgrade(req_))
         {
-            upgraded = true;
-            // Disable the timeout.
-            // The websocket::stream uses its own timeout settings.
+            // Disable the timeout. The websocket::stream uses its own timeout settings.
             boost::beast::get_lowest_layer(derived().stream()).expires_never();
+
+            upgraded = true;
             return derived().upgrade();
         }
 

src/web/impl/WsBase.h

@@ -121,6 +121,10 @@ public:
     void
     maybeSendNext()
     {
+        // cleanup if needed. can't do this in destructor so it's here
+        if (dead())
+            (*handler_)(ec_, derived().shared_from_this());
+
         if (ec_ || sending_ || messages_.empty())
             return;
 
@@ -204,8 +208,8 @@ public:
         if (dead())
             return;
 
-        // Clear the buffer
-        buffer_.consume(buffer_.size());
+        // Note: use entirely new buffer so previously used, potentially large, capacity is deallocated
+        buffer_ = boost::beast::flat_buffer{};
 
         derived().ws().async_read(buffer_, boost::beast::bind_front_handler(&WsBase::onRead, this->shared_from_this()));
     }

unittests/etl/LedgerPublisherTests.cpp

@@ -0,0 +1,304 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of clio: https://github.com/XRPLF/clio
+    Copyright (c) 2023, the clio developers.
+
+    Permission to use, copy, modify, and 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 <etl/impl/LedgerPublisher.h>
+#include <util/Fixtures.h>
+#include <util/MockCache.h>
+#include <util/TestObject.h>
+
+#include <fmt/core.h>
+#include <gtest/gtest.h>
+
+#include <chrono>
+
+using namespace testing;
+using namespace etl;
+namespace json = boost::json;
+using namespace std::chrono;
+
+static auto constexpr ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
+static auto constexpr ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
+static auto constexpr LEDGERHASH = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652";
+static auto constexpr SEQ = 30;
+static auto constexpr AGE = 800;
+
+class ETLLedgerPublisherTest : public MockBackendTest, public SyncAsioContextTest, public MockSubscriptionManagerTest
+{
+    void
+    SetUp() override
+    {
+        MockBackendTest::SetUp();
+        SyncAsioContextTest::SetUp();
+        MockSubscriptionManagerTest::SetUp();
+    }
+
+    void
+    TearDown() override
+    {
+        MockSubscriptionManagerTest::TearDown();
+        SyncAsioContextTest::TearDown();
+        MockBackendTest::TearDown();
+    }
+
+protected:
+    util::Config cfg{json::parse("{}")};
+    MockCache mockCache;
+};
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoIsWritingFalse)
+{
+    SystemState dummyState;
+    dummyState.isWriting = false;
+    auto const dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, AGE);
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    publisher.publish(dummyLedgerInfo);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    ASSERT_NE(rawBackendPtr, nullptr);
+
+    ON_CALL(*rawBackendPtr, fetchLedgerDiff(SEQ, _)).WillByDefault(Return(std::vector<LedgerObject>{}));
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(SEQ, _)).Times(1);
+
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    EXPECT_CALL(mockCache, updateImp).Times(1);
+
+    ctx.run();
+    EXPECT_TRUE(rawBackendPtr->fetchLedgerRange());
+    EXPECT_EQ(rawBackendPtr->fetchLedgerRange().value().minSequence, SEQ);
+    EXPECT_EQ(rawBackendPtr->fetchLedgerRange().value().maxSequence, SEQ);
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoIsWritingTrue)
+{
+    SystemState dummyState;
+    dummyState.isWriting = true;
+    auto const dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, AGE);
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    publisher.publish(dummyLedgerInfo);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(_, _)).Times(0);
+
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    ctx.run();
+    EXPECT_FALSE(rawBackendPtr->fetchLedgerRange());
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoInRange)
+{
+    SystemState dummyState;
+    dummyState.isWriting = true;
+
+    auto const dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, 0);  // age is 0
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    mockBackendPtr->updateRange(SEQ - 1);
+    mockBackendPtr->updateRange(SEQ);
+
+    publisher.publish(dummyLedgerInfo);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(_, _)).Times(0);
+
+    // mock fetch fee
+    EXPECT_CALL(*rawBackendPtr, doFetchLedgerObject).Times(1);
+    ON_CALL(*rawBackendPtr, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
+        .WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
+
+    // mock fetch transactions
+    EXPECT_CALL(*rawBackendPtr, fetchAllTransactionsInLedger).Times(1);
+    TransactionAndMetadata t1;
+    t1.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 100, 3, SEQ).getSerializer().peekData();
+    t1.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30).getSerializer().peekData();
+    t1.ledgerSequence = SEQ;
+    ON_CALL(*rawBackendPtr, fetchAllTransactionsInLedger(SEQ, _))
+        .WillByDefault(Return(std::vector<TransactionAndMetadata>{t1}));
+
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    MockSubscriptionManager* rawSubscriptionManagerPtr =
+        dynamic_cast<MockSubscriptionManager*>(mockSubscriptionManagerPtr.get());
+
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubLedger(_, _, fmt::format("{}-{}", SEQ - 1, SEQ), 1)).Times(1);
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubBookChanges).Times(1);
+    // mock 1 transaction
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubTransaction).Times(1);
+
+    ctx.run();
+    // last publish time should be set
+    EXPECT_TRUE(publisher.lastPublishAgeSeconds() <= 1);
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoCloseTimeGreaterThanNow)
+{
+    SystemState dummyState;
+    dummyState.isWriting = true;
+
+    ripple::LedgerInfo dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, 0);
+    auto const nowPlus10 = system_clock::now() + seconds(10);
+    auto const closeTime = duration_cast<seconds>(nowPlus10.time_since_epoch()).count() - rippleEpochStart;
+    dummyLedgerInfo.closeTime = ripple::NetClock::time_point{seconds{closeTime}};
+
+    mockBackendPtr->updateRange(SEQ - 1);
+    mockBackendPtr->updateRange(SEQ);
+
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    publisher.publish(dummyLedgerInfo);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(_, _)).Times(0);
+
+    // mock fetch fee
+    EXPECT_CALL(*rawBackendPtr, doFetchLedgerObject).Times(1);
+    ON_CALL(*rawBackendPtr, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
+        .WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
+
+    // mock fetch transactions
+    EXPECT_CALL(*rawBackendPtr, fetchAllTransactionsInLedger).Times(1);
+    TransactionAndMetadata t1;
+    t1.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 100, 3, SEQ).getSerializer().peekData();
+    t1.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30).getSerializer().peekData();
+    t1.ledgerSequence = SEQ;
+    ON_CALL(*rawBackendPtr, fetchAllTransactionsInLedger(SEQ, _))
+        .WillByDefault(Return(std::vector<TransactionAndMetadata>{t1}));
+
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    MockSubscriptionManager* rawSubscriptionManagerPtr =
+        dynamic_cast<MockSubscriptionManager*>(mockSubscriptionManagerPtr.get());
+
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubLedger(_, _, fmt::format("{}-{}", SEQ - 1, SEQ), 1)).Times(1);
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubBookChanges).Times(1);
+    // mock 1 transaction
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubTransaction).Times(1);
+
+    ctx.run();
+    // last publish time should be set
+    EXPECT_TRUE(publisher.lastPublishAgeSeconds() <= 1);
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsTrue)
+{
+    SystemState dummyState;
+    dummyState.isStopping = true;
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    EXPECT_FALSE(publisher.publish(SEQ, {}));
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqMaxAttampt)
+{
+    SystemState dummyState;
+    dummyState.isStopping = false;
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+
+    static auto constexpr MAX_ATTEMPT = 2;
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    EXPECT_CALL(*rawBackendPtr, hardFetchLedgerRange).Times(MAX_ATTEMPT);
+
+    LedgerRange const range{.minSequence = SEQ - 1, .maxSequence = SEQ - 1};
+    ON_CALL(*rawBackendPtr, hardFetchLedgerRange(_)).WillByDefault(Return(range));
+    EXPECT_FALSE(publisher.publish(SEQ, MAX_ATTEMPT));
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishLedgerSeqStopIsFalse)
+{
+    SystemState dummyState;
+    dummyState.isStopping = false;
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    LedgerRange const range{.minSequence = SEQ, .maxSequence = SEQ};
+    ON_CALL(*rawBackendPtr, hardFetchLedgerRange(_)).WillByDefault(Return(range));
+    EXPECT_CALL(*rawBackendPtr, hardFetchLedgerRange).Times(1);
+
+    auto const dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, AGE);
+    ON_CALL(*rawBackendPtr, fetchLedgerBySequence(SEQ, _)).WillByDefault(Return(dummyLedgerInfo));
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence).Times(1);
+
+    ON_CALL(*rawBackendPtr, fetchLedgerDiff(SEQ, _)).WillByDefault(Return(std::vector<LedgerObject>{}));
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(SEQ, _)).Times(1);
+    EXPECT_CALL(mockCache, updateImp).Times(1);
+
+    EXPECT_TRUE(publisher.publish(SEQ, {}));
+    ctx.run();
+}
+
+TEST_F(ETLLedgerPublisherTest, PublishMultipleTxInOrder)
+{
+    SystemState dummyState;
+    dummyState.isWriting = true;
+
+    auto const dummyLedgerInfo = CreateLedgerInfo(LEDGERHASH, SEQ, 0);  // age is 0
+    detail::LedgerPublisher publisher(ctx, mockBackendPtr, mockCache, mockSubscriptionManagerPtr, dummyState);
+    mockBackendPtr->updateRange(SEQ - 1);
+    mockBackendPtr->updateRange(SEQ);
+
+    publisher.publish(dummyLedgerInfo);
+
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    EXPECT_CALL(*rawBackendPtr, fetchLedgerDiff(_, _)).Times(0);
+
+    // mock fetch fee
+    EXPECT_CALL(*rawBackendPtr, doFetchLedgerObject).Times(1);
+    ON_CALL(*rawBackendPtr, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
+        .WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
+
+    // mock fetch transactions
+    EXPECT_CALL(*rawBackendPtr, fetchAllTransactionsInLedger).Times(1);
+    // t1 index > t2 index
+    TransactionAndMetadata t1;
+    t1.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 100, 3, SEQ).getSerializer().peekData();
+    t1.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30, 2).getSerializer().peekData();
+    t1.ledgerSequence = SEQ;
+    t1.date = 1;
+    TransactionAndMetadata t2;
+    t2.transaction = CreatePaymentTransactionObject(ACCOUNT, ACCOUNT2, 100, 3, SEQ).getSerializer().peekData();
+    t2.metadata = CreatePaymentTransactionMetaObject(ACCOUNT, ACCOUNT2, 110, 30, 1).getSerializer().peekData();
+    t2.ledgerSequence = SEQ;
+    t2.date = 2;
+    ON_CALL(*rawBackendPtr, fetchAllTransactionsInLedger(SEQ, _))
+        .WillByDefault(Return(std::vector<TransactionAndMetadata>{t1, t2}));
+
+    // setLastPublishedSequence not in strand, should verify before run
+    EXPECT_TRUE(publisher.getLastPublishedSequence());
+    EXPECT_EQ(publisher.getLastPublishedSequence().value(), SEQ);
+
+    MockSubscriptionManager* rawSubscriptionManagerPtr =
+        dynamic_cast<MockSubscriptionManager*>(mockSubscriptionManagerPtr.get());
+
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubLedger(_, _, fmt::format("{}-{}", SEQ - 1, SEQ), 2)).Times(1);
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubBookChanges).Times(1);
+    // should call pubTransaction t2 first (greater tx index)
+    Sequence const s;
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubTransaction(t2, _)).InSequence(s);
+    EXPECT_CALL(*rawSubscriptionManagerPtr, pubTransaction(t1, _)).InSequence(s);
+
+    ctx.run();
+    // last publish time should be set
+    EXPECT_TRUE(publisher.lastPublishAgeSeconds() <= 1);
+}

unittests/rpc/handlers/AccountObjectsTests.cpp

@@ -85,7 +85,7 @@ generateTestValuesForParametersTest()
             "TypeInvalid",
             R"({"account":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "type":"wrong"})",
             "invalidParams",
-            "Invalid parameters."},
+            "Invalid field 'type'."},
         AccountObjectsParamTestCaseBundle{
             "LedgerHashInvalid",
             R"({"account":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", "ledger_hash":"1"})",

unittests/rpc/handlers/AccountTxTests.cpp

@@ -47,7 +47,7 @@ struct AccountTxParamTestCaseBundle
     std::string testJson;
     std::optional<std::string> expectedError;
     std::optional<std::string> expectedErrorMessage;
-    std::uint32_t apiVersion = 2;
+    std::uint32_t apiVersion = 2u;
 };
 
 // parameterized test cases for parameters check
@@ -353,10 +353,9 @@ TEST_P(AccountTxParameterTest, CheckParams)
     mockBackendPtr->updateRange(MINSEQ);  // min
     mockBackendPtr->updateRange(MAXSEQ);  // max
     auto const testBundle = GetParam();
-    auto* rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
-    std::cout << "Before parse" << std::endl;
+    auto* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    ASSERT_NE(rawBackendPtr, nullptr);
     auto const req = json::parse(testBundle.testJson);
-    std::cout << "After parse" << std::endl;
     if (testBundle.expectedError.has_value())
     {
         ASSERT_TRUE(testBundle.expectedErrorMessage.has_value());
@@ -372,14 +371,6 @@ TEST_P(AccountTxParameterTest, CheckParams)
     }
     else
     {
-        if (req.as_object().contains("ledger_hash"))
-        {
-            EXPECT_CALL(*rawBackendPtr, fetchLedgerByHash).WillOnce(testing::Return(ripple::LedgerHeader{}));
-        }
-        else if (req.as_object().contains("ledger_index"))
-        {
-            EXPECT_CALL(*rawBackendPtr, fetchLedgerBySequence).WillOnce(testing::Return(ripple::LedgerHeader{}));
-        }
         EXPECT_CALL(*rawBackendPtr, fetchAccountTransactions);
 
         runSpawn([&, this](auto yield) {
@@ -656,7 +647,7 @@ TEST_F(RPCAccountTxHandlerTest, BinaryTrue)
             "144B4E9C06F24296074F7BC48F92A97916C6DC5EA98314D31252CF902EF8DD8451"
             "243869B38667CBD89DF3");
         EXPECT_FALSE(output->at("transactions").as_array()[0].as_object().contains("date"));
-
+        EXPECT_FALSE(output->at("transactions").as_array()[0].as_object().contains("inLedger"));
         EXPECT_FALSE(output->as_object().contains("limit"));
     });
 }
@@ -962,7 +953,236 @@ TEST_F(RPCAccountTxHandlerTest, TxLargerThanMaxSeq)
     });
 }
 
-TEST_F(RPCAccountTxHandlerTest, NFTTxs)
+TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v1)
+{
+    auto const OUT = R"({
+            "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+            "ledger_index_min": 10,
+            "ledger_index_max": 30,
+            "transactions": [
+                {
+                    "meta": {
+                        "AffectedNodes": 
+                        [
+                            {
+                                "ModifiedNode": 
+                                {
+                                    "FinalFields": 
+                                    {
+                                        "NFTokens": 
+                                        [
+                                            {
+                                                "NFToken": 
+                                                {
+                                                    "NFTokenID": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF",
+                                                    "URI": "7465737475726C"
+                                                }
+                                            },
+                                            {
+                                                "NFToken": 
+                                                {
+                                                    "NFTokenID": "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC",
+                                                    "URI": "7465737475726C"
+                                                }
+                                            }
+                                        ]
+                                    },
+                                    "LedgerEntryType": "NFTokenPage",
+                                    "PreviousFields": 
+                                    {
+                                        "NFTokens": 
+                                        [
+                                            {
+                                                "NFToken": 
+                                                {
+                                                    "NFTokenID": "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC",
+                                                    "URI": "7465737475726C"
+                                                }
+                                            }
+                                        ]
+                                    }
+                                }
+                            }
+                        ],
+                        "TransactionIndex": 0,
+                        "TransactionResult": "tesSUCCESS",
+                        "nftoken_id": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF"
+                    },
+                    "tx": 
+                    {
+                        "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                        "Fee": "50",
+                        "NFTokenTaxon": 123,
+                        "Sequence": 1,
+                        "SigningPubKey": "74657374",
+                        "TransactionType": "NFTokenMint",
+                        "hash": "C74463F49CFDCBEF3E9902672719918CDE5042DC7E7660BEBD1D1105C4B6DFF4",
+                        "ledger_index": 11,
+                        "inLedger": 11,
+                        "date": 1
+                    },
+                    "validated": true
+                },
+                {
+                    "meta": 
+                    {
+                        "AffectedNodes": 
+                        [
+                            {
+                                "DeletedNode": 
+                                {
+                                    "FinalFields": 
+                                    {
+                                        "NFTokenID": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"
+                                    },
+                                    "LedgerEntryType": "NFTokenOffer"
+                                }
+                            }
+                        ],
+                        "TransactionIndex": 0,
+                        "TransactionResult": "tesSUCCESS",
+                        "nftoken_id": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"
+                    },
+                    "tx": 
+                    {
+                        "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                        "Fee": "50",
+                        "NFTokenBuyOffer": "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC",
+                        "Sequence": 1,
+                        "SigningPubKey": "74657374",
+                        "TransactionType": "NFTokenAcceptOffer",
+                        "hash": "7682BE6BCDE62F8142915DD852936623B68FC3839A8A424A6064B898702B0CDF",
+                        "ledger_index": 11,
+                        "inLedger": 11,
+                        "date": 2
+                    },
+                    "validated": true
+                },
+                {
+                    "meta": 
+                    {
+                        "AffectedNodes": 
+                        [
+                            {
+                                "DeletedNode": {
+                                    "FinalFields": 
+                                    {
+                                        "NFTokenID": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"
+                                    },
+                                    "LedgerEntryType": "NFTokenOffer"
+                                }
+                            },
+                            {
+                                "DeletedNode": 
+                                {
+                                    "FinalFields": 
+                                    {
+                                        "NFTokenID": "15FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF"
+                                    },
+                                    "LedgerEntryType": "NFTokenOffer"
+                                }
+                            }
+                        ],
+                        "TransactionIndex": 0,
+                        "TransactionResult": "tesSUCCESS",
+                        "nftoken_ids": 
+                        [
+                            "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA",
+                            "15FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF"
+                        ]
+                    },
+                    "tx": 
+                    {
+                        "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                        "Fee": "50",
+                        "NFTokenOffers": 
+                        [
+                            "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA",
+                            "15FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF"
+                        ],
+                        "Sequence": 1,
+                        "SigningPubKey": "74657374",
+                        "TransactionType": "NFTokenCancelOffer",
+                        "hash": "9F82743EEB30065FB9CB92C61F0F064B5859C5A590FA811FAAAD9C988E5B47DB",
+                        "ledger_index": 11,
+                        "inLedger": 11,
+                        "date": 3
+                    },
+                    "validated": true
+                },
+                {
+                    "meta": 
+                    {
+                        "AffectedNodes": 
+                        [
+                            {
+                                "CreatedNode": 
+                                {
+                                    "LedgerEntryType": "NFTokenOffer",
+                                    "LedgerIndex": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"
+                                }
+                            }
+                        ],
+                        "TransactionIndex": 0,
+                        "TransactionResult": "tesSUCCESS",
+                        "offer_id": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA"
+                    },
+                    "tx": 
+                    {
+                        "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                        "Amount": "123",
+                        "Fee": "50",
+                        "NFTokenID": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF",
+                        "Sequence": 1,
+                        "SigningPubKey": "74657374",
+                        "TransactionType": "NFTokenCreateOffer",
+                        "hash": "ECB1837EB7C7C0AC22ECDCCE59FDD4795C70E0B9D8F4E1C9A9408BB7EC75DA5C",
+                        "ledger_index": 11,
+                        "inLedger": 11,
+                        "date": 4
+                    },
+                    "validated": true
+                }
+            ],
+            "validated": true,
+            "marker": 
+            {
+                "ledger": 12,
+                "seq": 34
+            }
+        })";
+    mockBackendPtr->updateRange(MINSEQ);  // min
+    mockBackendPtr->updateRange(MAXSEQ);  // max
+    MockBackend* rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
+    auto const transactions = genNFTTransactions(MINSEQ + 1);
+    auto const transCursor = TransactionsAndCursor{transactions, TransactionsCursor{12, 34}};
+    ON_CALL(*rawBackendPtr, fetchAccountTransactions).WillByDefault(Return(transCursor));
+    EXPECT_CALL(
+        *rawBackendPtr,
+        fetchAccountTransactions(
+            testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_))
+        .Times(1);
+
+    runSpawn([&, this](auto yield) {
+        auto const handler = AnyHandler{AccountTxHandler{mockBackendPtr}};
+        auto const static input = json::parse(fmt::format(
+            R"({{
+                "account": "{}",
+                "ledger_index_min": {},
+                "ledger_index_max": {},
+                "forward": false,
+                "marker": {{"ledger": 10, "seq": 11}}
+            }})",
+            ACCOUNT,
+            -1,
+            -1));
+        auto const output = handler.process(input, Context{.yield = yield, .apiVersion = 1u});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(*output, json::parse(OUT));
+    });
+}
+
+TEST_F(RPCAccountTxHandlerTest, NFTTxs_API_v2)
 {
     auto const OUT = R"({
             "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
@@ -1158,7 +1378,8 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs)
         })";
     mockBackendPtr->updateRange(MINSEQ);  // min
     mockBackendPtr->updateRange(MAXSEQ);  // max
-    MockBackend* rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
+    MockBackend* rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    ASSERT_NE(rawBackendPtr, nullptr);
     auto const transactions = genNFTTransactions(MINSEQ + 1);
     auto const transCursor = TransactionsAndCursor{transactions, TransactionsCursor{12, 34}};
     ON_CALL(*rawBackendPtr, fetchAccountTransactions).WillByDefault(Return(transCursor));
@@ -1181,7 +1402,7 @@ TEST_F(RPCAccountTxHandlerTest, NFTTxs)
             ACCOUNT,
             -1,
             -1));
-        auto const output = handler.process(input, Context{yield});
+        auto const output = handler.process(input, Context{.yield = yield, .apiVersion = 2u});
         ASSERT_TRUE(output);
         EXPECT_EQ(*output, json::parse(OUT));
     });
@@ -1192,6 +1413,7 @@ struct AccountTxTransactionBundle
     std::string testName;
     std::string testJson;
     std::string result;
+    std::uint32_t apiVersion = 2u;
 };
 
 // parameterized test cases for parameters check
@@ -1351,7 +1573,57 @@ generateTransactionTypeTestValues()
             })",
             "[]"},
         AccountTxTransactionBundle{
-            "Payment",
+            "Payment_API_v1",
+            R"({
+                "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                "ledger_index": "validated",
+                "tx_type": "Payment"
+            })",
+            R"([
+                {
+                    "meta": {
+                        "AffectedNodes": [
+                        {
+                            "ModifiedNode": {
+                                "FinalFields": {
+                                    "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                                    "Balance": "22"
+                                },
+                                "LedgerEntryType": "AccountRoot"
+                            }
+                        },
+                        {
+                            "ModifiedNode": {
+                                "FinalFields": {
+                                    "Account": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun",
+                                    "Balance": "23"
+                                },
+                                "LedgerEntryType": "AccountRoot"
+                            }
+                        }],
+                        "TransactionIndex": 0,
+                        "TransactionResult": "tesSUCCESS",
+                        "delivered_amount": "unavailable"
+                    },
+                    "tx": {
+                        "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                        "Amount": "1",
+                        "Destination": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun",
+                        "Fee": "1",
+                        "Sequence": 32,
+                        "SigningPubKey": "74657374",
+                        "TransactionType": "Payment",
+                        "hash": "51D2AAA6B8E4E16EF22F6424854283D8391B56875858A711B8CE4D5B9A422CC2",
+                        "ledger_index": 30,
+                        "inLedger": 30,
+                        "date": 1
+                    },
+                    "validated": true
+                }
+            ])",
+            1u},
+        AccountTxTransactionBundle{
+            "Payment_API_v2",
             R"({
                 "account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
                 "ledger_index": "validated",
@@ -1397,7 +1669,8 @@ generateTransactionTypeTestValues()
                 },
                 "validated": true
                 }
-            ])"},
+            ])",
+            2u},
         AccountTxTransactionBundle{
             "PaymentChannelClaim",
             R"({
@@ -1484,7 +1757,7 @@ TEST_P(AccountTxTransactionTypeTest, SpecificTransactionType)
     runSpawn([&, this](auto yield) {
         auto const handler = AnyHandler{AccountTxHandler{mockBackendPtr}};
         auto const req = json::parse(testBundle.testJson);
-        auto const output = handler.process(req, Context{yield});
+        auto const output = handler.process(req, Context{.yield = yield, .apiVersion = testBundle.apiVersion});
         EXPECT_TRUE(output);
 
         auto const transactions = output->at("transactions").as_array();

unittests/rpc/handlers/BookOffersTests.cpp

@@ -256,7 +256,7 @@ generateParameterBookOffersTestBundles()
                 "taker": "123"
             })",
             "invalidParams",
-            "Invalid field 'taker'"},
+            "Invalid field 'taker'."},
         ParameterTestBundle{
             "TakerNotString",
             R"({
@@ -272,7 +272,7 @@ generateParameterBookOffersTestBundles()
                 "taker": 123
             })",
             "invalidParams",
-            "Invalid field 'taker'"},
+            "Invalid field 'taker'."},
         ParameterTestBundle{
             "LimitNotInt",
             R"({
@@ -384,8 +384,7 @@ generateParameterBookOffersTestBundles()
                 }
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
         ParameterTestBundle{
             "PaysCurrencyWithXRPIssuer",
             R"({
@@ -430,8 +429,7 @@ generateParameterBookOffersTestBundles()
                 }            
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
         ParameterTestBundle{
             "BadMarket",
             R"({

unittests/rpc/handlers/LedgerEntryTests.cpp

@@ -103,7 +103,7 @@ generateTestValuesForParametersTest()
             "invalidParams",
             "ledgerIndexMalformed"},
 
-        ParamTestCaseBundle{"UnknownOption", R"({})", "unknownOption", "Unknown option."},
+        ParamTestCaseBundle{"UnknownOption", R"({})", "invalidParams", "Invalid parameters."},
 
         ParamTestCaseBundle{
             "InvalidDepositPreauthType",
@@ -1094,8 +1094,8 @@ TEST_F(RPCLedgerEntryTest, InvalidEntryTypeVersion2)
         auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2});
         ASSERT_FALSE(output);
         auto const err = rpc::makeError(output.error());
-        EXPECT_EQ(err.at("error").as_string(), "unknownOption");
-        EXPECT_EQ(err.at("error_message").as_string(), "Unknown option.");
+        EXPECT_EQ(err.at("error").as_string(), "invalidParams");
+        EXPECT_EQ(err.at("error_message").as_string(), "Invalid parameters.");
     });
 }
 
@@ -1107,7 +1107,7 @@ TEST_F(RPCLedgerEntryTest, InvalidEntryTypeVersion1)
         auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1});
         ASSERT_FALSE(output);
         auto const err = rpc::makeError(output.error());
-        EXPECT_EQ(err.at("error").as_string(), "invalidParams");
-        EXPECT_EQ(err.at("error_message").as_string(), "Invalid parameters.");
+        EXPECT_EQ(err.at("error").as_string(), "unknownOption");
+        EXPECT_EQ(err.at("error_message").as_string(), "Unknown option.");
     });
 }

unittests/rpc/handlers/SubscribeTests.cpp

@@ -410,8 +410,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
         SubscribeParamTestCaseBundle{
             "BooksItemTakerPaysXRPHasIssuer",
             R"({
@@ -430,8 +429,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
         SubscribeParamTestCaseBundle{
             "BooksItemBadMartket",
             R"({

unittests/rpc/handlers/TxTests.cpp

@@ -28,12 +28,49 @@ using namespace rpc;
 namespace json = boost::json;
 using namespace testing;
 
-constexpr static auto TXNID = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DD";
-constexpr static auto NFTID = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF";
-constexpr static auto NFTID2 = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA";
-constexpr static auto ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
-constexpr static auto ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
-constexpr static auto CURRENCY = "0158415500000000C1F76FF6ECB0BAC600000000";
+auto constexpr static TXNID = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DD";
+auto constexpr static NFTID = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DF";
+auto constexpr static NFTID2 = "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DA";
+auto constexpr static ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
+auto constexpr static ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
+auto constexpr static CURRENCY = "0158415500000000C1F76FF6ECB0BAC600000000";
+
+auto constexpr static DEFAULT_OUT = R"({
+    "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+    "Fee": "2",
+    "Sequence": 100,
+    "SigningPubKey": "74657374",
+    "TakerGets": {
+        "currency": "0158415500000000C1F76FF6ECB0BAC600000000",
+        "issuer": "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun",
+        "value": "200"
+    },
+    "TakerPays": "300",
+    "TransactionType": "OfferCreate",
+    "hash": "2E2FBAAFF767227FE4381C4BE9855986A6B9F96C62F6E443731AB36F7BBB8A08",
+    "meta": {
+        "AffectedNodes": [
+            {
+                "CreatedNode": {
+                    "LedgerEntryType": "Offer",
+                    "NewFields": {
+                        "TakerGets": "200",
+                        "TakerPays": {
+                            "currency": "0158415500000000C1F76FF6ECB0BAC600000000",
+                            "issuer": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
+                            "value": "300"
+                        }
+                    }
+                }
+            }
+        ],
+        "TransactionIndex": 100,
+        "TransactionResult": "tesSUCCESS"
+    },
+    "date": 123456,
+    "ledger_index": 100,
+    "validated": true
+})";
 
 class RPCTxTest : public HandlerBaseTest
 {
@@ -48,7 +85,7 @@ TEST_F(RPCTxTest, ExcessiveLgrRange)
                 "command": "tx",
                 "transaction": "{}",
                 "min_ledger": 1,
-                "max_ledger":1002
+                "max_ledger": 1002
             }})",
             TXNID));
         auto const output = handler.process(req, Context{yield});
@@ -119,7 +156,7 @@ TEST_F(RPCTxTest, TxnNotFoundInGivenRangeSearchAllFalse)
                 "command": "tx",
                 "transaction": "{}",
                 "min_ledger": 1,
-                "max_ledger":1000
+                "max_ledger": 1000
             }})",
             TXNID));
         auto const output = handler.process(req, Context{yield});
@@ -147,7 +184,7 @@ TEST_F(RPCTxTest, TxnNotFoundInGivenRangeSearchAllTrue)
                 "command": "tx",
                 "transaction": "{}",
                 "min_ledger": 1,
-                "max_ledger":1000
+                "max_ledger": 1000
             }})",
             TXNID));
         auto const output = handler.process(req, Context{yield});
@@ -160,53 +197,21 @@ TEST_F(RPCTxTest, TxnNotFoundInGivenRangeSearchAllTrue)
     });
 }
 
-TEST_F(RPCTxTest, DefaultParameter)
+TEST_F(RPCTxTest, DefaultParameter_API_v1)
 {
-    auto constexpr static OUT = R"({
-            "Account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
-            "Fee":"2",
-            "Sequence":100,
-            "SigningPubKey":"74657374",
-            "TakerGets":{
-                "currency":"0158415500000000C1F76FF6ECB0BAC600000000",
-                "issuer":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun",
-                "value":"200"
-            },
-            "TakerPays":"300",
-            "TransactionType":"OfferCreate",
-            "hash":"2E2FBAAFF767227FE4381C4BE9855986A6B9F96C62F6E443731AB36F7BBB8A08",
-            "meta":{
-                "AffectedNodes":[
-                    {
-                        "CreatedNode":{
-                        "LedgerEntryType":"Offer",
-                        "NewFields":{
-                            "TakerGets":"200",
-                            "TakerPays":{
-                                "currency":"0158415500000000C1F76FF6ECB0BAC600000000",
-                                "issuer":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
-                                "value":"300"
-                            }
-                        }
-                        }
-                    }
-                ],
-                "TransactionIndex":100,
-                "TransactionResult":"tesSUCCESS"
-            },
-            "date":123456,
-            "ledger_index":100,
-            "validated": true
-    })";
-    auto const rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
+    auto const rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    ASSERT_NE(rawBackendPtr, nullptr);
+
     TransactionAndMetadata tx;
     tx.metadata = CreateMetaDataForCreateOffer(CURRENCY, ACCOUNT, 100, 200, 300).getSerializer().peekData();
     tx.transaction =
         CreateCreateOfferTransactionObject(ACCOUNT, 2, 100, CURRENCY, ACCOUNT2, 200, 300).getSerializer().peekData();
     tx.date = 123456;
     tx.ledgerSequence = 100;
+
     ON_CALL(*rawBackendPtr, fetchTransaction(ripple::uint256{TXNID}, _)).WillByDefault(Return(tx));
     EXPECT_CALL(*rawBackendPtr, fetchTransaction).Times(1);
+
     runSpawn([this](auto yield) {
         auto const handler = AnyHandler{TxHandler{mockBackendPtr}};
         auto const req = json::parse(fmt::format(
@@ -215,20 +220,54 @@ TEST_F(RPCTxTest, DefaultParameter)
                 "transaction": "{}"
             }})",
             TXNID));
-        auto const output = handler.process(req, Context{yield});
+        auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 1u});
         ASSERT_TRUE(output);
-        EXPECT_EQ(*output, json::parse(OUT));
+
+        auto v1Output = json::parse(DEFAULT_OUT);
+        v1Output.as_object()[JS(inLedger)] = v1Output.as_object()[JS(ledger_index)];
+        EXPECT_EQ(*output, v1Output);
+    });
+}
+
+TEST_F(RPCTxTest, DefaultParameter_API_v2)
+{
+    auto const rawBackendPtr = dynamic_cast<MockBackend*>(mockBackendPtr.get());
+    ASSERT_NE(rawBackendPtr, nullptr);
+
+    TransactionAndMetadata tx;
+    tx.metadata = CreateMetaDataForCreateOffer(CURRENCY, ACCOUNT, 100, 200, 300).getSerializer().peekData();
+    tx.transaction =
+        CreateCreateOfferTransactionObject(ACCOUNT, 2, 100, CURRENCY, ACCOUNT2, 200, 300).getSerializer().peekData();
+    tx.date = 123456;
+    tx.ledgerSequence = 100;
+
+    ON_CALL(*rawBackendPtr, fetchTransaction(ripple::uint256{TXNID}, _)).WillByDefault(Return(tx));
+    EXPECT_CALL(*rawBackendPtr, fetchTransaction).Times(1);
+
+    runSpawn([this](auto yield) {
+        auto const handler = AnyHandler{TxHandler{mockBackendPtr}};
+        auto const req = json::parse(fmt::format(
+            R"({{ 
+                "command": "tx",
+                "transaction": "{}"
+            }})",
+            TXNID));
+        auto const output = handler.process(req, Context{.yield = yield, .apiVersion = 2u});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(*output, json::parse(DEFAULT_OUT));
     });
 }
 
 TEST_F(RPCTxTest, ReturnBinary)
 {
+    // Note: `inLedger` is API v1 only. See DefaultOutput_*
     auto constexpr static OUT = R"({
-        "meta":"201C00000064F8E311006FE864D50AA87BEE5380000158415500000000C1F76FF6ECB0BAC6000000004B4E9C06F24296074F7BC48F92A97916C6DC5EA96540000000000000C8E1E1F1031000",
-        "tx":"120007240000006464400000000000012C65D5071AFD498D00000158415500000000C1F76FF6ECB0BAC600000000D31252CF902EF8DD8451243869B38667CBD89DF368400000000000000273047465737481144B4E9C06F24296074F7BC48F92A97916C6DC5EA9",
-        "hash":"05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DD",
-        "date":123456,
-        "ledger_index":100,
+        "meta": "201C00000064F8E311006FE864D50AA87BEE5380000158415500000000C1F76FF6ECB0BAC6000000004B4E9C06F24296074F7BC48F92A97916C6DC5EA96540000000000000C8E1E1F1031000",
+        "tx": "120007240000006464400000000000012C65D5071AFD498D00000158415500000000C1F76FF6ECB0BAC600000000D31252CF902EF8DD8451243869B38667CBD89DF368400000000000000273047465737481144B4E9C06F24296074F7BC48F92A97916C6DC5EA9",
+        "hash": "05FB0EB4B899F056FA095537C5817163801F544BAFCEA39C995D76DB4D16F9DD",
+        "date": 123456,
+        "ledger_index": 100,
+        "inLedger": 100,
         "validated": true
     })";
     auto const rawBackendPtr = static_cast<MockBackend*>(mockBackendPtr.get());
@@ -257,6 +296,7 @@ TEST_F(RPCTxTest, ReturnBinary)
 
 TEST_F(RPCTxTest, MintNFT)
 {
+    // Note: `inLedger` is API v1 only. See DefaultOutput_*
     auto const static OUT = fmt::format(
         R"({{
             "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
@@ -307,9 +347,10 @@ TEST_F(RPCTxTest, MintNFT)
                 "TransactionResult": "tesSUCCESS",
                 "nftoken_id": "{}"
             }},
-            "validated": true,
             "date": 123456,
-            "ledger_index": 100
+            "ledger_index": 100,
+            "inLedger": 100,
+            "validated": true
         }})",
         NFTID,
         NFTID);

unittests/rpc/handlers/UnsubscribeTests.cpp

@@ -405,8 +405,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "dstIsrMalformed",
-            "Unneeded field 'taker_gets.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_gets.issuer' for XRP currency specification."},
         UnsubscribeParamTestCaseBundle{
             "BooksItemTakerPaysXRPHasIssuer",
             R"({
@@ -426,8 +425,7 @@ generateTestValuesForParametersTest()
                 ]
             })",
             "srcIsrMalformed",
-            "Unneeded field 'taker_pays.issuer' for XRP currency "
-            "specification."},
+            "Unneeded field 'taker_pays.issuer' for XRP currency specification."},
         UnsubscribeParamTestCaseBundle{
             "BooksItemBadMartket",
             R"({

unittests/util/TestObject.cpp

@@ -106,7 +106,8 @@ CreatePaymentTransactionMetaObject(
     std::string_view accountId1,
     std::string_view accountId2,
     int finalBalance1,
-    int finalBalance2)
+    int finalBalance2,
+    uint32_t transactionIndex)
 {
     ripple::STObject finalFields(ripple::sfFinalFields);
     finalFields.setAccountID(ripple::sfAccount, GetAccountIDWithString(accountId1));
@@ -128,7 +129,7 @@ CreatePaymentTransactionMetaObject(
     metaArray.push_back(node2);
     metaObj.setFieldArray(ripple::sfAffectedNodes, metaArray);
     metaObj.setFieldU8(ripple::sfTransactionResult, ripple::tesSUCCESS);
-    metaObj.setFieldU32(ripple::sfTransactionIndex, 0);
+    metaObj.setFieldU32(ripple::sfTransactionIndex, transactionIndex);
     return metaObj;
 }
 

unittests/util/TestObject.h

@@ -75,7 +75,8 @@ CreatePaymentTransactionMetaObject(
     std::string_view accountId1,
     std::string_view accountId2,
     int finalBalance1,
-    int finalBalance2);
+    int finalBalance2,
+    uint32_t transactionIndex = 0);
 
 /*
  * Create an account root ledger object

unittests/web/RPCServerHandlerTests.cpp

@@ -549,7 +549,7 @@ TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableNotArray)
     EXPECT_EQ(session->lastStatus, boost::beast::http::status::bad_request);
 }
 
-TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableEmptyArray)
+TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableArrayWithDigit)
 {
     static auto constexpr response = "params unparseable";
 
@@ -558,7 +558,7 @@ TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableEmptyArray)
 
     static auto constexpr requestJSON = R"({
                                             "method": "ledger",
-                                            "params": []
+                                            "params": [1]
                                         })";
 
     EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);