From a2488820d5f82f699090487f4cfdb3108bd93d3e Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Tue, 1 Sep 2026 17:43:35 +0100 Subject: [PATCH] Assert on incorrect shortcut --- src/rpc/RPCHelpers.cpp | 10 +++++-- src/rpc/RPCHelpers.hpp | 12 +++++---- tests/unit/rpc/RPCHelpersTests.cpp | 43 +++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp index e3ccc9528..80227143d 100644 --- a/src/rpc/RPCHelpers.cpp +++ b/src/rpc/RPCHelpers.cpp @@ -569,8 +569,14 @@ getLedgerHeaderFromLedgerSpecifier( return *lgrInfo; } - // A shortcut means the latest validated ledger; see the declaration for why that holds - // for all three of them. + if (resolved.isShortcut()) { + auto const shortcut = std::get(resolved.value); + ASSERT( + shortcut == spec::LedgerShortcut::Validated, + "current/closed ledgers must be forwarded before dispatch" + ); + } + auto const ledgerSequence = resolved.isSequence() ? std::get(resolved.value) : maxSeq; // return without hitting the db diff --git a/src/rpc/RPCHelpers.hpp b/src/rpc/RPCHelpers.hpp index a9a77fa8f..823e45b1f 100644 --- a/src/rpc/RPCHelpers.hpp +++ b/src/rpc/RPCHelpers.hpp @@ -59,6 +59,7 @@ #include #include #include +#include #include #include #include @@ -308,11 +309,12 @@ getLedgerHeaderFromHashOrSeq( * Behaviour matches that overload: a hash or sequence beyond @p maxSeq, or one absent from * the backend, yields @c ledgerNotFound. * - * All three shortcuts resolve to @p maxSeq. Clio only serves validated data, so - * @c validated is the latest validated sequence by definition, and @c current / @c closed - * never arrive here — @ref specifiesCurrentOrClosedLedger forwards those upstream before - * dispatch. An unspecified ledger resolves via @c LedgerSpecifier::resolved(), which the - * spec library fixes to @c validated for Clio. + * 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 diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp index 48ca1b794..eb73aed08 100644 --- a/tests/unit/rpc/RPCHelpersTests.cpp +++ b/tests/unit/rpc/RPCHelpersTests.cpp @@ -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" @@ -2140,7 +2141,7 @@ TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierBySequenceBeyondMaxSeqSkipsBacke }); } -TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierShortcutUsesMaxSeq) +TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierValidatedUsesMaxSeq) { EXPECT_CALL(*backend_, fetchLedgerBySequence(kSpecifierRangeMax, _)) .WillOnce(Return(createLedgerHeader(kIndex1, kSpecifierRangeMax))); @@ -2157,6 +2158,46 @@ TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierShortcutUsesMaxSeq) }); } +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, + spec::LedgerSpecifier{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, + spec::LedgerSpecifier{spec::LedgerShortcut::Closed}, + kSpecifierRangeMax + ); + }, + "must be forwarded before dispatch" + ); + }); +} + TEST_F(RPCHelpersTest, LedgerHeaderFromSpecifierUnspecifiedResolvesToMaxSeq) { // an unspecified ledger resolves via LedgerSpecifier::resolved(), which the spec library