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
2025-11-04 03:45:50 +00:00 · 2025-02-04 17:10:12 +00:00
parent 47c0a6a297 8a418bfe00
commit 3555b7d998
3 changed files with 143 additions and 77 deletions
--- a/src/rpc/handlers/GatewayBalances.hpp
+++ b/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;
    }

    /**
--- a/tests/unit/rpc/handlers/BookOffersTests.cpp
+++ b/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)
 {
--- a/tests/unit/rpc/handlers/GatewayBalancesTests.cpp
+++ b/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);
@@ -337,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);