From f18ddd95c12f3d6e4a7b077fd89c096381e2c55b Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 29 Apr 2026 19:54:23 +0100 Subject: [PATCH 1/2] fix(telemetry): address clang-tidy errors on phase4 consensus tracing files - Add [[nodiscard]] to getConsensusTraceStrategy, getYays, getNays - Add missing , SpanGuard.h, SpanNames.h includes - Fix widening cast placement (cast before arithmetic, not after) - Replace nested ternary with lambda for const dir variable - Add braces to if/else-if chains in Consensus.h - Concatenate nested namespaces in ConsensusSpanNames.h Co-Authored-By: Claude Opus 4.6 --- include/xrpl/telemetry/Telemetry.h | 2 +- src/libxrpl/telemetry/NullTelemetry.cpp | 3 ++- .../libxrpl/telemetry/SpanGuardFactory.cpp | 1 + src/xrpld/app/consensus/RCLConsensus.cpp | 24 +++++++++++++------ src/xrpld/consensus/Consensus.h | 6 +++++ src/xrpld/consensus/ConsensusSpanNames.h | 8 ++----- src/xrpld/consensus/DisputedTx.h | 4 ++-- 7 files changed, 31 insertions(+), 17 deletions(-) diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h index 92f87f7a70..1b965345a9 100644 --- a/include/xrpl/telemetry/Telemetry.h +++ b/include/xrpl/telemetry/Telemetry.h @@ -252,7 +252,7 @@ public: shouldTraceLedger() const = 0; /** @return The configured consensus trace correlation strategy. */ - virtual std::string const& + [[nodiscard]] virtual std::string const& getConsensusTraceStrategy() const = 0; #ifdef XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/NullTelemetry.cpp b/src/libxrpl/telemetry/NullTelemetry.cpp index a957330a1a..8283ff97bc 100644 --- a/src/libxrpl/telemetry/NullTelemetry.cpp +++ b/src/libxrpl/telemetry/NullTelemetry.cpp @@ -18,6 +18,7 @@ #endif #include +#include #include namespace xrpl::telemetry { @@ -87,7 +88,7 @@ public: return false; } - std::string const& + [[nodiscard]] std::string const& getConsensusTraceStrategy() const override { return setup_.consensusTraceStrategy; diff --git a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp index 8567b61d82..aa935b2151 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardFactory.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include using namespace xrpl; diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index dffdc9c8bc..b479c5a105 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -63,6 +63,8 @@ #include #include #include +#include +#include #include #include @@ -357,7 +359,7 @@ RCLConsensus::Adaptor::onClose( telemetry::cons_span::op::ledgerClose); span.setAttribute( telemetry::cons_span::attr::ledgerSeq, - static_cast(ledger.ledger_->header().seq + 1)); + static_cast(ledger.ledger_->header().seq) + 1); span.setAttribute(telemetry::cons_span::attr::mode, toDisplayString(mode).c_str()); bool const wrongLCL = mode == ConsensusMode::wrongLedger; @@ -556,7 +558,7 @@ RCLConsensus::Adaptor::doAccept( ? acceptSpan->childSpan(telemetry::cons_span::acceptApply) : telemetry::SpanGuard::childSpan(telemetry::cons_span::acceptApply, roundSpanContext_); doAcceptSpan.setAttribute( - telemetry::cons_span::attr::ledgerSeq, static_cast(prevLedger.seq() + 1)); + telemetry::cons_span::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( telemetry::cons_span::attr::closeTime, static_cast(consensusCloseTime.time_since_epoch().count())); @@ -582,9 +584,17 @@ RCLConsensus::Adaptor::doAccept( static_cast(rawCloseTimes.peers.size())); { auto const prevRes = prevLedger.closeTimeResolution(); - std::string dir = (closeResolution > prevRes) ? "increased" - : (closeResolution < prevRes) ? "decreased" - : "unchanged"; + auto const dir = [&]() -> std::string { + if (closeResolution > prevRes) + { + return "increased"; + } + if (closeResolution < prevRes) + { + return "decreased"; + } + return "unchanged"; + }(); doAcceptSpan.setAttribute(telemetry::cons_span::attr::resolutionDirection, std::move(dir)); } @@ -1218,10 +1228,10 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr) return; roundSpan_->setAttribute(cons_span::attr::ledgerId, to_string(prevLgr.id()).c_str()); - roundSpan_->setAttribute(cons_span::attr::ledgerSeq, static_cast(prevLgr.seq() + 1)); + roundSpan_->setAttribute(cons_span::attr::ledgerSeq, static_cast(prevLgr.seq()) + 1); roundSpan_->setAttribute(cons_span::attr::mode, toDisplayString(mode_.load()).c_str()); roundSpan_->setAttribute(cons_span::attr::traceStrategy, strategy.c_str()); - roundSpan_->setAttribute(cons_span::attr::roundId, static_cast(prevLgr.seq() + 1)); + roundSpan_->setAttribute(cons_span::attr::roundId, static_cast(prevLgr.seq()) + 1); roundSpanContext_ = roundSpan_->captureContext(); } diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index a32cdd2c0c..f6bb1ecb22 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -1801,11 +1801,17 @@ Consensus::haveConsensus(std::unique_ptr const& clog char const* stateStr = "no"; if (result_->state == ConsensusState::Yes) + { stateStr = "yes"; + } else if (result_->state == ConsensusState::MovedOn) + { stateStr = "moved_on"; + } else if (result_->state == ConsensusState::Expired) + { stateStr = "expired"; + } span.setAttribute(cons_span::attr::result, stateStr); CLOG(clog) << "Consensus has been reached. "; diff --git a/src/xrpld/consensus/ConsensusSpanNames.h b/src/xrpld/consensus/ConsensusSpanNames.h index 868f730860..b3112af1ec 100644 --- a/src/xrpld/consensus/ConsensusSpanNames.h +++ b/src/xrpld/consensus/ConsensusSpanNames.h @@ -78,9 +78,7 @@ #include -namespace xrpl { -namespace telemetry { -namespace cons_span { +namespace xrpl::telemetry::cons_span { // ===== Span name segments ==================================================== @@ -240,6 +238,4 @@ inline constexpr auto decreased = makeStr("decreased"); inline constexpr auto unchanged = makeStr("unchanged"); } // namespace val -} // namespace cons_span -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry::cons_span diff --git a/src/xrpld/consensus/DisputedTx.h b/src/xrpld/consensus/DisputedTx.h index 2629feef5e..fcd2b0ebac 100644 --- a/src/xrpld/consensus/DisputedTx.h +++ b/src/xrpld/consensus/DisputedTx.h @@ -177,14 +177,14 @@ public: getJson() const; //! Number of peers voting yes. - int + [[nodiscard]] int getYays() const { return yays_; } //! Number of peers voting no. - int + [[nodiscard]] int getNays() const { return nays_; From cf18032e7f55e1adc7e7d0b130f98d0a584073d7 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 29 Apr 2026 19:52:01 +0100 Subject: [PATCH 2/2] fix(telemetry): address clang-tidy errors on phase3 transaction tracing files - Add [[maybe_unused]] to RAII span variables in TxQ.cpp - Remove unused st.h include, add missing to_string header in TxQ.cpp - Concatenate nested namespaces in TxQSpanNames.h, TxSpanNames.h, ConsensusReceiveTracing.h, PropagationHelpers.h, TxTracing.h - Remove unused TraceContextPropagator.h include from RCLConsensus.cpp Co-Authored-By: Claude Opus 4.6 --- src/xrpld/app/consensus/RCLConsensus.cpp | 2 +- src/xrpld/app/misc/TxSpanNames.h | 8 ++------ src/xrpld/app/misc/detail/TxQ.cpp | 6 +++--- src/xrpld/app/misc/detail/TxQSpanNames.h | 8 ++------ src/xrpld/telemetry/ConsensusReceiveTracing.h | 6 ++---- src/xrpld/telemetry/PropagationHelpers.h | 6 ++---- src/xrpld/telemetry/TxTracing.h | 6 ++---- 7 files changed, 14 insertions(+), 28 deletions(-) diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 4a50cc696c..eb040f231f 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -62,7 +62,7 @@ #include #include #include -#include +#include // IWYU pragma: keep #include diff --git a/src/xrpld/app/misc/TxSpanNames.h b/src/xrpld/app/misc/TxSpanNames.h index 2cfd6527d0..091213959e 100644 --- a/src/xrpld/app/misc/TxSpanNames.h +++ b/src/xrpld/app/misc/TxSpanNames.h @@ -17,9 +17,7 @@ #include -namespace xrpl { -namespace telemetry { -namespace tx_span { +namespace xrpl::telemetry::tx_span { // ===== Span prefixes ======================================================= @@ -72,6 +70,4 @@ inline constexpr auto async = makeStr("async"); inline constexpr auto knownBad = makeStr("known_bad"); } // namespace val -} // namespace tx_span -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry::tx_span diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 32842ab9ad..e263eba22a 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -30,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -532,7 +532,7 @@ TxQ::tryClearAccountQueueUpThruTx( beast::Journal j) { using namespace telemetry; - auto span = SpanGuard::span( + [[maybe_unused]] auto span = SpanGuard::span( TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::batchClear); SeqProxy const tSeqProx{tx.getSeqProxy()}; @@ -1681,7 +1681,7 @@ TxQ::tryDirectApply( beast::Journal j) { using namespace telemetry; - auto span = SpanGuard::span( + [[maybe_unused]] auto span = SpanGuard::span( TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::applyDirect); auto const account = (*tx)[sfAccount]; diff --git a/src/xrpld/app/misc/detail/TxQSpanNames.h b/src/xrpld/app/misc/detail/TxQSpanNames.h index 6989674341..4f18e82ae7 100644 --- a/src/xrpld/app/misc/detail/TxQSpanNames.h +++ b/src/xrpld/app/misc/detail/TxQSpanNames.h @@ -48,9 +48,7 @@ #include -namespace xrpl { -namespace telemetry { -namespace txq_span { +namespace xrpl::telemetry::txq_span { // ===== Span prefixes ======================================================= @@ -110,6 +108,4 @@ inline constexpr auto failed = makeStr("failed"); inline constexpr auto retried = makeStr("retried"); } // namespace val -} // namespace txq_span -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry::txq_span diff --git a/src/xrpld/telemetry/ConsensusReceiveTracing.h b/src/xrpld/telemetry/ConsensusReceiveTracing.h index a53f2685f8..6da9900fab 100644 --- a/src/xrpld/telemetry/ConsensusReceiveTracing.h +++ b/src/xrpld/telemetry/ConsensusReceiveTracing.h @@ -39,8 +39,7 @@ #include #include -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { // Inline span name constants for consensus receive spans. // Phase 4 will provide these via ConsensusSpanNames.h; these are @@ -123,5 +122,4 @@ validationReceiveSpan([[maybe_unused]] protocol::TMValidation const& msg) return SpanGuard::span(TraceCategory::Consensus, "consensus", "validation.receive"); } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/xrpld/telemetry/PropagationHelpers.h b/src/xrpld/telemetry/PropagationHelpers.h index c051026b74..44959ae039 100644 --- a/src/xrpld/telemetry/PropagationHelpers.h +++ b/src/xrpld/telemetry/PropagationHelpers.h @@ -32,8 +32,7 @@ #include #include -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { /** Inject trace context from an active SpanGuard into a protobuf * TraceContext message for cross-node propagation. @@ -58,5 +57,4 @@ injectSpanContext(SpanGuard const& span, protocol::TraceContext& proto) proto.set_trace_flags(bytes.traceFlags); } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry diff --git a/src/xrpld/telemetry/TxTracing.h b/src/xrpld/telemetry/TxTracing.h index e466c45a6c..68b4903f88 100644 --- a/src/xrpld/telemetry/TxTracing.h +++ b/src/xrpld/telemetry/TxTracing.h @@ -16,8 +16,7 @@ #include #include -namespace xrpl { -namespace telemetry { +namespace xrpl::telemetry { /** Create a "tx.receive" span for a transaction received from a peer. * trace_id is derived from txID[0:16]. If the incoming message carries @@ -59,5 +58,4 @@ txProcessSpan(uint256 const& txID) TraceCategory::Transactions, tx_span::process, txID.data(), txID.bytes); } -} // namespace telemetry -} // namespace xrpl +} // namespace xrpl::telemetry