fix: Skip deleted book directories and non-root modifications in ValidBookDirectory invariant (#7312)

This commit is contained in:
Peter Chen
2026-05-24 16:37:16 -04:00
committed by GitHub
parent 30de556224
commit e34c2667d7
2 changed files with 77 additions and 5 deletions

View File

@@ -40,19 +40,27 @@ badExchangeRate(SLE const& dir)
void
ValidBookDirectory::visitEntry(
bool,
bool isDelete,
std::shared_ptr<SLE const> const& before,
std::shared_ptr<SLE const> const& after)
{
// New root directories must have matching exchange-rate metadata. New
// child directories must point to an existing root.
// child directories, and modified directories that change sfRootIndex, must
// point to an existing root.
// Only validate newly-created directories; LedgerStateFix handles legacy
// bad exchange-rate metadata.
if (badBookDirectory_ || before || !after || after->getType() != ltDIR_NODE)
// Only validate newly-created directories and sfRootIndex changes;
// LedgerStateFix handles legacy bad exchange-rate metadata. Skip deletions
// because `after` is not guaranteed to be null.
if (badBookDirectory_ || isDelete || !after || after->getType() != ltDIR_NODE)
return;
auto const rootIndex = after->getFieldH256(sfRootIndex);
// Ignore ordinary modifications that do not change which root this
// directory belongs to. That tolerates legacy bad exchange-rate metadata
// during normal operation while still checking sfRootIndex changes.
if (before && before->getFieldH256(sfRootIndex) == rootIndex)
return;
if (after->key() == rootIndex && !badBookDirectory_)
{
badBookDirectory_ = badBookDirectory_ || badExchangeRate(*after);

View File

@@ -2137,6 +2137,70 @@ class Invariants_test : public beast::unit_test::Suite
BEAST_EXPECT(
invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
}
// A bad root is rejected when added, ignored when a legacy bad root is
// modified without changing sfRootIndex or deleted, and checked when a
// modified directory changes sfRootIndex.
{
Env env{*this, defaultAmendments()};
Account const a1{"A1"};
env.fund(XRP(1000), a1);
env.close();
OpenView view{*env.current()};
auto const directoryQuality = STAmount::kURateOne;
auto const rootDir = getBookRootKey(a1, directoryQuality);
auto const missingRootDir = getBookRootKey(a1, directoryQuality + 1);
auto const badRoot = makeRootPage(rootDir, directoryQuality + 1);
view.rawInsert(badRoot);
test::StreamSink sink{beast::Severity::Warning};
beast::Journal const jlog{sink};
{
// add
ValidBookDirectory invariant;
invariant.visitEntry(false, nullptr, badRoot);
BEAST_EXPECT(
!invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
}
{
// modify (without changing the sfRootIndex)
ValidBookDirectory invariant;
invariant.visitEntry(false, badRoot, badRoot);
BEAST_EXPECT(
invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
}
{
// modify (changing sfRootIndex to a missing root)
auto const childBefore = makeChildPage(rootDir);
auto const childAfter = std::make_shared<SLE>(*childBefore, childBefore->key());
childAfter->setFieldH256(sfRootIndex, missingRootDir.key);
ValidBookDirectory invariant;
invariant.visitEntry(false, childBefore, childAfter);
test::StreamSink missingRootSink{beast::Severity::Warning};
beast::Journal const missingRootJlog{missingRootSink};
BEAST_EXPECT(!invariant.finalize(
makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, missingRootJlog));
BEAST_EXPECT(
missingRootSink.messages().str().find("book directory root missing") !=
std::string::npos);
}
{
// delete
view.rawErase(badRoot);
BEAST_EXPECT(!view.exists(rootDir));
ValidBookDirectory invariant;
invariant.visitEntry(true, badRoot, badRoot);
BEAST_EXPECT(
invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
}
}
}
Keylet