Skip to content

Commit

Permalink
Change amm_info error message in API version 3
Browse files Browse the repository at this point in the history
  • Loading branch information
@Bronek
Bronek committed Feb 21, 2024
1 parent 7e4049a commit 99ea4fe
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 5 deletions.
15 changes: 13 additions & 2 deletions src/ripple/rpc/handlers/AMMInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,17 @@ doAMMInfo(RPC::JsonContext& context)
std::optional<Issue> issue2;
std::optional<uint256> 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))
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion src/test/jtx/AMM.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ class AMM
std::optional<Issue> issue1 = std::nullopt,
std::optional<Issue> issue2 = std::nullopt,
std::optional<AccountID> const& ammAccount = std::nullopt,
bool ignoreParams = false) const;
bool ignoreParams = false,
unsigned apiVersion = RPC::apiInvalidVersion) const;

/** Verify the AMM balances.
*/
Expand Down
8 changes: 6 additions & 2 deletions src/test/jtx/impl/AMM.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ AMM::ammRpcInfo(
std::optional<Issue> issue1,
std::optional<Issue> issue2,
std::optional<AccountID> const& ammAccount,
bool ignoreParams) const
bool ignoreParams,
unsigned apiVersion) const
{
Json::Value jv;
if (account)
Expand Down Expand Up @@ -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];
Expand Down
101 changes: 101 additions & 0 deletions src/test/rpc/AMMInfo_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::tuple<
std::optional<Issue>,
std::optional<Issue>,
std::optional<AccountID>,
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::tuple<
std::optional<Issue>,
std::optional<Issue>,
std::optional<AccountID>,
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");
Expand All @@ -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::tuple<
std::optional<Issue>,
std::optional<Issue>,
std::optional<AccountID>,
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::tuple<
std::optional<Issue>,
std::optional<Issue>,
std::optional<AccountID>,
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
Expand Down

0 comments on commit 99ea4fe

Please sign in to comment.