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 all 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
2 changes: 1 addition & 1 deletion src/ripple/protocol/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ enum error_code_i {
// unused 27,
// unused 28,
rpcTXN_NOT_FOUND = 29,
// unused 30,
rpcINVALID_HOTWALLET = 30,

// Malformed command
rpcINVALID_PARAMS = 31,
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
{rpcINTERNAL, "internal", "Internal error.", 500},
{rpcINVALID_LGR_RANGE, "invalidLgrRange", "Ledger range is invalid.", 400},
{rpcINVALID_PARAMS, "invalidParams", "Invalid parameters.", 400},
{rpcINVALID_HOTWALLET, "invalidHotWallet", "Invalid hotwallet.", 400},
{rpcJSON_RPC, "json_rpc", "JSON-RPC transport error.", 500},
{rpcLGR_IDXS_INVALID, "lgrIdxsInvalid", "Ledger indexes invalid.", 400},
{rpcLGR_IDX_MALFORMED, "lgrIdxMalformed", "Ledger index malformed.", 400},
Expand Down
19 changes: 18 additions & 1 deletion 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 > 1u)
{
RPC::inject_error(rpcACT_NOT_FOUND, result);
return result;
}

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

Expand Down Expand Up @@ -116,7 +122,18 @@ doGatewayBalances(RPC::JsonContext& context)

if (!valid)
{
result[jss::error] = "invalidHotWallet";
// The documentation states that invalidParams is used when
// One or more fields are specified incorrectly.
// invalidHotwallet should be used when the account exists, but does
// not have currency issued by the account from the request.
if (context.apiVersion < 2u)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add rpcINALID_HOT_WALLET error and inject it into the result.

{
RPC::inject_error(rpcINVALID_HOTWALLET, result);
}
else
{
RPC::inject_error(rpcINVALID_PARAMS, result);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

brackets here as well

return result;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/rpc/handlers/PayChanClaim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ 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.apiVersion);
if (RPC::contains_error(result))
return result;

Expand Down
10 changes: 8 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,
unsigned int apiVersion)
{
bool const has_key_type = params.isMember(jss::key_type);

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

if (!keyType)
{
error = RPC::invalid_field_error(jss::key_type);
if (apiVersion > 1u)
error = RPC::make_error(rpcBAD_KEY_TYPE);
else
error = RPC::invalid_field_error(jss::key_type);
return {};
}

Expand Down
9 changes: 6 additions & 3 deletions src/ripple/rpc/impl/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,6 @@ getSeedFromRPC(Json::Value const& params, Json::Value& error);
std::optional<Seed>
parseRippleLibSeed(Json::Value const& params);

std::pair<PublicKey, SecretKey>
keypairForSignature(Json::Value const& params, Json::Value& error);

/**
* API version numbers used in API version 1
*/
Expand Down Expand Up @@ -293,6 +290,12 @@ getAPIVersionNumber(const Json::Value& value, bool betaEnabled);
std::variant<std::shared_ptr<Ledger const>, Json::Value>
getLedgerByContext(RPC::JsonContext& context);

std::pair<PublicKey, SecretKey>
keypairForSignature(
Json::Value const& params,
Json::Value& error,
uint apiVersion = apiVersionIfUnspecified);

} // namespace RPC
} // namespace ripple

Expand Down
46 changes: 44 additions & 2 deletions src/test/app/PayChan_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
#include <ripple/protocol/PayChan.h>
#include <ripple/protocol/TxFlags.h>
#include <ripple/protocol/jss.h>
#include <test/jtx.h>

#include <ripple/rpc/impl/RPCHelpers.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change the headers order? The original order and the new line looks right to me.

#include <chrono>
#include <test/jtx.h>

namespace ripple {
namespace test {
Expand Down Expand Up @@ -1160,6 +1160,47 @@ struct PayChan_test : public beast::unit_test::suite
bob.human());
}

void
testAccountChannelAuthorize(FeatureBitset features)
{
using namespace jtx;
using namespace std::literals::chrono_literals;

Env env{*this, features};
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();

Json::Value args{Json::objectValue};
args[jss::channel_id] = chan1Str;
args[jss::key_type] = "ed255191";
args[jss::seed] = "snHq1rzQoN2qiUkC3XF5RyxBzUtN";
args[jss::amount] = 51110000;

// test for all api versions
for (auto apiVersion = RPC::apiMinimumSupportedVersion;
apiVersion <= RPC::apiBetaVersion;
++apiVersion)
{
testcase(
"PayChan Channel_Auth RPC Api " + std::to_string(apiVersion));
args[jss::api_version] = apiVersion;
auto const rs = env.rpc(
"json",
"channel_authorize",
args.toStyledString())[jss::result];
auto const error = apiVersion < 2u ? "invalidParams" : "badKeyType";
BEAST_EXPECT(rs[jss::error] == error);
}
}

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);
testAccountChannelAuthorize(features);
testAuthVerifyRPC(features);
testOptionalFields(features);
testMalformedPK(features);
Expand Down
Loading