From 554aa8938b3854073fc2c6cedc99d69b676a3d14 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 27 May 2020 17:44:20 -0400 Subject: [PATCH] Consolidate "Not Synced" Error Messages * Fixes #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. --- RELEASENOTES.md | 5 +++++ src/ripple/app/main/Application.cpp | 8 ++++--- src/ripple/app/main/GRPCServer.cpp | 3 ++- src/ripple/app/main/GRPCServer.h | 2 ++ src/ripple/net/impl/RPCCall.cpp | 8 +++++-- src/ripple/protocol/ErrorCodes.h | 2 +- src/ripple/protocol/impl/ErrorCodes.cpp | 3 ++- src/ripple/rpc/Context.h | 2 +- src/ripple/rpc/handlers/AccountTx.cpp | 10 +++++++-- src/ripple/rpc/handlers/AccountTxOld.cpp | 8 +++++-- src/ripple/rpc/handlers/LedgerRequest.cpp | 6 ++++- src/ripple/rpc/handlers/RipplePathFind.cpp | 4 +++- src/ripple/rpc/impl/Handler.h | 16 +++++++++---- src/ripple/rpc/impl/RPCHandler.cpp | 22 ++++++++++++++---- src/ripple/rpc/impl/RPCHelpers.cpp | 26 +++++++++++++++++----- src/ripple/rpc/impl/RPCHelpers.h | 2 +- src/ripple/rpc/impl/ServerHandlerImp.cpp | 8 +++---- src/ripple/rpc/impl/TransactionSign.cpp | 14 ++++++++---- src/test/app/Path_test.cpp | 12 ++++++---- src/test/rpc/AccountTx_test.cpp | 4 ++-- src/test/rpc/LedgerRequestRPC_test.cpp | 4 ++-- src/test/rpc/RPCCall_test.cpp | 8 +++---- 22 files changed, 128 insertions(+), 49 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 1cc76688909..e2d22e15a7e 100644 --- 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 `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. + # Releases ## Version 1.5.0 diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 3b37c736963..398212fa556 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1759,9 +1759,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 index acf33784f38..007e5f0c499 100644 --- a/src/ripple/app/main/GRPCServer.cpp +++ b/src/ripple/app/main/GRPCServer.cpp @@ -142,7 +142,8 @@ GRPCServerImpl::CallData::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 index 5175e2e256d..bb06784c24f 100644 --- a/src/ripple/app/main/GRPCServer.h +++ b/src/ripple/app/main/GRPCServer.h @@ -105,6 +105,8 @@ class GRPCServerImpl final template using Handler = std::function( RPC::GRPCContext&)>; + // 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 index a565d4bc321..16d24664aa3 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -314,7 +314,9 @@ class RPCParser if (uLedgerMax != -1 && uLedgerMax < uLedgerMin) { - return rpcError(rpcLGR_IDXS_INVALID); + // This is an api_version 2 response + // because it is a command line request + return rpcError(rpcNOT_SYNCED); } jvRequest[jss::ledger_index_min] = jvParams[1u].asInt(); @@ -384,7 +386,9 @@ class RPCParser if (uLedgerMax != -1 && uLedgerMax < uLedgerMin) { - return rpcError(rpcLGR_IDXS_INVALID); + // This is an api_version 2 response + // because it is a command line request + 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 index a3fb9e590bb..68c6c4395f5 100644 --- 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 index 9110daf40f7..3df10624655 100644 --- 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 index 1188006362e..7a22ed9fe0c 100644 --- a/src/ripple/rpc/Context.h +++ b/src/ripple/rpc/Context.h @@ -47,6 +47,7 @@ struct Context Role role; std::shared_ptr 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 index f4e3a6b9d3f..77c7285d2a6 100644 --- 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 { diff --git a/src/ripple/rpc/handlers/AccountTxOld.cpp b/src/ripple/rpc/handlers/AccountTxOld.cpp index 472f999b621..5950f474d36 100644 --- a/src/ripple/rpc/handlers/AccountTxOld.cpp +++ b/src/ripple/rpc/handlers/AccountTxOld.cpp @@ -105,7 +105,9 @@ 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; @@ -113,7 +115,9 @@ doAccountTxOld(RPC::JsonContext& context) 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 index f7b12e95eb9..5d2d11b00aa 100644 --- 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 index 4b03bb93f53..5e23a47bd52 100644 --- 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 index 09acd6ba165..6b6fa71e7cb 100644 --- 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 index 9d4885b1ebf..cd15e91e0a7 100644 --- 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 index 1104caa59f2..060862bede9 100644 --- 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,7 +396,11 @@ 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; @@ -394,7 +408,9 @@ getLedger(T& ledger, LedgerShortcut shortcut, Context& context) 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/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index bb596f80c52..bb51d0f91af 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -211,7 +211,7 @@ extern beast::SemanticVersion const lastVersion; constexpr unsigned int APIInvalidVersion = 0; constexpr unsigned int APIVersionIfUnspecified = 1; constexpr unsigned int ApiMinimumSupportedVersion = 1; -constexpr unsigned int ApiMaximumSupportedVersion = 1; +constexpr unsigned int ApiMaximumSupportedVersion = 2; constexpr unsigned int APINumberVersionSupported = ApiMaximumSupportedVersion - ApiMinimumSupportedVersion + 1; diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index 3f404321bd0..0fb0018f390 100644 --- 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 index 0ebeef36326..0cceca14223 100644 --- 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 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); diff --git a/src/test/app/Path_test.cpp b/src/test/app/Path_test.cpp index 7b273666ac5..17e15c95043 100644 --- a/src/test/app/Path_test.cpp +++ b/src/test/app/Path_test.cpp @@ -223,9 +223,11 @@ class Path_test : public beast::unit_test::suite app.getOPs(), app.getLedgerMaster(), c, - Role::USER}, + Role::USER, + {}, + {}, + RPC::APIVersionIfUnspecified}, {}, - RPC::APIVersionIfUnspecified, {}}; Json::Value params = Json::objectValue; @@ -329,9 +331,11 @@ class Path_test : public beast::unit_test::suite app.getOPs(), app.getLedgerMaster(), c, - Role::USER}, + Role::USER, + {}, + {}, + RPC::APIVersionIfUnspecified}, {}, - RPC::APIVersionIfUnspecified, {}}; Json::Value result; gate g; diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 534dd8235f8..661bff18eca 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -175,7 +175,7 @@ class AccountTx_test : public beast::unit_test::suite p[jss::ledger_index_max] = 1; BEAST_EXPECT(isErr( env.rpc("json", "account_tx", to_string(p)), - rpcLGR_IDXS_INVALID)); + rpcINVALID_LGR_RANGE)); } // Ledger index min only @@ -190,7 +190,7 @@ class AccountTx_test : public beast::unit_test::suite p[jss::ledger_index_min] = env.current()->info().seq; BEAST_EXPECT(isErr( env.rpc("json", "account_tx", to_string(p)), - rpcLGR_IDXS_INVALID)); + rpcINVALID_LGR_RANGE)); } // Ledger index max only diff --git a/src/test/rpc/LedgerRequestRPC_test.cpp b/src/test/rpc/LedgerRequestRPC_test.cpp index c7d84009969..39a2b24316f 100644 --- a/src/test/rpc/LedgerRequestRPC_test.cpp +++ b/src/test/rpc/LedgerRequestRPC_test.cpp @@ -297,10 +297,10 @@ class LedgerRequestRPC_test : public beast::unit_test::suite // date check to trigger env.timeKeeper().adjustCloseTime(weeks{3}); result = env.rpc("ledger_request", "1")[jss::result]; - BEAST_EXPECT(result[jss::error] == "noCurrent"); + BEAST_EXPECT(result[jss::error] == "notSynced"); BEAST_EXPECT(result[jss::status] == "error"); BEAST_EXPECT( - result[jss::error_message] == "Current ledger is unavailable."); + result[jss::error_message] == "Not synced to the network."); } void diff --git a/src/test/rpc/RPCCall_test.cpp b/src/test/rpc/RPCCall_test.cpp index 174feb0b7f8..e1fe0549003 100644 --- a/src/test/rpc/RPCCall_test.cpp +++ b/src/test/rpc/RPCCall_test.cpp @@ -1441,9 +1441,9 @@ static RPCCallTestData const rpcCallTestArray[] = { "method" : "account_tx", "params" : [ { - "error" : "lgrIdxsInvalid", + "error" : "notSynced", "error_code" : 55, - "error_message" : "Ledger indexes invalid." + "error_message" : "Not synced to the network." } ] })", @@ -5909,9 +5909,9 @@ static RPCCallTestData const rpcCallTestArray[] = { "method" : "tx_account", "params" : [ { - "error" : "lgrIdxsInvalid", + "error" : "notSynced", "error_code" : 55, - "error_message" : "Ledger indexes invalid." + "error_message" : "Not synced to the network." } ] })",