code review changes

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2026-07-06 20:37:08 +01:00
parent e129b0793c
commit 7a6ff619e5
13 changed files with 195 additions and 33 deletions

View File

@@ -47,6 +47,7 @@ libxrpl.shamap > xrpl.protocol
libxrpl.shamap > xrpl.shamap
libxrpl.telemetry > xrpl.basics
libxrpl.telemetry > xrpl.config
libxrpl.telemetry > xrpl.protocol
libxrpl.telemetry > xrpl.telemetry
libxrpl.tx > xrpl.basics
libxrpl.tx > xrpl.conditions

View File

@@ -378,32 +378,46 @@ The following data is explicitly **excluded** from telemetry collection:
#### Privacy Protection Mechanisms
| Mechanism | Description |
| ----------------------------- | ------------------------------------------------------------------------- |
| **Account Hashing** | `tx_account` is hashed at collector level before storage |
| **Configurable Redaction** | Sensitive fields can be excluded via `[telemetry]` config section |
| **Sampling** | Only 10% of traces recorded by default, reducing data exposure |
| **Local Control** | Node operators have full control over what gets exported |
| **No Raw Payloads** | Transaction content is never recorded, only metadata (hash, type, result) |
| **Collector-Level Filtering** | Additional redaction/hashing can be configured at OTel Collector |
| Mechanism | Description |
| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| **Account Hashing** | Account addresses are hashed both SDK-side (`pathfind_source_account`, `pathfind_dest_account` — always hashed before emission) and again at the collector level, so raw addresses never reach storage |
| **Sampling** | Only 10% of traces recorded by default, reducing data exposure |
| **Local Control** | Node operators have full control over what gets exported |
| **No Raw Payloads** | Transaction content is never recorded, only metadata (hash, type, result) |
| **Collector-Level Filtering** | Additional redaction/hashing can be configured at OTel Collector |
#### Account Address Hashing
Account addresses are **always** hashed before they reach the telemetry
backend — there is no opt-out flag and therefore no insecure-by-default
failure mode. Protection is applied in two independent layers:
1. **SDK-side** (this node): the path-finding RPC handlers call
`redactAccount()` (`xrpl::telemetry`, `Redaction.h`) before setting the
`pathfind_source_account` / `pathfind_dest_account` span attributes. The
helper emits the first 16 characters of `sha512Half(address)` as
lowercase hex — deterministic (spans for one account still correlate)
but non-reversible.
2. **Collector-side** (defense-in-depth): an `attributes/hash` processor in
the OpenTelemetry Collector re-hashes those same attributes, so any node
that emitted a raw value is still redacted before storage.
#### Collector-Level Data Protection
The OpenTelemetry Collector can be configured (via an `attributes` processor)
to hash or redact sensitive attributes before export — for example, hashing
`tx_account`, deleting `peer_address` to drop IP addresses, and deleting
`params` to redact request parameters.
`pathfind_source_account` / `pathfind_dest_account`, deleting `peer_address`
to drop IP addresses, and deleting `params` to redact request parameters.
#### Configuration Options for Privacy
In `xrpld.cfg`, operators control data collection granularity through the
`[telemetry]` section. Besides `enabled`, per-component toggles
(`trace_transactions`, `trace_consensus`, `trace_rpc`, `trace_peer` — the last
often disabled due to high volume) select which spans are emitted, and
redaction flags (`redact_account` to hash account addresses, `redact_peer_address`
to remove peer IP addresses) control SDK-level redaction before export.
> **Note**: The `redact_account` configuration in `xrpld.cfg` controls SDK-level redaction before export, while collector-level filtering (see [Collector-Level Data Protection](#collector-level-data-protection) above) provides an additional defense-in-depth layer. Both can operate independently.
often disabled due to high volume) select which spans are emitted. Account
address hashing is not configurable: addresses are hashed unconditionally by
the SDK helper described above, with collector-level hashing as a second
layer.
> **Key Principle**: Telemetry collects **operational metadata** (timing, counts, hashes) — never **sensitive content** (keys, balances, amounts, raw payloads).

View File

@@ -169,11 +169,12 @@ Node health (`amendment_blocked`, `server_state`) is not part of the telemetry s
| `rpc.process` | `batch_size` | int64 | `params.size()` (only when batch) |
| `rpc.ws_message` | `command` | string | `jv[command]` or `jv[method]` |
| `rpc.command.*` | `load_type` | string | `context.loadType.label()` |
| `pathfind.compute` | `pathfind_dest_amount` | string | `saDstAmount_.getFullText()` |
| `pathfind.compute` | `pathfind_dest_currency` | string | `to_string(saDstAmount_.asset())` |
| `pathfind.discover` | `pathfind_num_source_assets` | int64 | `sourceAssets.size()` |
**New attr keys**: `RpcSpanNames.h` (`isBatch`, `batchSize`, `loadType`), `PathFindSpanNames.h` (`destAmount`, `destCurrency`, `numSourceAssets`).
_Note: `pathfind_dest_amount` was removed — the destination amount is a financial value excluded by the privacy policy (design §2.4.4)._
**New attr keys**: `RpcSpanNames.h` (`isBatch`, `batchSize`, `loadType`), `PathFindSpanNames.h` (`destCurrency`, `numSourceAssets`).
**Modified files**:

View File

@@ -206,10 +206,13 @@ target_link_libraries(xrpl.libxrpl.tx PUBLIC xrpl.libxrpl.ledger)
# When telemetry=ON, links the Conan-provided umbrella target
# opentelemetry-cpp::opentelemetry-cpp (individual component targets like
# ::api, ::sdk are not available in the Conan package).
#
# Links xrpl.libxrpl.protocol PRIVATELY for sha512Half (digest.h)
add_module(xrpl telemetry)
target_link_libraries(
xrpl.libxrpl.telemetry
PUBLIC xrpl.libxrpl.basics xrpl.libxrpl.beast xrpl.libxrpl.config
PRIVATE xrpl.libxrpl.protocol
)
if(telemetry)
target_link_libraries(

View File

@@ -48,6 +48,15 @@ processors:
action: delete
- key: telemetry.sdk.version
action: delete
# Defense-in-depth: hash path-finding account attributes. The xrpld SDK
# already hashes these before export, but a node that emitted raw values
# is caught here so raw addresses never reach the backend.
attributes/hash:
actions:
- key: pathfind_source_account
action: hash
- key: pathfind_dest_account
action: hash
exporters:
debug:
@@ -66,5 +75,5 @@ service:
pipelines:
traces:
receivers: [otlp]
processors: [resource/tier, resource/stripsdk, batch]
processors: [resource/tier, resource/stripsdk, attributes/hash, batch]
exporters: [debug, otlp/tempo]

View File

@@ -0,0 +1,56 @@
#pragma once
/** Account-address redaction for telemetry span attributes.
Path-finding RPC handlers would otherwise emit the caller's raw
account addresses as span attributes. To keep addresses out of the
telemetry backend, they are hashed at the point of emission. This
header exposes a single pure helper that turns an address into a
short, stable, non-reversible token.
Data flow:
handler -> redactAccount(addr) -> span attribute -> OTLP export
The returned token is the first 16 hex characters (lowercase) of the
SHA-512Half digest of the address. It is deterministic (same address
always maps to the same token) so operators can still correlate spans
for a given account, but the original address cannot be recovered.
A second, independent hashing layer runs in the OpenTelemetry
Collector (an `attributes/hash` processor) as defense-in-depth for
any node that emits a raw value.
@note This function is pure and reentrant: it holds no global state,
performs no I/O, and is safe to call concurrently from any thread.
Usage example:
@code
#include <xrpl/telemetry/Redaction.h>
using namespace xrpl::telemetry;
span.setAttribute(
pathfind_span::attr::sourceAccount, redactAccount(src.asString()));
@endcode
Edge case (empty input yields empty output):
@code
assert(redactAccount("") == "");
@endcode
*/
#include <string>
#include <string_view>
namespace xrpl::telemetry {
/** Hash an account address into a short, stable, non-reversible token.
@param addr The account address to redact (e.g. an r-address).
@return The first 16 lowercase hex characters of sha512Half(addr),
or an empty string when @p addr is empty.
*/
[[nodiscard]] std::string
redactAccount(std::string_view addr);
} // namespace xrpl::telemetry

View File

@@ -0,0 +1,32 @@
#include <xrpl/telemetry/Redaction.h>
#include <xrpl/basics/Slice.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/protocol/digest.h>
#include <algorithm>
#include <cctype>
#include <string>
#include <string_view>
namespace xrpl::telemetry {
std::string
redactAccount(std::string_view addr)
{
// Empty in, empty out: nothing to hash and no token to emit.
if (addr.empty())
return {};
// sha512Half yields a uint256; to_string renders it as uppercase hex.
// Keep the first 16 chars (64 bits) — enough to correlate spans while
// staying non-reversible — and lowercase them for a stable token.
auto const digest = sha512Half(Slice(addr.data(), addr.size()));
std::string token = to_string(digest).substr(0, 16);
std::transform(token.begin(), token.end(), token.begin(), [](unsigned char c) {
return static_cast<char>(std::tolower(c));
});
return token;
}
} // namespace xrpl::telemetry

View File

@@ -0,0 +1,44 @@
#include <xrpl/telemetry/Redaction.h>
#include <gtest/gtest.h>
#include <algorithm>
#include <cctype>
#include <string>
using namespace xrpl;
using namespace xrpl::telemetry;
// Empty input must produce empty output (edge / negative path).
TEST(Redaction, empty_input_returns_empty)
{
EXPECT_EQ(redactAccount(""), "");
}
// Success path: assert the EXACT expected token. The value is the first
// 16 chars of sha512Half(addr) rendered as lowercase hex, computed once
// and hard-coded here so any change to the hashing recipe is caught.
TEST(Redaction, known_account_exact_hash)
{
EXPECT_EQ(redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"), "a513895f49311f54");
}
// Structural invariants: length, lowercase-hex alphabet, and stability.
TEST(Redaction, token_is_16_lowercase_hex_and_stable)
{
auto const token = redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh");
EXPECT_EQ(token.size(), 16u);
EXPECT_TRUE(std::all_of(token.begin(), token.end(), [](unsigned char c) {
return std::isdigit(c) || (c >= 'a' && c <= 'f');
}));
// Deterministic: hashing the same input twice yields the same token.
EXPECT_EQ(token, redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"));
}
// Distinct inputs must produce distinct tokens (no accidental constant).
TEST(Redaction, distinct_inputs_distinct_tokens)
{
EXPECT_NE(
redactAccount("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"),
redactAccount("rDifferentAccount1234567890abcdefgh"));
}

View File

@@ -84,8 +84,6 @@ inline constexpr auto numPaths = makeStr("pathfind_num_paths");
inline constexpr auto numRequests = makeStr("pathfind_num_requests");
/// "pathfind_ledger_index" — pathfind target ledger index.
inline constexpr auto ledgerIndex = makeStr("pathfind_ledger_index");
/// "pathfind_dest_amount" — requested destination amount as string.
inline constexpr auto destAmount = makeStr("pathfind_dest_amount");
/// "pathfind_dest_currency" — destination currency code.
inline constexpr auto destCurrency = makeStr("pathfind_dest_currency");
/// "pathfind_num_source_assets" — candidate source assets count.

View File

@@ -742,7 +742,6 @@ PathRequest::doUpdate(
auto span = SpanGuard::span(
TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::compute);
span.setAttribute(pathfind_span::attr::fast, fast);
span.setAttribute(pathfind_span::attr::destAmount, saDstAmount_.getFullText().c_str());
span.setAttribute(pathfind_span::attr::destCurrency, to_string(saDstAmount_.asset()).c_str());
JLOG(journal_.debug()) << iIdentifier_ << " update " << (fast ? "fast" : "normal");

View File

@@ -73,15 +73,16 @@ PathRequestManager::updateAll(std::shared_ptr<ReadView const> const& inLedger)
cache = getAssetCache(inLedger, true);
}
// updateAll runs on every ledger close; skip span emission entirely when
// there are no active path subscriptions to avoid a steady stream of empty
// spans at mainnet close cadence.
if (requests.empty())
return;
using namespace telemetry;
auto span = SpanGuard::span(
TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::updateAll);
// updateAll runs on every ledger close. Skip span emission when there are
// no active path subscriptions, to avoid a steady stream of empty spans at
// mainnet close cadence. A null guard is used in that case; all other work
// still runs unchanged (notably the isNewPathRequest() flag reset below),
// so behaviour matches the pre-span code path.
auto span = requests.empty()
? SpanGuard{}
: SpanGuard::span(
TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::updateAll);
span.setAttribute(pathfind_span::attr::ledgerIndex, static_cast<int64_t>(inLedger->seq()));
span.setAttribute(pathfind_span::attr::numRequests, static_cast<int64_t>(requests.size()));

View File

@@ -10,6 +10,7 @@
#include <xrpl/protocol/jss.h>
#include <xrpl/resource/Fees.h>
#include <xrpl/server/InfoSub.h>
#include <xrpl/telemetry/Redaction.h>
#include <xrpl/telemetry/SpanGuard.h>
namespace xrpl {
@@ -20,10 +21,11 @@ doPathFind(RPC::JsonContext& context)
using namespace telemetry;
auto span = SpanGuard::span(
TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request);
// Addresses are hashed before emission for privacy.
if (auto const& src = context.params[jss::source_account]; src.isString())
span.setAttribute(pathfind_span::attr::sourceAccount, src.asString());
span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString()));
if (auto const& dst = context.params[jss::destination_account]; dst.isString())
span.setAttribute(pathfind_span::attr::destAccount, dst.asString());
span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString()));
if (context.app.config().pathSearchMax == 0)
return rpcError(RpcNotSupported);

View File

@@ -14,6 +14,7 @@
#include <xrpl/protocol/RPCErr.h>
#include <xrpl/protocol/jss.h>
#include <xrpl/resource/Fees.h>
#include <xrpl/telemetry/Redaction.h>
#include <xrpl/telemetry/SpanGuard.h>
#include <memory>
@@ -28,10 +29,11 @@ doRipplePathFind(RPC::JsonContext& context)
using namespace telemetry;
auto span = SpanGuard::span(
TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request);
// Addresses are hashed before emission for privacy.
if (auto const& src = context.params[jss::source_account]; src.isString())
span.setAttribute(pathfind_span::attr::sourceAccount, src.asString());
span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString()));
if (auto const& dst = context.params[jss::destination_account]; dst.isString())
span.setAttribute(pathfind_span::attr::destAccount, dst.asString());
span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString()));
if (context.app.config().pathSearchMax == 0)
return rpcError(RpcNotSupported);