Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ language: en-US

reviews:
profile: chill
review_status: true

review_status: false
auto_review:
# Automatic review is off: reviews are requested on demand with
# "@coderabbitai review" in a PR comment. The settings below still apply
Expand Down
27 changes: 27 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,32 @@
# Changes

## 10.12.0.0 08/16/2026

* **`Request(Dictionary<string, object>)` never delivered the API version, so one client spoke two protocol versions** (**breaking**) — it stamped the version under `nameof(ApiVersion)`, literally `"ApiVersion"`. A dictionary is serialized verbatim, and rippled knows only `api_version`: it ignores unknown fields and answers on its default, API v1. Measured on mainnet, the three spellings are not equivalent — `api_version: 2` returns the v2 shape, while `"ApiVersion": 2` and no version field at all both return v1. So `client.AccountInfo(…)` went out as v2 while `client.Request(new Dictionary { ["command"] = "account_info" })` on the *same client* went out as v1, and response shapes differed between the two with nothing to signal it. The typed path was never affected: `BaseRequest.ApiVersion` carries `[JsonPropertyName("api_version")]`.
* the key is now the wire name, and a version the caller put in the dictionary themselves is still respected. The junk `"ApiVersion"` field no longer rides along on every request
* **breaking:** callers of the untyped path move from API v1 to whatever `ApiVersion` says, which defaults to 2 — response shapes change under code that did not change. This is the fix, not a side effect: the previous behaviour ignored the setting entirely. Callers who want v1 can put `["api_version"] = 1` in the dictionary or set `ApiVersion` on the client
* `TestURequestApiVersion` reads what the client actually puts on the wire through a request-capturing WebSocket server — a field the node ignores cannot be seen from the response, which is how this survived. It pins the wire name on the untyped path, that an explicit `api_version` is not overwritten, and that both request paths of one client carry the same version. `WebSocketTestServerBase` gained the client-frame reader that `PagedResponseServer` had kept private, rather than a third copy of it
* **`TransactionStream` re-parsed the transaction on every read of it, and lost the hash under API v1** (**breaking**) — the same defect `TransactionSummary` was fixed for in 10.9.1.0, left standing on the stream side. `Transaction` was an expression-bodied property over two `object` members holding `JsonElement`s: `JsonSerializer.Deserialize<TransactionResponse>((TransactionJson ?? Proposed).ToString(), …)`. Three things wrong with that one line, on the busiest path the client has — every transaction of a `transactions` subscription:
* **the transaction was rendered back to a string and parsed a second time.** It was already parsed: `TransactionJson`/`Proposed` are `object`, which System.Text.Json fills with a self-contained `JsonElement`. Same round trip as the one removed from `RequestManager.Resolve` below
* **nothing was cached**, so the expression ran again on every access. Measured over 300 real mainnet stream messages: one read cost 4.94 KB (API v1) / 3.96 KB (v2), three reads cost exactly three times that — 14.82 KB and 11.89 KB. A consumer reading `TransactionType` and then `Hash` paid twice, and nothing in the property's signature said so
* **the hash was unreachable under API v1.** rippled reports it at the top level under v2 but only inside the envelope under v1, and `Hash` was mapped to the top-level field alone, so `tx.Hash` was always `null` on v1 — which is what left the `Blazor-WebAssembly` demo printing no hash, since it requests `"ApiVersion": 1`. Verified against mainnet in both directions
* a message carrying neither envelope threw `NullReferenceException` straight out of the property

`TransactionStream` now follows `TransactionSummary`: `Transaction` is typed `TransactionResponse` and mapped to `tx_json`, a private set-only `TransactionV1` alias catches the API v1 `transaction` envelope, and `Hash` falls back to the envelope. The transaction is deserialized **once, with the message that carries it** — there is no second parse left to cache, and reading the property back is a field read. `ledger_index` and `ledger_hash` needed no fallback: rippled reports both at the top level in either version, which the captures confirm.
* measured over the same 300 messages per version, allocation per message for the whole consumer flow — deserialize the message, then read the transaction off it: **16.44 → 13.75 KB** at one read and **26.32 → 13.75 KB** at three (API v1); **15.24 → 13.07 KB** and **23.17 → 13.07 KB** (API v2). The figure no longer moves with the number of reads at all, which is the point. Timings did not separate reliably on the measuring machine and are not quoted
* the trade-off, stated plainly: deserializing the message alone went **up**, 11.50 → 13.75 KB (v1), because the transaction is now materialized eagerly instead of being left as a lazy `JsonElement`. A consumer that never touches `Transaction` pays about 2.25 KB more per message; one that touches it once or more pays 2.7–12.6 KB less
* **breaking:** the public `object` properties `TransactionJson` and `Proposed` are gone — they existed only as raw envelopes for the getter to re-parse, and there is nothing left to re-parse. `Transaction` keeps its name and type and gains a setter. Consistent with the removal of `Path.TypeHex` in 10.11.0.0, no `[Obsolete]` grace period
* `TestUTransactionStreamEnvelope` pins both envelopes, the hash under both versions, the message carrying neither, and that repeated reads allocate nothing and hand back the same instance
* **Every response was parsed twice and copied to UTF-16 twice** — the cost of reading a response, measured rather than reasoned about. `RequestManager.Resolve` did `JsonSerializer.Deserialize(response.Result?.ToString() ?? "{}", taskInfo.Type, ...)`. `BaseResponse.Result` is typed `object`, which System.Text.Json fills with a `JsonElement` that already owns a private copy of the `result` bytes — so `.ToString()` rendered that element back into a UTF-16 string and the serializer parsed the string a second time. On a `ledger_data` page at `limit=2048` (~1 MB) the four stages measured, per response, at: 1.97 MB for the UTF-16 copy of the message, 1.68 MB for the document built over it, 1.97 MB for the UTF-16 copy of the `result`, 1.68 MB for the second document — **7.30 MB, 7.42x the response size**, all four allocations past the 85 KB large-object threshold. Both halves are now gone:
* `DeserializeResult` works off the parsed node: `element.Deserialize(type, options)` for a typed model, and the element itself when the request asked for `JsonElement` or `object`, which is what a consumer that needs the raw ledger objects asks for (the typed `LOLedgerData.State` drops unknown fields). A `BaseResponse` assembled by hand rather than parsed off the wire keeps the old string path. Behaviour is otherwise unchanged, including a missing or JSON-`null` `result`, which still yields what deserializing `"{}"` yielded
* the socket path carries the frame as it arrived. `Connection` binds `OnBinaryMessage` instead of `OnMessageReceived`, `IsLikelyResponse` and `RequestManager.HandleResponse` have `ReadOnlySpan<byte>` overloads, and the UTF-16 string is materialized — once, lazily — only for what genuinely needs text: stream messages and the `OnWarning`/`OnServerWarning`/`OnError` callbacks. The `string` overloads stay for `Connection.OnMessage(string)` and for external callers
* the warning callbacks no longer pay for listeners that are not there. rippled attaches `warning`/`warnings` to responses under load and on a reporting-mode server, and the dispatch built the UTF-16 text for them before checking whether `OnWarning`/`OnServerWarning` were subscribed — on such a server that is the removed allocation, back on every page. Measured with warnings on all 20 pages and nothing subscribed: 4.28x → 2.08x
* the failure report survives the failure. A response that will not parse is most often a heap that has just run out, and materializing the message for `OnError` is then the largest allocation left on the path — if it throws, the notification is lost inside the handler and the consumer sees silence. The text is now built only when a handler is attached, and an `OutOfMemoryException` while building it falls back to a literal placeholder so the classification still goes out. `Connection.OnMessage(null)` also keeps its old route through `OnError` instead of throwing `ArgumentNullException` out of the entry point
* measured end to end against a local WebSocket server, 600 `ledger_data` pages of ~1 MB: **8.32 → 2.68 MB allocated per response** (8.46x → 2.72x the payload), 11.49 → 7.96 ms per response, 87 → 126 responses/s, peak managed heap 42.4 → 20.8 MB, peak LOH 39.2 → 17.3 MB, peak working set 203.4 → 71.8 MB. Under a lowered `DOTNET_GCHeapHardLimit` the pre-fix path reproduced the production failure exactly — `XrplException: Failed to deserialize response for request <id>: Exception of type 'System.OutOfMemoryException' was thrown`, with `JsonElement.ToString()` at the top of the inner stack — at a ceiling the fixed path completes 15/15 pages under
* the win is not specific to `ledger_data` or to `JsonElement`: the second parse was on the path of every command. The repo's own `BenchmarkLedgerDataCrawl`, which goes through `Request` → `Dictionary<string, object>`, drops from 22.9 to 14.8 MiB allocated per 2 MiB page (11.4x → 7.4x) with LOH ending at 38.2 instead of 115.4 MiB — it stays above the `JsonElement` figure because building a `Dictionary<string, object>` boxes every value, which this change does not address
* `TestUResponseParsing` pins the behaviour that had to survive — the untyped node handed through is self-contained and readable after a forced gen2 collection, a typed model deserializes to the same values, the `string` and UTF-8 overloads agree, a missing `result` still completes, an `error` status still rejects with the parsed `ErrorResponse` attached, a null message does not throw out of the entry point — and holds two allocation budgets at 4x the response size. The first measures `RequestManager` alone, per thread so the class-parallel run cannot perturb it (1.89x now). The second runs 20 pages through `Connection` over a real socket, because nothing else in the suite can see *which* overload the client picks: it reads the process-wide counter and is therefore kept out of the parallel pass, and it separates the two paths with room on both sides — 2.18x as bound, 4.84x with the string callback bound instead. `PagedResponseServer` reuses one response frame per connection and rewrites the id in place so the server contributes nothing to what the client is measured on
* **`error` responses were deserialized a third time** — the `status == "error"` branch of `HandleResponse` re-parsed the whole message into an `ErrorResponse` inside a `try`/`catch` that swallowed everything, to build the exception's `Response`. The message had already been deserialized into an `ErrorResponse` at the top of the same method; the second parse only produced an equal copy, and on a large error payload it was a second large-object allocation on a path that is already failing

## 10.11.1.0 08/13/2026

* **Fix infinite recursion in `LONFTokenConverter.Write` — the metadata of an NFT transaction could not be serialized at all** — regression introduced in 10.3.0.0 with the `Newtonsoft.Json` → `System.Text.Json` migration; affects every release from 10.3.0.0 on. `JsonSerializer.Serialize(tx.Meta)` threw `JsonException: A possible object cycle was detected` for any transaction whose `AffectedNodes` contain an `NFTokenPage`, which is every `NFTokenMint`, `NFTokenBurn`, `NFTokenAcceptOffer` and `NFTokenModify` that touched a page. Verified against mainnet on all six NFT transaction types — the four above failed, `NFTokenCreateOffer` and `NFTokenCancelOffer` (no page in their metadata) went through:
Expand Down
76 changes: 76 additions & 0 deletions Tests/Xrpl.Tests/Client/BenchmarkLedgerDataCrawl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
using System.Diagnostics;
using System.Threading.Tasks;

using System.Text.Json;

using Xrpl.Client;
using Xrpl.Models.Common;
using Xrpl.Models.Methods;

namespace Xrpl.Tests.ClientLib
{
Expand Down Expand Up @@ -112,6 +116,78 @@ public async Task BenchmarkSequentialPaging()
Console.WriteLine("decile profile (ms/page): " + string.Join(" | ", DecileProfile(pageMs)));
}

/// <summary>
/// Same crawl through <c>GRequest&lt;JsonElement, …&gt;</c> — the path a consumer takes when
/// it needs the raw ledger objects, because the typed <c>LOLedgerData.State</c> drops
/// fields the models do not know. This is where the response path's own cost shows up
/// undiluted: nothing is materialized into a model, so what is measured is receive,
/// route and parse.
/// </summary>
[TestMethod]
public async Task BenchmarkSequentialPagingUntyped()
{
int pages = EnvInt("CRAWL_PAGES", 2000);
int payloadBytes = EnvInt("CRAWL_PAYLOAD_BYTES", 2 * 1024 * 1024);
int fragments = EnvInt("CRAWL_FRAGMENTS", 32);

using PagedResponseServer server = new PagedResponseServer(payloadBytes, fragments);
using XrplClient client = new XrplClient(server.Url);

await client.Connect().ConfigureAwait(false);
await RequestUntypedPageAsync(client).ConfigureAwait(false);

GC.Collect(2, GCCollectionMode.Forced, blocking: true);
GC.WaitForPendingFinalizers();
GC.Collect(2, GCCollectionMode.Forced, blocking: true);

long allocatedBefore = GC.GetTotalAllocatedBytes(precise: true);
int gen2Before = GC.CollectionCount(2);
long lohBefore = LohBytes();
long startTicks = Stopwatch.GetTimestamp();

for (int i = 0; i < pages; i++)
{
await RequestUntypedPageAsync(client).ConfigureAwait(false);
}

double totalSeconds = (Stopwatch.GetTimestamp() - startTicks) / (double)Stopwatch.Frequency;
long allocated = GC.GetTotalAllocatedBytes(precise: true) - allocatedBefore;
long lohAfter = LohBytes();

await client.Disconnect().ConfigureAwait(false);

Console.WriteLine("=== ledger_data crawl benchmark (untyped JsonElement result) ===");
Console.WriteLine($"pages : {pages}");
Console.WriteLine($"payload : {payloadBytes / 1024.0 / 1024.0:F2} MiB");
Console.WriteLine($"total time : {totalSeconds:F2} s ({pages / totalSeconds:F2} pages/s)");
Console.WriteLine($"allocated per page: {allocated / (double)pages / 1024.0 / 1024.0:F1} MiB " +
$"({allocated / (double)pages / payloadBytes:F1}x payload)");
Console.WriteLine($"gen2 collections : {GC.CollectionCount(2) - gen2Before}");
Console.WriteLine($"LOH size : {lohBefore / 1024.0 / 1024.0:F1} -> {lohAfter / 1024.0 / 1024.0:F1} MiB");
}

private static async Task RequestUntypedPageAsync(XrplClient client)
{
JsonElement result = await client
.GRequest<JsonElement, LedgerDataRequest>(new LedgerDataRequest
{
LedgerIndex = new LedgerIndex(96000000),
Binary = true,
Limit = 2048
})
.ConfigureAwait(false);

if (result.ValueKind != JsonValueKind.Object || !result.TryGetProperty("state", out JsonElement state))
{
throw new InvalidOperationException("empty ledger_data response");
}

if (state.GetArrayLength() == 0)
{
throw new InvalidOperationException("ledger_data page carried no objects");
}
}

private static async Task RequestPageAsync(XrplClient client)
{
Dictionary<string, object> request = new Dictionary<string, object>
Expand Down
Loading
Loading