Consolidate "Not Synced" Error Messages

* Fixes XRPLF#3269 * Work on a version 2 of the XRP Network API has begun. The new API returns the code `notSynced` in place of `noClosed`, `noCurrent`, and `noNetwork`. And `invalidParams` is returned in place of `lgrIdxInvalid`. * The version 2 API can be specified by adding "api_version" = 2 to your json request. The default version remains 1 (if unspecified), except for the command line interface which tracks version 2. * It can be re-enabled by setting ApiMaximumSupportedVersion to 2.
manojsdoshi · Jun 25, 2020 · 11b6d3f · 11b6d3f
1 parent 84bd3b9
commit 11b6d3f
Show file tree

Hide file tree

Showing 21 changed files with 169 additions and 47 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -7,6 +7,11 @@ This document contains the release notes for `rippled`, the reference server imp
 
 Have new ideas? Need help with setting up your node? Come visit us [here](https://github.com/ripple/rippled/issues/new/choose)
 
+# Change Log
+
+- Work on a version 2 of the XRP Network API has begun.  The new API returns the code `notSynced` in place of `noClosed`, `noCurrent`, and `noNetwork`.  And `invalidLgrRange` is returned in place of `lgrIdxInvalid`.
+- The version 2 API can be specified by adding "api_version" : 2 to your json request.  The default version remains 1 (if unspecified), except for the command line interface which always uses the latest verison.
+
 # Releases
 
 ## Version 1.5.0

diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp
@@ -1756,9 +1756,11 @@ ApplicationImp::setup()
              getOPs(),
              getLedgerMaster(),
              c,
-             Role::ADMIN},
-            jvCommand,
-            RPC::ApiMaximumSupportedVersion};
+             Role::ADMIN,
+             {},
+             {},
+             RPC::ApiMaximumSupportedVersion},
+            jvCommand};
 
         Json::Value jvResult;
         RPC::doCommand(context, jvResult);

diff --git a/src/ripple/app/main/GRPCServer.cpp b/src/ripple/app/main/GRPCServer.cpp
@@ -142,7 +142,8 @@ GRPCServerImpl::CallData<Request, Response>::process(
                  usage,
                  role,
                  coro,
-                 InfoSub::pointer()},
+                 InfoSub::pointer(),
+                 apiVersion},
                 request_};
 
             // Make sure we can currently handle the rpc

diff --git a/src/ripple/app/main/GRPCServer.h b/src/ripple/app/main/GRPCServer.h
@@ -105,6 +105,8 @@ class GRPCServerImpl final
     template <class Request, class Response>
     using Handler = std::function<std::pair<Response, grpc::Status>(
         RPC::GRPCContext<Request>&)>;
+    // This implementation is currently limited to v1 of the API
+    static unsigned constexpr apiVersion = 1;
 
 public:
     explicit GRPCServerImpl(Application& app);

diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp
@@ -314,7 +314,10 @@ class RPCParser
 
             if (uLedgerMax != -1 && uLedgerMax < uLedgerMin)
             {
-                return rpcError(rpcLGR_IDXS_INVALID);
+                // The command line always follows ApiMaximumSupportedVersion
+                if (RPC::ApiMaximumSupportedVersion == 1)
+                    return rpcError(rpcLGR_IDXS_INVALID);
+                return rpcError(rpcNOT_SYNCED);
             }
 
             jvRequest[jss::ledger_index_min] = jvParams[1u].asInt();
@@ -384,7 +387,10 @@ class RPCParser
 
             if (uLedgerMax != -1 && uLedgerMax < uLedgerMin)
             {
-                return rpcError(rpcLGR_IDXS_INVALID);
+                // The command line always follows ApiMaximumSupportedVersion
+                if (RPC::ApiMaximumSupportedVersion == 1)
+                    return rpcError(rpcLGR_IDXS_INVALID);
+                return rpcError(rpcNOT_SYNCED);
             }
 
             jvRequest[jss::ledger_index_min] = jvParams[1u].asInt();

diff --git a/src/ripple/protocol/ErrorCodes.h b/src/ripple/protocol/ErrorCodes.h
@@ -64,9 +64,9 @@ enum error_code_i {
     rpcNO_CLOSED = 15,
     rpcNO_CURRENT = 16,
     rpcNO_NETWORK = 17,
+    rpcNOT_SYNCED = 18,
 
     // Ledger state
-    // unused                  18,
     rpcACT_NOT_FOUND = 19,
     // unused                  20,
     rpcLGR_NOT_FOUND = 21,

diff --git a/src/ripple/protocol/impl/ErrorCodes.cpp b/src/ripple/protocol/impl/ErrorCodes.cpp
@@ -90,8 +90,9 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
     {rpcNOT_SUPPORTED, "notSupported", "Operation not supported."},
     {rpcNO_CLOSED, "noClosed", "Closed ledger is unavailable."},
     {rpcNO_CURRENT, "noCurrent", "Current ledger is unavailable."},
+    {rpcNOT_SYNCED, "notSynced", "Not synced to the network."},
     {rpcNO_EVENTS, "noEvents", "Current transport does not support events."},
-    {rpcNO_NETWORK, "noNetwork", "Not synced to Ripple network."},
+    {rpcNO_NETWORK, "noNetwork", "Not synced to the network."},
     {rpcNO_PERMISSION,
      "noPermission",
      "You don't have permission for this command."},

diff --git a/src/ripple/rpc/Context.h b/src/ripple/rpc/Context.h
@@ -47,6 +47,7 @@ struct Context
     Role role;
     std::shared_ptr<JobQueue::Coro> coro{};
     InfoSub::pointer infoSub{};
+    unsigned int apiVersion;
 };
 
 struct JsonContext : public Context
@@ -62,7 +63,6 @@ struct JsonContext : public Context
 
     Json::Value params;
 
-    unsigned int apiVersion;
     Headers headers{};
 };
 

diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp
@@ -214,7 +214,9 @@ getLedgerRange(
     if (!bValidated)
     {
         // Don't have a validated ledger range.
-        return rpcLGR_IDXS_INVALID;
+        if (context.apiVersion == 1)
+            return rpcLGR_IDXS_INVALID;
+        return rpcNOT_SYNCED;
     }
 
     std::uint32_t uLedgerMin = uValidatedMin;
@@ -236,7 +238,11 @@ getLedgerRange(
                         uLedgerMax = ls.max;
                     }
                     if (uLedgerMax < uLedgerMin)
-                        return rpcLGR_IDXS_INVALID;
+                    {
+                        if (context.apiVersion == 1)
+                            return rpcLGR_IDXS_INVALID;
+                        return rpcINVALID_LGR_RANGE;
+                    }
                 }
                 else
                 {
@@ -330,6 +336,10 @@ populateProtoResponse(
         {
             status = {grpc::StatusCode::NOT_FOUND, error.message()};
         }
+        else if (error.toErrorCode() == rpcNOT_SYNCED)
+        {
+            status = {grpc::StatusCode::FAILED_PRECONDITION, error.message()};
+        }
         else
         {
             status = {grpc::StatusCode::INVALID_ARGUMENT, error.message()};

diff --git a/src/ripple/rpc/handlers/AccountTxOld.cpp b/src/ripple/rpc/handlers/AccountTxOld.cpp
@@ -105,15 +105,19 @@ doAccountTxOld(RPC::JsonContext& context)
         if (!bValidated && (iLedgerMin == -1 || iLedgerMax == -1))
         {
             // Don't have a validated ledger range.
-            return rpcError(rpcLGR_IDXS_INVALID);
+            if (context.apiVersion == 1)
+                return rpcError(rpcLGR_IDXS_INVALID);
+            return rpcError(rpcNOT_SYNCED);
         }
 
         uLedgerMin = iLedgerMin == -1 ? uValidatedMin : iLedgerMin;
         uLedgerMax = iLedgerMax == -1 ? uValidatedMax : iLedgerMax;
 
         if (uLedgerMax < uLedgerMin)
         {
-            return rpcError(rpcLGR_IDXS_INVALID);
+            if (context.apiVersion == 1)
+                return rpcError(rpcLGR_IDXS_INVALID);
+            return rpcError(rpcNOT_SYNCED);
         }
     }
     else

diff --git a/src/ripple/rpc/handlers/LedgerRequest.cpp b/src/ripple/rpc/handlers/LedgerRequest.cpp
@@ -67,7 +67,11 @@ doLedgerRequest(RPC::JsonContext& context)
         // We need a validated ledger to get the hash from the sequence
         if (ledgerMaster.getValidatedLedgerAge() >
             RPC::Tuning::maxValidatedLedgerAge)
-            return rpcError(rpcNO_CURRENT);
+        {
+            if (context.apiVersion == 1)
+                return rpcError(rpcNO_CURRENT);
+            return rpcError(rpcNOT_SYNCED);
+        }
 
         ledgerIndex = jsonIndex.asInt();
         auto ledger = ledgerMaster.getValidatedLedger();

diff --git a/src/ripple/rpc/handlers/RipplePathFind.cpp b/src/ripple/rpc/handlers/RipplePathFind.cpp
@@ -49,7 +49,9 @@ doRipplePathFind(RPC::JsonContext& context)
         if (context.app.getLedgerMaster().getValidatedLedgerAge() >
             RPC::Tuning::maxValidatedLedgerAge)
         {
-            return rpcError(rpcNO_NETWORK);
+            if (context.apiVersion == 1)
+                return rpcError(rpcNO_NETWORK);
+            return rpcError(rpcNOT_SYNCED);
         }
 
         PathRequest::pointer request;

diff --git a/src/ripple/rpc/impl/Handler.h b/src/ripple/rpc/impl/Handler.h
@@ -83,7 +83,9 @@ conditionMet(Condition condition_required, T& context)
         JLOG(context.j.info()) << "Insufficient network mode for RPC: "
                                << context.netOps.strOperatingMode();
 
-        return rpcNO_NETWORK;
+        if (context.apiVersion == 1)
+            return rpcNO_NETWORK;
+        return rpcNOT_SYNCED;
     }
 
     if (context.app.getOPs().isAmendmentBlocked() &&
@@ -99,7 +101,9 @@ conditionMet(Condition condition_required, T& context)
         if (context.ledgerMaster.getValidatedLedgerAge() >
             Tuning::maxValidatedLedgerAge)
         {
-            return rpcNO_CURRENT;
+            if (context.apiVersion == 1)
+                return rpcNO_CURRENT;
+            return rpcNOT_SYNCED;
         }
 
         auto const cID = context.ledgerMaster.getCurrentLedgerIndex();
@@ -110,14 +114,18 @@ conditionMet(Condition condition_required, T& context)
             JLOG(context.j.debug())
                 << "Current ledger ID(" << cID
                 << ") is less than validated ledger ID(" << vID << ")";
-            return rpcNO_CURRENT;
+            if (context.apiVersion == 1)
+                return rpcNO_CURRENT;
+            return rpcNOT_SYNCED;
         }
     }
 
     if ((condition_required & NEEDS_CLOSED_LEDGER) &&
         !context.ledgerMaster.getClosedLedger())
     {
-        return rpcNO_CLOSED;
+        if (context.apiVersion == 1)
+            return rpcNO_CLOSED;
+        return rpcNOT_SYNCED;
     }
 
     return rpcSUCCESS;

diff --git a/src/ripple/rpc/impl/RPCHandler.cpp b/src/ripple/rpc/impl/RPCHandler.cpp
@@ -65,9 +65,16 @@ namespace {
      Failure:
         {
            "result" : {
+              // api_version == 1
               "error" : "noNetwork",
-              "error_code" : 16,
-              "error_message" : "Not synced to Ripple network.",
+              "error_code" : 17,
+              "error_message" : "Not synced to the network.",
+
+              // api_version == 2
+              "error" : "notSynced",
+              "error_code" : 18,
+              "error_message" : "Not synced to the network.",
+
               "request" : {
                  "command" : "ledger",
                  "ledger_index" : 10300865
@@ -95,9 +102,16 @@ namespace {
 
      Failure:
         {
+          // api_version == 1
           "error" : "noNetwork",
-          "error_code" : 16,
-          "error_message" : "Not synced to Ripple network.",
+          "error_code" : 17,
+          "error_message" : "Not synced to the network.",
+
+          // api_version == 2
+          "error" : "notSynced",
+          "error_code" : 18,
+          "error_message" : "Not synced to the network.",
+
           "request" : {
              "command" : "ledger",
              "ledger_index" : 10300865

diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp
@@ -347,7 +347,9 @@ getLedger(T& ledger, uint32_t ledgerIndex, Context& context)
         isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
     {
         ledger.reset();
-        return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        if (context.apiVersion == 1)
+            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        return {rpcNOT_SYNCED, "notSynced"};
     }
 
     return Status::OK;
@@ -358,13 +360,21 @@ Status
 getLedger(T& ledger, LedgerShortcut shortcut, Context& context)
 {
     if (isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
-        return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+    {
+        if (context.apiVersion == 1)
+            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        return {rpcNOT_SYNCED, "notSynced"};
+    }
 
     if (shortcut == LedgerShortcut::VALIDATED)
     {
         ledger = context.ledgerMaster.getValidatedLedger();
         if (ledger == nullptr)
-            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        {
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
+        }
 
         assert(!ledger->open());
     }
@@ -386,15 +396,21 @@ getLedger(T& ledger, LedgerShortcut shortcut, Context& context)
         }
 
         if (ledger == nullptr)
-            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        {
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
+        }
 
         static auto const minSequenceGap = 10;
 
         if (ledger->info().seq + minSequenceGap <
             context.ledgerMaster.getValidLedgerIndex())
         {
             ledger.reset();
-            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
         }
     }
     return Status::OK;

diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp
@@ -443,9 +443,9 @@ ServerHandlerImp::processSession(
                  is->getConsumer(),
                  role,
                  coro,
-                 is},
+                 is,
+                 apiVersion},
                 jv,
-                apiVersion,
                 {is->user(), is->forwarded_for()}};
 
             RPC::doCommand(context, jr[jss::result]);
@@ -829,9 +829,9 @@ ServerHandlerImp::processRequest(
              usage,
              role,
              coro,
-             InfoSub::pointer()},
+             InfoSub::pointer(),
+             apiVersion},
             params,
-            apiVersion,
             {user, forwardedFor}};
         Json::Value result;
         RPC::doCommand(context, result);

diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp
@@ -270,7 +270,8 @@ checkTxJsonFields(
     bool const verify,
     std::chrono::seconds validatedLedgerAge,
     Config const& config,
-    LoadFeeTrack const& feeTrack)
+    LoadFeeTrack const& feeTrack,
+    unsigned apiVersion)
 {
     std::pair<Json::Value, AccountID> ret;
 
@@ -308,7 +309,10 @@ checkTxJsonFields(
     if (verify && !config.standalone() &&
         (validatedLedgerAge > Tuning::maxValidatedLedgerAge))
     {
-        ret.first = rpcError(rpcNO_CURRENT);
+        if (apiVersion == 1)
+            ret.first = rpcError(rpcNO_CURRENT);
+        else
+            ret.first = rpcError(rpcNOT_SYNCED);
         return ret;
     }
 
@@ -384,7 +388,8 @@ transactionPreProcessImpl(
         verify,
         validatedLedgerAge,
         app.config(),
-        app.getFeeTrack());
+        app.getFeeTrack(),
+        getAPIVersionNumber(params));
 
     if (RPC::contains_error(txJsonResult))
         return std::move(txJsonResult);
@@ -1068,7 +1073,8 @@ transactionSubmitMultiSigned(
         true,
         validatedLedgerAge,
         app.config(),
-        app.getFeeTrack());
+        app.getFeeTrack(),
+        getAPIVersionNumber(jvRequest));
 
     if (RPC::contains_error(txJsonResult))
         return std::move(txJsonResult);