APIv2: remove tx_history and ledger_header (#4759)

Remove `tx_history` and `ledger_header` methods from API version 2.

Update `RPC::Handler` to allow for methods (or method implementations)
to be API version specific. This partially resolves #4727. We can now
store multiple handlers with the same name, as long as they belong to
different (non-overlapping) API versions. This necessarily impacts the
handler lookup algorithm and its complexity; however, there is no
performance loss on x86_64 architecture, and only minimal performance
loss on arm64 (around 10ns). This design change gives us extra
flexibility evolving the API in the future, including other parts of
#4727.

In API version 2, `tx_history` and `ledger_header` are no longer
recognised; if they are called, `rippled` will return error
`unknownCmd`

Resolve #3638

Resolve #3539
This commit is contained in:
Bronek Kozicki
2023-10-24 23:57:49 +01:00
committed by GitHub
parent 3e5f770a38
commit 1eac4d2c07
14 changed files with 367 additions and 60 deletions

View File

@@ -134,6 +134,13 @@ Currently (prior to the release of 2.0), it is available as a "beta" version, me
Since `api_version` 2 is in "beta", breaking changes to V2 can currently be made at any time.
#### Removed methods
In API version 2, the following methods are no longer available:
- `tx_history` - Instead, use other methods such as `account_tx` or `ledger` with the `transactions` field set to `true`.
- `ledger_header` - Instead, use the `ledger` method.
#### Modifications to account_info response in V2
- `signer_lists` is returned in the root of the response. Previously, in API version 1, it was nested under `account_data`. (https://github.com/XRPLF/rippled/pull/3770)

View File

@@ -1062,6 +1062,7 @@ if (tests)
src/test/rpc/KeyGeneration_test.cpp
src/test/rpc/LedgerClosed_test.cpp
src/test/rpc/LedgerData_test.cpp
src/test/rpc/LedgerHeader_test.cpp
src/test/rpc/LedgerRPC_test.cpp
src/test/rpc/LedgerRequestRPC_test.cpp
src/test/rpc/ManifestRPC_test.cpp
@@ -1085,6 +1086,7 @@ if (tests)
src/test/rpc/ValidatorInfo_test.cpp
src/test/rpc/ValidatorRPC_test.cpp
src/test/rpc/Version_test.cpp
src/test/rpc/Handler_test.cpp
#[===============================[
test sources:
subdir: server

View File

@@ -43,7 +43,7 @@ namespace ripple {
namespace perf {
PerfLogImp::Counters::Counters(
std::vector<char const*> const& labels,
std::set<char const*> const& labels,
JobTypes const& jobTypes)
{
{

View File

@@ -113,9 +113,7 @@ class PerfLogImp : public PerfLog
std::unordered_map<std::uint64_t, MethodStart> methods_;
mutable std::mutex methodsMutex_;
Counters(
std::vector<char const*> const& labels,
JobTypes const& jobTypes);
Counters(std::set<char const*> const& labels, JobTypes const& jobTypes);
Json::Value
countersJson() const;
Json::Value

View File

@@ -30,6 +30,7 @@
#include <ripple/rpc/Role.h>
#include <ripple/rpc/Status.h>
#include <ripple/rpc/impl/Handler.h>
#include <ripple/rpc/impl/RPCHelpers.h>
namespace Json {
class Object;
@@ -58,23 +59,15 @@ public:
void
writeResult(Object&);
static char const*
name()
{
return "ledger";
}
static constexpr char name[] = "ledger";
static Role
role()
{
return Role::USER;
}
static constexpr unsigned minApiVer = RPC::apiMinimumSupportedVersion;
static Condition
condition()
{
return NO_CONDITION;
}
static constexpr unsigned maxApiVer = RPC::apiMaximumValidVersion;
static constexpr Role role = Role::USER;
static constexpr Condition condition = NO_CONDITION;
private:
JsonContext& context_;

View File

@@ -46,23 +46,15 @@ public:
setVersion(obj, apiVersion_, betaEnabled_);
}
static char const*
name()
{
return "version";
}
static constexpr char const* name = "version";
static Role
role()
{
return Role::USER;
}
static constexpr unsigned minApiVer = RPC::apiMinimumSupportedVersion;
static Condition
condition()
{
return NO_CONDITION;
}
static constexpr unsigned maxApiVer = RPC::apiMaximumValidVersion;
static constexpr Role role = Role::USER;
static constexpr Condition condition = NO_CONDITION;
private:
unsigned int apiVersion_;

View File

@@ -17,11 +17,14 @@
*/
//==============================================================================
#include <ripple/basics/contract.h>
#include <ripple/rpc/handlers/Handlers.h>
#include <ripple/rpc/handlers/Version.h>
#include <ripple/rpc/impl/Handler.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <map>
namespace ripple {
namespace RPC {
namespace {
@@ -47,6 +50,9 @@ template <class Object, class HandlerImpl>
Status
handle(JsonContext& context, Object& object)
{
assert(
context.apiVersion >= HandlerImpl::minApiVer &&
context.apiVersion <= HandlerImpl::maxApiVer);
HandlerImpl handler(context);
auto status = handler.check();
@@ -55,7 +61,20 @@ handle(JsonContext& context, Object& object)
else
handler.writeResult(object);
return status;
};
}
template <typename HandlerImpl>
Handler
handlerFrom()
{
return {
HandlerImpl::name,
&handle<Json::Value, HandlerImpl>,
HandlerImpl::role,
HandlerImpl::condition,
HandlerImpl::minApiVer,
HandlerImpl::maxApiVer};
}
Handler const handlerArray[]{
// Some handlers not specified here are added to the table via addHandler()
@@ -110,7 +129,7 @@ Handler const handlerArray[]{
NEEDS_CURRENT_LEDGER},
{"ledger_data", byRef(&doLedgerData), Role::USER, NO_CONDITION},
{"ledger_entry", byRef(&doLedgerEntry), Role::USER, NO_CONDITION},
{"ledger_header", byRef(&doLedgerHeader), Role::USER, NO_CONDITION},
{"ledger_header", byRef(&doLedgerHeader), Role::USER, NO_CONDITION, 1, 1},
{"ledger_request", byRef(&doLedgerRequest), Role::ADMIN, NO_CONDITION},
{"log_level", byRef(&doLogLevel), Role::ADMIN, NO_CONDITION},
{"logrotate", byRef(&doLogRotate), Role::ADMIN, NO_CONDITION},
@@ -156,7 +175,7 @@ Handler const handlerArray[]{
NEEDS_CURRENT_LEDGER},
{"transaction_entry", byRef(&doTransactionEntry), Role::USER, NO_CONDITION},
{"tx", byRef(&doTxJson), Role::USER, NEEDS_NETWORK_CONNECTION},
{"tx_history", byRef(&doTxHistory), Role::USER, NO_CONDITION},
{"tx_history", byRef(&doTxHistory), Role::USER, NO_CONDITION, 1, 1},
{"tx_reduce_relay", byRef(&doTxReduceRelay), Role::USER, NO_CONDITION},
{"unl_list", byRef(&doUnlList), Role::ADMIN, NO_CONDITION},
{"validation_create",
@@ -178,14 +197,42 @@ Handler const handlerArray[]{
class HandlerTable
{
private:
using handler_table_t = std::multimap<std::string, Handler>;
// Use with equal_range to enforce that API range of a newly added handler
// does not overlap with API range of an existing handler with same name
[[nodiscard]] bool
overlappingApiVersion(
std::pair<handler_table_t::iterator, handler_table_t::iterator> range,
unsigned minVer,
unsigned maxVer)
{
assert(minVer <= maxVer);
assert(maxVer <= RPC::apiMaximumValidVersion);
return std::any_of(
range.first,
range.second, //
[minVer, maxVer](auto const& item) {
return item.second.minApiVer_ <= maxVer &&
item.second.maxApiVer_ >= minVer;
});
}
template <std::size_t N>
explicit HandlerTable(const Handler (&entries)[N])
{
for (std::size_t i = 0; i < N; ++i)
for (auto const& entry : entries)
{
auto const& entry = entries[i];
assert(table_.find(entry.name_) == table_.end());
table_[entry.name_] = entry;
if (overlappingApiVersion(
table_.equal_range(entry.name_),
entry.minApiVer_,
entry.maxApiVer_))
LogicError(
std::string("Handler for ") + entry.name_ +
" overlaps with an existing handler");
table_.insert({entry.name_, entry});
}
// This is where the new-style handlers are added.
@@ -201,7 +248,7 @@ public:
return handlerTable;
}
Handler const*
[[nodiscard]] Handler const*
getHandler(unsigned version, bool betaEnabled, std::string const& name)
const
{
@@ -209,36 +256,48 @@ public:
version > (betaEnabled ? RPC::apiBetaVersion
: RPC::apiMaximumSupportedVersion))
return nullptr;
auto i = table_.find(name);
return i == table_.end() ? nullptr : &i->second;
auto const range = table_.equal_range(name);
auto const i = std::find_if(
range.first, range.second, [version](auto const& entry) {
return entry.second.minApiVer_ <= version &&
version <= entry.second.maxApiVer_;
});
return i == range.second ? nullptr : &i->second;
}
std::vector<char const*>
[[nodiscard]] std::set<char const*>
getHandlerNames() const
{
std::vector<char const*> ret;
ret.reserve(table_.size());
std::set<char const*> ret;
for (auto const& i : table_)
ret.push_back(i.second.name_);
ret.insert(i.second.name_);
return ret;
}
private:
std::map<std::string, Handler> table_;
handler_table_t table_;
template <class HandlerImpl>
void
addHandler()
{
assert(table_.find(HandlerImpl::name()) == table_.end());
static_assert(HandlerImpl::minApiVer <= HandlerImpl::maxApiVer);
static_assert(HandlerImpl::maxApiVer <= RPC::apiMaximumValidVersion);
static_assert(
RPC::apiMinimumSupportedVersion <= HandlerImpl::minApiVer);
Handler h;
h.name_ = HandlerImpl::name();
h.valueMethod_ = &handle<Json::Value, HandlerImpl>;
h.role_ = HandlerImpl::role();
h.condition_ = HandlerImpl::condition();
if (overlappingApiVersion(
table_.equal_range(HandlerImpl::name),
HandlerImpl::minApiVer,
HandlerImpl::maxApiVer))
LogicError(
std::string("Handler for ") + HandlerImpl::name +
" overlaps with an existing handler");
table_[HandlerImpl::name()] = h;
table_.insert({HandlerImpl::name, handlerFrom<HandlerImpl>()});
}
};
@@ -250,11 +309,11 @@ getHandler(unsigned version, bool betaEnabled, std::string const& name)
return HandlerTable::instance().getHandler(version, betaEnabled, name);
}
std::vector<char const*>
std::set<char const*>
getHandlerNames()
{
return HandlerTable::instance().getHandlerNames();
};
}
} // namespace RPC
} // namespace ripple

View File

@@ -25,6 +25,7 @@
#include <ripple/core/Config.h>
#include <ripple/rpc/RPCHandler.h>
#include <ripple/rpc/Status.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <ripple/rpc/impl/Tuning.h>
#include <vector>
@@ -52,6 +53,9 @@ struct Handler
Method<Json::Value> valueMethod_;
Role role_;
RPC::Condition condition_;
unsigned minApiVer_ = apiMinimumSupportedVersion;
unsigned maxApiVer_ = apiMaximumValidVersion;
};
Handler const*
@@ -70,7 +74,7 @@ makeObjectValue(
}
/** Return names of all methods. */
std::vector<char const*>
std::set<char const*>
getHandlerNames();
template <class T>

View File

@@ -242,10 +242,12 @@ constexpr unsigned int apiVersionIfUnspecified = 1;
constexpr unsigned int apiMinimumSupportedVersion = 1;
constexpr unsigned int apiMaximumSupportedVersion = 1;
constexpr unsigned int apiBetaVersion = 2;
constexpr unsigned int apiMaximumValidVersion = apiBetaVersion;
static_assert(apiMinimumSupportedVersion >= apiVersionIfUnspecified);
static_assert(apiMaximumSupportedVersion >= apiMinimumSupportedVersion);
static_assert(apiBetaVersion >= apiMaximumSupportedVersion);
static_assert(apiMaximumValidVersion >= apiMaximumSupportedVersion);
template <class Object>
void

View File

@@ -25,6 +25,7 @@
#include <ripple/protocol/jss.h>
#include <ripple/rpc/impl/Handler.h>
#include <test/jtx/Env.h>
#include <test/jtx/TestHelpers.h>
#include <atomic>
#include <chrono>
@@ -309,7 +310,8 @@ public:
// Get the all the labels we can use for RPC interfaces without
// causing an assert.
std::vector<char const*> labels{ripple::RPC::getHandlerNames()};
std::vector<char const*> labels =
test::jtx::make_vector(ripple::RPC::getHandlerNames());
std::shuffle(labels.begin(), labels.end(), default_prng());
// Get two IDs to associate with each label. Errors tend to happen at

View File

@@ -27,10 +27,19 @@
#include <ripple/protocol/jss.h>
#include <test/jtx/Env.h>
#include <ranges>
namespace ripple {
namespace test {
namespace jtx {
// Helper to make vector from iterable
auto
make_vector(auto const& input) requires std::ranges::range<decltype(input)>
{
return std::vector(std::ranges::begin(input), std::ranges::end(input));
}
// Functions used in debugging
Json::Value
getAccountOffers(Env& env, AccountID const& acct, bool current = false);

View File

@@ -0,0 +1,132 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <ripple/beast/unit_test.h>
#include <ripple/rpc/impl/Handler.h>
#include <test/jtx.h>
#include <chrono>
#include <iostream>
#include <limits>
#include <numeric>
#include <random>
namespace ripple::test {
// NOTE: there should be no need for this function;
// `std::cout << some_duration` should just work if built with a compliant
// C++20 compiler. Sadly, we are not using one, as of today
// TODO: remove this operator<< overload when we bump compiler version
std::ostream&
operator<<(std::ostream& os, std::chrono::nanoseconds ns)
{
return (os << ns.count() << "ns");
}
// NOTE This is a rather naive effort at a microbenchmark. Ideally we want
// Google Benchmark, or something similar. Also, this actually does not belong
// to unit tests, as it makes little sense to run it in conditions very
// dissimilar to how rippled will normally work.
// TODO as https://github.com/XRPLF/rippled/issues/4765
class Handler_test : public beast::unit_test::suite
{
auto
time(std::size_t n, auto f, auto prng) -> auto
{
using clock = std::chrono::steady_clock;
assert(n > 0);
double sum = 0;
double sum_squared = 0;
std::size_t j = 0;
while (j < n)
{
// Generate 100 inputs upfront, separated from the inner loop
std::array<decltype(prng()), 100> inputs = {};
for (auto& i : inputs)
{
i = prng();
}
// Take 100 samples, then sort and throw away 35 from each end,
// using only middle 30. This helps to reduce measurement noise.
std::array<long, 100> samples = {};
for (std::size_t k = 0; k < 100; ++k)
{
auto start = std::chrono::steady_clock::now();
f(inputs[k]);
samples[k] = (std::chrono::steady_clock::now() - start).count();
}
std::sort(samples.begin(), samples.end());
for (std::size_t k = 35; k < 65; ++k)
{
j += 1;
sum += samples[k];
sum_squared += (samples[k] * samples[k]);
}
}
const double mean_squared = (sum * sum) / (j * j);
return std::make_tuple(
clock::duration{static_cast<long>(sum / j)},
clock::duration{
static_cast<long>(std::sqrt((sum_squared / j) - mean_squared))},
j);
}
void
reportLookupPerformance()
{
testcase("Handler lookup performance");
std::random_device dev;
std::ranlux48 prng(dev());
std::vector<const char*> names =
test::jtx::make_vector(ripple::RPC::getHandlerNames());
std::uniform_int_distribution<std::size_t> distr{0, names.size() - 1};
std::size_t dummy = 0;
auto const [mean, stdev, n] = time(
1'000'000,
[&](std::size_t i) {
auto const d = RPC::getHandler(1, false, names[i]);
dummy = dummy + i + (int)d->role_;
},
[&]() -> std::size_t { return distr(prng); });
std::cout << "mean=" << mean << " stdev=" << stdev << " N=" << n
<< '\n';
BEAST_EXPECT(dummy != 0);
}
public:
void
run() override
{
reportLookupPerformance();
}
};
BEAST_DEFINE_TESTSUITE_MANUAL(Handler, test, ripple);
} // namespace ripple::test

View File

@@ -0,0 +1,91 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <ripple/protocol/jss.h>
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
namespace ripple {
class LedgerHeader_test : public beast::unit_test::suite
{
void
testSimpleCurrent()
{
testcase("Current ledger");
using namespace test::jtx;
Env env{*this, envconfig(no_admin)};
Json::Value params{Json::objectValue};
params[jss::api_version] = 1;
params[jss::ledger_index] = "current";
auto const result =
env.client().invoke("ledger_header", params)[jss::result];
BEAST_EXPECT(result[jss::status] == "success");
BEAST_EXPECT(result.isMember("ledger"));
BEAST_EXPECT(result[jss::ledger][jss::closed] == false);
BEAST_EXPECT(result[jss::validated] == false);
}
void
testSimpleValidated()
{
testcase("Validated ledger");
using namespace test::jtx;
Env env{*this, envconfig(no_admin)};
Json::Value params{Json::objectValue};
params[jss::api_version] = 1;
params[jss::ledger_index] = "validated";
auto const result =
env.client().invoke("ledger_header", params)[jss::result];
BEAST_EXPECT(result[jss::status] == "success");
BEAST_EXPECT(result.isMember("ledger"));
BEAST_EXPECT(result[jss::ledger][jss::closed] == true);
BEAST_EXPECT(result[jss::validated] == true);
}
void
testCommandRetired()
{
testcase("Command retired from API v2");
using namespace test::jtx;
Env env{*this, envconfig(no_admin)};
Json::Value params{Json::objectValue};
params[jss::api_version] = 2;
auto const result =
env.client().invoke("ledger_header", params)[jss::result];
BEAST_EXPECT(result[jss::error] == "unknownCmd");
BEAST_EXPECT(result[jss::status] == "error");
}
public:
void
run() override
{
testSimpleCurrent();
testSimpleValidated();
testCommandRetired();
}
};
BEAST_DEFINE_TESTSUITE(LedgerHeader, rpc, ripple);
} // namespace ripple

View File

@@ -54,6 +54,21 @@ class TransactionHistory_test : public beast::unit_test::suite
}
}
void
testCommandRetired()
{
testcase("Command retired from API v2");
using namespace test::jtx;
Env env{*this, envconfig(no_admin)};
Json::Value params{Json::objectValue};
params[jss::api_version] = 2;
auto const result =
env.client().invoke("tx_history", params)[jss::result];
BEAST_EXPECT(result[jss::error] == "unknownCmd");
BEAST_EXPECT(result[jss::status] == "error");
}
void
testRequest()
{
@@ -148,6 +163,7 @@ public:
{
testBadInput();
testRequest();
testCommandRetired();
}
};