Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Error messages on Gateway_Balance and Channel_Authorize #4577

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f7c5c65
Error messages for ApiV2
PeterChen13579 Jun 20, 2023
2b78768
added clang format
PeterChen13579 Jun 20, 2023
2eb175e
changed ledger->read to exist
PeterChen13579 Jun 20, 2023
e493534
quick fix
PeterChen13579 Jun 20, 2023
690ce23
changed env unit test based on ED's commit
PeterChen13579 Jun 22, 2023
0cd5145
Revert "changed env unit test based on ED's commit"
PeterChen13579 Jun 22, 2023
c843248
Merge remote-tracking branch 'upstream/master' into Error_messages
PeterChen13579 Jun 22, 2023
614cea4
Merge remote-tracking branch 'upstream/develop' into Error_messages
PeterChen13579 Jun 22, 2023
29971ca
changed unit test based on ED's API Beta commit
PeterChen13579 Jun 22, 2023
311e66c
Update src/test/app/PayChan_test.cpp
PeterChen13579 Jun 22, 2023
3adae5b
changed unittest based on Arihant's comment
PeterChen13579 Jun 27, 2023
dad2e8e
added clang format
PeterChen13579 Jun 27, 2023
7344b30
Fixed issue #4548 as dealing with same rpc
PeterChen13579 Jun 28, 2023
b1bae4d
Fix for arihant's comment
PeterChen13579 Jun 28, 2023
da46159
compare to 2u, for faster step
PeterChen13579 Jun 29, 2023
41fcf31
sign commit attempt
PeterChen13579 Jun 30, 2023
682a1a4
quick fix on Arihant's comment
PeterChen13579 Jul 5, 2023
920cd81
Merge remote-tracking branch 'upstream/develop' into Error_messages
PeterChen13579 Jul 10, 2023
21c5ebd
fixes based on Greg's comments
PeterChen13579 Jul 10, 2023
38e7f8e
move account/HW outside of loop
PeterChen13579 Jul 10, 2023
ce680b5
typo fix
PeterChen13579 Jul 10, 2023
092f6c3
refactor based on shawn's comments
PeterChen13579 Jul 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/ripple/rpc/handlers/GatewayBalances.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ doGatewayBalances(RPC::JsonContext& context)

result[jss::account] = toBase58(accountID);

if (!ledger->exists(keylet::account(accountID)) && context.apiVersion > 1)
{
RPC::inject_error(rpcACT_NOT_FOUND, result);
return result;
}

// Parse the specified hotwallet(s), if any
std::set<AccountID> hotWallets;

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/handlers/PayChanClaim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ doChannelAuthorize(RPC::JsonContext& context)
return RPC::missing_field_error(jss::secret);

Json::Value result;
auto const [pk, sk] = RPC::keypairForSignature(params, result);
auto const [pk, sk] = RPC::keypairForSignature(params, result, context);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider passing context.apiVersion instead.

if (RPC::contains_error(result))
return result;

Expand Down
14 changes: 12 additions & 2 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,10 @@ getSeedFromRPC(Json::Value const& params, Json::Value& error)
}

std::pair<PublicKey, SecretKey>
keypairForSignature(Json::Value const& params, Json::Value& error)
keypairForSignature(
Json::Value const& params,
Json::Value& error,
std::optional<std::reference_wrapper<JsonContext>> context)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider std::optional<uint> const& apiVersion instead.

{
bool const has_key_type = params.isMember(jss::key_type);

Expand Down Expand Up @@ -900,7 +903,14 @@ keypairForSignature(Json::Value const& params, Json::Value& error)

if (!keyType)
{
error = RPC::invalid_field_error(jss::key_type);
if (context.has_value() && context.value().get().apiVersion > 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: keep it consistent - use 1u if you are using it everywhere else in the PR. Or keep it 1 if you want to use that everywhere but keep it consistent.

{
error = RPC::make_error(rpcBAD_KEY_TYPE);
}
else
{
error = RPC::invalid_field_error(jss::key_type);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the brackets needed here?

return {};
}

Expand Down
5 changes: 4 additions & 1 deletion src/ripple/rpc/impl/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,10 @@ std::optional<Seed>
parseRippleLibSeed(Json::Value const& params);

std::pair<PublicKey, SecretKey>
keypairForSignature(Json::Value const& params, Json::Value& error);
keypairForSignature(
Json::Value const& params,
Json::Value& error,
std::optional<std::reference_wrapper<JsonContext>> context = std::nullopt);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider changing to std::optional<uint> const& apiVersion. Actually, maybe we can even do this: uint apiVersion = RPC::apiVersionIfUnspecified?


/**
* API version numbers used in API version 1
Expand Down
42 changes: 42 additions & 0 deletions src/test/app/PayChan_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,47 @@ struct PayChan_test : public beast::unit_test::suite
bob.human());
}

void
testAccountChannelAuthorizeApiV2(FeatureBitset features)
PeterChen13579 marked this conversation as resolved.
Show resolved Hide resolved
{
testcase("PayChan Channel_Auth RPC Api V2");
using namespace jtx;
using namespace std::literals::chrono_literals;

Env env{
*this,
envconfig([](std::unique_ptr<Config> c) {
c->loadFromString("\n[beta_rpc_api]\n1\n");
return c;
}),
features};
PeterChen13579 marked this conversation as resolved.
Show resolved Hide resolved
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const charlie = Account("charlie", KeyType::ed25519);
env.fund(XRP(10000), alice, bob, charlie);
auto const pk = alice.pk();
auto const settleDelay = 3600s;
auto const channelFunds = XRP(1000);
auto const chan1Str = to_string(channel(alice, bob, env.seq(alice)));
env(create(alice, bob, channelFunds, settleDelay, pk));
env.close();

// test for api_version 2
Json::Value args{Json::objectValue};
args[jss::api_version] = 2;
{
args[jss::channel_id] = chan1Str;
args[jss::key_type] = "ed255191";
args[jss::seed] = "snHq1rzQoN2qiUkC3XF5RyxBzUtN";
args[jss::amount] = 51110000;
auto const rs = env.rpc(
"json",
"channel_authorize",
args.toStyledString())[jss::result];
BEAST_EXPECT(rs[jss::error] == "badKeyType");
}
}

void
testAuthVerifyRPC(FeatureBitset features)
{
Expand Down Expand Up @@ -2139,6 +2180,7 @@ struct PayChan_test : public beast::unit_test::suite
testAccountChannelsRPC(features);
testAccountChannelsRPCMarkers(features);
testAccountChannelsRPCSenderOnly(features);
testAccountChannelAuthorizeApiV2(features);
testAuthVerifyRPC(features);
testOptionalFields(features);
testMalformedPK(features);
Expand Down
25 changes: 25 additions & 0 deletions src/test/rpc/GatewayBalances_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ class GatewayBalances_test : public beast::unit_test::suite
}
}

void
testGWBApiV2(FeatureBitset features)
{
using namespace std::chrono_literals;
using namespace jtx;
Env env{*this, features};
env.close();

// Gateway account and assets
Account const alice{"alice"};
Account const hw{"hw"};
auto wsc = makeWSClient(env.app().config());

Json::Value qry2;
qry2[jss::api_version] = 2;
qry2[jss::account] = "rNGQLojaFxTYphuQUJ24QUhyGBUMMbqBMx";
qry2[jss::hotwallet] = hw.human();
auto jv = wsc->invoke("gateway_balances", qry2);
expect(jv[jss::status] == "error");

auto response = jv[jss::result];
expect(response[jss::error] == "actNotFound");
}

void
testGWBOverflow()
{
Expand Down Expand Up @@ -209,6 +233,7 @@ class GatewayBalances_test : public beast::unit_test::suite
auto const sa = supported_amendments();
testGWB(sa - featureFlowCross);
testGWB(sa);
testGWBApiV2(sa);

testGWBOverflow();
}
Expand Down