From 8f6c14f8564afb4b10e86f4178ad75b9d0f20664 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Mon, 31 Aug 2026 16:42:54 +0700 Subject: [PATCH] Bound recoverable validator manifests --- src/test/app/Manifest_test.cpp | 83 ++++++++++++ src/test/app/SetHookTSH_test.cpp | 43 +++++-- src/xrpld/app/misc/Manifest.h | 54 ++++++-- src/xrpld/app/misc/detail/Manifest.cpp | 154 ++++++++++++++++++++--- src/xrpld/app/rdb/Wallet.h | 2 + src/xrpld/app/rdb/detail/Wallet.cpp | 3 +- src/xrpld/overlay/detail/OverlayImpl.cpp | 6 +- 7 files changed, 307 insertions(+), 38 deletions(-) diff --git a/src/test/app/Manifest_test.cpp b/src/test/app/Manifest_test.cpp index 9a1cd39ea9..8285d86846 100644 --- a/src/test/app/Manifest_test.cpp +++ b/src/test/app/Manifest_test.cpp @@ -1106,6 +1106,89 @@ public: ManifestDisposition::badMasterKey); } + { + testcase("bounded evictable retention"); + + ManifestCache bounded{ + beast::Journal(beast::Journal::getNullSink()), 2}; + + struct TestManifest + { + Manifest manifest; + PublicKey signingKey; + }; + auto make = [this]() { + auto const master = randomSecretKey(); + auto const signing = randomKeyPair(KeyType::secp256k1); + return TestManifest{ + makeManifest( + master, + KeyType::ed25519, + signing.second, + KeyType::secp256k1, + 0), + signing.first}; + }; + + auto a = make(); + auto b = make(); + auto c = make(); + auto protectedManifest = make(); + + BEAST_EXPECT( + bounded.applyManifest( + clone(a.manifest), ManifestRetention::evictable) == + ManifestDisposition::accepted); + BEAST_EXPECT( + bounded.applyManifest( + clone(b.manifest), ManifestRetention::evictable) == + ManifestDisposition::accepted); + + // Touch a after b, then admit c. The complete b row must go: + // master bytes, reverse signing-key mapping, and recency entry. + BEAST_EXPECT(bounded.getRawManifest(a.manifest.masterKey)); + BEAST_EXPECT( + bounded.applyManifest( + clone(c.manifest), ManifestRetention::evictable) == + ManifestDisposition::accepted); + BEAST_EXPECT(!bounded.getManifestSnapshot(b.manifest.masterKey)); + BEAST_EXPECT(bounded.getMasterKey(b.signingKey) == b.signingKey); + + // Protected rows do not consume residue capacity and survive + // churn. pin() is the sole policy boundary that can demote them. + BEAST_EXPECT( + bounded.applyManifest(clone(protectedManifest.manifest)) == + ManifestDisposition::accepted); + bounded.pin({protectedManifest.manifest.masterKey}); + + auto d = make(); + BEAST_EXPECT( + bounded.applyManifest( + clone(d.manifest), ManifestRetention::evictable) == + ManifestDisposition::accepted); + BEAST_EXPECT(bounded.getManifestSnapshot( + protectedManifest.manifest.masterKey)); + + bounded.pin({}); + std::size_t retained = 0; + bounded.for_each_manifest( + [&](std::size_t n) { retained = n; }, [](Manifest const&) {}); + BEAST_EXPECT(retained == 2); + BEAST_EXPECT(bounded.getManifestSnapshot( + protectedManifest.manifest.masterKey)); + + // An evicted identity is recoverable and re-enters only by paying + // the ordinary manifest verification path again. + BEAST_EXPECT( + bounded.applyManifest( + clone(b.manifest), ManifestRetention::evictable) == + ManifestDisposition::accepted); + BEAST_EXPECT(bounded.getManifestSnapshot(b.manifest.masterKey)); + bounded.for_each_manifest( + [&](std::size_t n) { retained = n; }, [](Manifest const&) {}); + BEAST_EXPECT(retained == 2); + } + testLoadStore(cache); testGetSignature(); testGetKeys(); diff --git a/src/test/app/SetHookTSH_test.cpp b/src/test/app/SetHookTSH_test.cpp index af422f79e3..04f884928d 100644 --- a/src/test/app/SetHookTSH_test.cpp +++ b/src/test/app/SetHookTSH_test.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -8483,15 +8484,38 @@ private: return std::string(static_cast(s.data()), s.size()); } - // A manifest transaction carries no account signature, so it cannot be - // submitted through env() the way a signed transaction can. Returns the - // resulting transaction id so the caller can inspect its metadata. + // First registration requires the manifest owner's ordinary account + // signature. Build that valid envelope here while retaining the manifest's + // own master/signing-key authority for the stake-holder relationship under + // test. Returns the transaction id so the caller can inspect its metadata. uint256 - submitManifest(jtx::Env& env, std::string const& manifest) + submitManifest( + jtx::Env& env, + jtx::Account const& account, + std::string const& manifest) { - Json::Value params; - params[jss::manifest] = strHex(manifest); - auto const jrr = env.rpc("json", "submit", to_string(params)); + auto const build = [&](XRPAmount fee) { + auto tx = + std::make_shared(ttMANIFEST_SET, [&](STObject& obj) { + obj.setAccountID(sfAccount, account.id()); + obj.setFieldU32(sfSequence, env.seq(account)); + obj.setFieldU32(sfNetworkID, env.app().config().NETWORK_ID); + obj.setFieldAmount(sfFee, fee); + obj.setFieldVL(sfSigningPubKey, account.pk().slice()); + + SerialIter mit{makeSlice(manifest)}; + obj.peekFieldObject(sfManifest).set(mit); + }); + tx->sign(account.pk(), account.sk()); + return tx; + }; + + auto const probe = build(XRPAmount{0}); + auto const tx = + build(SetManifest::calculateBaseFee(*env.current(), *probe)); + Serializer s; + tx->add(s); + auto const jrr = env.rpc("submit", strHex(s.slice())); auto const& result = jrr[jss::result]; @@ -8555,8 +8579,8 @@ private: setTSHHook(env, ephemeral, testStrong); - auto const txHash = - submitManifest(env, makeManifestString(master, ephemeral, 1)); + auto const txHash = submitManifest( + env, master, makeManifestString(master, ephemeral, 1)); env.close(); // A strong hook on a weak stake holder is never reached. @@ -8582,6 +8606,7 @@ private: auto const txHash = submitManifest( env, + master, makeManifestString( master, ephemeral, diff --git a/src/xrpld/app/misc/Manifest.h b/src/xrpld/app/misc/Manifest.h index 342523ddbd..8de0aee7d2 100644 --- a/src/xrpld/app/misc/Manifest.h +++ b/src/xrpld/app/misc/Manifest.h @@ -246,6 +246,14 @@ enum class ManifestDisposition { invalid }; +/** Retention class for an accepted validator manifest. + + Protected entries are required by current local policy or configuration. + Everything else is recoverable residue and shares one bounded population. + Retention does not grant validator-list membership or consensus weight. +*/ +enum class ManifestRetention : std::uint8_t { evictable, protected_ }; + inline std::string to_string(ManifestDisposition m) { @@ -301,6 +309,12 @@ private: /** Master public keys stored by current ephemeral public key. */ hash_map signingToMasterKeys_; + /** Retained masters not protected by current local policy. */ + hash_set evictable_; + + /** Masters supplied directly by local validator configuration. */ + hash_set configured_; + /** Master keys always offered to a peer, whatever their recency. Set by pin(); in practice the master keys on the configured validator @@ -311,10 +325,10 @@ private: /** Recency of use, keyed by master public key. The structure of this map is guarded by mutex_ exactly as map_ is: - entries are created next to it in applyManifest() and are never - removed. The counters themselves are atomic, so recording a hit is a - write to an atomic rather than a structural modification and is legal - while only a shared lock is held. + entries are created next to it in applyManifest() and removed only + with the corresponding manifest row. The counters themselves are + atomic, so recording a hit is a write to an atomic rather than a + structural modification and is legal while only a shared lock is held. This is deliberately not a second mutex. A second mutex would have to be ordered against mutex_, and that ordering would be an unenforced @@ -323,6 +337,8 @@ private: hash_map> mutable lastUsed_; std::atomic mutable tick_{0}; + std::size_t const evictableLimit_; + /** Ephemeral keys already probed against the ledger, and where. Bounds the reads driven by incoming validations to one per key per @@ -344,7 +360,18 @@ private: touch(PublicKey const& masterKey) const; ManifestDisposition - applyManifest(Manifest m, bool ledgerAuthoritative); + applyManifest( + Manifest m, + bool ledgerAuthoritative, + ManifestRetention retention); + + /** Remove one complete cache row while holding mutex_ exclusively. */ + void + eraseUnlocked(PublicKey const& masterKey); + + /** Remove the least recently used evictable row. */ + bool + evictOneUnlocked(); std::atomic seq_{0}; @@ -354,6 +381,9 @@ private: bool allowSameMasterSigningKey = false) const; public: + /** Maximum recoverable residue retained by a production cache. */ + static constexpr std::size_t evictableLimit = 1000; + /** Ceiling on the unpinned manifests offered to a newly connected peer. */ static constexpr std::size_t gossipLimit = 64; @@ -366,8 +396,9 @@ public: static constexpr std::size_t probeLimit = 4096; explicit ManifestCache( - beast::Journal j = beast::Journal(beast::Journal::getNullSink())) - : j_(j) + beast::Journal j = beast::Journal(beast::Journal::getNullSink()), + std::size_t const maxEvictable = evictableLimit) + : j_(j), evictableLimit_(maxEvictable) { } @@ -480,6 +511,15 @@ public: ManifestDisposition applyManifest(Manifest m); + /** Add a manifest with an explicit retention class. + + New evictable identities displace the least recently used evictable + row at capacity. Existing protected identities are never demoted by + ingress; pin() alone reconciles protection with local policy. + */ + ManifestDisposition + applyManifest(Manifest m, ManifestRetention retention); + /** Set the master keys that are always offered to peers. Replaces any previous set. Bumps sequence() when the set actually diff --git a/src/xrpld/app/misc/detail/Manifest.cpp b/src/xrpld/app/misc/detail/Manifest.cpp index 705bf9ba69..130c00a7da 100644 --- a/src/xrpld/app/misc/detail/Manifest.cpp +++ b/src/xrpld/app/misc/detail/Manifest.cpp @@ -378,6 +378,9 @@ ManifestCache::getManifestSnapshot(PublicKey const& pk) const if (manifest == map_.end()) return std::nullopt; + // This is the live validation prerequisite path, so it is the strongest + // evidence that recoverable residue remains useful. + touch(masterKey); auto const& current = manifest->second; return Snapshot{ current.masterKey, @@ -426,19 +429,76 @@ ManifestCache::touch(PublicKey const& masterKey) const iter->second.store(++tick_, std::memory_order_relaxed); } +void +ManifestCache::eraseUnlocked(PublicKey const& masterKey) +{ + auto const iter = map_.find(masterKey); + if (iter == map_.end()) + return; + + if (iter->second.signingKey) + signingToMasterKeys_.erase(*iter->second.signingKey); + map_.erase(iter); + evictable_.erase(masterKey); + lastUsed_.erase(masterKey); +} + +bool +ManifestCache::evictOneUnlocked() +{ + if (evictable_.empty()) + return false; + + auto victim = evictable_.begin(); + auto victimTick = std::numeric_limits::max(); + for (auto iter = evictable_.begin(); iter != evictable_.end(); ++iter) + { + auto const used = lastUsed_.find(*iter); + auto const tick = used == lastUsed_.end() + ? 0 + : used->second.load(std::memory_order_relaxed); + if (tick < victimTick) + { + victim = iter; + victimTick = tick; + } + } + + auto const masterKey = *victim; + eraseUnlocked(masterKey); + return true; +} + void ManifestCache::pin(hash_set keys) { std::lock_guard lock{mutex_}; - if (keys == pinned_) - return; - + bool changed = keys != pinned_; pinned_ = std::move(keys); - // The pinned set is part of what a gossip message contains, so a change to - // it has to invalidate any message cached against this sequence. - ++seq_; + // pin() is the sole demotion boundary. An incoming unlisted manifest can + // never demote protected state, while identities removed from local policy + // become recoverable residue and therefore count against the hard cap. + for (auto const& [masterKey, manifest] : map_) + { + (void)manifest; + if (configured_.contains(masterKey) || pinned_.contains(masterKey)) + changed = evictable_.erase(masterKey) != 0 || changed; + else + changed = evictable_.insert(masterKey).second || changed; + } + + while (evictable_.size() > evictableLimit_) + { + if (!evictOneUnlocked()) + break; + changed = true; + } + + // Protection changes affect both resolution and the bounded gossip view. + if (changed) + ++seq_; } namespace { @@ -498,7 +558,8 @@ ManifestCache::applyLedger( if (auto mo = manifestFromSLE(*sle, j_); mo && mo->masterKey == pk && sle->getAccountID(sfAccount) == calcAccountID(mo->masterKey) && - applyManifest(std::move(*mo), true) == + applyManifest( + std::move(*mo), true, ManifestRetention::protected_) == ManifestDisposition::accepted) ++accepted; } @@ -569,7 +630,7 @@ ManifestCache::applyLedgerSigningKey( *mo->signingKey == signingKey && keylet::manifest(mo->masterKey).key == manifestID && sleManifest->getAccountID(sfAccount) == calcAccountID(mo->masterKey)) - applyManifest(std::move(*mo), true); + applyManifest(std::move(*mo), true, ManifestRetention::evictable); // Only a signing key resolves: a master key is its own master. return held(); @@ -631,12 +692,22 @@ ManifestCache::checkKeyRoles(Manifest const& m) const ManifestDisposition ManifestCache::applyManifest(Manifest m) { - return applyManifest(std::move(m), false); + return applyManifest(std::move(m), false, ManifestRetention::protected_); } ManifestDisposition -ManifestCache::applyManifest(Manifest m, bool ledgerAuthoritative) +ManifestCache::applyManifest(Manifest m, ManifestRetention retention) { + return applyManifest(std::move(m), false, retention); +} + +ManifestDisposition +ManifestCache::applyManifest( + Manifest m, + bool ledgerAuthoritative, + ManifestRetention retention) +{ + bool const protect = retention == ManifestRetention::protected_; // Check the manifest against the conditions that do not require a // `unique_lock` (write lock) on the `mutex_`. Since the signature can be // relatively expensive, the `checkSignature` parameter determines if the @@ -706,13 +777,28 @@ ManifestCache::applyManifest(Manifest m, bool ledgerAuthoritative) { std::shared_lock sl{mutex_}; - if (auto d = - prewriteCheck(map_.find(m.masterKey), /*checkSig*/ true, sl)) - return *d; + auto const iter = map_.find(m.masterKey); + if (auto d = prewriteCheck(iter, /*checkSig*/ true, sl)) + { + // A stale protected application still promotes a retained + // evictable row. Defer that policy mutation to the write lock. + if (!protect || *d != ManifestDisposition::stale || + !evictable_.contains(m.masterKey)) + return *d; + } } std::unique_lock sl{mutex_}; auto const iter = map_.find(m.masterKey); + + if (protect && iter != map_.end() && m.sequence <= iter->second.sequence && + !(ledgerAuthoritative && m.sequence == iter->second.sequence && + m.serialized != iter->second.serialized)) + { + if (evictable_.erase(m.masterKey) != 0) + ++seq_; + return ManifestDisposition::stale; + } // Since we released the previously held read lock, it's possible that the // collections have been written to. This means we need to run // `prewriteCheck` again. This re-does work, but `prewriteCheck` is @@ -725,6 +811,14 @@ ManifestCache::applyManifest(Manifest m, bool ledgerAuthoritative) if (auto d = prewriteCheck(iter, /*checkSig*/ false, sl)) return *d; + if (iter == map_.end() && !protect) + { + if (evictableLimit_ == 0) + return ManifestDisposition::stale; + if (evictable_.size() >= evictableLimit_ && !evictOneUnlocked()) + return ManifestDisposition::stale; + } + bool const revoked = m.revoked(); // This is the first manifest we are seeing for a master key. This should // only ever happen once per validator run. @@ -737,10 +831,13 @@ ManifestCache::applyManifest(Manifest m, bool ledgerAuthoritative) signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); // Kept in step with map_ so touch() never has to insert; see - // lastUsed_. - lastUsed_.try_emplace(m.masterKey, 0); + // lastUsed_. New rows begin most-recent so capacity admission cannot + // immediately discard the row it just paid to verify. + lastUsed_.try_emplace(m.masterKey, ++tick_); auto masterKey = m.masterKey; + if (!protect) + evictable_.insert(masterKey); map_.emplace(std::move(masterKey), std::move(m)); // Increment sequence to invalidate cached manifest messages @@ -766,6 +863,9 @@ ManifestCache::applyManifest(Manifest m, bool ledgerAuthoritative) signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); iter->second = std::move(m); + if (protect) + evictable_.erase(iter->first); + touch(iter->first); // Something has changed. Keep track of it. seq_++; @@ -776,7 +876,8 @@ void ManifestCache::load(DatabaseCon& dbCon, std::string const& dbTable) { auto db = dbCon.checkoutDb(); - ripple::getManifests(*db, dbTable, *this, j_); + ripple::getManifests( + *db, dbTable, *this, ManifestRetention::protected_, j_); } bool @@ -786,7 +887,14 @@ ManifestCache::load( std::string const& configManifest, std::vector const& configRevocation) { - load(dbCon, dbTable); + { + // Wallet rows reflect a previous trust view. Load them directly into + // the bounded residue; current lists will promote relevant masters at + // the first pin() reconciliation. + auto db = dbCon.checkoutDb(); + ripple::getManifests( + *db, dbTable, *this, ManifestRetention::evictable, j_); + } if (!configManifest.empty()) { @@ -802,11 +910,15 @@ ManifestCache::load( JLOG(j_.warn()) << "Configured manifest revokes public key"; } - if (applyManifest(std::move(*mo)) == ManifestDisposition::invalid) + auto const masterKey = mo->masterKey; + if (applyManifest(std::move(*mo), ManifestRetention::protected_) == + ManifestDisposition::invalid) { JLOG(j_.error()) << "Manifest in config was rejected"; return false; } + std::unique_lock lock{mutex_}; + configured_.insert(masterKey); } if (!configRevocation.empty()) @@ -831,11 +943,15 @@ ManifestCache::load( return false; } - if (applyManifest(std::move(*mo)) == ManifestDisposition::invalid) + auto const masterKey = mo->masterKey; + if (applyManifest(std::move(*mo), ManifestRetention::protected_) == + ManifestDisposition::invalid) { JLOG(j_.error()) << "Invalid validator key revocation in config"; return false; } + std::unique_lock lock{mutex_}; + configured_.insert(masterKey); } return true; diff --git a/src/xrpld/app/rdb/Wallet.h b/src/xrpld/app/rdb/Wallet.h index 6130c9fc26..07ac9a89a7 100644 --- a/src/xrpld/app/rdb/Wallet.h +++ b/src/xrpld/app/rdb/Wallet.h @@ -58,6 +58,7 @@ makeTestWalletDB( * @param dbTable Name of the database table from which the manifest will be * extracted. * @param mCache Cache for storing the manifest. + * @param retention Retention assigned to loaded rows. * @param j Journal. */ void @@ -65,6 +66,7 @@ getManifests( soci::session& session, std::string const& dbTable, ManifestCache& mCache, + ManifestRetention retention, beast::Journal j); /** diff --git a/src/xrpld/app/rdb/detail/Wallet.cpp b/src/xrpld/app/rdb/detail/Wallet.cpp index ffb2859691..90f9d7d2de 100644 --- a/src/xrpld/app/rdb/detail/Wallet.cpp +++ b/src/xrpld/app/rdb/detail/Wallet.cpp @@ -46,6 +46,7 @@ getManifests( soci::session& session, std::string const& dbTable, ManifestCache& mCache, + ManifestRetention const retention, beast::Journal j) { // Load manifests stored in database @@ -65,7 +66,7 @@ getManifests( continue; } - mCache.applyManifest(std::move(*mo)); + mCache.applyManifest(std::move(*mo), retention); } else { diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index 6d83979e3d..e7fb4a8600 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -678,8 +678,10 @@ OverlayImpl::onManifests( continue; } - auto const result = - app_.validatorManifests().applyManifest(std::move(*mo)); + auto const result = app_.validatorManifests().applyManifest( + std::move(*mo), + listed ? ManifestRetention::protected_ + : ManifestRetention::evictable); if (result == ManifestDisposition::invalid) from->charge(