ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-27 07:26:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

merge: bring the resolveNodeIdentity() decision tests forward from phase8-log-correlation

Browse Source
This commit is contained in:
Pratik Mankawde
2026-09-14 23:34:57 +01:00
parent 6c218f9d19 b8dbba917a
commit 6dc961f195
5 changed files with 346 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
include/xrpl/server/Wallet.h
View File

@@ -7,6 +7,7 @@
#include <xrpl/core/PeerReservationTable.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Seed.h>
#include <xrpl/rdb/DatabaseCon.h>
#include <xrpl/server/Manifest.h>
@@ -83,6 +84,46 @@ saveManifests(
void
addValidatorManifest(soci::session& session, std::string const& serialized);
/**
* The seed a configured [node_seed] or --nodeid names.
*
* The command-line value wins when both are set. A cmdline value is parsed
* with parseGenericSeed(rfc1751=false); a config value is parsed as Base58
* only. Empty inputs count as supplied-but-invalid and throw.
*
* @param cmdlineSeed The --nodeid value, or std::nullopt when not passed.
* @param configSeed The first line of [node_seed], or std::nullopt when the
* section is absent. An empty section is passed as "".
* @return The parsed seed, or std::nullopt when neither is set.
* @throws std::runtime_error if the value present is malformed.
*/
[[nodiscard]] std::optional<Seed>
parseNodeIdentitySeed(
std::optional<std::string> const& cmdlineSeed,
std::optional<std::string> const& configSeed);
/**
* Pick this node's keypair from a pre-parsed seed and a stored-key reader.
*
* A configured seed wins outright and the reader is not consulted. When
* newNodeId is set, mint a fresh pair and skip the reader too. Otherwise
* consult the reader; return its pair if it has one, else mint.
*
* Lifting the decision out of the Application layer lets libxrpl-level
* tests drive every branch without an xrpld Config.
*
* @param configuredSeed Seed from parseNodeIdentitySeed(), or std::nullopt.
* @param newNodeId True when --newnodeid was passed.
* @param readStored Callable that returns the wallet's stored pair, or
* std::nullopt when nothing is stored.
* @return This node's keypair.
*/
[[nodiscard]] std::pair<PublicKey, SecretKey>
selectNodeIdentity(
std::optional<Seed> const& configuredSeed,
bool newNodeId,
std::function<std::optional<std::pair<PublicKey, SecretKey>>()> const& readStored);
/**
* Delete any saved public/private key associated with this node.
*/

53
src/libxrpl/server/Wallet.cpp
View File

@@ -3,13 +3,16 @@
#include <xrpl/basics/Log.h>
#include <xrpl/basics/UnorderedContainers.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/contract.h>
#include <xrpl/basics/safe_cast.h>
#include <xrpl/beast/hash/uhash.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/config/Constants.h>
#include <xrpl/core/PeerReservationTable.h>
#include <xrpl/protocol/KeyType.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Seed.h>
#include <xrpl/protocol/tokens.h>
#include <xrpl/rdb/DBInit.h>
#include <xrpl/rdb/DatabaseCon.h>
@@ -33,6 +36,7 @@
#include <functional>
#include <memory>
#include <optional>
#include <stdexcept>
#include <string>
#include <unordered_set>
#include <utility>
@@ -142,6 +146,55 @@ addValidatorManifest(soci::session& session, std::string const& serialized)
tr.commit();
}
std::optional<Seed>
parseNodeIdentitySeed(
std::optional<std::string> const& cmdlineSeed,
std::optional<std::string> const& configSeed)
{
if (cmdlineSeed)
{
auto seed = parseGenericSeed(*cmdlineSeed, false);
if (!seed)
Throw<std::runtime_error>("Invalid 'nodeid' in command line");
return seed;
}
if (configSeed)
{
auto seed = parseBase58<Seed>(*configSeed);
if (!seed)
{
Throw<std::runtime_error>(
std::string("Invalid [") + Sections::kNodeSeed + "] in configuration file");
}
return seed;
}
return std::nullopt;
}
std::pair<PublicKey, SecretKey>
selectNodeIdentity(
std::optional<Seed> const& configuredSeed,
bool newNodeId,
std::function<std::optional<std::pair<PublicKey, SecretKey>>()> const& readStored)
{
if (configuredSeed)
{
auto const sk = generateSecretKey(KeyType::Secp256k1, *configuredSeed);
return {derivePublicKey(KeyType::Secp256k1, sk), sk};
}
// --newnodeid discards whatever is stored, so mint now.
if (!newNodeId)
{
if (auto stored = readStored())
return *stored;
}
return randomKeyPair(KeyType::Secp256k1);
}
void
clearNodeIdentity(soci::session& session)
{

9
src/libxrpl/telemetry/NullTelemetry.cpp
View File

@@ -25,7 +25,6 @@
#ifdef XRPL_ENABLE_TELEMETRY
#include <opentelemetry/context/context.h>
#include <opentelemetry/metrics/meter.h>
#include <opentelemetry/metrics/noop.h>
#include <opentelemetry/nostd/shared_ptr.h>
#include <opentelemetry/trace/noop.h>
#include <opentelemetry/trace/span.h>
@@ -141,11 +140,11 @@ public:
}
opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter>
getMeter(std::string_view) override
getMeter(std::string_view name) override
{
static auto noopMeter = opentelemetry::nostd::shared_ptr<opentelemetry::metrics::Meter>(
new opentelemetry::metrics::NoopMeter());
return noopMeter;
// Route through the shared helper so the meter identity (name +
// kMeterVersion) matches every other noop path in the process.
return noopMeter(name);
}
#endif
};

257
src/tests/libxrpl/server/NodeIdentity.cpp
View File

@@ -1,32 +1,46 @@
/**
* @file NodeIdentity.cpp
* GTest unit tests for the wallet database's node-identity storage.
* GTest unit tests for the wallet's node-identity helpers and the pure
* decision logic behind resolveNodeIdentity().
*
* Three functions share one table, `NodeIdentity`, and the split between them
* is what the telemetry startup order depends on: `readNodeIdentity()` only
* reads, `storeNodeIdentity()` only writes, and `getNodeIdentity()` reads then
* writes a fresh key when the table is empty. `xrpld` resolves its identity
* before the Application exists and persists it later, so the store step has
* to be callable on its own and has to be idempotent-by-read: a second run
* must return the first run's key, not a new one.
* Two groups of functions share the NodeIdentity table:
* - readNodeIdentity() / storeNodeIdentity() / clearNodeIdentity() are the
* wallet-layer primitives xrpld composes at startup.
* - getNodeIdentity(session&) is the read-or-mint helper. It persists a
* freshly minted pair, so a second call after a mint returns the same
* key: that is the property that keeps a node's identity stable across
* restarts.
*
* Each test gets its own database file in a temporary directory, so nothing
* here depends on order or on the developer's data directory.
* parseNodeIdentitySeed() and selectNodeIdentity() are the libxrpl-level
* decision helpers that xrpld's resolveNodeIdentity() marshals its inputs
* into. Every outcome branch of resolveNodeIdentity() reduces to one of
* these two, so testing them here covers the decision tree without an
* xrpld Config.
*
* Each database-backed test gets its own file in a temporary directory, so
* nothing here depends on order or on the developer's data directory.
*/
#include <xrpl/basics/Log.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/protocol/KeyType.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Seed.h>
#include <xrpl/protocol/tokens.h>
#include <xrpl/rdb/DatabaseCon.h>
#include <xrpl/server/Wallet.h>
#include <gtest/gtest.h>
#include <soci/into.h>
#include <soci/session.h>
#include <filesystem>
#include <functional>
#include <memory>
#include <optional>
#include <stdexcept>
#include <string>
#include <system_error>
#include <utility>
using namespace xrpl;
@@ -82,7 +96,7 @@ TEST(WalletNodeIdentity, store_then_read_returns_the_same_pair)
// The store step exists so a key minted before the Application is built
// can be persisted afterwards. Reading it back must give the same pair, or
// the two halves of one run report two identities.
TempWalletDb wallet("store-then-read");
TempWalletDb const wallet("store-then-read");
auto const minted = randomKeyPair(KeyType::Secp256k1);
{
@@ -98,25 +112,32 @@ TEST(WalletNodeIdentity, store_then_read_returns_the_same_pair)
EXPECT_EQ(stored->second, minted.second);
}
TEST(WalletNodeIdentity, store_does_not_replace_an_existing_identity)
TEST(WalletNodeIdentity, store_appends_rather_than_replacing)
{
// getNodeIdentity() is the read-or-mint path and must keep the first key,
// so a restart does not change the node's identity on the network. The
// stored pair wins over anything a later caller offers.
TempWalletDb wallet("no-replace");
auto db = (*wallet).checkoutDb();
auto const first = getNodeIdentity(*db);
auto const other = randomKeyPair(KeyType::Secp256k1);
ASSERT_NE(first.first, other.first)
// Catches storeNodeIdentity being changed into an UPSERT: the wallet
// helper deliberately inserts without clearing, and the caller
// (getNodeIdentity(session&) or resolveNodeIdentity+setup) is what makes
// sure the table is empty first. Two stores must leave two rows.
TempWalletDb const wallet("store-appends");
auto const first = randomKeyPair(KeyType::Secp256k1);
auto const second = randomKeyPair(KeyType::Secp256k1);
ASSERT_NE(first.first, second.first)
<< "the two pairs must differ for this test to mean anything";
storeNodeIdentity(*db, other);
auto db = (*wallet).checkoutDb();
storeNodeIdentity(*db, first);
storeNodeIdentity(*db, second);
int rowCount = 0;
*db << "SELECT COUNT(*) FROM NodeIdentity;", soci::into(rowCount);
EXPECT_EQ(rowCount, 2) << "storeNodeIdentity must not clear the table";
// The read has no ORDER BY, so pin only that ONE of the two stored keys
// comes back -- not which one.
auto const stored = readNodeIdentity(*db);
ASSERT_TRUE(stored.has_value());
EXPECT_EQ(stored->first, first.first);
EXPECT_EQ(getNodeIdentity(*db).first, first.first);
EXPECT_TRUE(stored->first == first.first || stored->first == second.first)
<< "readNodeIdentity must return one of the two stored pairs";
}
TEST(WalletNodeIdentity, clear_then_store_installs_the_new_pair)
@@ -124,7 +145,7 @@ TEST(WalletNodeIdentity, clear_then_store_installs_the_new_pair)
// --newnodeid clears the row and then persists the freshly minted pair.
// Both steps are needed: clearing alone would leave the node with no
// stored identity at all.
TempWalletDb wallet("clear-then-store");
TempWalletDb const wallet("clear-then-store");
auto db = (*wallet).checkoutDb();
auto const first = getNodeIdentity(*db);
@@ -146,7 +167,7 @@ TEST(WalletNodeIdentity, get_mints_and_persists_when_the_table_is_empty)
// The mint path must persist, not just return: a second call has to give
// the same key. This is the property --newnodeid relies on to be
// meaningful, and the one a caller that only reads would break.
TempWalletDb wallet("mint-and-persist");
TempWalletDb const wallet("mint-and-persist");
auto db = (*wallet).checkoutDb();
auto const minted = getNodeIdentity(*db);
@@ -156,3 +177,185 @@ TEST(WalletNodeIdentity, get_mints_and_persists_when_the_table_is_empty)
EXPECT_EQ(stored->first, minted.first);
EXPECT_EQ(getNodeIdentity(*db).first, minted.first);
}
// -----------------------------------------------------------------------------
// parseNodeIdentitySeed()
//
// The seed-parsing half of resolveNodeIdentity(). Each test drives one branch
// of the review's outcome list: cmdline valid, cmdline malformed, config valid,
// config malformed, neither, and cmdline-wins-over-config.
// -----------------------------------------------------------------------------
namespace {
// A base58 seed known to parse (from the "masterpassphrase" node in
// src/test/protocol/Seed_test.cpp).
constexpr auto kValidSeed = "snoPBrXtMeMyMHUVTgbuqAfg1SUTb";
// Public key derived from kValidSeed (secp256k1), same source.
constexpr auto kValidSeedPublic = "n94a1u4jAz288pZLtw6yFWVbi89YamiC6JBXPVUj5zmExe5fTVg9";
// A second base58 seed to prove cmdline-wins-over-config.
constexpr auto kOtherSeed = "snMKnVku798EnBwUfxeSD8953sLYA";
} // namespace
TEST(ParseNodeIdentitySeed, both_absent_returns_nullopt)
{
// Catches replacing the fallthrough with a throw, or making it mint a
// random seed. resolveNodeIdentity() then falls through to the wallet.
EXPECT_FALSE(parseNodeIdentitySeed(std::nullopt, std::nullopt).has_value());
}
TEST(ParseNodeIdentitySeed, valid_cmdline_returns_that_seed)
{
// Catches swapping parseGenericSeed to always return nullopt, or reading
// configSeed instead of cmdlineSeed. Pin the concrete public key derived
// from the seed so the returned optional cannot silently be a different
// valid seed.
auto const seed = parseNodeIdentitySeed(std::string{kValidSeed}, std::nullopt);
ASSERT_TRUE(seed.has_value());
auto const sk = generateSecretKey(KeyType::Secp256k1, *seed);
auto const pk = derivePublicKey(KeyType::Secp256k1, sk);
EXPECT_EQ(toBase58(TokenType::NodePublic, pk), std::string{kValidSeedPublic});
}
TEST(ParseNodeIdentitySeed, valid_config_returns_that_seed)
{
// Catches ignoring the config branch. Same public-key pin as above so the
// result cannot silently drift to another seed.
auto const seed = parseNodeIdentitySeed(std::nullopt, std::string{kValidSeed});
ASSERT_TRUE(seed.has_value());
auto const sk = generateSecretKey(KeyType::Secp256k1, *seed);
auto const pk = derivePublicKey(KeyType::Secp256k1, sk);
EXPECT_EQ(toBase58(TokenType::NodePublic, pk), std::string{kValidSeedPublic});
}
TEST(ParseNodeIdentitySeed, malformed_cmdline_throws)
{
// Catches removing the throw. An empty string flunks parseGenericSeed(
// rfc1751=false) because the first check inside is str.empty(). A base58
// public key would too, but empty is the shorter probe.
EXPECT_THROW(
static_cast<void>(parseNodeIdentitySeed(std::string{}, std::nullopt)), std::runtime_error);
}
TEST(ParseNodeIdentitySeed, malformed_config_throws)
{
// Catches removing the throw. "garbage" is neither valid base58 nor a
// valid seed encoding, so parseBase58<Seed> returns nullopt and the
// config branch throws.
EXPECT_THROW(
static_cast<void>(parseNodeIdentitySeed(std::nullopt, std::string{"garbage"})),
std::runtime_error);
}
TEST(ParseNodeIdentitySeed, cmdline_wins_over_config)
{
// Catches swapping the two if-branches. Passing DIFFERENT valid seeds on
// each input and asserting the returned seed derives to kValidSeed's
// public key proves which one won.
auto const seed = parseNodeIdentitySeed(std::string{kValidSeed}, std::string{kOtherSeed});
ASSERT_TRUE(seed.has_value());
auto const sk = generateSecretKey(KeyType::Secp256k1, *seed);
auto const pk = derivePublicKey(KeyType::Secp256k1, sk);
EXPECT_EQ(toBase58(TokenType::NodePublic, pk), std::string{kValidSeedPublic});
}
// -----------------------------------------------------------------------------
// selectNodeIdentity()
//
// The decision half of resolveNodeIdentity(). Every remaining branch from the
// review's outcome list reduces to one of these four cases, because
// storedIdentity() catches its own exceptions and returns std::nullopt for
// every failure mode (standalone non-Load, wallet absent, invalid row, read
// throws) -- see storedIdentity() in NodeIdentity.cpp.
// -----------------------------------------------------------------------------
namespace {
// Reader that records whether it was called. resolveNodeIdentity()'s reader
// is a filesystem-touching lambda, so pinning "was it consulted" catches the
// mutations that flip which cases open the wallet.
struct TrackingReader
{
bool called{false};
std::optional<std::pair<PublicKey, SecretKey>> value;
std::function<std::optional<std::pair<PublicKey, SecretKey>>()>
fn()
{
return [this] {
called = true;
return value;
};
}
};
} // namespace
TEST(SelectNodeIdentity, seed_wins_and_reader_not_consulted)
{
// Catches removing the seed branch (or checking newNodeId first). The
// returned pair must be keysFromSeed(kValidSeed); if the seed branch is
// gone the reader gets called and its recorded pair or a fresh mint
// comes back instead.
TrackingReader reader;
reader.value = randomKeyPair(KeyType::Secp256k1);
auto const seed = parseBase58<Seed>(std::string{kValidSeed});
ASSERT_TRUE(seed.has_value());
auto const result = selectNodeIdentity(seed, /*newNodeId=*/false, reader.fn());
EXPECT_FALSE(reader.called) << "the reader must not run when a seed is configured";
EXPECT_EQ(toBase58(TokenType::NodePublic, result.first), std::string{kValidSeedPublic});
}
TEST(SelectNodeIdentity, newnodeid_mints_fresh_and_skips_reader)
{
// Catches removing the `!newNodeId` guard. With a stored pair available,
// the mint path must still run and the stored pair must not come back.
TrackingReader reader;
reader.value = randomKeyPair(KeyType::Secp256k1);
auto const storedPair = *reader.value;
auto const result = selectNodeIdentity(std::nullopt, /*newNodeId=*/true, reader.fn());
EXPECT_FALSE(reader.called) << "--newnodeid must not open the wallet";
EXPECT_NE(result.first, storedPair.first) << "the stored pair must be discarded";
}
TEST(SelectNodeIdentity, returns_stored_when_reader_has_one)
{
// Catches replacing the stored-return with a mint. Also catches the
// reader being called but its result discarded.
TrackingReader reader;
reader.value = randomKeyPair(KeyType::Secp256k1);
auto const storedPair = *reader.value;
auto const result = selectNodeIdentity(std::nullopt, /*newNodeId=*/false, reader.fn());
EXPECT_TRUE(reader.called);
EXPECT_EQ(result.first, storedPair.first);
EXPECT_EQ(result.second, storedPair.second);
}
TEST(SelectNodeIdentity, mints_when_nothing_stored)
{
// Catches removing the mint fallback. With the reader returning
// nullopt, selectNodeIdentity must still produce a keypair, and it must
// differ from anything it could have accidentally reused.
TrackingReader reader; // value stays std::nullopt.
auto const result = selectNodeIdentity(std::nullopt, /*newNodeId=*/false, reader.fn());
EXPECT_TRUE(reader.called);
// The mint path is randomKeyPair(), so the two calls must yield distinct
// keys. Same probe the wallet-side tests use.
auto const another = randomKeyPair(KeyType::Secp256k1);
EXPECT_NE(result.first, another.first);
}

82
src/xrpld/app/main/NodeIdentity.cpp
View File

@@ -4,15 +4,11 @@
#include <xrpld/core/Config.h>
#include <xrpl/basics/Log.h>
#include <xrpl/basics/contract.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/config/Constants.h>
#include <xrpl/core/StartUpType.h>
#include <xrpl/protocol/KeyType.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Seed.h>
#include <xrpl/protocol/tokens.h>
#include <xrpl/rdb/DBInit.h>
#include <xrpl/rdb/DatabaseCon.h>
#include <xrpl/server/Wallet.h>
@@ -23,7 +19,6 @@
#include <exception>
#include <filesystem>
#include <optional>
#include <stdexcept>
#include <string>
#include <system_error>
#include <utility>
@@ -32,53 +27,6 @@ namespace xrpl {
namespace {
/**
* The seed a configured `[node_seed]` or `--nodeid` names.
*
* @param config The server configuration.
* @param cmdline The command line parameters passed into the application.
* @return The seed, or std::nullopt when neither is configured.
* @throws std::runtime_error if the configured value is malformed.
*/
std::optional<Seed>
configuredSeed(Config const& config, boost::program_options::variables_map const& cmdline)
{
if (cmdline.contains("nodeid"))
{
auto seed = parseGenericSeed(cmdline["nodeid"].as<std::string>(), false);
if (!seed)
Throw<std::runtime_error>("Invalid 'nodeid' in command line");
return seed;
}
if (config.exists(Sections::kNodeSeed))
{
auto const& lines = config.section(Sections::kNodeSeed).lines();
auto seed = lines.empty() ? std::nullopt : parseBase58<Seed>(lines.front());
if (!seed)
{
Throw<std::runtime_error>(
std::string("Invalid [") + Sections::kNodeSeed + "] in configuration file");
}
return seed;
}
return std::nullopt;
}
/**
* The keypair a seed defines.
*
* @param seed The configured seed.
* @return The derived secp256k1 keypair.
*/
std::pair<PublicKey, SecretKey>
keysFromSeed(Seed const& seed)
{
auto const secretKey = generateSecretKey(KeyType::Secp256k1, seed);
return {derivePublicKey(KeyType::Secp256k1, secretKey), secretKey};
}
/**
* The stored identity, read without creating or modifying anything.
*
@@ -138,22 +86,28 @@ resolveNodeIdentity(
boost::program_options::variables_map const& cmdline,
beast::Journal journal)
{
// A configured seed decides the identity outright, and nothing is stored.
if (auto const seed = configuredSeed(config, cmdline))
return keysFromSeed(*seed);
// Marshal Config and the cmdline into the libxrpl-level primitives the
// decision helpers take. Keeping the decision in libxrpl lets its tests
// cover every branch without an xrpld Config.
std::optional<std::string> cmdlineSeed;
if (cmdline.contains("nodeid"))
cmdlineSeed = cmdline["nodeid"].as<std::string>();
// --newnodeid discards whatever is stored, so mint now; getNodeIdentity()
// clears the old row and stores this pair.
if (!cmdline.contains("newnodeid"))
std::optional<std::string> configSeedLine;
if (config.exists(Sections::kNodeSeed))
{
if (auto const stored = storedIdentity(config, journal))
return *stored;
auto const& lines = config.section(Sections::kNodeSeed).lines();
// Present-but-empty stays as an empty string, so parseNodeIdentitySeed
// throws the same "invalid [node_seed]" error the old code did.
configSeedLine = lines.empty() ? std::string{} : lines.front();
}
// Nothing to read: a first boot, or a standalone run's temporary database.
// Mint here so telemetry has an identity from construction; setup()
// persists this pair if there is a database to hold it.
return randomKeyPair(KeyType::Secp256k1);
auto const seed = parseNodeIdentitySeed(cmdlineSeed, configSeedLine);
bool const newNodeId = cmdline.contains("newnodeid");
// storedIdentity() catches its own exceptions and returns std::nullopt on
// any read failure, so a wallet that will not open collapses into "mint".
return selectNodeIdentity(seed, newNodeId, [&] { return storedIdentity(config, journal); });
}
std::pair<PublicKey, SecretKey>