refactor: Dual path for rpc handlers (#3198)

This commit is contained in:
Alex Kremer
2026-09-09 14:57:01 +01:00
committed by GitHub
parent 08255c4f9e
commit 81efb226ac
9 changed files with 465 additions and 17 deletions

View File

@@ -27,6 +27,7 @@
#include <boost/lexical_cast/bad_lexical_cast.hpp>
#include <fmt/format.h>
#include <rpcspec/Errors.hpp>
#include <rpcspec/Ledger.hpp>
#include <xrpl/basics/Slice.h>
#include <xrpl/basics/StringUtilities.h>
#include <xrpl/basics/base_uint.h>
@@ -547,6 +548,47 @@ getLedgerHeaderFromHashOrSeq(
return *lgrInfo;
}
std::expected<xrpl::LedgerHeader, Status>
getLedgerHeaderFromLedgerSpecifier(
BackendInterface const& backend,
boost::asio::yield_context yield,
rpc::spec::LedgerSpecifier const& ledger,
uint32_t maxSeq
)
{
auto const err = std::unexpected{Status{RippledError::RpcLgrNotFound, "ledgerNotFound"}};
auto const resolved = ledger.resolved();
if (resolved.isHash()) {
auto const maybeLgrInfo =
backend.fetchLedgerByHash(std::get<xrpl::uint256>(resolved.value), yield);
if (not maybeLgrInfo.has_value() or maybeLgrInfo->seq > maxSeq)
return err;
return *maybeLgrInfo;
}
if (resolved.isShortcut()) {
auto const shortcut = std::get<rpc::spec::LedgerShortcut>(resolved.value);
ASSERT(
shortcut == rpc::spec::LedgerShortcut::Validated,
"current/closed ledgers must be forwarded before dispatch"
);
}
auto const ledgerSequence = resolved.isSequence() ? std::get<uint32_t>(resolved.value) : maxSeq;
// return without hitting the db
if (ledgerSequence > maxSeq)
return err;
auto const maybeLgrInfo = backend.fetchLedgerBySequence(ledgerSequence, yield);
if (not maybeLgrInfo.has_value())
return err;
return *maybeLgrInfo;
}
std::vector<unsigned char>
ledgerHeaderToBlob(xrpl::LedgerHeader const& info, bool includeHash)
{

View File

@@ -22,6 +22,7 @@
#include <boost/regex.hpp>
#include <boost/regex/v5/regex_match.hpp>
#include <fmt/format.h>
#include <rpcspec/Ledger.hpp>
#include <xrpl/basics/Number.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/json/json_value.h>
@@ -58,6 +59,7 @@
#include <memory>
#include <optional>
#include <string>
#include <string_view>
#include <tuple>
#include <utility>
#include <vector>
@@ -299,6 +301,35 @@ getLedgerHeaderFromHashOrSeq(
uint32_t maxSeq
);
/**
* @brief Get ledger header from a spec-library ledger specifier.
*
* The strong-typed counterpart of @ref getLedgerHeaderFromHashOrSeq, for handlers whose
* spec produces a @c LedgerSpecifier instead of a ledger_hash / ledger_index pair.
* Behaviour matches that overload: a hash or sequence beyond @p maxSeq, or one absent from
* the backend, yields @c ledgerNotFound.
*
* A @c validated shortcut resolves to @p maxSeq, which is what it means for a server that
* only serves validated data. An unspecified ledger resolves via
* @c LedgerSpecifier::resolved(), which the spec library fixes to @c validated for Clio.
*
* @c current and @c closed cannot reach here: @ref specifiesCurrentOrClosedLedger forwards
* those upstream before dispatch.
*
* @param backend The backend to use
* @param yield The coroutine context
* @param ledger The ledger the request selected
* @param maxSeq The maximum sequence to search
* @return The ledger header or an error status
*/
std::expected<xrpl::LedgerHeader, Status>
getLedgerHeaderFromLedgerSpecifier(
BackendInterface const& backend,
boost::asio::yield_context yield,
rpc::spec::LedgerSpecifier const& ledger,
uint32_t maxSeq
);
/**
* @brief Traverse nodes owned by an account
*

View File

@@ -7,8 +7,12 @@
#include <boost/json/value.hpp>
#include <boost/json/value_from.hpp>
#include <boost/json/value_to.hpp>
#include <rpcspec/Errors.hpp>
#include <rpcspec/RpcSpecView.hpp>
#include <concepts>
#include <cstdint>
#include <expected>
#include <optional>
#include <string>
@@ -71,17 +75,46 @@ concept SomeHandlerWithInput = requires(T a, uint32_t version) {
{ a.spec(version) } -> std::same_as<RpcSpec const&>;
} and SomeContextProcessWithInput<T> and boost::json::has_value_to<typename T::Input>::value;
/**
* @brief Specifies what a Handler validated by the shared consteval spec must provide.
*
* Such a handler inherits @c rpc::spec::HandlerFor<Input> from the spec library, which
* supplies a static @c parseInput (validate and deserialise in one pass) and a static
* @c spec returning a type-erased @c RpcSpecView. Presence of @c parseInput is what
* selects this path over @c SomeHandlerWithInput.
*
* The two input paths are mutually exclusive by construction: a legacy handler returns
* @c RpcSpec @c const& from a non-static @c spec and needs a @c value_to for its Input,
* neither of which holds here. @c kIsSingleInputPath asserts that below.
*/
template <typename T>
concept SomeHandlerWithTypedInput = requires(uint32_t version, boost::json::value jv) {
typename T::Input;
{ T::parseInput(jv, version) } -> std::same_as<std::expected<typename T::Input, Status>>;
{ T::spec(version) } -> std::same_as<rpc::spec::RpcSpecView>;
} and SomeContextProcessWithInput<T>;
/**
* @brief Specifies what a Handler without Input must provide.
*/
template <typename T>
concept SomeHandlerWithoutInput = SomeContextProcessWithoutInput<T>;
/**
* @brief True when @p T does not straddle the legacy and typed input paths.
*
* Guards the @c if @c constexpr chain in @c DefaultProcessor - were a handler to satisfy
* both, the dispatch order alone would silently decide which spec ran.
*/
template <typename T>
constexpr bool kIsSingleInputPath = not(SomeHandlerWithInput<T> and SomeHandlerWithTypedInput<T>);
/**
* @brief Specifies what a Handler type must provide.
*/
template <typename T>
concept SomeHandler = (SomeHandlerWithInput<T> or SomeHandlerWithoutInput<T>) and
concept SomeHandler =
(SomeHandlerWithInput<T> or SomeHandlerWithTypedInput<T> or SomeHandlerWithoutInput<T>) and
boost::json::has_value_from<typename T::Output>::value;
} // namespace rpc

View File

@@ -2,9 +2,11 @@
#include "rpc/common/Concepts.hpp"
#include "rpc/common/Types.hpp"
#include "util/UnsupportedType.hpp"
#include <boost/json/value.hpp>
#include <rpcspec/WarningsToJson.hpp>
#include <utility>
namespace rpc::impl {
@@ -19,37 +21,61 @@ struct DefaultProcessor final {
{
using boost::json::value_from;
using boost::json::value_to;
if constexpr (SomeHandlerWithInput<HandlerType>) {
// first we run validation against specified API version
static_assert(
kIsSingleInputPath<HandlerType>,
"handler satisfies both the legacy and the typed input path; dispatch would be "
"decided by the order of the branches below rather than by the handler"
);
static_assert(
SomeHandlerWithTypedInput<HandlerType> or SomeHandlerWithInput<HandlerType> or
SomeHandlerWithoutInput<HandlerType>,
"handler matches none of the branches below"
);
// New `rpc-spec`-based handler
if constexpr (SomeHandlerWithTypedInput<HandlerType>) {
auto input = HandlerType::parseInput(value, ctx.apiVersion);
auto warnings = rpc::spec::toJsonArray(HandlerType::spec(ctx.apiVersion).check(value));
if (not input.has_value())
return ReturnType{Error{std::move(input).error()}, std::move(warnings)};
auto ret = handler.process(*input, ctx);
if (not ret.has_value())
return ReturnType{Error{std::move(ret).error()}, std::move(warnings)};
return ReturnType{value_from(std::move(ret).value()), std::move(warnings)};
}
if constexpr (SomeHandlerWithInput<HandlerType>) {
// Old spec-based handler: first we run validation against specified API version
// TODO: This will be eventually removed once fully migraded to new rpc-spec system.
auto const spec = handler.spec(ctx.apiVersion);
auto warnings = spec.check(value);
auto input = value; // copy here, spec require mutable data
if (auto const ret = spec.process(input); not ret)
if (auto const ret = spec.process(input); not ret.has_value())
return ReturnType{Error{ret.error()}, std::move(warnings)}; // forward Status
auto const inData = value_to<typename HandlerType::Input>(input);
auto ret = handler.process(inData, ctx);
// real handler is given expected Input, not json
if (!ret) {
return ReturnType{
Error{std::move(ret).error()}, std::move(warnings)
}; // forward Status
}
if (not ret.has_value())
return ReturnType{Error{std::move(ret).error()}, std::move(warnings)};
return ReturnType{value_from(std::move(ret).value()), std::move(warnings)};
} else if constexpr (SomeHandlerWithoutInput<HandlerType>) {
}
if constexpr (SomeHandlerWithoutInput<HandlerType>) {
// no input to pass, ignore the value
auto const ret = handler.process(ctx);
if (not ret) {
if (not ret.has_value())
return ReturnType{Error{ret.error()}}; // forward Status
}
return ReturnType{value_from(ret.value())};
} else {
// when concept SomeHandlerWithInput and SomeHandlerWithoutInput not cover all Handler
// case
static_assert(util::Unsupported<HandlerType>);
}
}
};

View File

@@ -3,6 +3,7 @@ add_library(clio_testing_common)
target_sources(
clio_testing_common
PRIVATE
rpc/FakesAndMocks.cpp
util/AssignRandomPort.cpp
util/BinaryTestObject.cpp
util/CallWithTimeout.cpp

View File

@@ -0,0 +1,6 @@
#include "rpc/FakesAndMocks.hpp"
#include <rpcspec/HandlerFor.hpp>
#include <rpcspec/HandlerForDefs.hpp> // IWYU pragma: keep
template struct rpc::spec::HandlerFor<tests::common::typed_fake::TypedInput>;

View File

@@ -10,6 +10,13 @@
#include <boost/json/value_from.hpp>
#include <boost/json/value_to.hpp>
#include <gmock/gmock.h>
#include <rpcspec/Aliases.hpp>
#include <rpcspec/Converters.hpp>
#include <rpcspec/Errors.hpp>
#include <rpcspec/FieldSpec.hpp>
#include <rpcspec/HandlerFor.hpp>
#include <rpcspec/Typed.hpp>
#include <rpcspec/VersionedSpec.hpp>
#include <cstdint>
#include <optional>
@@ -153,4 +160,54 @@ struct HandlerWithoutInputMock {
MOCK_METHOD(Result, process, (rpc::Context const&), (const));
};
// The shared consteval spec resolves a handler's spec from its Input type via an ADL
// `specFor` hook, so the fake Input below needs its own namespace to host that hook.
namespace typed_fake {
// input data for TypedHandlerFake; mirrors TestInput so the two paths stay comparable
struct TypedInput {
std::string hello;
std::optional<uint32_t> limit;
};
inline constexpr auto kInputSpec = rpc::spec::spec<TypedInput>(
rpc::spec::field("hello", &TypedInput::hello, rpc::spec::required, rpc::spec::asString),
rpc::spec::field("limit", &TypedInput::limit, rpc::spec::asUint32),
rpc::spec::field("old_field", rpc::spec::deprecated)
);
inline constexpr auto kSpec = rpc::spec::versioned<TypedInput>(kInputSpec);
[[nodiscard]] constexpr auto const&
specFor(TypedInput const*) noexcept
{
return kSpec;
}
} // namespace typed_fake
class TypedHandlerFake : public rpc::spec::HandlerFor<typed_fake::TypedInput> {
public:
using Output = TestOutput;
using Result = rpc::HandlerReturnType<Output>;
static Result
process(Input const& input, [[maybe_unused]] rpc::Context const& ctx)
{
return Output{input.hello + '_' + std::to_string(input.limit.value_or(0))};
}
};
class FailingTypedHandlerFake : public rpc::spec::HandlerFor<typed_fake::TypedInput> {
public:
using Output = TestOutput;
using Result = rpc::HandlerReturnType<Output>;
static Result
process([[maybe_unused]] Input const& input, [[maybe_unused]] rpc::Context const& ctx)
{
return rpc::Error{rpc::Status{"Very custom error"}};
}
};
} // namespace tests::common

View File

@@ -7,6 +7,7 @@
#include "util/AsioContextTestFixture.hpp"
#include "util/LoggerFixtures.hpp"
#include "util/MockAmendmentCenter.hpp"
#include "util/MockAssert.hpp"
#include "util/MockBackendTestFixture.hpp"
#include "util/MockPrometheus.hpp"
#include "util/NameGenerator.hpp"
@@ -26,6 +27,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <rpcspec/Errors.hpp>
#include <rpcspec/Ledger.hpp>
#include <xrpl/basics/Blob.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/AccountID.h>
@@ -2058,3 +2060,153 @@ INSTANTIATE_TEST_SUITE_P(
),
tests::util::kNameGenerator
);
namespace {
constexpr auto kSpecifierRangeMax = 300u;
} // namespace
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierByHash)
{
auto const expected = createLedgerHeader(kIndex1, 30);
EXPECT_CALL(*backend_, fetchLedgerByHash(xrpl::uint256{kIndex1}, _)).WillOnce(Return(expected));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_, yield, rpc::spec::LedgerSpecifier{xrpl::uint256{kIndex1}}, kSpecifierRangeMax
);
ASSERT_TRUE(res.has_value());
EXPECT_EQ(res->seq, 30);
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierByHashNotFound)
{
EXPECT_CALL(*backend_, fetchLedgerByHash(xrpl::uint256{kIndex1}, _))
.WillOnce(Return(std::nullopt));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_, yield, rpc::spec::LedgerSpecifier{xrpl::uint256{kIndex1}}, kSpecifierRangeMax
);
ASSERT_FALSE(res.has_value());
EXPECT_EQ(res.error().message, "ledgerNotFound");
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierByHashBeyondMaxSeq)
{
// present in the backend, but newer than the range the caller may serve
EXPECT_CALL(*backend_, fetchLedgerByHash(xrpl::uint256{kIndex1}, _))
.WillOnce(Return(createLedgerHeader(kIndex1, kSpecifierRangeMax + 1)));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_, yield, rpc::spec::LedgerSpecifier{xrpl::uint256{kIndex1}}, kSpecifierRangeMax
);
ASSERT_FALSE(res.has_value());
EXPECT_EQ(res.error().message, "ledgerNotFound");
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierBySequence)
{
EXPECT_CALL(*backend_, fetchLedgerBySequence(30, _))
.WillOnce(Return(createLedgerHeader(kIndex1, 30)));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_, yield, rpc::spec::LedgerSpecifier{uint32_t{30}}, kSpecifierRangeMax
);
ASSERT_TRUE(res.has_value());
EXPECT_EQ(res->seq, 30);
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierBySequenceBeyondMaxSeqSkipsBackend)
{
EXPECT_CALL(*backend_, fetchLedgerBySequence).Times(0);
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_,
yield,
rpc::spec::LedgerSpecifier{uint32_t{kSpecifierRangeMax + 1}},
kSpecifierRangeMax
);
ASSERT_FALSE(res.has_value());
EXPECT_EQ(res.error().message, "ledgerNotFound");
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierValidatedUsesMaxSeq)
{
EXPECT_CALL(*backend_, fetchLedgerBySequence(kSpecifierRangeMax, _))
.WillOnce(Return(createLedgerHeader(kIndex1, kSpecifierRangeMax)));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_,
yield,
rpc::spec::LedgerSpecifier{rpc::spec::LedgerShortcut::Validated},
kSpecifierRangeMax
);
ASSERT_TRUE(res.has_value());
EXPECT_EQ(res->seq, kSpecifierRangeMax);
});
}
struct RPCHelpersAssertTest : RPCHelpersTest, common::util::WithMockAssert {};
TEST_F(RPCHelpersAssertTest, LedgerHeaderFromSpecifierCurrentAsserts)
{
EXPECT_CALL(*backend_, fetchLedgerBySequence).Times(0);
runSpawn([&, this](auto yield) {
EXPECT_CLIO_ASSERT_FAIL_WITH_MESSAGE(
{
[[maybe_unused]] auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_,
yield,
rpc::spec::LedgerSpecifier{rpc::spec::LedgerShortcut::Current},
kSpecifierRangeMax
);
},
"must be forwarded before dispatch"
);
});
}
TEST_F(RPCHelpersAssertTest, LedgerHeaderFromSpecifierClosedAsserts)
{
EXPECT_CALL(*backend_, fetchLedgerBySequence).Times(0);
runSpawn([&, this](auto yield) {
EXPECT_CLIO_ASSERT_FAIL_WITH_MESSAGE(
{
[[maybe_unused]] auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_,
yield,
rpc::spec::LedgerSpecifier{rpc::spec::LedgerShortcut::Closed},
kSpecifierRangeMax
);
},
"must be forwarded before dispatch"
);
});
}
TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierUnspecifiedResolvesToMaxSeq)
{
// an unspecified ledger resolves via LedgerSpecifier::resolved(), which the spec library
// fixes to `validated` under RPCSPEC_IS_CLIO
EXPECT_CALL(*backend_, fetchLedgerBySequence(kSpecifierRangeMax, _))
.WillOnce(Return(createLedgerHeader(kIndex1, kSpecifierRangeMax)));
runSpawn([&, this](auto yield) {
auto const res = getLedgerHeaderFromLedgerSpecifier(
*backend_, yield, rpc::spec::LedgerSpecifier{}, kSpecifierRangeMax
);
ASSERT_TRUE(res.has_value());
EXPECT_EQ(res->seq, kSpecifierRangeMax);
});
}

View File

@@ -1,4 +1,6 @@
#include "rpc/Errors.hpp"
#include "rpc/FakesAndMocks.hpp"
#include "rpc/common/Concepts.hpp"
#include "rpc/common/Specs.hpp"
#include "rpc/common/Types.hpp"
#include "rpc/common/Validators.hpp"
@@ -67,3 +69,101 @@ TEST_F(RPCDefaultProcessorTest, InvalidInput)
EXPECT_TRUE(ret.warnings.empty());
});
}
static_assert(SomeHandlerWithTypedInput<TypedHandlerFake>);
static_assert(not SomeHandlerWithInput<TypedHandlerFake>);
static_assert(SomeHandlerWithTypedInput<FailingTypedHandlerFake>);
static_assert(SomeHandlerWithInput<HandlerMock>);
static_assert(not SomeHandlerWithTypedInput<HandlerMock>);
// The four tests below exercise the typed path — a handler whose spec, validation and
// deserialization all come from the shared consteval spec via HandlerFor<Input>. They run
// against the same DefaultProcessor as the legacy tests above, which is the point: the
// dual path is a dispatch detail, not a second processor.
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_HappyPath)
{
runSpawn([](auto yield) {
TypedHandlerFake const handler;
rpc::impl::DefaultProcessor<TypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "hello": "world", "limit": 42 })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_TRUE(ret);
EXPECT_TRUE(ret.warnings.empty());
EXPECT_EQ(ret.result.value().at("computed").as_string(), "world_42");
});
}
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_MissingRequiredField_ReturnsError)
{
runSpawn([](auto yield) {
TypedHandlerFake const handler;
rpc::impl::DefaultProcessor<TypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "limit": 42 })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_FALSE(ret);
EXPECT_TRUE(ret.warnings.empty());
});
}
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_DeprecatedField_WarningsForwarded)
{
runSpawn([](auto yield) {
TypedHandlerFake const handler;
rpc::impl::DefaultProcessor<TypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "hello": "world", "old_field": true })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_TRUE(ret);
EXPECT_EQ(ret.warnings.size(), 1);
});
}
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_HandlerReturnsError_ForwardsError)
{
runSpawn([](auto yield) {
FailingTypedHandlerFake const handler;
rpc::impl::DefaultProcessor<FailingTypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "hello": "world", "limit": 42 })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_FALSE(ret);
EXPECT_EQ(rpc::makeError(ret.result.error()).at("error").as_string(), "Very custom error");
EXPECT_TRUE(ret.warnings.empty());
});
}
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_HandlerReturnsError_StillForwardsWarnings)
{
runSpawn([](auto yield) {
FailingTypedHandlerFake const handler;
rpc::impl::DefaultProcessor<FailingTypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "hello": "world", "old_field": true })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_FALSE(ret);
EXPECT_EQ(rpc::makeError(ret.result.error()).at("error").as_string(), "Very custom error");
EXPECT_EQ(ret.warnings.size(), 1);
});
}
TEST_F(RPCDefaultProcessorTest, NewSpecHandler_DeprecatedFieldAbsent_NoWarnings)
{
runSpawn([](auto yield) {
TypedHandlerFake const handler;
rpc::impl::DefaultProcessor<TypedHandlerFake> const processor;
auto const input = boost::json::parse(R"JSON({ "hello": "world" })JSON");
auto const ret = processor(handler, input, Context{yield});
ASSERT_TRUE(ret);
EXPECT_TRUE(ret.warnings.empty());
});
}