From 12e6fcc97e191746f98dd8d0f9eb599d62c9ce8e Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Wed, 22 Jan 2025 09:48:13 +0000 Subject: [PATCH] fix: gateway_balance discrepancy (#1839) Fix https://github.com/XRPLF/clio/issues/1832 rippled code: https://github.com/XRPLF/rippled/blob/develop/src/xrpld/rpc/handlers/GatewayBalances.cpp#L129 --- src/rpc/handlers/GatewayBalances.hpp | 67 ++++++++------- tests/unit/rpc/handlers/BookOffersTests.cpp | 14 +-- .../rpc/handlers/GatewayBalancesTests.cpp | 85 +++++++++++++++---- 3 files changed, 113 insertions(+), 53 deletions(-) diff --git a/src/rpc/handlers/GatewayBalances.hpp b/src/rpc/handlers/GatewayBalances.hpp index 2822c64c..08a9e97d 100644 --- 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 kHOT_WALLET_VALIDATOR = - 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 { - if (j.is_string()) { - auto const pk = util::parseBase58Wrapper( - ripple::TokenType::AccountPublic, boost::json::value_to(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 { + if (j.is_string()) { + auto const pk = util::parseBase58Wrapper( + ripple::TokenType::AccountPublic, boost::json::value_to(j) + ); - if (pk) - return ripple::calcAccountID(*pk); + if (pk) + return ripple::calcAccountID(*pk); - return util::parseBase58Wrapper(boost::json::value_to(j)); + return util::parseBase58Wrapper(boost::json::value_to(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 kRPC_SPEC = RpcSpec{ - {JS(account), validation::Required{}, validation::CustomValidators::accountValidator}, - {JS(ledger_hash), validation::CustomValidators::uint256HexStringValidator}, - {JS(ledger_index), validation::CustomValidators::ledgerIndexValidator}, - {JS(hotwallet), kHOT_WALLET_VALIDATOR} + }; }; - return kRPC_SPEC; + 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; } /** diff --git a/tests/unit/rpc/handlers/BookOffersTests.cpp b/tests/unit/rpc/handlers/BookOffersTests.cpp index 88d6f3be..88d3056d 100644 --- a/tests/unit/rpc/handlers/BookOffersTests.cpp +++ b/tests/unit/rpc/handlers/BookOffersTests.cpp @@ -68,6 +68,13 @@ constexpr auto kPAYS20_XRP_GETS10_USD_BOOK_DIR = "7B1767D41DBCE79D9585CF9D0262A5 // transfer rate x2 constexpr auto kTRANSFER_RATE_X2 = 2000000000; +struct ParameterTestBundle { + std::string testName; + std::string testJson; + std::string expectedError; + std::string expectedErrorMessage; +}; + } // namespace using namespace rpc; @@ -81,13 +88,6 @@ struct RPCBookOffersHandlerTest : HandlerBaseTest { } }; -struct ParameterTestBundle { - std::string testName; - std::string testJson; - std::string expectedError; - std::string expectedErrorMessage; -}; - struct RPCBookOffersParameterTest : RPCBookOffersHandlerTest, WithParamInterface {}; TEST_P(RPCBookOffersParameterTest, CheckError) diff --git a/tests/unit/rpc/handlers/GatewayBalancesTests.cpp b/tests/unit/rpc/handlers/GatewayBalancesTests.cpp index 39706cd0..d9e319be 100644 --- a/tests/unit/rpc/handlers/GatewayBalancesTests.cpp +++ b/tests/unit/rpc/handlers/GatewayBalancesTests.cpp @@ -60,6 +60,13 @@ constexpr auto kINDEX1 = "1B8590C01B0006EDFA9ED60296DD052DC5E90F99659B25014D08E1 constexpr auto kINDEX2 = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321"; constexpr auto kTXN_ID = "E3FE6EA3D48F0C2B639448020EA4F03D4F4F8FFDB243A852A0F59177921B4879"; +struct ParameterTestBundle { + std::string testName; + std::string testJson; + std::string expectedError; + std::string expectedErrorMessage; + std::uint32_t apiVersion = 1u; +}; } // namespace struct RPCGatewayBalancesHandlerTest : HandlerBaseTest { @@ -69,13 +76,6 @@ struct RPCGatewayBalancesHandlerTest : HandlerBaseTest { } }; -struct ParameterTestBundle { - std::string testName; - std::string testJson; - std::string expectedError; - std::string expectedErrorMessage; -}; - struct ParameterTest : public RPCGatewayBalancesHandlerTest, public WithParamInterface {}; TEST_P(ParameterTest, CheckError) @@ -83,7 +83,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); @@ -155,7 +156,55 @@ generateParameterTestBundles() .expectedErrorMessage = "ledger_hashNotString" }, ParameterTestBundle{ - .testName = "WalletsNotStringOrArray", + .testName = "WalletsNotStringOrArrayV1", + .testJson = fmt::format( + R"({{ + "account": "{}", + "hotwallet": 12 + }})", + kACCOUNT + ), + .expectedError = "invalidHotWallet", + .expectedErrorMessage = "hotwalletNotStringOrArray" + }, + ParameterTestBundle{ + .testName = "WalletsNotStringAccountV1", + .testJson = fmt::format( + R"({{ + "account": "{}", + "hotwallet": [12] + }})", + kACCOUNT + ), + .expectedError = "invalidHotWallet", + .expectedErrorMessage = "hotwalletMalformed" + }, + ParameterTestBundle{ + .testName = "WalletsInvalidAccountV1", + .testJson = fmt::format( + R"({{ + "account": "{}", + "hotwallet": ["12"] + }})", + kACCOUNT + ), + .expectedError = "invalidHotWallet", + .expectedErrorMessage = "hotwalletMalformed" + }, + ParameterTestBundle{ + .testName = "WalletInvalidAccountV1", + .testJson = fmt::format( + R"({{ + "account": "{}", + "hotwallet": "12" + }})", + kACCOUNT + ), + .expectedError = "invalidHotWallet", + .expectedErrorMessage = "hotwalletMalformed" + }, + ParameterTestBundle{ + .testName = "WalletsNotStringOrArrayV2", .testJson = fmt::format( R"({{ "account": "{}", @@ -164,10 +213,11 @@ generateParameterTestBundles() kACCOUNT ), .expectedError = "invalidParams", - .expectedErrorMessage = "hotwalletNotStringOrArray" + .expectedErrorMessage = "hotwalletNotStringOrArray", + .apiVersion = 2u }, ParameterTestBundle{ - .testName = "WalletsNotStringAccount", + .testName = "WalletsNotStringAccountV2", .testJson = fmt::format( R"({{ "account": "{}", @@ -176,10 +226,11 @@ generateParameterTestBundles() kACCOUNT ), .expectedError = "invalidParams", - .expectedErrorMessage = "hotwalletMalformed" + .expectedErrorMessage = "hotwalletMalformed", + .apiVersion = 2u }, ParameterTestBundle{ - .testName = "WalletsInvalidAccount", + .testName = "WalletsInvalidAccountV2", .testJson = fmt::format( R"({{ "account": "{}", @@ -188,10 +239,11 @@ generateParameterTestBundles() kACCOUNT ), .expectedError = "invalidParams", - .expectedErrorMessage = "hotwalletMalformed" + .expectedErrorMessage = "hotwalletMalformed", + .apiVersion = 2u }, ParameterTestBundle{ - .testName = "WalletInvalidAccount", + .testName = "WalletInvalidAccountV2", .testJson = fmt::format( R"({{ "account": "{}", @@ -200,7 +252,8 @@ generateParameterTestBundles() kACCOUNT ), .expectedError = "invalidParams", - .expectedErrorMessage = "hotwalletMalformed" + .expectedErrorMessage = "hotwalletMalformed", + .apiVersion = 2u }, }; }