fix(manifest): reconcile trust entrants and make save policies explicit

This commit is contained in:
Nicholas Dudfield
2026-09-09 11:17:56 +07:00
parent c480adb8c2
commit 2fdcd7fd68
11 changed files with 241 additions and 85 deletions

View File

@@ -433,11 +433,7 @@ public:
*db << sql, soci::into(n);
return n;
};
auto save = [&]() {
cache.save(wallet, "ValidatorManifests", [](PublicKey const&) {
return false;
});
};
auto save = [&]() { cache.saveListed(); };
bool failed = false;
try
{

View File

@@ -21,11 +21,13 @@
#include <test/jtx/network.h>
#include <xrpld/app/ledger/OpenLedger.h>
#include <xrpld/app/misc/Manifest.h>
#include <xrpld/app/misc/ValidatorList.h>
#include <xrpld/app/tx/apply.h>
#include <xrpld/app/tx/detail/SetManifest.h>
#include <xrpld/core/Config.h>
#include <xrpld/ledger/OpenView.h>
#include <xrpl/basics/StringUtilities.h>
#include <xrpl/basics/base64.h>
#include <xrpl/basics/strHex.h>
#include <xrpl/json/to_string.h>
#include <xrpl/protocol/Feature.h>
@@ -427,6 +429,119 @@ struct SetManifest_test : public beast::unit_test::suite
BEAST_EXPECT(cache.revoked(master.pk()));
}
void
testListedAfterLedgerRevocation(
FeatureBitset features,
bool scheduled = false)
{
testcase(
scheduled
? "scheduled listing checks revocation before publishing trust"
: "newly listed warm identity honours the ledger revocation");
using namespace jtx;
Env env{*this, makeConfig(), features};
auto const master = Account("late-listed-master", KeyType::ed25519);
auto const signing = Account("late-listed-signing", KeyType::secp256k1);
auto const control = Account("late-list-control", KeyType::ed25519);
auto const controlSigning =
Account("late-list-control-signing", KeyType::secp256k1);
auto& cache = env.app().validatorManifests();
auto& lists = env.app().validators();
BEAST_EXPECT(
cache.applyManifest(*deserializeManifest(makeManifest(
control, controlSigning, 1))) == ManifestDisposition::accepted);
BEAST_EXPECT(lists.load(
{}, {toBase58(TokenType::NodePublic, control.pk())}, {}));
env.fund(XRP(1000), master);
env.close();
BEAST_EXPECT(
engineResult(submit(env, makeManifest(master, signing, 1))) ==
"tesSUCCESS");
env.close();
BEAST_EXPECT(!lists.listed(master.pk()));
BEAST_EXPECT(
cache.applyLedgerSigningKey(*env.closed(), signing.pk()) ==
master.pk());
BEAST_EXPECT(cache.getSequence(master.pk()) == 1);
BEAST_EXPECT(
engineResult(submit(
env,
makeManifest(
master,
signing,
std::numeric_limits<std::uint32_t>::max()))) ==
"tesSUCCESS");
env.close();
BEAST_EXPECT(
env.le(keylet::manifest(master.pk()))->getFieldU32(sfSequence) ==
std::numeric_limits<std::uint32_t>::max());
BEAST_EXPECT(!cache.revoked(master.pk()));
auto const effective =
env.timeKeeper().now() + std::chrono::seconds{120};
if (scheduled)
{
auto const publisher =
Account("late-list-publisher", KeyType::ed25519);
auto const pubSigning =
Account("late-list-pub-signing", KeyType::secp256k1);
BEAST_EXPECT(lists.load({}, {}, {strHex(publisher.pk())}));
auto list = [&](bool future) {
Json::Value body(Json::objectValue);
body["sequence"] = future ? 2 : 1;
body["expiration"] = static_cast<Json::UInt>(
(effective + std::chrono::seconds{3600})
.time_since_epoch()
.count());
if (future)
body["effective"] = static_cast<Json::UInt>(
effective.time_since_epoch().count());
body["validators"] = Json::Value(Json::arrayValue);
auto add = [&](Account const& m, Account const& s) {
Json::Value entry(Json::objectValue);
entry["validation_public_key"] = strHex(m.pk());
entry["manifest"] = base64_encode(makeManifest(m, s, 1));
body["validators"].append(entry);
};
add(control, controlSigning);
if (future)
add(master, signing);
auto const raw = to_string(body);
return ValidatorBlobInfo{
base64_encode(raw),
strHex(
sign(pubSigning.pk(), pubSigning.sk(), makeSlice(raw))),
{}};
};
auto const result = lists.applyLists(
base64_encode(makeManifest(publisher, pubSigning, 1)),
2,
{list(false), list(true)},
"test");
BEAST_EXPECT(result.bestDisposition() == ListDisposition::accepted);
BEAST_EXPECT(lists.listed(control.pk()));
BEAST_EXPECT(!lists.listed(master.pk()));
}
else
BEAST_EXPECT(lists.load(
{}, {toBase58(TokenType::NodePublic, master.pk())}, {}));
// Run the actual beginConsensus path, not a test-side cache refresh.
if (scheduled)
BEAST_EXPECT(env.close(effective + std::chrono::seconds{1}));
else
BEAST_EXPECT(env.close());
BEAST_EXPECT(lists.listed(master.pk()));
BEAST_EXPECT(cache.revoked(master.pk()));
BEAST_EXPECT(!lists.trusted(master.pk()));
BEAST_EXPECT(!lists.getQuorumKeys().second.contains(signing.pk()));
BEAST_EXPECT(lists.trusted(control.pk()));
BEAST_EXPECT(env.close());
BEAST_EXPECT(cache.revoked(master.pk()));
BEAST_EXPECT(!lists.trusted(master.pk()));
}
void
testSigningKeyRetrieval(FeatureBitset features)
{
@@ -843,6 +958,8 @@ public:
testUpdate(sa);
testRevocation(sa);
testRetrieval(sa);
testListedAfterLedgerRevocation(sa);
testListedAfterLedgerRevocation(sa, true);
testSigningKeyRetrieval(sa);
testMalformed(sa);
testEnvelopeRejections(sa);

View File

@@ -287,9 +287,7 @@ private:
cold.loadListed(wallet);
BEAST_EXPECT(!cold.getRawManifest(master.first));
BEAST_EXPECT(!cold.getRawManifest(unrelated.masterPublic));
cold.save(wallet, "ValidatorManifests", [](PublicKey const&) {
return false;
});
cold.saveListed();
}
BEAST_EXPECT(rows() == saved);
@@ -325,9 +323,7 @@ private:
manifests.applyGossipManifest(*deserializeManifest(base64_decode(
validator.manifest))) == ManifestDisposition::stale);
BEAST_EXPECT(!manifests.getRawManifest(unrelated.masterPublic));
manifests.save(wallet, "ValidatorManifests", [&](PublicKey const& key) {
return lists.listed(key);
});
manifests.saveListed();
BEAST_EXPECT(
rows() ==
saved - 1); // One row replaces the old version and revocation.
@@ -336,10 +332,7 @@ private:
BEAST_EXPECT(manifests.loadConfig(local.manifest, {}));
for (int i = 0; i < 2; ++i)
{
manifests.save(
wallet, "ValidatorManifests", [&](PublicKey const& key) {
return lists.listed(key);
});
manifests.saveListed();
BEAST_EXPECT(rows() == saved);
}
@@ -447,9 +440,7 @@ private:
*db, "ValidatorManifests", {master.first}, env.journal);
BEAST_EXPECT(saved.at(master.first).serialized == revokedBytes);
}
cache.save(wallet, "ValidatorManifests", [](PublicKey const&) {
return false;
});
cache.saveListed();
}
ManifestCache restarted{env.journal, 0};
ManifestCache publishers;

View File

@@ -1643,10 +1643,7 @@ ApplicationImp::run()
validatorSites_->stop();
// TODO Store manifests in manifests.sqlite instead of wallet.db
validatorManifests_->save(
getWalletDB(), "ValidatorManifests", [this](PublicKey const& pubKey) {
return validators().listed(pubKey);
});
validatorManifests_->saveListed();
// List updates restore wallet history while holding the list lock. Never
// call back into that lock from a save that already holds the wallet.

View File

@@ -570,11 +570,10 @@ public:
void
loadListed(DatabaseCon& dbCon);
/** Save cached manifests to database.
/** Replace a table with selected cached manifests and all revocations.
With a wallet attached, save listed/configured and pending history,
preserve unrelated wallet rows, and ignore isTrusted. Otherwise use
isTrusted and retain revocations, as for the publisher cache.
This is the legacy/publisher save policy, independent of wallet
attachment. Use saveListed() for selective validator history.
@param dbCon Database containing dbTable
@@ -590,6 +589,15 @@ public:
std::string const& dbTable,
std::function<bool(PublicKey const&)> const& isTrusted);
/** Save listed/configured and pending history to the attached wallet.
Preserve unrelated rows. Requires loadListed() to have attached the
wallet; no caller-supplied trust predicate or ValidatorList lock is
used.
*/
void
saveListed();
/** Invokes the callback once for every populated manifest.
@note Do not call ManifestCache member functions from within the

View File

@@ -2071,7 +2071,11 @@ NetworkOPsImp::beginConsensus(
closingInfo.parentCloseTime,
*this,
app_.overlay(),
app_.getHashRouter());
app_.getHashRouter(),
[this, &prevLedger](hash_set<PublicKey> const& candidates) {
if (prevLedger->rules().enabled(featureOnChainManifests))
app_.validatorManifests().applyLedger(*prevLedger, candidates);
});
if (!changes.added.empty() || !changes.removed.empty())
{

View File

@@ -31,6 +31,7 @@
#include <boost/iterator/counting_iterator.hpp>
#include <boost/range/adaptors.hpp>
#include <boost/thread/shared_mutex.hpp>
#include <functional>
#include <mutex>
#include <numeric>
#include <shared_mutex>
@@ -474,6 +475,10 @@ public:
@param seenValidators Set of NodeIDs of validators that have signed
recently received validations
@param reconcileCandidates Optional synchronous reconciliation before
new trust is published, after pending lists rotate. Called under the
list lock; must not re-enter ValidatorList.
@return TrustedKeyChanges instance with newly trusted or untrusted
node identities.
@@ -487,7 +492,9 @@ public:
NetClock::time_point closeTime,
NetworkOPs& ops,
Overlay& overlay,
HashRouter& hashRouter);
HashRouter& hashRouter,
std::function<void(hash_set<PublicKey> const&)> const&
reconcileCandidates = {});
/** Get quorum value for current trusted key set

View File

@@ -432,13 +432,12 @@ ManifestCache::pin(hash_set<PublicKey> keys)
try
{
auto db = wallet->checkoutDb();
saveManifests(
compactManifests(
*db,
"ValidatorManifests",
[](PublicKey const&) { return true; },
departing,
j_,
true);
j_);
}
catch (soci::soci_error const& e)
{
@@ -886,20 +885,28 @@ ManifestCache::save(
std::shared_lock lock{mutex_};
auto db = dbCon.checkoutDb();
saveManifests(
saveManifests(*db, dbTable, isTrusted, map_, j_);
}
void
ManifestCache::saveListed()
{
std::shared_lock lock{mutex_};
if (!wallet_)
Throw<std::logic_error>("Validator manifest wallet is not attached");
auto db = wallet_->checkoutDb();
compactManifests(
*db,
dbTable,
[this, &isTrusted](PublicKey const& key) {
"ValidatorManifests",
[this](PublicKey const& key) {
// Membership is already mirrored here. Do not take ValidatorList's
// lock while holding the cache lock (pin() takes them in reverse).
if (!wallet_)
return isTrusted(key);
return pinned_.contains(key) || configured_.contains(key) ||
pendingSave_.contains(key);
},
map_,
j_,
wallet_ != nullptr);
j_);
}
// Clean up macros to avoid namespace pollution

View File

@@ -1938,7 +1938,8 @@ ValidatorList::updateTrusted(
NetClock::time_point closeTime,
NetworkOPs& ops,
Overlay& overlay,
HashRouter& hashRouter)
HashRouter& hashRouter,
std::function<void(hash_set<PublicKey> const&)> const& reconcileCandidates)
{
using namespace std::chrono_literals;
if (timeKeeper_.now() > closeTime + 30s)
@@ -2030,6 +2031,20 @@ ValidatorList::updateTrusted(
if (good)
ops.clearUNLBlocked();
// The per-round ledger pass covered the previously trusted set. A key can
// become eligible between rounds or in the pending-list rotation above;
// reconcile it now so a warm stale binding is never trusted for one round.
if (reconcileCandidates)
{
hash_set<PublicKey> candidates;
for (auto const& [key, count] : keyListings_)
if (count >= listThreshold_ && !trustedMasterKeys_.contains(key) &&
!validatorManifests_.revoked(key))
candidates.insert(key);
if (!candidates.empty())
reconcileCandidates(candidates);
}
TrustChanges trustChanges;
auto it = trustedMasterKeys_.cbegin();

View File

@@ -84,8 +84,6 @@ getManifestsForKeys(
* @param isTrusted Callback that returns true if the key is trusted.
* @param map Maps public keys to manifests.
* @param j Journal.
* @param preserveUnloaded Keep unloaded history and save only local keys,
* without duplicating versions already saved (validator cache).
*/
void
saveManifests(
@@ -93,8 +91,21 @@ saveManifests(
std::string const& dbTable,
std::function<bool(PublicKey const&)> const& isTrusted,
hash_map<PublicKey, Manifest> const& map,
beast::Journal j,
bool preserveUnloaded = false);
beast::Journal j);
/** Save selected identities without deleting unrelated wallet history.
Keep the highest valid cache/disk version per selected master and compact
its old rows in one transaction. shouldRetain selects from map; unlike the
legacy saveManifests(), this never rewrites the whole table.
*/
void
compactManifests(
soci::session& session,
std::string const& dbTable,
std::function<bool(PublicKey const&)> const& shouldRetain,
hash_map<PublicKey, Manifest> const& map,
beast::Journal j);
/**
* @brief addValidatorManifest Saves the manifest of a validator to the

View File

@@ -135,56 +135,59 @@ saveManifest(
soci::use(rawData);
}
void
compactManifests(
soci::session& session,
std::string const& dbTable,
std::function<bool(PublicKey const&)> const& shouldRetain,
hash_map<PublicKey, Manifest> const& map,
beast::Journal j)
{
soci::transaction tr(session);
hash_map<PublicKey, Manifest> retained;
for (auto const& [key, manifest] : map)
if (shouldRetain(key))
retained.emplace(key, manifest.clone());
if (!retained.empty())
{
// Stage row IDs in SQLite, not an unbounded C++ vector. Keep the
// source table untouched while its read cursor is active. This
// table is created and dropped in one transaction, with no schema
// migration or persistent staging state.
auto const obsolete = dbTable + "_Compacting";
session << "CREATE TABLE " + obsolete + " (RowID INTEGER PRIMARY KEY);";
readManifests(
session,
dbTable,
[&](Manifest m, std::int64_t rowid) {
auto const it = retained.find(m.masterKey);
if (it == retained.end())
return;
if (m.sequence > it->second.sequence && m.verify())
it->second = std::move(m);
session << "INSERT INTO " + obsolete + " (RowID) VALUES (:id);",
soci::use(rowid);
},
j);
session << "DELETE FROM " + dbTable +
" WHERE rowid IN (SELECT RowID FROM " + obsolete + ");";
for (auto const& [key, manifest] : retained)
saveManifest(session, dbTable, manifest.serialized);
session << "DROP TABLE " + obsolete + ";";
}
tr.commit();
}
void
saveManifests(
soci::session& session,
std::string const& dbTable,
std::function<bool(PublicKey const&)> const& isTrusted,
hash_map<PublicKey, Manifest> const& map,
beast::Journal j,
bool preserveUnloaded)
beast::Journal j)
{
soci::transaction tr(session);
if (preserveUnloaded)
{
hash_map<PublicKey, Manifest> retained;
for (auto const& [key, manifest] : map)
if (isTrusted(key))
retained.emplace(key, manifest.clone());
if (!retained.empty())
{
// Stage row IDs in SQLite, not an unbounded C++ vector. Keep the
// source table untouched while its read cursor is active. This
// table is created and dropped in one transaction, with no schema
// migration or persistent staging state.
auto const obsolete = dbTable + "_Compacting";
session << "CREATE TABLE " + obsolete +
" (RowID INTEGER PRIMARY KEY);";
readManifests(
session,
dbTable,
[&](Manifest m, std::int64_t rowid) {
auto const it = retained.find(m.masterKey);
if (it == retained.end())
return;
if (m.sequence > it->second.sequence && m.verify())
it->second = std::move(m);
session
<< "INSERT INTO " + obsolete + " (RowID) VALUES (:id);",
soci::use(rowid);
},
j);
session << "DELETE FROM " + dbTable +
" WHERE rowid IN (SELECT RowID FROM " + obsolete + ");";
for (auto const& [key, manifest] : retained)
saveManifest(session, dbTable, manifest.serialized);
session << "DROP TABLE " + obsolete + ";";
}
tr.commit();
return;
}
session << "DELETE FROM " << dbTable;
for (auto const& v : map)
{