APIv2(gateway_balances, channel_authorize): update errors (XRPLF#4618)

gateway_balances * When `account` does not exist in the ledger, return `actNotFound` * (Previously, a normal response was returned) * Fix XRPLF#4290 * When required field(s) are missing, return `invalidParams` * (Previously, `invalidHotWallet` was incorrectly returned) * Fix XRPLF#4548 channel_authorize * When the specified `key_type` is invalid, return `badKeyType` * (Previously, `invalidParams` was returned) * Fix XRPLF#4289 Since these are breaking changes, they apply only to API version 2. Supersedes XRPLF#4577
ckeshava · Sep 22, 2023 · 05ac9c9 · 05ac9c9
1 parent c7d0ee4
commit 05ac9c9
Show file tree

Hide file tree

Showing 8 changed files with 225 additions and 114 deletions.
diff --git 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
@@ -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},
     {rpcISSUE_MALFORMED,        "issueMalformed",       "Issue is malformed.", 400},
     {rpcJSON_RPC,               "json_rpc",             "JSON-RPC transport error.", 500},
     {rpcLGR_IDXS_INVALID,       "lgrIdxsInvalid",       "Ledger indexes invalid.", 400},

diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp
@@ -78,6 +78,12 @@ doGatewayBalances(RPC::JsonContext& context)
 
     result[jss::account] = toBase58(accountID);
 
+    if (context.apiVersion > 1u && !ledger->exists(keylet::account(accountID)))
+    {
+        RPC::inject_error(rpcACT_NOT_FOUND, result);
+        return result;
+    }
+
     // Parse the specified hotwallet(s), if any
     std::set<AccountID> hotWallets;
 
@@ -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)
+            {
+                RPC::inject_error(rpcINVALID_HOTWALLET, result);
+            }
+            else
+            {
+                RPC::inject_error(rpcINVALID_PARAMS, result);
+            }
             return result;
         }
     }

diff --git 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);
+    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
@@ -847,7 +847,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);
 
@@ -901,7 +904,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 {};
         }
 

diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h
@@ -208,9 +208,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
  */
@@ -295,6 +292,11 @@ 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);
 /** Helper to parse submit_mode parameter to RPC submit.
  *
  * @param params RPC parameters

diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp
@@ -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>
 #include <chrono>
+#include <test/jtx.h>
 
 namespace ripple {
 namespace test {
@@ -1063,6 +1063,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)
     {
@@ -2042,6 +2083,7 @@ struct PayChan_test : public beast::unit_test::suite
         testAccountChannelsRPC(features);
         testAccountChannelsRPCMarkers(features);
         testAccountChannelsRPCSenderOnly(features);
+        testAccountChannelAuthorize(features);
         testAuthVerifyRPC(features);
         testOptionalFields(features);
         testMalformedPK(features);