diff --git a/src/ripple/rpc/handlers/AMMInfo.cpp b/src/ripple/rpc/handlers/AMMInfo.cpp index 534bc106ba7..5e994f22cef 100644 --- a/src/ripple/rpc/handlers/AMMInfo.cpp +++ b/src/ripple/rpc/handlers/AMMInfo.cpp @@ -96,6 +96,17 @@ doAMMInfo(RPC::JsonContext& context) std::optional issue2; std::optional ammID; + constexpr auto invalid = [](Json::Value const& params) -> bool { + return (params.isMember(jss::asset) != + params.isMember(jss::asset2)) || + (params.isMember(jss::asset) == + params.isMember(jss::amm_account)); + }; + + // NOTE, identical check for apVersion >= 3 below + if (context.apiVersion < 3 && invalid(params)) + return Unexpected(rpcINVALID_PARAMS); + if (params.isMember(jss::asset)) { if (auto const i = getIssue(params[jss::asset], context.j)) @@ -130,8 +141,8 @@ doAMMInfo(RPC::JsonContext& context) return Unexpected(rpcACT_MALFORMED); } - if ((params.isMember(jss::asset) != params.isMember(jss::asset2)) || - (params.isMember(jss::asset) == params.isMember(jss::amm_account))) + // NOTE, identical check for apVersion < 3 above + if (context.apiVersion >= 3 && invalid(params)) return Unexpected(rpcINVALID_PARAMS); assert( diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index 4c6e8d78a4e..3b222412685 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -176,7 +176,8 @@ class AMM std::optional issue1 = std::nullopt, std::optional issue2 = std::nullopt, std::optional const& ammAccount = std::nullopt, - bool ignoreParams = false) const; + bool ignoreParams = false, + unsigned apiVersion = RPC::apiInvalidVersion) const; /** Verify the AMM balances. */ diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index 2d5ce90d306..ad6e1301b35 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -163,7 +163,8 @@ AMM::ammRpcInfo( std::optional issue1, std::optional issue2, std::optional const& ammAccount, - bool ignoreParams) const + bool ignoreParams, + unsigned apiVersion) const { Json::Value jv; if (account) @@ -191,7 +192,10 @@ AMM::ammRpcInfo( if (ammAccount) jv[jss::amm_account] = to_string(*ammAccount); } - auto jr = env_.rpc("json", "amm_info", to_string(jv)); + auto jr = + (apiVersion == RPC::apiInvalidVersion + ? env_.rpc("json", "amm_info", to_string(jv)) + : env_.rpc(apiVersion, "json", "amm_info", to_string(jv))); if (jr.isObject() && jr.isMember(jss::result) && jr[jss::result].isMember(jss::status)) return jr[jss::result]; diff --git a/src/test/rpc/AMMInfo_test.cpp b/src/test/rpc/AMMInfo_test.cpp index 1d9642539a1..dfeae88d85f 100644 --- a/src/test/rpc/AMMInfo_test.cpp +++ b/src/test/rpc/AMMInfo_test.cpp @@ -75,6 +75,52 @@ class AMMInfo_test : public jtx::AMMTestBase } }); + // Invalid parameters *and* invalid LP account, default API version + testAMM([&](AMM& ammAlice, Env&) { + Account bogie("bogie"); + std::vector, + std::optional, + std::optional, + bool>> + vals = { + {xrpIssue(), std::nullopt, std::nullopt, false}, + {std::nullopt, USD.issue(), std::nullopt, false}, + {xrpIssue(), std::nullopt, ammAlice.ammAccount(), false}, + {std::nullopt, USD.issue(), ammAlice.ammAccount(), false}, + {xrpIssue(), USD.issue(), ammAlice.ammAccount(), false}, + {std::nullopt, std::nullopt, std::nullopt, true}}; + for (auto const& [iss1, iss2, acct, ignoreParams] : vals) + { + auto const jv = ammAlice.ammRpcInfo( + bogie, std::nullopt, iss1, iss2, acct, ignoreParams); + BEAST_EXPECT(jv[jss::error_message] == "Invalid parameters."); + } + }); + + // Invalid parameters *and* invalid LP account, API version 3 + testAMM([&](AMM& ammAlice, Env&) { + Account bogie("bogie"); + std::vector, + std::optional, + std::optional, + bool>> + vals = { + {xrpIssue(), std::nullopt, std::nullopt, false}, + {std::nullopt, USD.issue(), std::nullopt, false}, + {xrpIssue(), std::nullopt, ammAlice.ammAccount(), false}, + {std::nullopt, USD.issue(), ammAlice.ammAccount(), false}, + {xrpIssue(), USD.issue(), ammAlice.ammAccount(), false}, + {std::nullopt, std::nullopt, std::nullopt, true}}; + for (auto const& [iss1, iss2, acct, ignoreParams] : vals) + { + auto const jv = ammAlice.ammRpcInfo( + bogie, std::nullopt, iss1, iss2, acct, ignoreParams, 3); + BEAST_EXPECT(jv[jss::error_message] == "Account malformed."); + } + }); + // Invalid AMM account id testAMM([&](AMM& ammAlice, Env&) { Account bogie("bogie"); @@ -86,6 +132,61 @@ class AMMInfo_test : public jtx::AMMTestBase bogie.id()); BEAST_EXPECT(jv[jss::error_message] == "Account malformed."); }); + + // Invalid parameters *and* invalid AMM account, default API version + testAMM([&](AMM& ammAlice, Env&) { + Account bogie("bogie"); + std::vector, + std::optional, + std::optional, + bool>> + vals = { + {xrpIssue(), std::nullopt, std::nullopt, false}, + {std::nullopt, USD.issue(), std::nullopt, false}, + {xrpIssue(), std::nullopt, bogie, false}, + {std::nullopt, USD.issue(), bogie, false}, + {xrpIssue(), USD.issue(), bogie, false}, + {std::nullopt, std::nullopt, std::nullopt, true}}; + for (auto const& [iss1, iss2, acct, ignoreParams] : vals) + { + auto const jv = ammAlice.ammRpcInfo( + std::nullopt, std::nullopt, iss1, iss2, acct, ignoreParams); + BEAST_EXPECT(jv[jss::error_message] == "Invalid parameters."); + } + }); + + // Invalid parameters *and* invalid AMM account, API version 3 + testAMM([&](AMM& ammAlice, Env&) { + Account bogie("bogie"); + std::vector, + std::optional, + std::optional, + bool>> + vals = { + {xrpIssue(), std::nullopt, std::nullopt, false}, + {std::nullopt, USD.issue(), std::nullopt, false}, + {xrpIssue(), std::nullopt, bogie, false}, + {std::nullopt, USD.issue(), bogie, false}, + {xrpIssue(), USD.issue(), bogie, false}, + {std::nullopt, std::nullopt, std::nullopt, true}}; + for (auto const& [iss1, iss2, acct, ignoreParams] : vals) + { + auto const jv = ammAlice.ammRpcInfo( + std::nullopt, + std::nullopt, + iss1, + iss2, + acct, + ignoreParams, + 3); + BEAST_EXPECT( + jv[jss::error_message] == + (acct ? std::string("Account malformed.") + : std::string("Invalid parameters."))); + } + }); } void