diff --git a/CHANGES.md b/CHANGES.md index 4413bbcc..288538ea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,10 @@ ## 10.12.0.0 08/16/2026 +* **`Request(Dictionary)` 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((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 diff --git a/Tests/Xrpl.Tests/Client/PagedResponseServer.cs b/Tests/Xrpl.Tests/Client/PagedResponseServer.cs index 1d682772..d53fc90c 100644 --- a/Tests/Xrpl.Tests/Client/PagedResponseServer.cs +++ b/Tests/Xrpl.Tests/Client/PagedResponseServer.cs @@ -1,5 +1,4 @@ -using System; -using System.Buffers.Binary; +using System; using System.Net.Sockets; using System.Text; using System.Threading; @@ -176,75 +175,5 @@ private static string ExtractId(string message) return message.Substring(start, stop - start).Trim(); } - /// - /// Reads one client frame. Returns the decoded text of the first text frame seen, or null - /// once the peer closes. Control frames other than Close are skipped. - /// - private async Task ReadTextFrameAsync(NetworkStream stream) - { - while (true) - { - byte[] head = new byte[2]; - if (!await ReadExactAsync(stream, head, 2).ConfigureAwait(false)) - { - return null; - } - - int opcode = head[0] & 0x0F; - bool masked = (head[1] & 0x80) != 0; - long length = head[1] & 0x7F; - - if (length == 126) - { - byte[] extended = new byte[2]; - if (!await ReadExactAsync(stream, extended, 2).ConfigureAwait(false)) - { - return null; - } - - length = BinaryPrimitives.ReadUInt16BigEndian(extended); - } - else if (length == 127) - { - byte[] extended = new byte[8]; - if (!await ReadExactAsync(stream, extended, 8).ConfigureAwait(false)) - { - return null; - } - - length = (long)BinaryPrimitives.ReadUInt64BigEndian(extended); - } - - byte[] mask = new byte[4]; - if (masked && !await ReadExactAsync(stream, mask, 4).ConfigureAwait(false)) - { - return null; - } - - byte[] payload = new byte[length]; - if (length > 0 && !await ReadExactAsync(stream, payload, (int)length).ConfigureAwait(false)) - { - return null; - } - - if (masked) - { - for (int i = 0; i < payload.Length; i++) - { - payload[i] ^= mask[i % 4]; - } - } - - if (opcode == 0x8) - { - return null; - } - - if (opcode == 0x1 || opcode == 0x2) - { - return Encoding.UTF8.GetString(payload); - } - } - } } } diff --git a/Tests/Xrpl.Tests/Client/RequestCapturingServer.cs b/Tests/Xrpl.Tests/Client/RequestCapturingServer.cs new file mode 100644 index 00000000..580fbdea --- /dev/null +++ b/Tests/Xrpl.Tests/Client/RequestCapturingServer.cs @@ -0,0 +1,78 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; + +namespace Xrpl.Tests +{ + /// + /// WebSocket server that records the raw text of every request it receives and answers each + /// one with an empty success. Lets a test assert on what the client actually put on the wire, + /// which is the only way to see fields the node would silently ignore. + /// + internal sealed class RequestCapturingServer : WebSocketTestServerBase + { + private readonly ConcurrentQueue _requests = new ConcurrentQueue(); + + public RequestCapturingServer() + { + StartAccepting(); + } + + /// Every request seen so far, in arrival order. + public IReadOnlyCollection Requests => _requests; + + /// The last request whose command is . + public string LastRequestFor(string command) + { + string found = null; + foreach (string request in _requests) + { + using JsonDocument document = JsonDocument.Parse(request); + if (document.RootElement.TryGetProperty("command", out JsonElement value) && + value.ValueKind == JsonValueKind.String && + value.GetString() == command) + { + found = request; + } + } + + return found; + } + + protected override async Task ServeAsync(NetworkStream stream) + { + while (!Token.IsCancellationRequested) + { + string request = await ReadTextFrameAsync(stream).ConfigureAwait(false); + if (request == null) + { + return; + } + + _requests.Enqueue(request); + + string id = ExtractId(request); + byte[] response = Encoding.UTF8.GetBytes( + "{\"id\":" + id + ",\"status\":\"success\",\"type\":\"response\",\"result\":{}}"); + + await WriteFragmentedMessageAsync(stream, response, fragments: 1).ConfigureAwait(false); + } + } + + /// Echoes the request's id back verbatim, quotes included. + private static string ExtractId(string request) + { + using JsonDocument document = JsonDocument.Parse(request); + if (!document.RootElement.TryGetProperty("id", out JsonElement id)) + { + return "\"0\""; + } + + return id.ValueKind == JsonValueKind.String ? "\"" + id.GetString() + "\"" : id.ToString(); + } + } +} diff --git a/Tests/Xrpl.Tests/Client/TestURequestApiVersion.cs b/Tests/Xrpl.Tests/Client/TestURequestApiVersion.cs new file mode 100644 index 00000000..13ce6edf --- /dev/null +++ b/Tests/Xrpl.Tests/Client/TestURequestApiVersion.cs @@ -0,0 +1,112 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using System.Collections.Generic; +using System.Text.Json; +using System.Threading.Tasks; + +using Xrpl.Client; +using Xrpl.Models.Methods; + +namespace Xrpl.Tests.ClientLib +{ + /// + /// The untyped + /// used to stamp the version under nameof(ApiVersion) — literally "ApiVersion". + /// rippled knows only api_version, ignores anything else and falls back to API v1, so + /// the client's configured version never reached the node and the two request paths of one + /// client spoke different protocol versions. These tests read what actually goes on the wire, + /// because a field the node ignores is invisible from the response. + /// + /// + /// The untyped calls below deliberately use a different command from the typed ones: + /// issues a typed server_info of its own, so matching + /// on that command alone cannot tell the two paths apart. + /// + [TestClass] + public class TestURequestApiVersion + { + private const string UntypedCommand = "ledger_current"; + + private static uint? ApiVersionOf(string request) + { + using JsonDocument document = JsonDocument.Parse(request); + return document.RootElement.TryGetProperty("api_version", out JsonElement version) + ? version.GetUInt32() + : null; + } + + private static void AssertNoMemberNameOnTheWire(string request) + { + using JsonDocument document = JsonDocument.Parse(request); + Assert.IsFalse( + document.RootElement.TryGetProperty("ApiVersion", out _), + $"the C# member name must not reach the wire, rippled ignores it: {request}"); + } + + [TestMethod] + public async Task TestUntypedRequestSendsTheWireFieldName() + { + using RequestCapturingServer server = new RequestCapturingServer(); + using XrplClient client = new XrplClient(server.Url, new XrplClient.ClientOptions { ApiVersion = 2 }); + + await client.Connect().ConfigureAwait(false); + await client.Request(new Dictionary { ["command"] = UntypedCommand }).ConfigureAwait(false); + await client.Disconnect().ConfigureAwait(false); + + string sent = server.LastRequestFor(UntypedCommand); + Assert.IsNotNull(sent, "the server saw no untyped request"); + + Assert.AreEqual(2u, ApiVersionOf(sent), $"the untyped path dropped the client's version: {sent}"); + AssertNoMemberNameOnTheWire(sent); + } + + [TestMethod] + public async Task TestUntypedRequestKeepsAVersionTheCallerSet() + { + using RequestCapturingServer server = new RequestCapturingServer(); + using XrplClient client = new XrplClient(server.Url, new XrplClient.ClientOptions { ApiVersion = 2 }); + + await client.Connect().ConfigureAwait(false); + await client.Request(new Dictionary + { + ["command"] = UntypedCommand, + ["api_version"] = 1 + }).ConfigureAwait(false); + await client.Disconnect().ConfigureAwait(false); + + string sent = server.LastRequestFor(UntypedCommand); + Assert.IsNotNull(sent); + + Assert.AreEqual(1u, ApiVersionOf(sent), $"an explicit api_version must not be overwritten: {sent}"); + AssertNoMemberNameOnTheWire(sent); + } + + /// + /// The typed path was always correct — carries + /// [JsonPropertyName("api_version")]. Both paths are pinned together here because + /// the defect was precisely that one client spoke two protocol versions depending on which + /// method the caller reached for. + /// + [TestMethod] + public async Task TestBothRequestPathsSendTheSameVersion() + { + using RequestCapturingServer server = new RequestCapturingServer(); + using XrplClient client = new XrplClient(server.Url, new XrplClient.ClientOptions { ApiVersion = 2 }); + + await client.Connect().ConfigureAwait(false); + await client.ServerInfo(new ServerInfoRequest()).ConfigureAwait(false); + await client.Request(new Dictionary { ["command"] = UntypedCommand }).ConfigureAwait(false); + await client.Disconnect().ConfigureAwait(false); + + string typed = server.LastRequestFor("server_info"); + string untyped = server.LastRequestFor(UntypedCommand); + + Assert.IsNotNull(typed, "the server saw no typed request"); + Assert.IsNotNull(untyped, "the server saw no untyped request"); + + Assert.AreEqual(2u, ApiVersionOf(typed), $"typed request: {typed}"); + Assert.AreEqual(2u, ApiVersionOf(untyped), $"untyped request: {untyped}"); + AssertNoMemberNameOnTheWire(untyped); + } + } +} diff --git a/Tests/Xrpl.Tests/Client/WebSocketTestServerBase.cs b/Tests/Xrpl.Tests/Client/WebSocketTestServerBase.cs index 9ac8d07a..baea79c7 100644 --- a/Tests/Xrpl.Tests/Client/WebSocketTestServerBase.cs +++ b/Tests/Xrpl.Tests/Client/WebSocketTestServerBase.cs @@ -1,4 +1,5 @@ -using System; +using System; +using System.Buffers.Binary; using System.Net; using System.Net.Sockets; using System.Text; @@ -201,6 +202,77 @@ private async Task ReadUntilHeadersEndAsync(NetworkStream stream) return request.ToString(); } + /// + /// Reads one client frame. Returns the decoded text of the first text frame seen, or null + /// once the peer closes. Control frames other than Close are skipped. + /// + protected async Task ReadTextFrameAsync(NetworkStream stream) + { + while (true) + { + byte[] head = new byte[2]; + if (!await ReadExactAsync(stream, head, 2).ConfigureAwait(false)) + { + return null; + } + + int opcode = head[0] & 0x0F; + bool masked = (head[1] & 0x80) != 0; + long length = head[1] & 0x7F; + + if (length == 126) + { + byte[] extended = new byte[2]; + if (!await ReadExactAsync(stream, extended, 2).ConfigureAwait(false)) + { + return null; + } + + length = BinaryPrimitives.ReadUInt16BigEndian(extended); + } + else if (length == 127) + { + byte[] extended = new byte[8]; + if (!await ReadExactAsync(stream, extended, 8).ConfigureAwait(false)) + { + return null; + } + + length = (long)BinaryPrimitives.ReadUInt64BigEndian(extended); + } + + byte[] mask = new byte[4]; + if (masked && !await ReadExactAsync(stream, mask, 4).ConfigureAwait(false)) + { + return null; + } + + byte[] payload = new byte[length]; + if (length > 0 && !await ReadExactAsync(stream, payload, (int)length).ConfigureAwait(false)) + { + return null; + } + + if (masked) + { + for (int i = 0; i < payload.Length; i++) + { + payload[i] ^= mask[i % 4]; + } + } + + if (opcode == 0x8) + { + return null; + } + + if (opcode == 0x1 || opcode == 0x2) + { + return Encoding.UTF8.GetString(payload); + } + } + } + public void Dispose() { try diff --git a/Xrpl/Client/IXrplClient.cs b/Xrpl/Client/IXrplClient.cs index 90633b2f..76230b73 100644 --- a/Xrpl/Client/IXrplClient.cs +++ b/Xrpl/Client/IXrplClient.cs @@ -469,6 +469,13 @@ public class ClientOptions : ConnectionOptions public string maxFeeXRP { get; set; } public uint? networkID { get; set; } + /// + /// rippled's name for the API version field. Typed requests get it from + /// 's [JsonPropertyName]; a dictionary request + /// has to spell it out, since its keys reach the wire exactly as written. + /// + private const string ApiVersionField = "api_version"; + /// /// The API version to use when making requests. /// @@ -900,10 +907,17 @@ public async Task> Request(Dictionary { //string account = request["Account"] ? EnsureClassicAddress((string)request["account"]) : null; //request["Account"] = account; - if(!request.TryGetValue(nameof(ApiVersion), out var value)){ + + // The key has to be the wire name. A dictionary is serialized verbatim, and rippled + // knows only `api_version` - it ignores anything else and answers on its default, + // API v1. Stamping `nameof(ApiVersion)` here meant this path never delivered the + // version at all, so the same client spoke v2 through its typed methods and v1 + // through this one. + if (!request.ContainsKey(ApiVersionField)) { - request[nameof(ApiVersion)] = ApiVersion; - }} + request[ApiVersionField] = ApiVersion; + } + var response = await this.connection.Request(request, cancellationToken: cancellationToken); // mutates `response` to add warnings