Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API versioning #3155

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,7 @@ else ()
src/test/rpc/TransactionEntry_test.cpp
src/test/rpc/TransactionHistory_test.cpp
src/test/rpc/ValidatorRPC_test.cpp
src/test/rpc/Version_test.cpp
#[===============================[
nounity, test sources:
subdir: server
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include <ripple/protocol/STParsedJSON.h>
#include <ripple/protocol/Protocol.h>
#include <ripple/resource/Fees.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <ripple/beast/asio/io_latency_probe.h>
#include <ripple/beast/core/LexicalCast.h>

Expand Down Expand Up @@ -1570,7 +1571,8 @@ bool ApplicationImp::setup()
Resource::Charge loadType = Resource::feeReferenceRPC;
Resource::Consumer c;
RPC::Context context { journal ("RPCHandler"), jvCommand, *this,
loadType, getOPs (), getLedgerMaster (), c, Role::ADMIN};
loadType, getOPs (), getLedgerMaster(), c, Role::ADMIN,
RPC::ApiMaximumSupportedVersion};

Json::Value jvResult;
RPC::doCommand (context, jvResult);
Expand Down
15 changes: 15 additions & 0 deletions src/ripple/net/impl/RPCCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <ripple/protocol/SystemParameters.h>
#include <ripple/protocol/UintTypes.h>
#include <ripple/rpc/ServerHandler.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <ripple/beast/core/LexicalCast.h>

#include <boost/algorithm/string/predicate.hpp>
Expand Down Expand Up @@ -1353,6 +1354,20 @@ rpcCmdLineToJson (std::vector<std::string> const& args,

jvRequest = rpParser.parseCommand (args[0], jvRpcParams, true);

auto insert_api_version = [](Json::Value & jr){
if( jr.isObject() &&
!jr.isMember(jss::error) &&
!jr.isMember(jss::api_version))
{
jr[jss::api_version] = RPC::ApiMaximumSupportedVersion;
}
};

if(jvRequest.isObject())
insert_api_version(jvRequest);
else if(jvRequest.isArray())
std::for_each(jvRequest.begin(), jvRequest.end(), insert_api_version);

JLOG (j.trace()) << "RPC Request: " << jvRequest << std::endl;
return jvRequest;
}
Expand Down
4 changes: 4 additions & 0 deletions src/ripple/protocol/jss.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ JSS ( alternatives ); // out: PathRequest, RipplePathFind
JSS ( amendment_blocked ); // out: NetworkOPs
JSS ( amendments ); // in: AccountObjects, out: NetworkOPs
JSS ( amount ); // out: AccountChannels
JSS ( api_version); // in: many, out: Version
JSS ( api_version_low); // out: Version
JSS ( asks ); // out: Subscribe
JSS ( assets ); // out: GatewayBalances
JSS ( authorized ); // out: AccountLines
Expand Down Expand Up @@ -261,6 +263,8 @@ JSS ( index ); // in: LedgerEntry, DownloadShard
// field
JSS ( info ); // out: ServerInfo, ConsensusInfo, FetchInfo
JSS ( internal_command ); // in: Internal
JSS ( invalid_API_version ); // out: Many, when a request has an invalid
// version
JSS ( io_latency_ms ); // out: NetworkOPs
JSS ( ip ); // in: Connect, out: OverlayImpl
JSS ( issuer ); // in: RipplePathFind, Subscribe,
Expand Down
1 change: 1 addition & 0 deletions src/ripple/rpc/Context.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ struct Context
LedgerMaster& ledgerMaster;
Resource::Consumer& consumer;
Role role;
unsigned int apiVersion;
std::shared_ptr<JobQueue::Coro> coro {};
InfoSub::pointer infoSub {};
Headers headers {};
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/RPCHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct Context;
/** Execute an RPC command and store the results in a Json::Value. */
Status doCommand (RPC::Context&, Json::Value&);

Role roleRequired (std::string const& method );
Role roleRequired (unsigned int version, std::string const& method );

} // RPC
} // ripple
Expand Down
64 changes: 42 additions & 22 deletions src/ripple/rpc/impl/Handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <ripple/rpc/impl/Handler.h>
#include <ripple/rpc/handlers/Handlers.h>
#include <ripple/rpc/handlers/Version.h>
#include <ripple/rpc/impl/RPCHelpers.h>

namespace ripple {
namespace RPC {
Expand Down Expand Up @@ -129,16 +130,21 @@ class HandlerTable {
explicit
HandlerTable (const Handler(&entries)[N])
{
for (std::size_t i = 0; i < N; ++i)
for(auto v = RPC::ApiMinimumSupportedVersion; v <= RPC::ApiMaximumSupportedVersion; ++v)
{
auto const& entry = entries[i];
assert (table_.find(entry.name_) == table_.end());
table_[entry.name_] = entry;
for (std::size_t i = 0; i < N; ++i)
{
auto & innerTable = table_[versionToIndex(v)];
auto const& entry = entries[i];
assert (innerTable.find(entry.name_) == innerTable.end());
innerTable[entry.name_] = entry;
}

// This is where the new-style handlers are added.
// This is also where different versions of handlers are added.
addHandler<LedgerHandler>(v);
addHandler<VersionHandler>(v);
}

// This is where the new-style handlers are added.
addHandler<LedgerHandler>();
addHandler<VersionHandler>();
}

public:
Expand All @@ -148,45 +154,59 @@ class HandlerTable {
return handlerTable;
}

Handler const* getHandler(std::string name) const
Handler const* getHandler(unsigned version, std::string name) const
{
auto i = table_.find(name);
return i == table_.end() ? nullptr : &i->second;
if(version > RPC::ApiMaximumSupportedVersion || version < RPC::ApiMinimumSupportedVersion)
return nullptr;
auto & innerTable = table_[versionToIndex(version)];
auto i = innerTable.find(name);
return i == innerTable.end() ? nullptr : &i->second;
}

std::vector<char const*>
getHandlerNames() const
{
std::vector<char const*> ret;
ret.reserve(table_.size());
for (auto const& i : table_)
ret.push_back(i.second.name_);
return ret;
std::unordered_set<char const*> name_set;
for ( int index = 0; index < table_.size(); ++index)
{
for(auto const& h : table_[index])
{
name_set.insert(h.second.name_);
}
}
return std::vector<char const*>(name_set.begin(), name_set.end());
}

private:
std::map<std::string, Handler> table_;
std::array<std::map<std::string, Handler>, APINumberVersionSupported> table_;

template <class HandlerImpl>
void addHandler()
void addHandler(unsigned version)
{
assert (table_.find(HandlerImpl::name()) == table_.end());
assert (version >= RPC::ApiMinimumSupportedVersion && version <= RPC::ApiMaximumSupportedVersion);
auto & innerTable = table_[versionToIndex(version)];
nbougalis marked this conversation as resolved.
Show resolved Hide resolved
assert (innerTable.find(HandlerImpl::name()) == innerTable.end());

Handler h;
h.name_ = HandlerImpl::name();
h.valueMethod_ = &handle<Json::Value, HandlerImpl>;
h.role_ = HandlerImpl::role();
h.condition_ = HandlerImpl::condition();

table_[HandlerImpl::name()] = h;
innerTable[HandlerImpl::name()] = h;
}

inline unsigned versionToIndex(unsigned version) const
{
return version - RPC::ApiMinimumSupportedVersion;
}
};

} // namespace

Handler const* getHandler(std::string const& name)
Handler const* getHandler(unsigned version, std::string const& name)
{
return HandlerTable::instance().getHandler(name);
return HandlerTable::instance().getHandler(version, name);
}

std::vector<char const*>
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/impl/Handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct Handler
RPC::Condition condition_;
};

Handler const* getHandler (std::string const&);
Handler const* getHandler (unsigned int version, std::string const&);

/** Return a Json::objectValue with a single entry. */
template <class Value>
Expand Down
6 changes: 3 additions & 3 deletions src/ripple/rpc/impl/RPCHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ error_code_i fillHandler (Context& context,

JLOG (context.j.trace()) << "COMMAND:" << strCommand;
JLOG (context.j.trace()) << "REQUEST:" << context.params;
auto handler = getHandler(strCommand);
auto handler = getHandler(context.apiVersion, strCommand);

if (!handler)
return rpcUNKNOWN_COMMAND;
Expand Down Expand Up @@ -296,9 +296,9 @@ Status doCommand (
return rpcUNKNOWN_COMMAND;
}

Role roleRequired (std::string const& method)
Role roleRequired (unsigned int version, std::string const& method)
{
auto handler = RPC::getHandler(method);
auto handler = RPC::getHandler(version, method);

if (!handler)
return Role::FORBID;
Expand Down
19 changes: 19 additions & 0 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,5 +716,24 @@ beast::SemanticVersion const firstVersion("1.0.0");
beast::SemanticVersion const goodVersion("1.0.0");
beast::SemanticVersion const lastVersion("1.0.0");

unsigned int getAPIVersionNumber(Json::Value const& jv)
{
static Json::Value const minVersion (RPC::ApiMinimumSupportedVersion);
static Json::Value const maxVersion (RPC::ApiMaximumSupportedVersion);
static Json::Value const invalidVersion (RPC::APIInvalidVersion);

Json::Value requestedVersion(RPC::APIVersionIfUnspecified);
if(jv.isObject())
{
requestedVersion = jv.get (jss::api_version, requestedVersion);
}
if( !(requestedVersion.isInt() || requestedVersion.isUInt()) ||
requestedVersion < minVersion || requestedVersion > maxVersion)
{
requestedVersion = invalidVersion;
}
return requestedVersion.asUInt();
}

} // RPC
} // ripple
46 changes: 46 additions & 0 deletions src/ripple/rpc/impl/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,42 @@ 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
*/
extern beast::SemanticVersion const firstVersion;
extern beast::SemanticVersion const goodVersion;
extern beast::SemanticVersion const lastVersion;

/**
* API version numbers used in later API versions
*
* Requests with a version number in the range
* [ApiMinimumSupportedVersion, ApiMaximumSupportedVersion]
* are supported.
*
* Network Requests without explicit version numbers use
* APIVersionIfUnspecified. APIVersionIfUnspecified is 1,
* because all the RPC requests with a version >= 2 must
* explicitly specify the version in the requests.
* Note that APIVersionIfUnspecified will be lower than
* ApiMinimumSupportedVersion when we stop supporting API
* version 1.
*
* Command line Requests use ApiMaximumSupportedVersion.
*/

constexpr unsigned int APIInvalidVersion = 0;
constexpr unsigned int APIVersionIfUnspecified = 1;
constexpr unsigned int ApiMinimumSupportedVersion = 1;
constexpr unsigned int ApiMaximumSupportedVersion = 1;
constexpr unsigned int APINumberVersionSupported = ApiMaximumSupportedVersion -
ApiMinimumSupportedVersion + 1;

static_assert (ApiMinimumSupportedVersion >= APIVersionIfUnspecified);
static_assert (ApiMaximumSupportedVersion >= ApiMinimumSupportedVersion);


nbougalis marked this conversation as resolved.
Show resolved Hide resolved
template <class Object>
void
setVersion(Object& parent)
Expand All @@ -131,6 +163,20 @@ setVersion(Object& parent)
std::pair<RPC::Status, LedgerEntryType>
chooseLedgerEntryType(Json::Value const& params);

/**
* Retrieve the api version number from the json value
*
* Note that APIInvalidVersion will be returned if
* 1) the version number field has a wrong format
* 2) the version number retrieved is out of the supported range
* 3) the version number is unspecified and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like case 3 should be covered by the static checks that @nbougalis suggested above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. But I think we need both the static check (checking we don't make a mistake in coding) and the check in the function (checking network requests are acceptable).

* APIVersionIfUnspecified is out of the supported range
*
* @param value a Json value that may or may not specifies
* the api version number
* @return the api version number
*/
unsigned int getAPIVersionNumber(const Json::Value & value);
} // RPC
} // ripple

Expand Down
Loading