From ad2ff82ab14aa22c4710c9836632fecd8ba915aa Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Wed, 23 Sep 2026 13:29:57 +0100 Subject: [PATCH] fix: Align error messages and rpc-spec 0.1.17 (#3223) --- conan.lock | 2 +- conanfile.py | 2 +- src/rpc/RPCHelpers.cpp | 4 +- src/rpc/handlers/VaultInfo.cpp | 10 +---- tests/unit/rpc/RPCHelpersTests.cpp | 6 +-- tests/unit/rpc/handlers/BookOffersTests.cpp | 42 ++++++++++++++++---- tests/unit/rpc/handlers/SubscribeTests.cpp | 2 +- tests/unit/rpc/handlers/UnsubscribeTests.cpp | 2 +- tests/unit/rpc/handlers/VaultInfoTests.cpp | 24 +++++------ 9 files changed, 57 insertions(+), 37 deletions(-) diff --git a/conan.lock b/conan.lock index 455fa5317..ba3034e03 100644 --- a/conan.lock +++ b/conan.lock @@ -3,7 +3,7 @@ "requires": [ "zlib/1.3.2#1cb806da49011867778ffb6ac7190fcb%1782392402.122708", "xxhash/0.8.3#681d36a0a6111fc56e5e45ea182c19cc%1782392402.420688", - "xrpl-rpc-spec/0.1.15#cd65b9ba58070dc851d67c88a0224e4f%1789705851.014228", + "xrpl-rpc-spec/0.1.17#ddfbc1f89da7797b6506fb9ee9488e17%1790136770.803908", "xrpl/3.4.0#e06b127b3ba92806a43fc7a53fc05fd3%1789577552.219154", "sqlite3/3.53.0#324ada52333108388a9a6108bfa96734%1782392403.185447", "spdlog/1.17.0#bcbaaf7147bda6ad24ffbd1ac3d7142c%1782736610.443882", diff --git a/conanfile.py b/conanfile.py index 07f97f916..339b0ade5 100644 --- a/conanfile.py +++ b/conanfile.py @@ -17,7 +17,7 @@ class ClioConan(ConanFile): "fmt/12.1.0", "libbacktrace/cci.20210118", "spdlog/1.17.0", - "xrpl-rpc-spec/0.1.15", + "xrpl-rpc-spec/0.1.17", "xrpl/3.4.0", ] diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index be3c34de0..b8d36f5e2 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -1538,7 +1538,7 @@ parseBook( } if (pays == gets) - return std::unexpected{Status{RippledError::RpcBadMarket, "badMarket"}}; + return std::unexpected{Status{RippledError::RpcBadMarket}}; return xrpl::Book{pays, gets, domainID}; } @@ -1677,7 +1677,7 @@ parseBook(boost::json::object const& request) } if (payCurrency == getCurrency && payIssuer == getIssuer) - return std::unexpected{Status{RippledError::RpcBadMarket, "badMarket"}}; + return std::unexpected{Status{RippledError::RpcBadMarket}}; std::optional domainID; if (request.contains("domain")) { diff --git a/src/rpc/handlers/VaultInfo.cpp b/src/rpc/handlers/VaultInfo.cpp index ac99804bc..9449ee880 100644 --- a/src/rpc/handlers/VaultInfo.cpp +++ b/src/rpc/handlers/VaultInfo.cpp @@ -58,14 +58,8 @@ VaultInfoHandler::VaultInfoHandler(std::shared_ptr sharedPtrBa VaultInfoHandler::Result VaultInfoHandler::process(VaultInfoHandler::Input const& input, Context const& ctx) const { - // vault info input must either have owner and sequence, or vault_id only. Wording and code - // match xrpld's VaultInfo.cpp parseVault(). - if (not validate(input)) { - return Error{Status{ - RippledError::RpcInvalidParams, - "Must specify either 'vault_id' or both 'owner' and 'seq'." - }}; - } + if (not validate(input)) + return Error{ClioError::RpcMalformedRequest}; auto const range = sharedPtrBackend_->fetchLedgerRange(); ASSERT(range.has_value(), "VaultInfo's ledger range must be available"); diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp index 07915facf..f1ace39a2 100644 --- a/tests/unit/rpc/RPCHelpersTests.cpp +++ b/tests/unit/rpc/RPCHelpersTests.cpp @@ -706,7 +706,7 @@ TEST_F(RPCHelpersTest, ParseBookBadMarket) auto const book = rpc::parseBook(usd, usd, std::nullopt); ASSERT_FALSE(book.has_value()); EXPECT_TRUE(book.error().code == CombinedError{RippledError::RpcBadMarket}); - EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "badMarket"); + EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "No such market."); } // Identical MPT assets on both sides. @@ -717,7 +717,7 @@ TEST_F(RPCHelpersTest, ParseBookBadMarket) auto const book = rpc::parseBook(mpt, mpt, std::nullopt); ASSERT_FALSE(book.has_value()); EXPECT_TRUE(book.error().code == CombinedError{RippledError::RpcBadMarket}); - EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "badMarket"); + EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "No such market."); } } @@ -769,7 +769,7 @@ TEST_F(RPCHelpersTest, ParseBookCurrencyOverloadDelegates) ); ASSERT_FALSE(book.has_value()); EXPECT_TRUE(book.error().code == CombinedError{RippledError::RpcBadMarket}); - EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "badMarket"); + EXPECT_EQ(rpc::makeError(book.error()).at("error_message").as_string(), "No such market."); } } diff --git a/tests/unit/rpc/handlers/BookOffersTests.cpp b/tests/unit/rpc/handlers/BookOffersTests.cpp index 591e9fe90..9b19b30c8 100644 --- a/tests/unit/rpc/handlers/BookOffersTests.cpp +++ b/tests/unit/rpc/handlers/BookOffersTests.cpp @@ -152,7 +152,7 @@ generateParameterBookOffersTestBundles() } })JSON", .expectedError = "invalidParams", - .expectedErrorMessage = "Required field 'currency' missing" + .expectedErrorMessage = "Missing field 'taker_pays.currency'." }, ParameterTestBundle{ .testName = "TakerGetsMissingCurrency", @@ -163,7 +163,7 @@ generateParameterBookOffersTestBundles() } })JSON", .expectedError = "invalidParams", - .expectedErrorMessage = "Required field 'currency' missing" + .expectedErrorMessage = "Missing field 'taker_gets.currency'." }, ParameterTestBundle{ .testName = "TakerGetsWrongCurrency", @@ -208,8 +208,8 @@ generateParameterBookOffersTestBundles() "currency": "XRP" } })JSON", - .expectedError = "dstAmtMalformed", - .expectedErrorMessage = "Destination amount/currency/issuer is malformed." + .expectedError = "invalidParams", + .expectedErrorMessage = "Invalid field 'taker_gets.currency', not string." }, ParameterTestBundle{ .testName = "TakerPaysCurrencyNotString", @@ -222,8 +222,8 @@ generateParameterBookOffersTestBundles() "currency": "XRP" } })JSON", - .expectedError = "srcCurMalformed", - .expectedErrorMessage = "Source currency is malformed." + .expectedError = "invalidParams", + .expectedErrorMessage = "Invalid field 'taker_pays.currency', not string." }, ParameterTestBundle{ .testName = "TakerGetsWrongIssuer", @@ -490,7 +490,33 @@ generateParameterBookOffersTestBundles() } })JSON", .expectedError = "badMarket", - .expectedErrorMessage = "badMarket" + .expectedErrorMessage = "No such market." + }, + ParameterTestBundle{ + .testName = "TakerGetsMptIdNotString", + .testJson = R"JSON({ + "taker_gets": { + "mpt_issuance_id": 123 + }, + "taker_pays": { + "currency": "XRP" + } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Invalid field 'taker_gets.mpt_issuance_id', not string." + }, + ParameterTestBundle{ + .testName = "TakerPaysMptIdNotString", + .testJson = R"JSON({ + "taker_gets": { + "currency": "XRP" + }, + "taker_pays": { + "mpt_issuance_id": true + } + })JSON", + .expectedError = "invalidParams", + .expectedErrorMessage = "Invalid field 'taker_pays.mpt_issuance_id', not string." }, ParameterTestBundle{ .testName = "TakerGetsMptIdAndCurrency", @@ -585,7 +611,7 @@ generateParameterBookOffersTestBundles() } })JSON", .expectedError = "badMarket", - .expectedErrorMessage = "badMarket" + .expectedErrorMessage = "No such market." }, // The "account one" issuer (rrrrrrrrrrrrrrrrrrrrBZbvji == xrpl::noAccount()) is rejected, // mirroring rippled's parseTakerIssuerJSON "bad issuer account one" check. diff --git a/tests/unit/rpc/handlers/SubscribeTests.cpp b/tests/unit/rpc/handlers/SubscribeTests.cpp index f8bf48970..0f83a3b0d 100644 --- a/tests/unit/rpc/handlers/SubscribeTests.cpp +++ b/tests/unit/rpc/handlers/SubscribeTests.cpp @@ -476,7 +476,7 @@ generateTestValuesForParametersTest() ] })JSON", .expectedError = "badMarket", - .expectedErrorMessage = "badMarket" + .expectedErrorMessage = "No such market." }, SubscribeParamTestCaseBundle{ .testName = "BooksItemInvalidSnapshot", diff --git a/tests/unit/rpc/handlers/UnsubscribeTests.cpp b/tests/unit/rpc/handlers/UnsubscribeTests.cpp index ecdfbecff..9d471f689 100644 --- a/tests/unit/rpc/handlers/UnsubscribeTests.cpp +++ b/tests/unit/rpc/handlers/UnsubscribeTests.cpp @@ -439,7 +439,7 @@ generateTestValuesForParametersTest() ] })JSON", .expectedError = "badMarket", - .expectedErrorMessage = "badMarket" + .expectedErrorMessage = "No such market." }, UnsubscribeParamTestCaseBundle{ .testName = "BooksItemInvalidBoth", diff --git a/tests/unit/rpc/handlers/VaultInfoTests.cpp b/tests/unit/rpc/handlers/VaultInfoTests.cpp index ac9bb482c..ef4f94bdb 100644 --- a/tests/unit/rpc/handlers/VaultInfoTests.cpp +++ b/tests/unit/rpc/handlers/VaultInfoTests.cpp @@ -70,27 +70,27 @@ generateTestValuesForParametersTest() .testJson = R"JSON({ "idk": "idk" })JSON", - .expectedError = "invalidParams", - .expectedErrorCode = RippledError::RpcInvalidParams, - .expectedErrorMessage = "Must specify either 'vault_id' or both 'owner' and 'seq'." + .expectedError = "malformedRequest", + .expectedErrorCode = rpc::ClioError::RpcMalformedRequest, + .expectedErrorMessage = "Malformed request." }, VaultInfoParamTestCaseBundle{ .testName = "MissingOwnerInVault", .testJson = R"JSON({ "seq": 4 })JSON", - .expectedError = "invalidParams", - .expectedErrorCode = RippledError::RpcInvalidParams, - .expectedErrorMessage = "Must specify either 'vault_id' or both 'owner' and 'seq'." + .expectedError = "malformedRequest", + .expectedErrorCode = rpc::ClioError::RpcMalformedRequest, + .expectedErrorMessage = "Malformed request." }, VaultInfoParamTestCaseBundle{ .testName = "MissingSeqInVault", .testJson = R"JSON({ "owner": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh" })JSON", - .expectedError = "invalidParams", - .expectedErrorCode = RippledError::RpcInvalidParams, - .expectedErrorMessage = "Must specify either 'vault_id' or both 'owner' and 'seq'." + .expectedError = "malformedRequest", + .expectedErrorCode = rpc::ClioError::RpcMalformedRequest, + .expectedErrorMessage = "Malformed request." }, VaultInfoParamTestCaseBundle{ .testName = "SeqNotAnInteger", @@ -150,9 +150,9 @@ generateTestValuesForParametersTest() kVaultId, kAccount ), - .expectedError = "invalidParams", - .expectedErrorCode = RippledError::RpcInvalidParams, - .expectedErrorMessage = "Must specify either 'vault_id' or both 'owner' and 'seq'." + .expectedError = "malformedRequest", + .expectedErrorCode = rpc::ClioError::RpcMalformedRequest, + .expectedErrorMessage = "Malformed request." } }; }