From f7c5c65636e9d424b1b9f980f12975f7868a2759 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 20 Jun 2023 10:51:09 -0400 Subject: [PATCH 01/19] Error messages for ApiV2 --- src/ripple/rpc/handlers/GatewayBalances.cpp | 7 ++++ src/ripple/rpc/handlers/PayChanClaim.cpp | 2 +- src/ripple/rpc/impl/RPCHelpers.cpp | 14 ++++++-- src/ripple/rpc/impl/RPCHelpers.h | 2 +- src/test/app/PayChan_test.cpp | 39 +++++++++++++++++++++ src/test/rpc/GatewayBalances_test.cpp | 28 +++++++++++++++ 6 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 77cec496ed0..650dfd26181 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -78,6 +78,13 @@ doGatewayBalances(RPC::JsonContext& context) result[jss::account] = toBase58(accountID); + auto const sleAccepted = ledger->read(keylet::account(accountID)); + if (!sleAccepted && context.apiVersion == 2) + { + RPC::inject_error(rpcACT_NOT_FOUND, result); + return result; + } + // Parse the specified hotwallet(s), if any std::set hotWallets; diff --git a/src/ripple/rpc/handlers/PayChanClaim.cpp b/src/ripple/rpc/handlers/PayChanClaim.cpp index 6353124cb39..7f93535df4b 100644 --- a/src/ripple/rpc/handlers/PayChanClaim.cpp +++ b/src/ripple/rpc/handlers/PayChanClaim.cpp @@ -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); if (RPC::contains_error(result)) return result; diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 44eebdcaba7..16384ac1f11 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -846,7 +846,10 @@ getSeedFromRPC(Json::Value const& params, Json::Value& error) } std::pair -keypairForSignature(Json::Value const& params, Json::Value& error) +keypairForSignature( + Json::Value const& params, + Json::Value& error, + std::optional> context) { bool const has_key_type = params.isMember(jss::key_type); @@ -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) + { + error = RPC::make_error(rpcBAD_KEY_TYPE); + } + else + { + error = RPC::invalid_field_error(jss::key_type); + } return {}; } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 6184b357515..5d1af2004ba 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -207,7 +207,7 @@ std::optional parseRippleLibSeed(Json::Value const& params); std::pair -keypairForSignature(Json::Value const& params, Json::Value& error); +keypairForSignature(Json::Value const& params, Json::Value& error, std::optional> context = std::nullopt); /** * API version numbers used in API version 1 diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 2a8ea360e6c..878b6db4a58 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1160,6 +1160,44 @@ struct PayChan_test : public beast::unit_test::suite bob.human()); } + void + testAccountChannelAuthorizeApiV2(FeatureBitset features) + { + testcase("PayChan Channel_Auth RPC Api V2"); + using namespace jtx; + using namespace std::literals::chrono_literals; + + Env env{*this, envconfig([](std::unique_ptr c) { + c->loadFromString("\n[beta_rpc_api]\n1\n"); + return c; + }), 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(); + + // 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) { @@ -2139,6 +2177,7 @@ struct PayChan_test : public beast::unit_test::suite testAccountChannelsRPC(features); testAccountChannelsRPCMarkers(features); testAccountChannelsRPCSenderOnly(features); + testAccountChannelAuthorizeApiV2(features); testAuthVerifyRPC(features); testOptionalFields(features); testMalformedPK(features); diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index c14ec0f043c..60752e57ae0 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -148,6 +148,33 @@ class GatewayBalances_test : public beast::unit_test::suite } } + void + testGWBApiV2(FeatureBitset features) + { + using namespace std::chrono_literals; + using namespace jtx; + Env env{*this, envconfig([](std::unique_ptr c) { + c->loadFromString("\n[beta_rpc_api]\n1\n"); + return c; + }), 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() { @@ -209,6 +236,7 @@ class GatewayBalances_test : public beast::unit_test::suite auto const sa = supported_amendments(); testGWB(sa - featureFlowCross); testGWB(sa); + testGWBApiV2(sa); testGWBOverflow(); } From 2b7876882c2ea4f205ca8dfe3fb5aff13bb9d46d Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 20 Jun 2023 11:22:54 -0400 Subject: [PATCH 02/19] added clang format --- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- src/ripple/rpc/impl/RPCHelpers.h | 5 ++++- src/test/app/PayChan_test.cpp | 11 +++++++---- src/test/rpc/GatewayBalances_test.cpp | 13 ++++++++----- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 650dfd26181..9aa51bcd99c 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -80,7 +80,7 @@ doGatewayBalances(RPC::JsonContext& context) auto const sleAccepted = ledger->read(keylet::account(accountID)); if (!sleAccepted && context.apiVersion == 2) - { + { RPC::inject_error(rpcACT_NOT_FOUND, result); return result; } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 5d1af2004ba..27d24fc740d 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -207,7 +207,10 @@ std::optional parseRippleLibSeed(Json::Value const& params); std::pair -keypairForSignature(Json::Value const& params, Json::Value& error, std::optional> context = std::nullopt); +keypairForSignature( + Json::Value const& params, + Json::Value& error, + std::optional> context = std::nullopt); /** * API version numbers used in API version 1 diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 878b6db4a58..3a50ab9d191 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1167,10 +1167,13 @@ struct PayChan_test : public beast::unit_test::suite using namespace jtx; using namespace std::literals::chrono_literals; - Env env{*this, envconfig([](std::unique_ptr c) { - c->loadFromString("\n[beta_rpc_api]\n1\n"); - return c; - }), features}; + Env env{ + *this, + envconfig([](std::unique_ptr c) { + c->loadFromString("\n[beta_rpc_api]\n1\n"); + return c; + }), + features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const charlie = Account("charlie", KeyType::ed25519); diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 60752e57ae0..559c286472c 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -153,10 +153,13 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace std::chrono_literals; using namespace jtx; - Env env{*this, envconfig([](std::unique_ptr c) { - c->loadFromString("\n[beta_rpc_api]\n1\n"); - return c; - }), features}; + Env env{ + *this, + envconfig([](std::unique_ptr c) { + c->loadFromString("\n[beta_rpc_api]\n1\n"); + return c; + }), + features}; env.close(); // Gateway account and assets @@ -170,7 +173,7 @@ class GatewayBalances_test : public beast::unit_test::suite 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"); } From 2eb175eb53e3525270a519a6594ea181eefeac36 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 20 Jun 2023 12:22:59 -0400 Subject: [PATCH 03/19] changed ledger->read to exist --- src/ripple/rpc/handlers/GatewayBalances.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 9aa51bcd99c..ea9c043f3f1 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -78,8 +78,7 @@ doGatewayBalances(RPC::JsonContext& context) result[jss::account] = toBase58(accountID); - auto const sleAccepted = ledger->read(keylet::account(accountID)); - if (!sleAccepted && context.apiVersion == 2) + if (!ledger->exists(keylet::account(accountID)) && context.apiVersion == 2) { RPC::inject_error(rpcACT_NOT_FOUND, result); return result; From e49353471b861105233b204c2fc4a8fecab993f2 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 20 Jun 2023 15:30:47 -0400 Subject: [PATCH 04/19] quick fix --- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index ea9c043f3f1..7d65e131edd 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -78,7 +78,7 @@ doGatewayBalances(RPC::JsonContext& context) result[jss::account] = toBase58(accountID); - if (!ledger->exists(keylet::account(accountID)) && context.apiVersion == 2) + if (!ledger->exists(keylet::account(accountID)) && context.apiVersion > 1) { RPC::inject_error(rpcACT_NOT_FOUND, result); return result; From 690ce23c00923bf5f9f6005a349d15d803ee9e12 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Thu, 22 Jun 2023 15:28:51 -0400 Subject: [PATCH 05/19] changed env unit test based on ED's commit --- src/test/rpc/AccountInfo_test.cpp | 11 +++++++++++ src/test/rpc/GatewayBalances_test.cpp | 8 +------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 6ec4740bac2..33fa330e4c8 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -220,6 +220,10 @@ class AccountInfo_test : public beast::unit_test::suite "\"api_version\": 2, \"account\": \"" + alice.human() + "\", " + "\"signer_lists\": true }"; + auto const withSignersAsString = std::string("{ ") + + "\"api_version\": 2, \"account\": \"" + alice.human() + "\", " + + "\"signer_lists\": asdfgh }"; + // Alice has no SignerList yet. { // account_info without the "signer_lists" argument. @@ -237,6 +241,13 @@ class AccountInfo_test : public beast::unit_test::suite BEAST_EXPECT(signerLists.isArray()); BEAST_EXPECT(signerLists.size() == 0); } + { + // expects error if string used for signer_lists + auto const info = + env.rpc("json", "account_info", withSignersAsString); + BEAST_EXPECT(info[jss::status] == "error"); + BEAST_EXPECT(info[jss::error] == "invalidParams"); + } // Give alice a SignerList. Account const bogie{"bogie"}; diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 559c286472c..64b4677c725 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -153,13 +153,7 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace std::chrono_literals; using namespace jtx; - Env env{ - *this, - envconfig([](std::unique_ptr c) { - c->loadFromString("\n[beta_rpc_api]\n1\n"); - return c; - }), - features}; + Env env{*this, features}; env.close(); // Gateway account and assets From 0cd5145af96712dedb62f5ac211d63054daca976 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Thu, 22 Jun 2023 15:45:18 -0400 Subject: [PATCH 06/19] Revert "changed env unit test based on ED's commit" This reverts commit 690ce23c00923bf5f9f6005a349d15d803ee9e12. --- src/test/rpc/AccountInfo_test.cpp | 11 ----------- src/test/rpc/GatewayBalances_test.cpp | 8 +++++++- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 33fa330e4c8..6ec4740bac2 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -220,10 +220,6 @@ class AccountInfo_test : public beast::unit_test::suite "\"api_version\": 2, \"account\": \"" + alice.human() + "\", " + "\"signer_lists\": true }"; - auto const withSignersAsString = std::string("{ ") + - "\"api_version\": 2, \"account\": \"" + alice.human() + "\", " + - "\"signer_lists\": asdfgh }"; - // Alice has no SignerList yet. { // account_info without the "signer_lists" argument. @@ -241,13 +237,6 @@ class AccountInfo_test : public beast::unit_test::suite BEAST_EXPECT(signerLists.isArray()); BEAST_EXPECT(signerLists.size() == 0); } - { - // expects error if string used for signer_lists - auto const info = - env.rpc("json", "account_info", withSignersAsString); - BEAST_EXPECT(info[jss::status] == "error"); - BEAST_EXPECT(info[jss::error] == "invalidParams"); - } // Give alice a SignerList. Account const bogie{"bogie"}; diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 64b4677c725..559c286472c 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -153,7 +153,13 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace std::chrono_literals; using namespace jtx; - Env env{*this, features}; + Env env{ + *this, + envconfig([](std::unique_ptr c) { + c->loadFromString("\n[beta_rpc_api]\n1\n"); + return c; + }), + features}; env.close(); // Gateway account and assets From 29971caf00596ababf7d10ceea180ad842ad0bd1 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Thu, 22 Jun 2023 18:35:43 -0400 Subject: [PATCH 07/19] changed unit test based on ED's API Beta commit --- src/test/rpc/GatewayBalances_test.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 559c286472c..64b4677c725 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -153,13 +153,7 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace std::chrono_literals; using namespace jtx; - Env env{ - *this, - envconfig([](std::unique_ptr c) { - c->loadFromString("\n[beta_rpc_api]\n1\n"); - return c; - }), - features}; + Env env{*this, features}; env.close(); // Gateway account and assets From 311e66cbae5174db86f7599dc5b1badf33b4e556 Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Thu, 22 Jun 2023 19:13:13 -0400 Subject: [PATCH 08/19] Update src/test/app/PayChan_test.cpp Co-authored-by: Arihant Kothari <40741486+arihantkothari@users.noreply.github.com> --- src/test/app/PayChan_test.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 3a50ab9d191..f17eb6b2abc 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1167,13 +1167,7 @@ struct PayChan_test : public beast::unit_test::suite using namespace jtx; using namespace std::literals::chrono_literals; - Env env{ - *this, - envconfig([](std::unique_ptr c) { - c->loadFromString("\n[beta_rpc_api]\n1\n"); - return c; - }), - features}; + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const charlie = Account("charlie", KeyType::ed25519); From 3adae5b77d2902d2ce2d3c1f1f5ef8e61b5324c9 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 27 Jun 2023 14:30:57 -0400 Subject: [PATCH 09/19] changed unittest based on Arihant's comment --- src/test/app/PayChan_test.cpp | 34 +++- src/test/rpc/GatewayBalances_test.cpp | 274 +++++++++++++------------- 2 files changed, 162 insertions(+), 146 deletions(-) diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index f17eb6b2abc..79e087866da 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -24,9 +24,9 @@ #include #include #include -#include - +#include #include +#include namespace ripple { namespace test { @@ -1161,9 +1161,9 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountChannelAuthorizeApiV2(FeatureBitset features) + testAccountChannelAuthorize(FeatureBitset features, unsigned int apiVersion) { - testcase("PayChan Channel_Auth RPC Api V2"); + testcase("PayChan Channel_Auth RPC Api " + std::to_string(apiVersion)); using namespace jtx; using namespace std::literals::chrono_literals; @@ -1181,12 +1181,21 @@ struct PayChan_test : public beast::unit_test::suite // test for api_version 2 Json::Value args{Json::objectValue}; - args[jss::api_version] = 2; + args[jss::api_version] = apiVersion; + args[jss::channel_id] = chan1Str; + args[jss::key_type] = "ed255191"; + args[jss::seed] = "snHq1rzQoN2qiUkC3XF5RyxBzUtN"; + args[jss::amount] = 51110000; + if (apiVersion < 2) + { + auto const rs = env.rpc( + "json", + "channel_authorize", + args.toStyledString())[jss::result]; + BEAST_EXPECT(rs[jss::error] == "invalidParams"); + } + else { - 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", @@ -2174,7 +2183,12 @@ struct PayChan_test : public beast::unit_test::suite testAccountChannelsRPC(features); testAccountChannelsRPCMarkers(features); testAccountChannelsRPCSenderOnly(features); - testAccountChannelAuthorizeApiV2(features); + for (auto testVersion = RPC::apiMinimumSupportedVersion; + testVersion <= RPC::apiBetaVersion; + ++testVersion) + { + testAccountChannelAuthorize(features, testVersion); + } testAuthVerifyRPC(features); testOptionalFields(features); testMalformedPK(features); diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 64b4677c725..b6c03adbcdd 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace ripple { namespace test { @@ -28,150 +29,147 @@ class GatewayBalances_test : public beast::unit_test::suite { public: void - testGWB(FeatureBitset features) + testGWB(FeatureBitset features, unsigned int apiVersion) { using namespace std::chrono_literals; using namespace jtx; Env env(*this, features); - // Gateway account and assets - Account const alice{"alice"}; - env.fund(XRP(10000), "alice"); - auto USD = alice["USD"]; - auto CNY = alice["CNY"]; - auto JPY = alice["JPY"]; - - // Create a hotwallet - Account const hw{"hw"}; - env.fund(XRP(10000), "hw"); - env(trust(hw, USD(10000))); - env(trust(hw, JPY(10000))); - env(pay(alice, hw, USD(5000))); - env(pay(alice, hw, JPY(5000))); - - // Create some clients - Account const bob{"bob"}; - env.fund(XRP(10000), "bob"); - env(trust(bob, USD(100))); - env(trust(bob, CNY(100))); - env(pay(alice, bob, USD(50))); - - Account const charley{"charley"}; - env.fund(XRP(10000), "charley"); - env(trust(charley, CNY(500))); - env(trust(charley, JPY(500))); - env(pay(alice, charley, CNY(250))); - env(pay(alice, charley, JPY(250))); - - Account const dave{"dave"}; - env.fund(XRP(10000), "dave"); - env(trust(dave, CNY(100))); - env(pay(alice, dave, CNY(30))); - - // give the gateway an asset - env(trust(alice, charley["USD"](50))); - env(pay(charley, alice, USD(10))); - - // freeze dave - env(trust(alice, dave["CNY"](0), dave, tfSetFreeze)); - - env.close(); - - auto wsc = makeWSClient(env.app().config()); - - Json::Value qry; - qry[jss::account] = alice.human(); - qry[jss::hotwallet] = hw.human(); - - auto jv = wsc->invoke("gateway_balances", qry); - expect(jv[jss::status] == "success"); - if (wsc->version() == 2) - { - expect(jv.isMember(jss::jsonrpc) && jv[jss::jsonrpc] == "2.0"); - expect(jv.isMember(jss::ripplerpc) && jv[jss::ripplerpc] == "2.0"); - expect(jv.isMember(jss::id) && jv[jss::id] == 5); - } - - auto const& result = jv[jss::result]; - expect(result[jss::account] == alice.human()); - expect(result[jss::status] == "success"); - - { - auto const& balances = result[jss::balances]; - expect(balances.isObject(), "balances is not an object"); - expect(balances.size() == 1, "balances size is not 1"); - - auto const& hwBalance = balances[hw.human()]; - expect(hwBalance.isArray(), "hwBalance is not an array"); - expect(hwBalance.size() == 2); - auto c1 = hwBalance[0u][jss::currency]; - auto c2 = hwBalance[1u][jss::currency]; - expect(c1 == "USD" || c2 == "USD"); - expect(c1 == "JPY" || c2 == "JPY"); - expect( - hwBalance[0u][jss::value] == "5000" && - hwBalance[1u][jss::value] == "5000"); - } - - { - auto const& fBalances = result[jss::frozen_balances]; - expect(fBalances.isObject()); - expect(fBalances.size() == 1); - - auto const& fBal = fBalances[dave.human()]; - expect(fBal.isArray()); - expect(fBal.size() == 1); - expect(fBal[0u].isObject()); - expect(fBal[0u][jss::currency] == "CNY"); - expect(fBal[0u][jss::value] == "30"); - } - + if (apiVersion < 2) { - auto const& assets = result[jss::assets]; - expect(assets.isObject(), "assets it not an object"); - expect(assets.size() == 1, "assets size is not 1"); - - auto const& cAssets = assets[charley.human()]; - expect(cAssets.isArray()); - expect(cAssets.size() == 1); - expect(cAssets[0u][jss::currency] == "USD"); - expect(cAssets[0u][jss::value] == "10"); + // Gateway account and assets + Account const alice{"alice"}; + env.fund(XRP(10000), "alice"); + auto USD = alice["USD"]; + auto CNY = alice["CNY"]; + auto JPY = alice["JPY"]; + + // Create a hotwallet + Account const hw{"hw"}; + env.fund(XRP(10000), "hw"); + env(trust(hw, USD(10000))); + env(trust(hw, JPY(10000))); + env(pay(alice, hw, USD(5000))); + env(pay(alice, hw, JPY(5000))); + + // Create some clients + Account const bob{"bob"}; + env.fund(XRP(10000), "bob"); + env(trust(bob, USD(100))); + env(trust(bob, CNY(100))); + env(pay(alice, bob, USD(50))); + + Account const charley{"charley"}; + env.fund(XRP(10000), "charley"); + env(trust(charley, CNY(500))); + env(trust(charley, JPY(500))); + env(pay(alice, charley, CNY(250))); + env(pay(alice, charley, JPY(250))); + + Account const dave{"dave"}; + env.fund(XRP(10000), "dave"); + env(trust(dave, CNY(100))); + env(pay(alice, dave, CNY(30))); + + // give the gateway an asset + env(trust(alice, charley["USD"](50))); + env(pay(charley, alice, USD(10))); + + // freeze dave + env(trust(alice, dave["CNY"](0), dave, tfSetFreeze)); + + env.close(); + + auto wsc = makeWSClient(env.app().config()); + + Json::Value qry; + qry[jss::account] = alice.human(); + qry[jss::hotwallet] = hw.human(); + + auto jv = wsc->invoke("gateway_balances", qry); + expect(jv[jss::status] == "success"); + if (wsc->version() == 2) + { + expect(jv.isMember(jss::jsonrpc) && jv[jss::jsonrpc] == "2.0"); + expect( + jv.isMember(jss::ripplerpc) && jv[jss::ripplerpc] == "2.0"); + expect(jv.isMember(jss::id) && jv[jss::id] == 5); + } + + auto const& result = jv[jss::result]; + expect(result[jss::account] == alice.human()); + expect(result[jss::status] == "success"); + + { + auto const& balances = result[jss::balances]; + expect(balances.isObject(), "balances is not an object"); + expect(balances.size() == 1, "balances size is not 1"); + + auto const& hwBalance = balances[hw.human()]; + expect(hwBalance.isArray(), "hwBalance is not an array"); + expect(hwBalance.size() == 2); + auto c1 = hwBalance[0u][jss::currency]; + auto c2 = hwBalance[1u][jss::currency]; + expect(c1 == "USD" || c2 == "USD"); + expect(c1 == "JPY" || c2 == "JPY"); + expect( + hwBalance[0u][jss::value] == "5000" && + hwBalance[1u][jss::value] == "5000"); + } + + { + auto const& fBalances = result[jss::frozen_balances]; + expect(fBalances.isObject()); + expect(fBalances.size() == 1); + + auto const& fBal = fBalances[dave.human()]; + expect(fBal.isArray()); + expect(fBal.size() == 1); + expect(fBal[0u].isObject()); + expect(fBal[0u][jss::currency] == "CNY"); + expect(fBal[0u][jss::value] == "30"); + } + + { + auto const& assets = result[jss::assets]; + expect(assets.isObject(), "assets it not an object"); + expect(assets.size() == 1, "assets size is not 1"); + + auto const& cAssets = assets[charley.human()]; + expect(cAssets.isArray()); + expect(cAssets.size() == 1); + expect(cAssets[0u][jss::currency] == "USD"); + expect(cAssets[0u][jss::value] == "10"); + } + + { + auto const& obligations = result[jss::obligations]; + expect(obligations.isObject(), "obligations is not an object"); + expect(obligations.size() == 3); + expect(obligations["CNY"] == "250"); + expect(obligations["JPY"] == "250"); + expect(obligations["USD"] == "50"); + } } - + else { - auto const& obligations = result[jss::obligations]; - expect(obligations.isObject(), "obligations is not an object"); - expect(obligations.size() == 3); - expect(obligations["CNY"] == "250"); - expect(obligations["JPY"] == "250"); - expect(obligations["USD"] == "50"); + // 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] = apiVersion; + 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 - 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() { @@ -231,9 +229,13 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testGWB(sa - featureFlowCross); - testGWB(sa); - testGWBApiV2(sa); + for (auto testVersion = RPC::apiMinimumSupportedVersion; + testVersion <= RPC::apiBetaVersion; + ++testVersion) + { + testGWB(sa - featureFlowCross, testVersion); + testGWB(sa, testVersion); + } testGWBOverflow(); } From dad2e8e2f583f393a3198c87d2ce53dc9e5525bc Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 27 Jun 2023 17:50:33 -0400 Subject: [PATCH 10/19] added clang format --- src/test/rpc/GatewayBalances_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index b6c03adbcdd..559468df927 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -18,9 +18,9 @@ #include #include #include +#include #include #include -#include namespace ripple { namespace test { From 7344b30193f4c64cd570f4e708bc507ac13daf63 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Wed, 28 Jun 2023 13:36:14 -0400 Subject: [PATCH 11/19] Fixed issue #4548 as dealing with same rpc --- src/ripple/rpc/handlers/GatewayBalances.cpp | 13 +++++++++- src/test/rpc/GatewayBalances_test.cpp | 27 ++++++++++++++++----- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 7d65e131edd..a109f1a8f92 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -122,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 == 1) + { + result[jss::error] = "invalidHotWallet"; + } + else + { + result[jss::error] = "invalidParams"; + } return result; } } diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 559468df927..9c811f8a147 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -155,18 +155,33 @@ class GatewayBalances_test : public beast::unit_test::suite { // Gateway account and assets Account const alice{"alice"}; + env.fund(XRP(10000), alice); Account const hw{"hw"}; + env.fund(XRP(10000), hw); + env.close(); + auto wsc = makeWSClient(env.app().config()); Json::Value qry2; qry2[jss::api_version] = apiVersion; - qry2[jss::account] = "rNGQLojaFxTYphuQUJ24QUhyGBUMMbqBMx"; - qry2[jss::hotwallet] = hw.human(); - auto jv = wsc->invoke("gateway_balances", qry2); - expect(jv[jss::status] == "error"); + { + 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"); + auto response = jv[jss::result]; + expect(response[jss::error] == "actNotFound"); + } + { + qry2[jss::account] = alice.human(); + qry2[jss::hotwallet] = "asdf"; + auto jv = wsc->invoke("gateway_balances", qry2); + expect(jv[jss::status] == "error"); + + auto response = jv[jss::result]; + expect(response[jss::error] == "invalidParams"); + } } } From b1bae4d7f5d716bd5a74763f83f0753246d1388b Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Wed, 28 Jun 2023 14:06:40 -0400 Subject: [PATCH 12/19] Fix for arihant's comment --- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index a109f1a8f92..8107d4abf7e 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -126,7 +126,7 @@ doGatewayBalances(RPC::JsonContext& context) // 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 == 1) + if (context.apiVersion < 2) { result[jss::error] = "invalidHotWallet"; } From da46159a7b83ed295653ba6600ea7b56a40b7875 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Thu, 29 Jun 2023 13:21:24 -0400 Subject: [PATCH 13/19] compare to 2u, for faster step --- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- src/test/app/PayChan_test.cpp | 2 +- src/test/rpc/GatewayBalances_test.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 8107d4abf7e..6f2835f02a7 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -126,7 +126,7 @@ doGatewayBalances(RPC::JsonContext& context) // 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 < 2) + if (context.apiVersion < 2u) { result[jss::error] = "invalidHotWallet"; } diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 79e087866da..8164f27a863 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1186,7 +1186,7 @@ struct PayChan_test : public beast::unit_test::suite args[jss::key_type] = "ed255191"; args[jss::seed] = "snHq1rzQoN2qiUkC3XF5RyxBzUtN"; args[jss::amount] = 51110000; - if (apiVersion < 2) + if (apiVersion < 2u) { auto const rs = env.rpc( "json", diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 9c811f8a147..d9e92b2f800 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -35,7 +35,7 @@ class GatewayBalances_test : public beast::unit_test::suite using namespace jtx; Env env(*this, features); - if (apiVersion < 2) + if (apiVersion < 2u) { // Gateway account and assets Account const alice{"alice"}; From 41fcf315669df574ed73932f3ed47d6aeee92685 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Fri, 30 Jun 2023 11:11:56 -0400 Subject: [PATCH 14/19] sign commit attempt --- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 6f2835f02a7..c8475718b95 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -78,7 +78,7 @@ doGatewayBalances(RPC::JsonContext& context) result[jss::account] = toBase58(accountID); - if (!ledger->exists(keylet::account(accountID)) && context.apiVersion > 1) + if (!ledger->exists(keylet::account(accountID)) && context.apiVersion > 1u) { RPC::inject_error(rpcACT_NOT_FOUND, result); return result; From 682a1a4a277c9face475cc70fd7b5045262223a1 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Wed, 5 Jul 2023 13:52:41 -0400 Subject: [PATCH 15/19] quick fix on Arihant's comment --- src/ripple/rpc/impl/RPCHelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 16384ac1f11..68b117aa2be 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -903,7 +903,7 @@ keypairForSignature( if (!keyType) { - if (context.has_value() && context.value().get().apiVersion > 1) + if (context.has_value() && context.value().get().apiVersion > 1u) { error = RPC::make_error(rpcBAD_KEY_TYPE); } From 21c5ebd73ba0dd68f80dc3526001b72763a0b273 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 10 Jul 2023 15:21:06 -0400 Subject: [PATCH 16/19] fixes based on Greg's comments --- src/ripple/protocol/ErrorCodes.h | 2 +- src/ripple/protocol/impl/ErrorCodes.cpp | 1 + src/ripple/rpc/handlers/GatewayBalances.cpp | 4 +- src/ripple/rpc/handlers/PayChanClaim.cpp | 3 +- src/ripple/rpc/impl/RPCHelpers.cpp | 4 +- src/ripple/rpc/impl/RPCHelpers.h | 12 ++-- src/test/app/PayChan_test.cpp | 32 ++++------ src/test/rpc/GatewayBalances_test.cpp | 67 ++++++++++----------- 8 files changed, 59 insertions(+), 66 deletions(-) diff --git a/src/ripple/protocol/ErrorCodes.h b/src/ripple/protocol/ErrorCodes.h index ee33eee0604..78f3fb61620 100644 --- a/src/ripple/protocol/ErrorCodes.h +++ b/src/ripple/protocol/ErrorCodes.h @@ -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, diff --git a/src/ripple/protocol/impl/ErrorCodes.cpp b/src/ripple/protocol/impl/ErrorCodes.cpp index bb3b2d47a89..d8f5135c2fc 100644 --- a/src/ripple/protocol/impl/ErrorCodes.cpp +++ b/src/ripple/protocol/impl/ErrorCodes.cpp @@ -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 hotwalloet.", 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}, diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index c8475718b95..cec8de9e24f 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -128,11 +128,11 @@ doGatewayBalances(RPC::JsonContext& context) // not have currency issued by the account from the request. if (context.apiVersion < 2u) { - result[jss::error] = "invalidHotWallet"; + RPC::inject_error(rpcINVALID_HOTWALLET, result); } else { - result[jss::error] = "invalidParams"; + RPC::inject_error(rpcINVALID_PARAMS, result); } return result; } diff --git a/src/ripple/rpc/handlers/PayChanClaim.cpp b/src/ripple/rpc/handlers/PayChanClaim.cpp index 7f93535df4b..23a4041bb35 100644 --- a/src/ripple/rpc/handlers/PayChanClaim.cpp +++ b/src/ripple/rpc/handlers/PayChanClaim.cpp @@ -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, context); + auto const [pk, sk] = + RPC::keypairForSignature(params, result, context.apiVersion); if (RPC::contains_error(result)) return result; diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 68b117aa2be..ffb33b7736d 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -849,7 +849,7 @@ std::pair keypairForSignature( Json::Value const& params, Json::Value& error, - std::optional> context) + uint apiVersion) { bool const has_key_type = params.isMember(jss::key_type); @@ -903,7 +903,7 @@ keypairForSignature( if (!keyType) { - if (context.has_value() && context.value().get().apiVersion > 1u) + if (apiVersion > 1u) { error = RPC::make_error(rpcBAD_KEY_TYPE); } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 27d24fc740d..ec951d34a58 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -206,12 +206,6 @@ getSeedFromRPC(Json::Value const& params, Json::Value& error); std::optional parseRippleLibSeed(Json::Value const& params); -std::pair -keypairForSignature( - Json::Value const& params, - Json::Value& error, - std::optional> context = std::nullopt); - /** * API version numbers used in API version 1 */ @@ -296,6 +290,12 @@ getAPIVersionNumber(const Json::Value& value, bool betaEnabled); std::variant, Json::Value> getLedgerByContext(RPC::JsonContext& context); +std::pair +keypairForSignature( + Json::Value const& params, + Json::Value& error, + uint apiVersion = apiVersionIfUnspecified); + } // namespace RPC } // namespace ripple diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 8164f27a863..e8a9dc29393 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -1161,9 +1161,8 @@ struct PayChan_test : public beast::unit_test::suite } void - testAccountChannelAuthorize(FeatureBitset features, unsigned int apiVersion) + testAccountChannelAuthorize(FeatureBitset features) { - testcase("PayChan Channel_Auth RPC Api " + std::to_string(apiVersion)); using namespace jtx; using namespace std::literals::chrono_literals; @@ -1179,28 +1178,26 @@ struct PayChan_test : public beast::unit_test::suite env(create(alice, bob, channelFunds, settleDelay, pk)); env.close(); - // test for api_version 2 Json::Value args{Json::objectValue}; - args[jss::api_version] = apiVersion; args[jss::channel_id] = chan1Str; args[jss::key_type] = "ed255191"; args[jss::seed] = "snHq1rzQoN2qiUkC3XF5RyxBzUtN"; args[jss::amount] = 51110000; - if (apiVersion < 2u) - { - auto const rs = env.rpc( - "json", - "channel_authorize", - args.toStyledString())[jss::result]; - BEAST_EXPECT(rs[jss::error] == "invalidParams"); - } - else + + // 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]; - BEAST_EXPECT(rs[jss::error] == "badKeyType"); + auto const error = apiVersion < 2u ? "invalidParams" : "badKeyType"; + BEAST_EXPECT(rs[jss::error] == error); } } @@ -2183,12 +2180,7 @@ struct PayChan_test : public beast::unit_test::suite testAccountChannelsRPC(features); testAccountChannelsRPCMarkers(features); testAccountChannelsRPCSenderOnly(features); - for (auto testVersion = RPC::apiMinimumSupportedVersion; - testVersion <= RPC::apiBetaVersion; - ++testVersion) - { - testAccountChannelAuthorize(features, testVersion); - } + testAccountChannelAuthorize(features); testAuthVerifyRPC(features); testOptionalFields(features); testMalformedPK(features); diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index d9e92b2f800..211fd09305f 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -29,13 +29,12 @@ class GatewayBalances_test : public beast::unit_test::suite { public: void - testGWB(FeatureBitset features, unsigned int apiVersion) + testGWB(FeatureBitset features) { using namespace std::chrono_literals; using namespace jtx; Env env(*this, features); - if (apiVersion < 2u) { // Gateway account and assets Account const alice{"alice"}; @@ -151,37 +150,39 @@ class GatewayBalances_test : public beast::unit_test::suite expect(obligations["USD"] == "50"); } } - else - { - // Gateway account and assets - Account const alice{"alice"}; - env.fund(XRP(10000), alice); - Account const hw{"hw"}; - env.fund(XRP(10000), hw); - env.close(); + } - auto wsc = makeWSClient(env.app().config()); + void + testGWBApiVersions(FeatureBitset features) + { + using namespace std::chrono_literals; + using namespace jtx; + Env env(*this, features); - Json::Value qry2; - qry2[jss::api_version] = apiVersion; - { - qry2[jss::account] = "rNGQLojaFxTYphuQUJ24QUhyGBUMMbqBMx"; - qry2[jss::hotwallet] = hw.human(); - auto jv = wsc->invoke("gateway_balances", qry2); - expect(jv[jss::status] == "error"); + // Gateway account and assets + Account const alice{"alice"}; + env.fund(XRP(10000), alice); + Account const hw{"hw"}; + env.fund(XRP(10000), hw); + env.close(); - auto response = jv[jss::result]; - expect(response[jss::error] == "actNotFound"); - } - { - qry2[jss::account] = alice.human(); - qry2[jss::hotwallet] = "asdf"; - auto jv = wsc->invoke("gateway_balances", qry2); - expect(jv[jss::status] == "error"); + auto wsc = makeWSClient(env.app().config()); - auto response = jv[jss::result]; - expect(response[jss::error] == "invalidParams"); - } + Json::Value qry2; + for (auto apiVersion = RPC::apiMinimumSupportedVersion; + apiVersion <= RPC::apiBetaVersion; + ++apiVersion) + { + qry2[jss::account] = alice.human(); + qry2[jss::hotwallet] = "asdf"; + qry2[jss::api_version] = apiVersion; + auto jv = wsc->invoke("gateway_balances", qry2); + expect(jv[jss::status] == "error"); + + auto response = jv[jss::result]; + auto const error = + apiVersion < 2u ? "invalidHotWallet" : "invalidParams"; + BEAST_EXPECT(response[jss::error] == error); } } @@ -244,12 +245,10 @@ class GatewayBalances_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - for (auto testVersion = RPC::apiMinimumSupportedVersion; - testVersion <= RPC::apiBetaVersion; - ++testVersion) + for (auto feature : {sa - featureFlowCross, sa}) { - testGWB(sa - featureFlowCross, testVersion); - testGWB(sa, testVersion); + testGWB(feature); + testGWBApiVersions(feature); } testGWBOverflow(); From 38e7f8e11094ddb38ee03f51af83a3013d8c23f7 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 10 Jul 2023 15:42:02 -0400 Subject: [PATCH 17/19] move account/HW outside of loop --- src/test/rpc/GatewayBalances_test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 211fd09305f..091b9f51686 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -169,12 +169,13 @@ class GatewayBalances_test : public beast::unit_test::suite auto wsc = makeWSClient(env.app().config()); Json::Value qry2; + qry2[jss::account] = alice.human(); + qry2[jss::hotwallet] = "asdf"; + for (auto apiVersion = RPC::apiMinimumSupportedVersion; apiVersion <= RPC::apiBetaVersion; ++apiVersion) { - qry2[jss::account] = alice.human(); - qry2[jss::hotwallet] = "asdf"; qry2[jss::api_version] = apiVersion; auto jv = wsc->invoke("gateway_balances", qry2); expect(jv[jss::status] == "error"); From ce680b52a18db4f83ef05c579955afd66a5a553d Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Mon, 10 Jul 2023 16:02:46 -0400 Subject: [PATCH 18/19] typo fix --- src/ripple/protocol/impl/ErrorCodes.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/ErrorCodes.cpp b/src/ripple/protocol/impl/ErrorCodes.cpp index d8f5135c2fc..277f60c5e38 100644 --- a/src/ripple/protocol/impl/ErrorCodes.cpp +++ b/src/ripple/protocol/impl/ErrorCodes.cpp @@ -76,7 +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 hotwalloet.", 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}, From 092f6c37a7ecda4f5fa862b1b866b79d785db539 Mon Sep 17 00:00:00 2001 From: Peter Chen Date: Tue, 11 Jul 2023 09:45:42 -0400 Subject: [PATCH 19/19] refactor based on shawn's comments --- src/ripple/rpc/impl/RPCHelpers.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index ffb33b7736d..60e72ed48ef 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -849,7 +849,7 @@ std::pair keypairForSignature( Json::Value const& params, Json::Value& error, - uint apiVersion) + unsigned int apiVersion) { bool const has_key_type = params.isMember(jss::key_type); @@ -904,13 +904,9 @@ keypairForSignature( if (!keyType) { if (apiVersion > 1u) - { error = RPC::make_error(rpcBAD_KEY_TYPE); - } else - { error = RPC::invalid_field_error(jss::key_type); - } return {}; }