From fcdbbabedfd0ee6f41ae5f3319205fc08ddd3056 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 26 Aug 2026 19:48:38 +0100 Subject: [PATCH] perf(rpc): hash pathfind accounts only when the span records them doPathFind and doRipplePathFind fill two span attributes from the request's source and destination accounts. Both values are call arguments, so they are built whatever the build: asString() copies the address out of the JSON and redactAccount() takes a SHA-512Half over it and formats 16 hex characters. That is two copies and two hashes on every pathfinding RPC, for pathfind_source_account and pathfind_dest_account, which nothing outside the span reads. Wrapping the block in "if (span)" drops that work when telemetry is compiled out, where the guard's operator bool() is a literal false, and also when telemetry is on but this span is not being recorded. The const-reference read of context.params moves inside the guard with the code that needs it, so a telemetry read still never inserts a null into the request. --- src/xrpld/rpc/handlers/orderbook/PathFind.cpp | 31 +++++++++++++------ .../rpc/handlers/orderbook/RipplePathFind.cpp | 31 +++++++++++++------ 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp index 6da4f2cdca..37a451142b 100644 --- a/src/xrpld/rpc/handlers/orderbook/PathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/PathFind.cpp @@ -25,16 +25,27 @@ doPathFind(rpc::JsonContext& context) // thread) nest under it. doPathFind does not yield, so scoping is safe. auto span = ScopedSpanGuard( TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request); - // Addresses are hashed before emission for privacy. Read through a const - // reference: the non-const json::Value::operator[] inserts a null for a - // missing key, which would make PathRequest::parseJson's isMember() checks - // see an absent field as present and return Malformed instead of Missing. - // Reading for telemetry must not alter what the request looks like. - auto const& params = std::as_const(context.params); - if (auto const& src = params[jss::source_account]; src.isString()) - span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); - if (auto const& dst = params[jss::destination_account]; dst.isString()) - span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + // Guarded on the span being live because setAttribute's arguments are + // evaluated whatever the build, and neither is free: asString() copies the + // address out of the JSON and redactAccount() takes a SHA-512Half over it. + // That is two copies and two hashes on every path_find call. The + // compiled-out guard's operator bool() is a literal false, so the block + // disappears entirely in that build; with telemetry compiled in it is + // skipped whenever this span is not being recorded. + if (span) + { + // Addresses are hashed before emission for privacy. Read through a + // const reference: the non-const json::Value::operator[] inserts a null + // for a missing key, which would make PathRequest::parseJson's + // isMember() checks see an absent field as present and return Malformed + // instead of Missing. Reading for telemetry must not alter what the + // request looks like. + auto const& params = std::as_const(context.params); + if (auto const& src = params[jss::source_account]; src.isString()) + span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); + if (auto const& dst = params[jss::destination_account]; dst.isString()) + span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + } if (context.app.config().pathSearchMax == 0) return rpcError(RpcNotSupported); diff --git a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp index 1b5e881d2b..2444e090a3 100644 --- a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp @@ -34,16 +34,27 @@ doRipplePathFind(rpc::JsonContext& context) // span's log lines stay trace-correlated. auto span = ScopedSpanGuard( TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request); - // Addresses are hashed before emission for privacy. Read through a const - // reference: the non-const json::Value::operator[] inserts a null for a - // missing key, which would make PathRequest::parseJson's isMember() checks - // see an absent field as present and return Malformed instead of Missing. - // Reading for telemetry must not alter what the request looks like. - auto const& params = std::as_const(context.params); - if (auto const& src = params[jss::source_account]; src.isString()) - span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); - if (auto const& dst = params[jss::destination_account]; dst.isString()) - span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + // Guarded on the span being live because setAttribute's arguments are + // evaluated whatever the build, and neither is free: asString() copies the + // address out of the JSON and redactAccount() takes a SHA-512Half over it. + // That is two copies and two hashes on every ripple_path_find call. The + // compiled-out guard's operator bool() is a literal false, so the block + // disappears entirely in that build; with telemetry compiled in it is + // skipped whenever this span is not being recorded. + if (span) + { + // Addresses are hashed before emission for privacy. Read through a + // const reference: the non-const json::Value::operator[] inserts a null + // for a missing key, which would make PathRequest::parseJson's + // isMember() checks see an absent field as present and return Malformed + // instead of Missing. Reading for telemetry must not alter what the + // request looks like. + auto const& params = std::as_const(context.params); + if (auto const& src = params[jss::source_account]; src.isString()) + span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString())); + if (auto const& dst = params[jss::destination_account]; dst.isString()) + span.setAttribute(pathfind_span::attr::destAccount, redactAccount(dst.asString())); + } if (context.app.config().pathSearchMax == 0) return rpcError(RpcNotSupported);