fix: gateway_balance discrepancy (#1839) (#1874)

Port of #1839 into 2.3.1.
Fixes #1832.

rippled code:


https://github.com/XRPLF/rippled/blob/develop/src/xrpld/rpc/handlers/GatewayBalances.cpp#L129

.github/workflows/build.yml

@@ -74,7 +74,7 @@ jobs:
             conan_profile: clang
             code_coverage: false
             static: true
-          - os: macos14
+          - os: macos15
             build_type: Release
             code_coverage: false
             static: false
@@ -197,7 +197,7 @@ jobs:
               image: rippleci/clio_ci:latest
             conan_profile: clang
             build_type: Debug
-          - os: macos14
+          - os: macos15
             conan_profile: apple_clang_15
             build_type: Release
     runs-on: [self-hosted, "${{ matrix.os }}"]

conanfile.py

@@ -28,7 +28,7 @@ class Clio(ConanFile):
         'protobuf/3.21.9',
         'grpc/1.50.1',
         'openssl/1.1.1u',
-        'xrpl/2.3.0-rc2',
+        'xrpl/2.3.1',
         'zlib/1.3.1',
         'libbacktrace/cci.20210118'
     ]

src/rpc/Errors.cpp

@@ -79,7 +79,6 @@ getErrorInfo(ClioError code)
         {ClioError::rpcMALFORMED_REQUEST, "malformedRequest", "Malformed request."},
         {ClioError::rpcMALFORMED_OWNER, "malformedOwner", "Malformed owner."},
         {ClioError::rpcMALFORMED_ADDRESS, "malformedAddress", "Malformed address."},
-        {ClioError::rpcINVALID_HOT_WALLET, "invalidHotWallet", "Invalid hot wallet."},
         {ClioError::rpcUNKNOWN_OPTION, "unknownOption", "Unknown option."},
         {ClioError::rpcFIELD_NOT_FOUND_TRANSACTION, "fieldNotFoundTransaction", "Missing field."},
         {ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID, "malformedDocumentID", "Malformed oracle_document_id."},

src/rpc/Errors.hpp

@@ -39,7 +39,6 @@ enum class ClioError {
     rpcMALFORMED_REQUEST = 5001,
     rpcMALFORMED_OWNER = 5002,
     rpcMALFORMED_ADDRESS = 5003,
-    rpcINVALID_HOT_WALLET = 5004,
     rpcUNKNOWN_OPTION = 5005,
     rpcFIELD_NOT_FOUND_TRANSACTION = 5006,
     rpcMALFORMED_ORACLE_DOCUMENT_ID = 5007,

src/rpc/common/APIVersion.hpp

@@ -34,16 +34,13 @@ static constexpr uint32_t API_VERSION_DEFAULT = 1u;
 
 /**
  * @brief Minimum API version supported by this build
- *
- * Note: Clio does not natively support v1 and only supports v2 or newer.
- * However, Clio will forward all v1 requests to rippled for backward compatibility.
  */
 static constexpr uint32_t API_VERSION_MIN = 1u;
 
 /**
  * @brief Maximum API version supported by this build
  */
-static constexpr uint32_t API_VERSION_MAX = 2u;
+static constexpr uint32_t API_VERSION_MAX = 3u;
 
 /**
  * @brief A baseclass for API version helper

src/rpc/handlers/AccountTx.hpp

@@ -39,7 +39,6 @@
 #include <xrpl/protocol/jss.h>
 
 #include <cstdint>
-#include <limits>
 #include <memory>
 #include <optional>
 #include <string>
@@ -57,8 +56,8 @@ class AccountTxHandler {
     std::shared_ptr<BackendInterface> sharedPtrBackend_;
 
 public:
-    // no max limit
     static auto constexpr LIMIT_MIN = 1;
+    static auto constexpr LIMIT_MAX = 1000;
     static auto constexpr LIMIT_DEFAULT = 200;
 
     /**
@@ -133,7 +132,7 @@ public:
             {JS(limit),
              validation::Type<uint32_t>{},
              validation::Min(1u),
-             modifiers::Clamp<int32_t>{LIMIT_MIN, std::numeric_limits<int32_t>::max()}},
+             modifiers::Clamp<int32_t>{LIMIT_MIN, LIMIT_MAX}},
             {JS(marker),
              meta::WithCustomError{
                  validation::Type<boost::json::object>{},

src/rpc/handlers/GatewayBalances.cpp

@@ -142,10 +142,6 @@ GatewayBalancesHandler::process(GatewayBalancesHandler::Input input, Context con
     if (auto status = std::get_if<Status>(&ret))
         return Error{*status};
 
-    auto inHotbalances = [&](auto const& hw) { return output.hotBalances.contains(hw); };
-    if (not std::all_of(input.hotWallets.begin(), input.hotWallets.end(), inHotbalances))
-        return Error{Status{ClioError::rpcINVALID_HOT_WALLET}};
-
     output.accountID = input.account;
     output.ledgerHash = ripple::strHex(lgrInfo.hash);
     output.ledgerIndex = lgrInfo.seq;

src/rpc/handlers/GatewayBalances.hpp

@@ -108,44 +108,51 @@ public:
     static RpcSpecConstRef
     spec([[maybe_unused]] uint32_t apiVersion)
     {
-        static auto const hotWalletValidator =
-            validation::CustomValidator{[](boost::json::value const& value, std::string_view key) -> MaybeError {
-                if (!value.is_string() && !value.is_array())
-                    return Error{Status{RippledError::rpcINVALID_PARAMS, std::string(key) + "NotStringOrArray"}};
+        auto const getHotWalletValidator = [](RippledError errCode) {
+            return validation::CustomValidator{
+                [errCode](boost::json::value const& value, std::string_view key) -> MaybeError {
+                    if (!value.is_string() && !value.is_array())
+                        return Error{Status{errCode, std::string(key) + "NotStringOrArray"}};
 
-                // wallet needs to be an valid accountID or public key
-                auto const wallets = value.is_array() ? value.as_array() : boost::json::array{value};
-                auto const getAccountID = [](auto const& j) -> std::optional<ripple::AccountID> {
-                    if (j.is_string()) {
-                        auto const pk = util::parseBase58Wrapper<ripple::PublicKey>(
-                            ripple::TokenType::AccountPublic, boost::json::value_to<std::string>(j)
-                        );
+                    // wallet needs to be an valid accountID or public key
+                    auto const wallets = value.is_array() ? value.as_array() : boost::json::array{value};
+                    auto const getAccountID = [](auto const& j) -> std::optional<ripple::AccountID> {
+                        if (j.is_string()) {
+                            auto const pk = util::parseBase58Wrapper<ripple::PublicKey>(
+                                ripple::TokenType::AccountPublic, boost::json::value_to<std::string>(j)
+                            );
 
-                        if (pk)
-                            return ripple::calcAccountID(*pk);
+                            if (pk)
+                                return ripple::calcAccountID(*pk);
 
-                        return util::parseBase58Wrapper<ripple::AccountID>(boost::json::value_to<std::string>(j));
+                            return util::parseBase58Wrapper<ripple::AccountID>(boost::json::value_to<std::string>(j));
+                        }
+
+                        return {};
+                    };
+
+                    for (auto const& wallet : wallets) {
+                        if (!getAccountID(wallet))
+                            return Error{Status{errCode, std::string(key) + "Malformed"}};
                     }
 
-                    return {};
-                };
-
-                for (auto const& wallet : wallets) {
-                    if (!getAccountID(wallet))
-                        return Error{Status{RippledError::rpcINVALID_PARAMS, std::string(key) + "Malformed"}};
+                    return MaybeError{};
                 }
-
-                return MaybeError{};
-            }};
-
-        static auto const rpcSpec = RpcSpec{
-            {JS(account), validation::Required{}, validation::CustomValidators::AccountValidator},
-            {JS(ledger_hash), validation::CustomValidators::Uint256HexStringValidator},
-            {JS(ledger_index), validation::CustomValidators::LedgerIndexValidator},
-            {JS(hotwallet), hotWalletValidator}
+            };
         };
 
-        return rpcSpec;
+        static auto const kSPEC_COMMON = RpcSpec{
+            {JS(account), validation::Required{}, validation::CustomValidators::AccountValidator},
+            {JS(ledger_hash), validation::CustomValidators::Uint256HexStringValidator},
+            {JS(ledger_index), validation::CustomValidators::LedgerIndexValidator}
+        };
+
+        auto static const kSPEC_V1 =
+            RpcSpec{kSPEC_COMMON, {{JS(hotwallet), getHotWalletValidator(ripple::rpcINVALID_HOTWALLET)}}};
+        auto static const kSPEC_V2 =
+            RpcSpec{kSPEC_COMMON, {{JS(hotwallet), getHotWalletValidator(ripple::rpcINVALID_PARAMS)}}};
+
+        return apiVersion == 1 ? kSPEC_V1 : kSPEC_V2;
     }
 
     /**

src/web/impl/ErrorHandling.hpp

@@ -87,7 +87,6 @@ public:
                     case rpc::ClioError::rpcMALFORMED_REQUEST:
                     case rpc::ClioError::rpcMALFORMED_OWNER:
                     case rpc::ClioError::rpcMALFORMED_ADDRESS:
-                    case rpc::ClioError::rpcINVALID_HOT_WALLET:
                     case rpc::ClioError::rpcFIELD_NOT_FOUND_TRANSACTION:
                     case rpc::ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID:
                     case rpc::ClioError::rpcMALFORMED_AUTHORIZED_CREDENTIALS:

src/web/impl/HttpBase.hpp

@@ -60,6 +60,14 @@
 
 namespace web::impl {
 
+static auto constexpr HealthCheckHTML = R"html(
+    <!DOCTYPE html>
+    <html>
+        <head><title>Test page for Clio</title></head>
+        <body><h1>Clio Test</h1><p>This page shows Clio http(s) connectivity is working.</p></body>
+    </html>
+)html";
+
 using tcp = boost::asio::ip::tcp;
 
 /**
@@ -205,6 +213,9 @@ public:
         if (ec)
             return httpFail(ec, "read");
 
+        if (req_.method() == http::verb::get and req_.target() == "/health")
+            return sender_(httpResponse(http::status::ok, "text/html", HealthCheckHTML));
+
         // Update isAdmin property of the connection
         ConnectionBase::isAdmin_ = adminVerification_->isAdmin(req_, this->clientIp);
 

tests/common/util/TestHttpClient.cpp

@@ -36,6 +36,7 @@
 #include <boost/beast/http.hpp>  // IWYU pragma: keep
 #include <boost/beast/http/field.hpp>
 #include <boost/beast/http/message.hpp>
+#include <boost/beast/http/status.hpp>
 #include <boost/beast/http/string_body.hpp>
 #include <boost/beast/http/verb.hpp>
 #include <boost/beast/http/write.hpp>  // IWYU pragma: keep
@@ -58,7 +59,7 @@ using tcp = boost::asio::ip::tcp;
 
 namespace {
 
-std::string
+std::pair<boost::beast::http::status, std::string>
 syncRequest(
     std::string const& host,
     std::string const& port,
@@ -96,7 +97,7 @@ syncRequest(
     boost::beast::error_code ec;
     stream.socket().shutdown(tcp::socket::shutdown_both, ec);
 
-    return res.body();
+    return {res.result(), res.body()};
 }
 
 }  // namespace
@@ -105,7 +106,7 @@ WebHeader::WebHeader(http::field name, std::string value) : name(name), value(st
 {
 }
 
-std::string
+std::pair<boost::beast::http::status, std::string>
 HttpSyncClient::post(
     std::string const& host,
     std::string const& port,
@@ -116,7 +117,7 @@ HttpSyncClient::post(
     return syncRequest(host, port, body, std::move(additionalHeaders), http::verb::post);
 }
 
-std::string
+std::pair<boost::beast::http::status, std::string>
 HttpSyncClient::get(
     std::string const& host,
     std::string const& port,

tests/common/util/TestHttpClient.hpp

@@ -26,6 +26,7 @@
 #include <boost/beast/core/tcp_stream.hpp>
 #include <boost/beast/http/field.hpp>
 #include <boost/beast/http/message.hpp>
+#include <boost/beast/http/status.hpp>
 #include <boost/beast/http/string_body.hpp>
 
 #include <chrono>
@@ -33,6 +34,7 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <vector>
 
 struct WebHeader {
@@ -43,7 +45,7 @@ struct WebHeader {
 };
 
 struct HttpSyncClient {
-    static std::string
+    static std::pair<boost::beast::http::status, std::string>
     post(
         std::string const& host,
         std::string const& port,
@@ -51,7 +53,7 @@ struct HttpSyncClient {
         std::vector<WebHeader> additionalHeaders = {}
     );
 
-    static std::string
+    static std::pair<boost::beast::http::status, std::string>
     get(std::string const& host,
         std::string const& port,
         std::string const& body,

tests/unit/rpc/handlers/AccountTxTests.cpp

@@ -769,14 +769,13 @@ TEST_F(RPCAccountTxHandlerTest, LimitAndMarker)
 
     auto const transactions = genTransactions(MINSEQ + 1, MAXSEQ - 1);
     auto const transCursor = TransactionsAndCursor{transactions, TransactionsCursor{12, 34}};
-    ON_CALL(*backend, fetchAccountTransactions).WillByDefault(Return(transCursor));
     EXPECT_CALL(
         *backend,
         fetchAccountTransactions(
             testing::_, testing::_, false, testing::Optional(testing::Eq(TransactionsCursor{10, 11})), testing::_
         )
     )
-        .Times(1);
+        .WillOnce(Return(transCursor));
 
     runSpawn([&, this](auto yield) {
         auto const handler = AnyHandler{AccountTxHandler{backend}};
@@ -804,6 +803,73 @@ TEST_F(RPCAccountTxHandlerTest, LimitAndMarker)
     });
 }
 
+TEST_F(RPCAccountTxHandlerTest, LimitIsCapped)
+{
+    backend->setRange(MINSEQ, MAXSEQ);
+
+    auto const transactions = genTransactions(MINSEQ + 1, MAXSEQ - 1);
+    auto const transCursor = TransactionsAndCursor{transactions, TransactionsCursor{12, 34}};
+    EXPECT_CALL(*backend, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_))
+        .WillOnce(Return(transCursor));
+
+    runSpawn([&, this](auto yield) {
+        auto const handler = AnyHandler{AccountTxHandler{backend}};
+        auto static const input = json::parse(fmt::format(
+            R"({{
+                "account": "{}",
+                "ledger_index_min": {},
+                "ledger_index_max": {},
+                "limit": 100000,
+                "forward": false
+            }})",
+            ACCOUNT,
+            -1,
+            -1
+        ));
+        auto const output = handler.process(input, Context{yield});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(output.result->at("account").as_string(), ACCOUNT);
+        EXPECT_EQ(output.result->at("ledger_index_min").as_uint64(), MINSEQ);
+        EXPECT_EQ(output.result->at("ledger_index_max").as_uint64(), MAXSEQ);
+        EXPECT_EQ(output.result->at("limit").as_uint64(), AccountTxHandler::LIMIT_MAX);
+        EXPECT_EQ(output.result->at("transactions").as_array().size(), 2);
+    });
+}
+
+TEST_F(RPCAccountTxHandlerTest, LimitAllowedUpToCap)
+{
+    backend->setRange(MINSEQ, MAXSEQ);
+
+    auto const transactions = genTransactions(MINSEQ + 1, MAXSEQ - 1);
+    auto const transCursor = TransactionsAndCursor{transactions, TransactionsCursor{12, 34}};
+    EXPECT_CALL(*backend, fetchAccountTransactions(testing::_, testing::_, false, testing::_, testing::_))
+        .WillOnce(Return(transCursor));
+
+    runSpawn([&, this](auto yield) {
+        auto const handler = AnyHandler{AccountTxHandler{backend}};
+        auto static const input = json::parse(fmt::format(
+            R"({{
+                "account": "{}",
+                "ledger_index_min": {},
+                "ledger_index_max": {},
+                "limit": {},
+                "forward": false
+            }})",
+            ACCOUNT,
+            -1,
+            -1,
+            AccountTxHandler::LIMIT_MAX - 1
+        ));
+        auto const output = handler.process(input, Context{yield});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(output.result->at("account").as_string(), ACCOUNT);
+        EXPECT_EQ(output.result->at("ledger_index_min").as_uint64(), MINSEQ);
+        EXPECT_EQ(output.result->at("ledger_index_max").as_uint64(), MAXSEQ);
+        EXPECT_EQ(output.result->at("limit").as_uint64(), AccountTxHandler::LIMIT_MAX - 1);
+        EXPECT_EQ(output.result->at("transactions").as_array().size(), 2);
+    });
+}
+
 TEST_F(RPCAccountTxHandlerTest, SpecificLedgerIndex)
 {
     backend->setRange(MINSEQ, MAXSEQ);

tests/unit/rpc/handlers/BookOffersTests.cpp

@@ -50,24 +50,19 @@
 #include <string>
 #include <vector>
 
-constexpr static auto ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
-constexpr static auto ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
+namespace {
+constexpr auto ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
+constexpr auto ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
 
-constexpr static auto LEDGERHASH = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652";
-constexpr static auto INDEX1 = "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC";
-constexpr static auto INDEX2 = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
+constexpr auto LEDGERHASH = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652";
+constexpr auto INDEX1 = "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC";
+constexpr auto INDEX2 = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
 // 20 USD : 10 XRP
-constexpr static auto PAYS20USDGETS10XRPBOOKDIR = "43B83ADC452B85FCBADA6CAEAC5181C255A213630D58FFD455071AFD498D0000";
+constexpr auto PAYS20USDGETS10XRPBOOKDIR = "43B83ADC452B85FCBADA6CAEAC5181C255A213630D58FFD455071AFD498D0000";
 // 20 XRP : 10 USD
-constexpr static auto PAYS20XRPGETS10USDBOOKDIR = "7B1767D41DBCE79D9585CF9D0262A5FEC45E5206FF524F8B55071AFD498D0000";
+constexpr auto PAYS20XRPGETS10USDBOOKDIR = "7B1767D41DBCE79D9585CF9D0262A5FEC45E5206FF524F8B55071AFD498D0000";
 // transfer rate x2
-constexpr static auto TRANSFERRATEX2 = 2000000000;
-
-using namespace rpc;
-namespace json = boost::json;
-using namespace testing;
-
-class RPCBookOffersHandlerTest : public HandlerBaseTest {};
+constexpr auto TRANSFERRATEX2 = 2000000000;
 
 struct ParameterTestBundle {
     std::string testName;
@@ -76,7 +71,15 @@ struct ParameterTestBundle {
     std::string expectedErrorMessage;
 };
 
-struct RPCBookOffersParameterTest : public RPCBookOffersHandlerTest, public WithParamInterface<ParameterTestBundle> {};
+}  // namespace
+
+using namespace rpc;
+namespace json = boost::json;
+using namespace testing;
+
+class RPCBookOffersHandlerTest : public HandlerBaseTest {};
+
+struct RPCBookOffersParameterTest : RPCBookOffersHandlerTest, WithParamInterface<ParameterTestBundle> {};
 
 TEST_P(RPCBookOffersParameterTest, CheckError)
 {

tests/unit/rpc/handlers/GatewayBalancesTests.cpp

@@ -49,22 +49,30 @@ using namespace rpc;
 namespace json = boost::json;
 using namespace testing;
 
-constexpr static auto ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
-constexpr static auto ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
-constexpr static auto ACCOUNT3 = "raHGBERMka3KZsfpTQUAtumxmvpqhFLyrk";
-constexpr static auto ISSUER = "rK9DrarGKnVEo2nYp5MfVRXRYf5yRX3mwD";
-constexpr static auto LEDGERHASH = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652";
-constexpr static auto INDEX1 = "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC";
-constexpr static auto INDEX2 = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
-constexpr static auto TXNID = "E3FE6EA3D48F0C2B639448020EA4F03D4F4F8FFDB243A852A0F59177921B4879";
-
-class RPCGatewayBalancesHandlerTest : public HandlerBaseTest {};
+namespace {
+constexpr auto ACCOUNT = "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn";
+constexpr auto ACCOUNT2 = "rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun";
+constexpr auto ACCOUNT3 = "raHGBERMka3KZsfpTQUAtumxmvpqhFLyrk";
+constexpr auto ISSUER = "rK9DrarGKnVEo2nYp5MfVRXRYf5yRX3mwD";
+constexpr auto LEDGERHASH = "4BC50C9B0D8515D3EAAE1E74B29A95804346C491EE1A95BF25E4AAB854A6A652";
+constexpr auto INDEX1 = "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1BC983515BC";
+constexpr auto INDEX2 = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
+constexpr auto TXNID = "E3FE6EA3D48F0C2B639448020EA4F03D4F4F8FFDB243A852A0F59177921B4879";
 
 struct ParameterTestBundle {
     std::string testName;
     std::string testJson;
     std::string expectedError;
     std::string expectedErrorMessage;
+    std::uint32_t apiVersion = 1u;
+};
+} // namespace
+
+struct RPCGatewayBalancesHandlerTest : HandlerBaseTest {
+    RPCGatewayBalancesHandlerTest()
+    {
+        backend->setRange(10, 300);
+    }
 };
 
 struct ParameterTest : public RPCGatewayBalancesHandlerTest, public WithParamInterface<ParameterTestBundle> {};
@@ -74,7 +82,8 @@ TEST_P(ParameterTest, CheckError)
     auto bundle = GetParam();
     auto const handler = AnyHandler{GatewayBalancesHandler{backend}};
     runSpawn([&](auto yield) {
-        auto const output = handler.process(json::parse(bundle.testJson), Context{yield});
+        auto const output =
+            handler.process(json::parse(bundle.testJson), Context{.yield = yield, .apiVersion = bundle.apiVersion});
         ASSERT_FALSE(output);
         auto const err = rpc::makeError(output.result.error());
         EXPECT_EQ(err.at("error").as_string(), bundle.expectedError);
@@ -146,52 +155,104 @@ generateParameterTestBundles()
             "ledger_hashNotString"
         },
         ParameterTestBundle{
-            "WalletsNotStringOrArray",
-            fmt::format(
+            .testName = "WalletsNotStringOrArrayV1",
+            .testJson = fmt::format(
                 R"({{
                     "account": "{}",
                     "hotwallet": 12
                 }})",
                 ACCOUNT
             ),
-            "invalidParams",
-            "hotwalletNotStringOrArray"
+            .expectedError = "invalidHotWallet",
+            .expectedErrorMessage = "hotwalletNotStringOrArray"
         },
         ParameterTestBundle{
-            "WalletsNotStringAccount",
-            fmt::format(
+            .testName = "WalletsNotStringAccountV1",
+            .testJson = fmt::format(
                 R"({{
                     "account": "{}",
                     "hotwallet": [12]
                 }})",
                 ACCOUNT
             ),
-            "invalidParams",
-            "hotwalletMalformed"
+            .expectedError = "invalidHotWallet",
+            .expectedErrorMessage = "hotwalletMalformed"
         },
         ParameterTestBundle{
-            "WalletsInvalidAccount",
-            fmt::format(
+            .testName = "WalletsInvalidAccountV1",
+            .testJson = fmt::format(
                 R"({{
                     "account": "{}",
                     "hotwallet": ["12"]
                 }})",
                 ACCOUNT
             ),
-            "invalidParams",
-            "hotwalletMalformed"
+            .expectedError = "invalidHotWallet",
+            .expectedErrorMessage = "hotwalletMalformed"
         },
         ParameterTestBundle{
-            "WalletInvalidAccount",
-            fmt::format(
+            .testName = "WalletInvalidAccountV1",
+            .testJson = fmt::format(
                 R"({{
                     "account": "{}",
                     "hotwallet": "12"
                 }})",
                 ACCOUNT
             ),
-            "invalidParams",
-            "hotwalletMalformed"
+            .expectedError = "invalidHotWallet",
+            .expectedErrorMessage = "hotwalletMalformed"
+        },
+        ParameterTestBundle{
+            .testName = "WalletsNotStringOrArrayV2",
+            .testJson = fmt::format(
+                R"({{
+                    "account": "{}",
+                    "hotwallet": 12
+                }})",
+                ACCOUNT
+            ),
+            .expectedError = "invalidParams",
+            .expectedErrorMessage = "hotwalletNotStringOrArray",
+            .apiVersion = 2u
+        },
+        ParameterTestBundle{
+            .testName = "WalletsNotStringAccountV2",
+            .testJson = fmt::format(
+                R"({{
+                    "account": "{}",
+                    "hotwallet": [12]
+                }})",
+                ACCOUNT
+            ),
+            .expectedError = "invalidParams",
+            .expectedErrorMessage = "hotwalletMalformed",
+            .apiVersion = 2u
+        },
+        ParameterTestBundle{
+            .testName = "WalletsInvalidAccountV2",
+            .testJson = fmt::format(
+                R"({{
+                    "account": "{}",
+                    "hotwallet": ["12"]
+                }})",
+                ACCOUNT
+            ),
+            .expectedError = "invalidParams",
+            .expectedErrorMessage = "hotwalletMalformed",
+            .apiVersion = 2u
+        },
+        ParameterTestBundle{
+            .testName = "WalletInvalidAccountV2",
+            .testJson = fmt::format(
+                R"({{
+                    "account": "{}",
+                    "hotwallet": "12"
+                }})",
+                ACCOUNT
+            ),
+            .expectedError = "invalidParams",
+            .expectedErrorMessage = "hotwalletMalformed",
+            .apiVersion = 2u
         },
     };
 }
@@ -207,7 +268,6 @@ TEST_F(RPCGatewayBalancesHandlerTest, LedgerNotFoundViaStringIndex)
 {
     auto const seq = 123;
 
-    backend->setRange(10, 300);
     EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
     // return empty ledgerHeader
     ON_CALL(*backend, fetchLedgerBySequence(seq, _)).WillByDefault(Return(std::optional<ripple::LedgerHeader>{}));
@@ -236,7 +296,6 @@ TEST_F(RPCGatewayBalancesHandlerTest, LedgerNotFoundViaIntIndex)
 {
     auto const seq = 123;
 
-    backend->setRange(10, 300);
     EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
     // return empty ledgerHeader
     ON_CALL(*backend, fetchLedgerBySequence(seq, _)).WillByDefault(Return(std::optional<ripple::LedgerHeader>{}));
@@ -263,7 +322,6 @@ TEST_F(RPCGatewayBalancesHandlerTest, LedgerNotFoundViaIntIndex)
 
 TEST_F(RPCGatewayBalancesHandlerTest, LedgerNotFoundViaHash)
 {
-    backend->setRange(10, 300);
     EXPECT_CALL(*backend, fetchLedgerByHash).Times(1);
     // return empty ledgerHeader
     ON_CALL(*backend, fetchLedgerByHash(ripple::uint256{LEDGERHASH}, _))
@@ -293,7 +351,6 @@ TEST_F(RPCGatewayBalancesHandlerTest, AccountNotFound)
 {
     auto const seq = 300;
 
-    backend->setRange(10, seq);
     EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
     // return valid ledgerHeader
     auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, seq);
@@ -322,54 +379,6 @@ TEST_F(RPCGatewayBalancesHandlerTest, AccountNotFound)
     });
 }
 
-TEST_F(RPCGatewayBalancesHandlerTest, InvalidHotWallet)
-{
-    auto const seq = 300;
-
-    backend->setRange(10, seq);
-    EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
-    // return valid ledgerHeader
-    auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, seq);
-    ON_CALL(*backend, fetchLedgerBySequence(seq, _)).WillByDefault(Return(ledgerHeader));
-
-    // return valid account
-    auto const accountKk = ripple::keylet::account(GetAccountIDWithString(ACCOUNT)).key;
-    ON_CALL(*backend, doFetchLedgerObject(accountKk, seq, _)).WillByDefault(Return(Blob{'f', 'a', 'k', 'e'}));
-
-    // return valid owner dir
-    auto const ownerDir = CreateOwnerDirLedgerObject({ripple::uint256{INDEX2}}, INDEX1);
-    auto const ownerDirKk = ripple::keylet::ownerDir(GetAccountIDWithString(ACCOUNT)).key;
-    ON_CALL(*backend, doFetchLedgerObject(ownerDirKk, seq, _))
-        .WillByDefault(Return(ownerDir.getSerializer().peekData()));
-    EXPECT_CALL(*backend, doFetchLedgerObject).Times(2);
-
-    // create a valid line, balance is 0
-    auto const line1 = CreateRippleStateLedgerObject("USD", ISSUER, 0, ACCOUNT, 10, ACCOUNT2, 20, TXNID, 123);
-    std::vector<Blob> bbs;
-    bbs.push_back(line1.getSerializer().peekData());
-    ON_CALL(*backend, doFetchLedgerObjects).WillByDefault(Return(bbs));
-    EXPECT_CALL(*backend, doFetchLedgerObjects).Times(1);
-
-    auto const handler = AnyHandler{GatewayBalancesHandler{backend}};
-    runSpawn([&](auto yield) {
-        auto const output = handler.process(
-            json::parse(fmt::format(
-                R"({{
-                    "account": "{}",
-                    "hotwallet": "{}"
-                }})",
-                ACCOUNT,
-                ACCOUNT2
-            )),
-            Context{yield}
-        );
-        ASSERT_FALSE(output);
-        auto const err = rpc::makeError(output.result.error());
-        EXPECT_EQ(err.at("error").as_string(), "invalidHotWallet");
-        EXPECT_EQ(err.at("error_message").as_string(), "Invalid hot wallet.");
-    });
-}
-
 struct NormalTestBundle {
     std::string testName;
     ripple::STObject mockedDir;
@@ -385,7 +394,6 @@ TEST_P(NormalPathTest, CheckOutput)
     auto const& bundle = GetParam();
     auto const seq = 300;
 
-    backend->setRange(10, seq);
     EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
     // return valid ledgerHeader
     auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, seq);

tests/unit/web/RPCServerHandlerTests.cpp

@@ -17,12 +17,14 @@
 //==============================================================================
 
 #include "rpc/Errors.hpp"
+#include "rpc/common/APIVersion.hpp"
 #include "rpc/common/Types.hpp"
 #include "util/AsioContextTestFixture.hpp"
 #include "util/MockBackendTestFixture.hpp"
 #include "util/MockETLService.hpp"
 #include "util/MockPrometheus.hpp"
 #include "util/MockRPCEngine.hpp"
+#include "util/NameGenerator.hpp"
 #include "util/Taggable.hpp"
 #include "util/config/Config.hpp"
 #include "web/RPCServerHandler.hpp"
@@ -30,17 +32,22 @@
 
 #include <boost/beast/http/status.hpp>
 #include <boost/json/parse.hpp>
+#include <fmt/core.h>
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <cstdint>
 #include <memory>
 #include <stdexcept>
 #include <string>
+#include <vector>
 
 using namespace web;
 
-constexpr static auto MINSEQ = 10;
-constexpr static auto MAXSEQ = 30;
+namespace {
+
+constexpr auto MINSEQ = 10;
+constexpr auto MAXSEQ = 30;
 
 struct MockWsBase : public web::ConnectionBase {
     std::string message;
@@ -466,54 +473,6 @@ TEST_F(WebRPCServerHandlerTest, WsNotReady)
     EXPECT_EQ(boost::json::parse(session->message), boost::json::parse(response));
 }
 
-TEST_F(WebRPCServerHandlerTest, HTTPInvalidAPIVersion)
-{
-    static auto constexpr request = R"({
-                                        "method": "server_info",
-                                        "params": [{
-                                            "api_version": null
-                                        }]
-                                    })";
-
-    backend->setRange(MINSEQ, MAXSEQ);
-
-    static auto constexpr response = "invalid_API_version";
-
-    EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);
-
-    (*handler)(request, session);
-    EXPECT_EQ(session->message, response);
-    EXPECT_EQ(session->lastStatus, boost::beast::http::status::bad_request);
-}
-
-TEST_F(WebRPCServerHandlerTest, WSInvalidAPIVersion)
-{
-    session->upgraded = true;
-    static auto constexpr request = R"({
-                                        "method": "server_info",
-                                        "api_version": null
-                                    })";
-
-    backend->setRange(MINSEQ, MAXSEQ);
-
-    static auto constexpr response = R"({
-                                        "error": "invalid_API_version",
-                                        "error_code": 6000,
-                                        "error_message": "API version must be an integer",
-                                        "status": "error",
-                                        "type": "response",
-                                        "request": {
-                                            "method": "server_info",
-                                            "api_version": null
-                                        }
-                                    })";
-
-    EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);
-
-    (*handler)(request, session);
-    EXPECT_EQ(boost::json::parse(session->message), boost::json::parse(response));
-}
-
 TEST_F(WebRPCServerHandlerTest, HTTPBadSyntaxWhenRequestSubscribe)
 {
     static auto constexpr request = R"({"method": "subscribe"})";
@@ -872,3 +831,84 @@ TEST_F(WebRPCServerHandlerTest, WsRequestNotJson)
     (*handler)(request, session);
     EXPECT_EQ(boost::json::parse(session->message), boost::json::parse(response));
 }
+
+struct InvalidAPIVersionTestBundle {
+    std::string testName;
+    std::string version;
+    std::string wsMessage;
+};
+
+// parameterized test cases for parameters check
+struct WebRPCServerHandlerInvalidAPIVersionParamTest : public WebRPCServerHandlerTest,
+                                                       public testing::WithParamInterface<InvalidAPIVersionTestBundle> {
+};
+
+auto
+generateInvalidVersions()
+{
+    return std::vector<InvalidAPIVersionTestBundle>{
+        {"v0", "0", fmt::format("Requested API version is lower than minimum supported ({})", rpc::API_VERSION_MIN)},
+        {"v4", "4", fmt::format("Requested API version is higher than maximum supported ({})", rpc::API_VERSION_MAX)},
+        {"null", "null", "API version must be an integer"},
+        {"str", "\"bogus\"", "API version must be an integer"},
+        {"bool", "false", "API version must be an integer"},
+        {"double", "12.34", "API version must be an integer"},
+    };
+}
+
+INSTANTIATE_TEST_CASE_P(
+    WebRPCServerHandlerAPIVersionGroup,
+    WebRPCServerHandlerInvalidAPIVersionParamTest,
+    testing::ValuesIn(generateInvalidVersions()),
+    tests::util::NameGenerator
+);
+
+TEST_P(WebRPCServerHandlerInvalidAPIVersionParamTest, HTTPInvalidAPIVersion)
+{
+    auto request = fmt::format(
+        R"({{
+            "method": "server_info",
+            "params": [{{
+                "api_version": {}
+            }}]
+        }})",
+        GetParam().version
+    );
+
+    backend->setRange(MINSEQ, MAXSEQ);
+
+    EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);
+
+    (*handler)(request, session);
+    EXPECT_EQ(session->message, "invalid_API_version");
+    EXPECT_EQ(session->lastStatus, boost::beast::http::status::bad_request);
+}
+
+TEST_P(WebRPCServerHandlerInvalidAPIVersionParamTest, WSInvalidAPIVersion)
+{
+    session->upgraded = true;
+    auto request = fmt::format(
+        R"({{
+            "method": "server_info",
+            "api_version": {}
+        }})",
+        GetParam().version
+    );
+
+    backend->setRange(MINSEQ, MAXSEQ);
+
+    EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);
+
+    (*handler)(request, session);
+
+    auto response = boost::json::parse(session->message);
+    EXPECT_TRUE(response.is_object());
+    EXPECT_TRUE(response.as_object().contains("error"));
+    EXPECT_EQ(response.at("error").as_string(), "invalid_API_version");
+    EXPECT_TRUE(response.as_object().contains("error_message"));
+    EXPECT_EQ(response.at("error_message").as_string(), GetParam().wsMessage);
+    EXPECT_TRUE(response.as_object().contains("error_code"));
+    EXPECT_EQ(response.at("error_code").as_int64(), static_cast<int64_t>(rpc::ClioError::rpcINVALID_API_VERSION));
+}
+
+}  // namespace

tests/unit/web/ServerTests.cpp

@@ -56,6 +56,7 @@
 #include <string>
 #include <string_view>
 #include <thread>
+#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -214,8 +215,9 @@ TEST_F(WebServerTest, Http)
 {
     auto e = std::make_shared<EchoExecutor>();
     auto const server = makeServerSync(cfg, ctx, dosGuard, e);
-    auto const res = HttpSyncClient::post("localhost", port, R"({"Hello":1})");
+    auto const [status, res] = HttpSyncClient::post("localhost", port, R"({"Hello":1})");
     EXPECT_EQ(res, R"({"Hello":1})");
+    EXPECT_EQ(status, boost::beast::http::status::ok);
 }
 
 TEST_F(WebServerTest, Ws)
@@ -233,11 +235,12 @@ TEST_F(WebServerTest, HttpInternalError)
 {
     auto e = std::make_shared<ExceptionExecutor>();
     auto const server = makeServerSync(cfg, ctx, dosGuard, e);
-    auto const res = HttpSyncClient::post("localhost", port, R"({})");
+    auto const [status, res] = HttpSyncClient::post("localhost", port, R"({})");
     EXPECT_EQ(
         res,
         R"({"error":"internal","error_code":73,"error_message":"Internal error.","status":"error","type":"response"})"
     );
+    EXPECT_EQ(status, boost::beast::http::status::internal_server_error);
 }
 
 TEST_F(WebServerTest, WsInternalError)
@@ -316,13 +319,16 @@ TEST_F(WebServerTest, HttpRequestOverload)
 {
     auto e = std::make_shared<EchoExecutor>();
     auto const server = makeServerSync(cfg, ctx, dosGuardOverload, e);
-    auto res = HttpSyncClient::post("localhost", port, R"({})");
+    auto [status, res] = HttpSyncClient::post("localhost", port, R"({})");
     EXPECT_EQ(res, "{}");
-    res = HttpSyncClient::post("localhost", port, R"({})");
+    EXPECT_EQ(status, boost::beast::http::status::ok);
+
+    std::tie(status, res) = HttpSyncClient::post("localhost", port, R"({})");
     EXPECT_EQ(
         res,
         R"({"error":"slowDown","error_code":10,"error_message":"You are placing too much load on the server.","status":"error","type":"response"})"
     );
+    EXPECT_EQ(status, boost::beast::http::status::service_unavailable);
 }
 
 TEST_F(WebServerTest, WsRequestOverload)
@@ -349,11 +355,12 @@ TEST_F(WebServerTest, HttpPayloadOverload)
     std::string const s100(100, 'a');
     auto e = std::make_shared<EchoExecutor>();
     auto server = makeServerSync(cfg, ctx, dosGuardOverload, e);
-    auto const res = HttpSyncClient::post("localhost", port, fmt::format(R"({{"payload":"{}"}})", s100));
+    auto const [status, res] = HttpSyncClient::post("localhost", port, fmt::format(R"({{"payload":"{}"}})", s100));
     EXPECT_EQ(
         res,
         R"({"payload":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","warning":"load","warnings":[{"id":2003,"message":"You are about to be rate limited"}]})"
     );
+    EXPECT_EQ(status, boost::beast::http::status::ok);
 }
 
 TEST_F(WebServerTest, WsPayloadOverload)
@@ -393,6 +400,26 @@ TEST_F(WebServerTest, WsTooManyConnection)
     EXPECT_TRUE(exceptionThrown);
 }
 
+TEST_F(WebServerTest, HealthCheck)
+{
+    auto e = std::make_shared<ExceptionExecutor>();  // request handled before we get to executor
+    auto const server = makeServerSync(cfg, ctx, dosGuard, e);
+    auto const [status, res] = HttpSyncClient::get("localhost", port, "", "/health");
+
+    EXPECT_FALSE(res.empty());
+    EXPECT_EQ(status, boost::beast::http::status::ok);
+}
+
+TEST_F(WebServerTest, GetOtherThanHealthCheck)
+{
+    auto e = std::make_shared<ExceptionExecutor>();  // request handled before we get to executor
+    auto const server = makeServerSync(cfg, ctx, dosGuard, e);
+    auto const [status, res] = HttpSyncClient::get("localhost", port, "", "/");
+
+    EXPECT_FALSE(res.empty());
+    EXPECT_EQ(status, boost::beast::http::status::bad_request);
+}
+
 std::string
 JSONServerConfigWithAdminPassword(uint32_t const port)
 {
@@ -500,8 +527,11 @@ TEST_P(WebServerAdminTest, HttpAdminCheck)
     auto server = makeServerSync(serverConfig, ctx, dosGuardOverload, e);
     std::string const request = "Why hello";
     uint32_t const webServerPort = serverConfig.value<uint32_t>("server.port");
-    auto const res = HttpSyncClient::post("localhost", std::to_string(webServerPort), request, GetParam().headers);
+    auto const [status, res] =
+        HttpSyncClient::post("localhost", std::to_string(webServerPort), request, GetParam().headers);
+
     EXPECT_EQ(res, fmt::format("{} {}", request, GetParam().expectedResponse));
+    EXPECT_EQ(status, boost::beast::http::status::ok);
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -618,8 +648,10 @@ TEST_F(WebServerPrometheusTest, rejectedWithoutAdminPassword)
     uint32_t const webServerPort = tests::util::generateFreePort();
     Config const serverConfig{boost::json::parse(JSONServerConfigWithAdminPassword(webServerPort))};
     auto server = makeServerSync(serverConfig, ctx, dosGuard, e);
-    auto const res = HttpSyncClient::get("localhost", std::to_string(webServerPort), "", "/metrics");
+    auto const [status, res] = HttpSyncClient::get("localhost", std::to_string(webServerPort), "", "/metrics");
+
     EXPECT_EQ(res, "Only admin is allowed to collect metrics");
+    EXPECT_EQ(status, boost::beast::http::status::unauthorized);
 }
 
 TEST_F(WebServerPrometheusTest, rejectedIfPrometheusIsDisabled)
@@ -641,7 +673,7 @@ TEST_F(WebServerPrometheusTest, rejectedIfPrometheusIsDisabled)
     Config const serverConfig{boost::json::parse(JSONServerConfigWithDisabledPrometheus)};
     PrometheusService::init(serverConfig);
     auto server = makeServerSync(serverConfig, ctx, dosGuard, e);
-    auto const res = HttpSyncClient::get(
+    auto const [status, res] = HttpSyncClient::get(
         "localhost",
         std::to_string(webServerPort),
         "",
@@ -652,6 +684,7 @@ TEST_F(WebServerPrometheusTest, rejectedIfPrometheusIsDisabled)
         )}
     );
     EXPECT_EQ(res, "Prometheus is disabled in clio config");
+    EXPECT_EQ(status, boost::beast::http::status::forbidden);
 }
 
 TEST_F(WebServerPrometheusTest, validResponse)
@@ -662,7 +695,7 @@ TEST_F(WebServerPrometheusTest, validResponse)
     auto e = std::make_shared<EchoExecutor>();
     Config const serverConfig{boost::json::parse(JSONServerConfigWithAdminPassword(webServerPort))};
     auto server = makeServerSync(serverConfig, ctx, dosGuard, e);
-    auto const res = HttpSyncClient::get(
+    auto const [status, res] = HttpSyncClient::get(
         "localhost",
         std::to_string(webServerPort),
         "",
@@ -673,4 +706,5 @@ TEST_F(WebServerPrometheusTest, validResponse)
         )}
     );
     EXPECT_EQ(res, "# TYPE test_counter counter\ntest_counter 1\n\n");
+    EXPECT_EQ(status, boost::beast::http::status::ok);
 }