Skip to content

Commit

Permalink
Consolidate "Not Synced" Error Messages
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
HowardHinnant committed Jun 22, 2020
1 parent f43aeda commit 554aa89
Show file tree
Hide file tree
Showing 22 changed files with 128 additions and 49 deletions.
5 changes: 5 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/app/main/GRPCServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/ripple/app/main/GRPCServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions src/ripple/net/impl/RPCCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/ErrorCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/impl/ErrorCodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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."},
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/Context.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct Context
Role role;
std::shared_ptr<JobQueue::Coro> coro{};
InfoSub::pointer infoSub{};
unsigned int apiVersion;
};

struct JsonContext : public Context
Expand All @@ -62,7 +63,6 @@ struct JsonContext : public Context

Json::Value params;

unsigned int apiVersion;
Headers headers{};
};

Expand Down
10 changes: 8 additions & 2 deletions src/ripple/rpc/handlers/AccountTx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{
Expand Down
8 changes: 6 additions & 2 deletions src/ripple/rpc/handlers/AccountTxOld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion src/ripple/rpc/handlers/LedgerRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/rpc/handlers/RipplePathFind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 12 additions & 4 deletions src/ripple/rpc/impl/Handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand All @@ -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();
Expand All @@ -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;
Expand Down
22 changes: 18 additions & 4 deletions src/ripple/rpc/impl/RPCHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/impl/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 4 additions & 4 deletions src/ripple/rpc/impl/ServerHandlerImp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 10 additions & 4 deletions src/ripple/rpc/impl/TransactionSign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -384,7 +388,8 @@ transactionPreProcessImpl(
verify,
validatedLedgerAge,
app.config(),
app.getFeeTrack());
app.getFeeTrack(),
getAPIVersionNumber(params));

if (RPC::contains_error(txJsonResult))
return std::move(txJsonResult);
Expand Down Expand Up @@ -1068,7 +1073,8 @@ transactionSubmitMultiSigned(
true,
validatedLedgerAge,
app.config(),
app.getFeeTrack());
app.getFeeTrack(),
getAPIVersionNumber(jvRequest));

if (RPC::contains_error(txJsonResult))
return std::move(txJsonResult);
Expand Down
Loading

0 comments on commit 554aa89

Please sign in to comment.