diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index e6c807ac95..4f45216cda 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -10,7 +10,7 @@ { "compiler": ["gcc", "clang"], - "build_type": ["Debug"], + "build_type": ["Debug", "Release"], "arch": ["amd64"], "sanitizers": ["address", "undefinedbehavior"] }, diff --git a/OpenTelemetryPlan/03-implementation-strategy.md b/OpenTelemetryPlan/03-implementation-strategy.md index 9e80db5d30..eecd144617 100644 --- a/OpenTelemetryPlan/03-implementation-strategy.md +++ b/OpenTelemetryPlan/03-implementation-strategy.md @@ -229,6 +229,14 @@ xychart-beta - Maximum memory is bounded: ~8.3 MB static (dominated by worker thread stack) + 2048 queued spans x ~500 bytes (~1 MB) + active spans (~0.8 MB) ≈ **~10 MB ceiling** - The worker thread stack (~8 MB) is virtual memory; actual RSS depends on stack usage (typically much less) +> **Measured outcome**: A perf-iac comparison (telemetry compiled-in + enabled vs compiled-out, +> 9 nodes — validators and client-handlers — under sustained payment load) recorded **no measurable +> RSS increase over the telemetry-off baseline** (~15 GiB mean / ~18–19 GiB peak on both sides), +> with no OOM, no swap, and no leak across the run. The ~10 MB ceiling above is therefore a +> provisioning safety margin (dominated by virtual thread-stack address space), not an expected +> resident-memory increase. Steady-state cost shows up as throughput (~3–4% at head sampling 1.0), +> not memory. + ### 3.5.4 Performance Data Sources The overhead estimates in Sections 3.3-3.5 are derived from the following sources: @@ -329,19 +337,19 @@ This section provides a detailed assessment of how intrusive the OpenTelemetry i ### 3.9.1 Files Modified Summary -| Component | Files Modified | Lines Added | Lines Changed | Architectural Impact | -| --------------------- | -------------- | ----------- | ------------- | -------------------- | -| **Core Telemetry** | 7 new files | ~800 | 0 | None (new module) | -| **Application Init** | 2 files | ~30 | ~5 | Minimal | -| **RPC Layer** | 3 files | ~80 | ~20 | Minimal | -| **Transaction Relay** | 4 files | ~120 | ~40 | Low | -| **Consensus** | 3 files | ~100 | ~30 | Low-Medium | -| **Protocol Buffers** | 1 file | ~25 | 0 | Low | -| **CMake/Build** | 3 files | ~50 | ~10 | Minimal | -| **PathFinding** | 2 | ~80 | ~5 | Minimal | -| **TxQ/Fee** | 2 | ~60 | ~5 | Minimal | -| **Validator/Amend** | 3 | ~40 | ~5 | Minimal | -| **Total** | **~27 files** | **~1,490** | **~120** | **Low** | +| Component | Files Modified | Architectural Impact | +| --------------------- | -------------- | -------------------- | +| **Core Telemetry** | 10 new files | None (new module) | +| **Application Init** | 2 files | Minimal | +| **RPC Layer** | 3 files | Minimal | +| **Transaction Relay** | 4 files | Low | +| **Consensus** | 3 files | Low-Medium | +| **Protocol Buffers** | 1 file | Low | +| **CMake/Build** | 3 files | Minimal | +| **PathFinding** | 2 | Minimal | +| **TxQ/Fee** | 2 | Minimal | +| **Validator/Amend** | 3 | Minimal | +| **Total** | **~33 files** | **Low** | ### 3.9.2 Detailed File Impact @@ -361,40 +369,43 @@ pie title Code Changes by Component #### New Files (No Impact on Existing Code) -| File | Lines | Purpose | -| ------------------------------------------- | ----- | ----------------------------------------------------- | -| `include/xrpl/telemetry/Telemetry.h` | ~160 | Main interface (global singleton) | -| `include/xrpl/telemetry/SpanGuard.h` | ~250 | RAII wrapper + factory methods + discard + no-op stub | -| `include/xrpl/telemetry/DiscardFlag.h` | ~28 | Thread-local discard flag | -| `include/xrpl/telemetry/TraceContext.h` | ~80 | Context propagation | -| `src/libxrpl/telemetry/Telemetry.cpp` | ~400 | Implementation + FilteringSpanProcessor | -| `src/libxrpl/telemetry/TelemetryConfig.cpp` | ~60 | Config parsing | -| `src/libxrpl/telemetry/NullTelemetry.cpp` | ~40 | No-op implementation | +| File | Purpose | +| ------------------------------------------- | ------------------------- | +| `include/xrpl/telemetry/Telemetry.h` | Main interface | +| `include/xrpl/telemetry/TelemetryConfig.h` | Configuration structures | +| `include/xrpl/telemetry/TraceContext.h` | Context propagation | +| `include/xrpl/telemetry/SpanGuard.h` | RAII wrapper | +| `include/xrpl/telemetry/DiscardFlag.h` | Thread-local discard flag | +| `include/xrpl/telemetry/SpanAttributes.h` | Attribute helpers | +| `src/libxrpl/telemetry/Telemetry.cpp` | Implementation | +| `src/libxrpl/telemetry/TelemetryConfig.cpp` | Config parsing | +| `src/libxrpl/telemetry/TraceContext.cpp` | Context serialization | +| `src/libxrpl/telemetry/NullTelemetry.cpp` | No-op implementation | #### Modified Files (Existing Xrpld Code) -| File | Lines Added | Lines Changed | Risk Level | -| ------------------------------------------------- | ----------- | ------------- | ---------- | -| `src/xrpld/app/main/Application.cpp` | ~15 | ~3 | Low | -| `include/xrpl/core/ServiceRegistry.h` | ~5 | ~2 | Low | -| `src/xrpld/rpc/detail/ServerHandler.cpp` | ~40 | ~10 | Low | -| `src/xrpld/rpc/handlers/*.cpp` | ~30 | ~8 | Low | -| `src/xrpld/overlay/detail/PeerImp.cpp` | ~60 | ~15 | Medium | -| `src/xrpld/overlay/detail/OverlayImpl.cpp` | ~30 | ~10 | Medium | -| `src/xrpld/app/consensus/RCLConsensus.cpp` | ~50 | ~15 | Medium | -| `src/xrpld/app/consensus/RCLConsensusAdaptor.cpp` | ~40 | ~12 | Medium | -| `src/xrpld/core/JobQueue.cpp` | ~20 | ~5 | Low | -| `src/xrpld/app/paths/PathRequest.cpp` | ~40 | ~3 | Low | -| `src/xrpld/app/paths/Pathfinder.cpp` | ~40 | ~2 | Low | -| `src/xrpld/app/misc/TxQ.cpp` | ~40 | ~3 | Low | -| `src/xrpld/app/main/LoadManager.cpp` | ~20 | ~2 | Low | -| `src/xrpld/app/misc/ValidatorList.cpp` | ~20 | ~2 | Low | -| `src/xrpld/app/misc/AmendmentTable.cpp` | ~10 | ~2 | Low | -| `src/xrpld/app/misc/Manifest.cpp` | ~10 | ~1 | Low | -| `src/xrpld/shamap/SHAMap.cpp` | ~20 | ~3 | Low | -| `src/xrpld/overlay/detail/ripple.proto` | ~25 | 0 | Low | -| `CMakeLists.txt` | ~40 | ~8 | Low | -| `cmake/FindOpenTelemetry.cmake` | ~50 | 0 | None (new) | +| File | Risk Level | +| ------------------------------------------------- | ---------- | +| `src/xrpld/app/main/Application.cpp` | Low | +| `include/xrpl/core/ServiceRegistry.h` | Low | +| `src/xrpld/rpc/detail/ServerHandler.cpp` | Low | +| `src/xrpld/rpc/handlers/*.cpp` | Low | +| `src/xrpld/overlay/detail/PeerImp.cpp` | Medium | +| `src/xrpld/overlay/detail/OverlayImpl.cpp` | Medium | +| `src/xrpld/app/consensus/RCLConsensus.cpp` | Medium | +| `src/xrpld/app/consensus/RCLConsensusAdaptor.cpp` | Medium | +| `src/xrpld/core/JobQueue.cpp` | Low | +| `src/xrpld/app/paths/PathRequest.cpp` | Low | +| `src/xrpld/app/paths/Pathfinder.cpp` | Low | +| `src/xrpld/app/misc/TxQ.cpp` | Low | +| `src/xrpld/app/main/LoadManager.cpp` | Low | +| `src/xrpld/app/misc/ValidatorList.cpp` | Low | +| `src/xrpld/app/misc/AmendmentTable.cpp` | Low | +| `src/xrpld/app/misc/Manifest.cpp` | Low | +| `src/xrpld/shamap/SHAMap.cpp` | Low | +| `src/xrpld/overlay/detail/ripple.proto` | Low | +| `CMakeLists.txt` | Low | +| `cmake/FindOpenTelemetry.cmake` | None (new) | ### 3.9.3 Risk Assessment by Component diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index 2eeca72dcb..c08d0510c9 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -36,13 +36,14 @@ The authoritative `[telemetry]` example lives in `cfg/xrpld-example.cfg`. Teleme documents but are not parsed by `TelemetryConfig.cpp` in Phase 1b and later phases. They will be added as the corresponding subsystems are instrumented: -| Option | Planned Phase | Purpose | -| ----------------- | ------------- | ---------------------------------------- | -| `exporter` | Future | Select between OTLP/HTTP and OTLP/gRPC | -| `trace_pathfind` | Phase 2 | Path computation tracing toggle | -| `trace_txq` | Phase 3 | Transaction queue tracing toggle | -| `trace_validator` | Future | Validator list / manifest update tracing | -| `trace_amendment` | Future | Amendment voting tracing | +| Option | Planned Phase | Purpose | +| -------------------------- | ------------- | ----------------------------------------------------------------------- | +| `exporter` | Future | Select between OTLP/HTTP and OTLP/gRPC | +| `trace_pathfind` | Phase 2 | Path computation tracing toggle | +| `trace_txq` | Phase 3 | Transaction queue tracing toggle | +| `trace_validator` | Future | Validator list / manifest update tracing | +| `trace_amendment` | Future | Amendment voting tracing | +| `consensus_trace_strategy` | Phase 4 | Trace ID strategy for consensus rounds (`deterministic` \| `attribute`) | --- diff --git a/OpenTelemetryPlan/07-observability-backends.md b/OpenTelemetryPlan/07-observability-backends.md index a8bd897723..d397fcb69d 100644 --- a/OpenTelemetryPlan/07-observability-backends.md +++ b/OpenTelemetryPlan/07-observability-backends.md @@ -258,10 +258,13 @@ Grafana provisions three TraceQL-based alert rules (group `xrpld-tracing-alerts` {resource.service.name="xrpld" && name="consensus.round"} | avg(duration) > 5s ``` -- **RPC Error Rate Spike** (critical, `for: 2m`): fires when the error rate across `rpc.command.*` spans exceeds 5%. +- **RPC Error Rate Spike** (critical, `for: 2m`): fires when the error rate across `rpc.command.*` spans exceeds 5%. Error _rate_ is a ratio, so it must divide the error-span rate by the total-span rate — a single TraceQL `rate()` returns spans/second, not a percentage, and would fire on traffic volume alone. This uses span metrics emitted by the collector's `spanmetrics` connector (Prometheus datasource), not a TraceQL query: ``` - {resource.service.name="xrpld" && name=~"rpc.command.*" && status.code=error} | rate() > 0.05 + sum(rate(calls_total{service_name="xrpld", span_name=~"rpc.command.*", status_code="STATUS_CODE_ERROR"}[5m])) + / + sum(rate(calls_total{service_name="xrpld", span_name=~"rpc.command.*"}[5m])) + > 0.05 ``` - **Transaction Throughput Drop** (warning, `for: 10m`): fires when the `tx.receive` span rate falls below 10/s. @@ -270,7 +273,7 @@ Grafana provisions three TraceQL-based alert rules (group `xrpld-tracing-alerts` {resource.service.name="xrpld" && name="tx.receive"} | rate() < 10 ``` -> **Note**: The first two rules use TraceQL aggregates (`avg(duration)`, `rate()`), which require Tempo 2.3+ with TraceQL metrics enabled. Verify aggregate query support in your Tempo version before provisioning. +> **Note**: The Consensus Round Slow and Transaction Throughput Drop rules use TraceQL aggregates (`avg(duration)`, `rate()`), which require Tempo 2.3+ with TraceQL metrics enabled. Verify aggregate query support in your Tempo version before provisioning. The RPC Error Rate Spike rule instead queries Prometheus span metrics (collector `spanmetrics` connector), so it needs that connector enabled in the collector pipeline. --- diff --git a/OpenTelemetryPlan/OpenTelemetryPlan.md b/OpenTelemetryPlan/OpenTelemetryPlan.md index d931b455b1..505a854e49 100644 --- a/OpenTelemetryPlan/OpenTelemetryPlan.md +++ b/OpenTelemetryPlan/OpenTelemetryPlan.md @@ -16,12 +16,12 @@ This document provides a comprehensive implementation plan for integrating OpenT ### Estimated Performance Overhead -| Metric | Overhead | Notes | -| ------------- | ---------- | ----------------------------------- | -| CPU | 1-3% | Span creation and attribute setting | -| Memory | 2-5 MB | Batch buffer for pending spans | -| Network | 10-50 KB/s | Compressed OTLP export to collector | -| Latency (p99) | <2% | With proper sampling configuration | +| Metric | Overhead | Notes | +| ------------- | ---------- | ------------------------------------------------ | +| CPU | 1-3% | Span creation and attribute setting | +| Memory | <10 MB | SDK statics + batch buffer + worker thread stack | +| Network | 10-50 KB/s | Compressed OTLP export to collector | +| Latency (p99) | <2% | With proper sampling configuration | --- diff --git a/OpenTelemetryPlan/presentation.md b/OpenTelemetryPlan/presentation.md index 535807a96e..bc83f82fc2 100644 --- a/OpenTelemetryPlan/presentation.md +++ b/OpenTelemetryPlan/presentation.md @@ -222,7 +222,7 @@ flowchart TB services --> Telemetry end - Telemetry -->|OTLP/gRPC| Collector["OTel Collector"] + Telemetry -->|OTLP/HTTP| Collector["OTel Collector"] Collector --> Tempo["Grafana Tempo"] Collector --> Elastic["Elastic APM"] @@ -237,7 +237,7 @@ flowchart TB - **Core Services (blue, top)**: RPC Server, Overlay, and Consensus are the three primary components that generate trace data — they represent the entry points for client requests, peer messages, and consensus rounds respectively. - **Telemetry Module (green, middle)**: The OpenTelemetry SDK sits below the core services and receives span data from all three; it acts as a single collection point within the xrpld process. -- **OTel Collector (orange, center)**: An external process that receives spans over OTLP/gRPC from the Telemetry Module; it decouples xrpld from backend choices and handles batching, sampling, and routing. +- **OTel Collector (orange, center)**: An external process that receives spans over OTLP/HTTP from the Telemetry Module; it decouples xrpld from backend choices and handles batching, sampling, and routing. - **Backends (bottom row)**: Tempo and Elastic APM are interchangeable — the Collector fans out to any combination, so operators can switch backends without modifying xrpld code. - **Top-to-bottom flow**: Data flows from instrumented code down through the SDK, out over the network to the Collector, and finally into storage/visualization backends. @@ -373,6 +373,13 @@ Per-transaction tracing cost breakdown: > recordable (~250 bytes base), and `std::map`-based attribute storage (~200-500 bytes for 3-5 > string attributes). See [Section 3.5.4](./03-implementation-strategy.md#354-performance-data-sources) for source references. +> **Measured (perf-iac, telemetry on vs off, 9 nodes under payment load)**: the ~10 MB +> above is a theoretical SDK-footprint ceiling, dominated by virtual (not resident) thread-stack +> memory. In practice, per-node RSS showed **no measurable increase over the telemetry-off +> baseline** (~15 GiB mean / ~18–19 GiB peak on both sides), with no OOM, swap, or leak over the +> run. Treat memory overhead as negligible; the ceiling is a provisioning safety margin, not an +> expected increase. + **Network (10-50 KB/s) — Calculation**: Two sources of network overhead: @@ -457,7 +464,7 @@ flowchart LR ```mermaid flowchart LR - A["Head Sampling
10% default"] --> B["Tail Sampling
Keep errors/slow"] --> C["Batch Export
Reduce I/O"] --> D["Conditional Compile
XRPL_ENABLE_TELEMETRY"] + A["Head Sampling
fixed 1.0 (record all)"] --> B["Tail Sampling
Keep errors/slow"] --> C["Batch Export
Reduce I/O"] --> D["Conditional Compile
XRPL_ENABLE_TELEMETRY"] style A fill:#1565c0,stroke:#0d47a1,color:#fff style B fill:#2e7d32,stroke:#1b5e20,color:#fff @@ -496,7 +503,7 @@ flowchart LR | Aspect | Details | | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Where it runs** | Inside xrpld (SDK-level). Configured via `sampling_ratio` in `xrpld.cfg`. | +| **Where it runs** | Inside xrpld (SDK-level). In xrpld the ratio is fixed at 1.0 and not read from config (tail sampling in the collector needs every span) — the example below shows the general head-sampling mechanism. | | **When the decision happens** | At trace creation time — before the first span is even populated. | | **How it works** | `sampling_ratio=0.1` means each trace has a 10% probability of being recorded. Dropped traces incur near-zero overhead (no spans created, no attributes set, no export). | | **Propagation** | Once a trace is sampled, the `trace_flags` field (1 byte in the context header) tells downstream nodes to also sample it. Unsampled traces propagate `trace_flags=0`, so downstream nodes skip them too. | @@ -508,8 +515,10 @@ flowchart LR ```ini [telemetry] -# Record 10% of traces (recommended for production) -sampling_ratio=0.1 +# xrpld fixes head sampling at 1.0 (record every trace). This value is +# NOT read from config — the collector performs tail sampling instead, +# which needs all spans to arrive. See Slide 9 (tail sampling) and §7.4.2. +sampling_ratio=1.0 ``` ### Tail Sampling (Decision at End) @@ -574,16 +583,16 @@ processors: ### Head vs. Tail — Side-by-Side -| | Head Sampling | Tail Sampling | -| ----------------------------- | ---------------------------------------- | ------------------------------------------------ | -| **Decision point** | Trace start (inside xrpld) | Trace end (in OTel Collector) | -| **Knows trace content?** | No (random coin flip) | Yes (evaluates completed trace) | -| **Overhead on xrpld** | Lowest (dropped traces = no-op) | Higher (must export 100% to Collector) | -| **Collector resource usage** | Low (receives only sampled traces) | Higher (buffers all traces before deciding) | -| **Captures all errors?** | No (only if trace was randomly selected) | **Yes** (error policy catches them) | -| **Captures slow operations?** | No (random) | **Yes** (latency policy catches them) | -| **Configuration** | `xrpld.cfg`: `sampling_ratio=0.1` | `otel-collector.yaml`: `tail_sampling` processor | -| **Best for** | High-throughput steady-state | Troubleshooting & anomaly detection | +| | Head Sampling | Tail Sampling | +| ----------------------------- | ----------------------------------------- | ------------------------------------------------ | +| **Decision point** | Trace start (inside xrpld) | Trace end (in OTel Collector) | +| **Knows trace content?** | No (random coin flip) | Yes (evaluates completed trace) | +| **Overhead on xrpld** | Lowest (dropped traces = no-op) | Higher (must export 100% to Collector) | +| **Collector resource usage** | Low (receives only sampled traces) | Higher (buffers all traces before deciding) | +| **Captures all errors?** | No (only if trace was randomly selected) | **Yes** (error policy catches them) | +| **Captures slow operations?** | No (random) | **Yes** (latency policy catches them) | +| **Configuration** | Fixed at `1.0` in xrpld (not config-read) | `otel-collector.yaml`: `tail_sampling` processor | +| **Best for** | High-throughput steady-state | Troubleshooting & anomaly detection | ### Recommended Strategy for xrpld @@ -610,7 +619,7 @@ flowchart LR style storage fill:#2e7d32,stroke:#1b5e20,color:#fff ``` -> **Why this works**: xrpld exports everything (no blind drops), the Collector applies intelligent filtering (keep errors/slow/anomalies, sample the rest), and only ~15-20% of traces reach storage. If Collector resource usage becomes a concern, add head sampling at `sampling_ratio=0.5` to halve the export volume while still giving the Collector enough data for good tail-sampling decisions. +> **Why this works**: xrpld exports everything (no blind drops), the Collector applies intelligent filtering (keep errors/slow/anomalies, sample the rest), and only ~15-20% of traces reach storage. xrpld's head sampling is fixed at 1.0 and not configurable, because tail sampling can only see traces that reach the Collector — any head drop would blind the error/slow policies. To reduce volume, tune the Collector's tail-sampling rules rather than adding head sampling. --- diff --git a/cspell.config.yaml b/cspell.config.yaml index 8c354b79c2..39f8371994 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -238,8 +238,10 @@ words: - pyenv - pyparsing - qalloc + - qbsprofile - queuable - Raphson + - rcflags - replayer - rerere - retriable diff --git a/nix/devshell.nix b/nix/devshell.nix index 1bd7ea4c0c..105033eb06 100644 --- a/nix/devshell.nix +++ b/nix/devshell.nix @@ -1,6 +1,26 @@ { pkgs, ... }: let - inherit (import ./packages.nix { inherit pkgs; }) commonPackages; + # conan is in the binary cache for Linux but not for Darwin, so on Darwin + # it is always built from source — and its bundled test suite is unreliable + # in the sandbox: `test_qbsprofile_rcflags` needs gcc (absent on Darwin, see + # https://github.com/NixOS/nixpkgs/pull/528995) and the patch tests are + # flaky from source. We only use conan as a build tool, so skip its tests on + # Darwin. Scoped to the dev shell (not the CI env, which builds conan on + # Linux from the cache). Drop once the fix reaches nixos-unstable and the + # lock is bumped. + pkgs_patched = + if pkgs.stdenv.isDarwin then + pkgs.extend ( + final: prev: { + conan = prev.conan.overridePythonAttrs (_: { + doCheck = false; + }); + } + ) + else + pkgs; + + inherit (import ./packages.nix { pkgs = pkgs_patched; }) commonPackages; # Supported compiler versions gccVersion = pkgs.lib.range 13 15; diff --git a/src/test/rpc/GetAggregatePrice_test.cpp b/src/test/rpc/GetAggregatePrice_test.cpp index 37ecc54172..214bd12183 100644 --- a/src/test/rpc/GetAggregatePrice_test.cpp +++ b/src/test/rpc/GetAggregatePrice_test.cpp @@ -3,12 +3,21 @@ #include #include +#include + #include +#include #include +#include +#include +#include #include +#include +#include #include #include +#include #include #include #include @@ -312,11 +321,91 @@ public: } } + void + testNullTxReadMeta() + { + testcase("Null txRead metadata"); + using namespace jtx; + + // Verify that iteratePriceData handles a null txRead result + // gracefully (returns early) rather than crashing with a + // nullptr dereference. This simulates local data corruption + // where a transaction referenced by sfPreviousTxnID is missing + // from the ledger's transaction map. + Env env(*this); + auto const baseFee = static_cast(env.current()->fees().base.drops()); + + Account const owner{"owner"}; + env.fund(XRP(1'000), owner); + + // Create oracle with XRP/USD and XRP/EUR + Oracle oracle( + env, + {.owner = owner, + .series = {{"XRP", "USD", 740, 1}, {"XRP", "EUR", 840, 1}}, + .fee = baseFee}); + + // Update oracle to only have XRP/EUR, pushing XRP/USD into + // history. iteratePriceData will need to read historical tx + // metadata to find the XRP/USD price. + oracle.set(UpdateArg{.series = {{"XRP", "EUR", 850, 1}}, .fee = baseFee}); + + OraclesData const oracles{{owner, oracle.documentID()}}; + + // Precondition: with an uncorrupted oracle, the historical + // traversal must succeed and produce a price for XRP/USD. + // This proves the test reaches iteratePriceData's history + // path; without it, a future change that breaks the setup + // could turn the post-corruption assertion into a vacuous + // pass (objectNotFound is reachable from many unrelated + // code paths). + { + auto const ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles); + BEAST_EXPECT(!ret.isMember(jss::error)); + BEAST_EXPECT(ret.isMember(jss::median)); + } + + // Simulate data corruption: modify the oracle SLE in the open + // ledger to have a bogus sfPreviousTxnID that doesn't exist in + // any ledger. sfPreviousTxnLgrSeq still points to a valid closed + // ledger, so getLedgerBySeq succeeds but txRead returns null. + auto const oracleKeylet = keylet::oracle(owner, oracle.documentID()); + uint256 const bogusTxnID{0xABCABCAB}; + bool const modified = env.app().getOpenLedger().modify( + [&oracleKeylet, &bogusTxnID](OpenView& view, beast::Journal) -> bool { + auto const sle = view.read(oracleKeylet); + if (!sle) + return false; + auto replacement = std::make_shared(*sle, sle->key()); + replacement->setFieldH256(sfPreviousTxnID, bogusTxnID); + view.rawReplace(replacement); + return true; + }); + + // Confirm the injection actually took effect: modify must + // report success, and re-reading the SLE must show the + // bogus hash. Otherwise the failure-mode assertion below + // would not be exercising the null-txRead path at all. + BEAST_EXPECT(modified); + if (auto const sle = env.current()->read(oracleKeylet); BEAST_EXPECT(sle)) + BEAST_EXPECT(sle->getFieldH256(sfPreviousTxnID) == bogusTxnID); + + // Query for XRP/USD using the "current" (open) ledger. + // The oracle SLE now has a bogus sfPreviousTxnID. The current + // oracle only has EUR, so iteratePriceData will try to read + // history. txRead returns null for the bogus hash, and the + // null check should cause a graceful early return instead of + // a nullptr dereference. + auto const ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles); + BEAST_EXPECT(ret[jss::error].asString() == "objectNotFound"); + } + void run() override { testErrors(); testRpc(); + testNullTxReadMeta(); } };