From 55930624c66a41308958a897907c29041a2ca198 Mon Sep 17 00:00:00 2001 From: "xrpl-release-watch[bot]" <308043467+xrpl-release-watch[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:14:44 -0300 Subject: [PATCH 1/7] ci: bump standalone stand to rippled 3.2.1 (#73) Co-authored-by: github-merge-queue --- .ci-config/docker-compose.ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci-config/docker-compose.ci.yml b/.ci-config/docker-compose.ci.yml index bf42c40a..edd8e90a 100644 --- a/.ci-config/docker-compose.ci.yml +++ b/.ci-config/docker-compose.ci.yml @@ -1,6 +1,6 @@ services: xrpld: - image: xrpllabsofficial/xrpld:3.2.0 + image: xrpllabsofficial/xrpld:3.2.1 container_name: rippled-service command: ["-a", "--start"] ports: From e63e9ef84627d18e048b9659ed3a7b1ef6330f3f Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:00:21 -0300 Subject: [PATCH 2/7] =?UTF-8?q?fix(client):=20=D0=B8=D1=81=D0=BA=D0=BB?= =?UTF-8?q?=D1=8E=D1=87=D0=B5=D0=BD=D0=B8=D0=B5=20=D0=B8=D0=B7=20OnConnect?= =?UTF-8?q?ed=20=D0=B1=D0=BE=D0=BB=D1=8C=D1=88=D0=B5=20=D0=BD=D0=B5=20?= =?UTF-8?q?=D1=83=D0=B1=D0=B8=D0=B2=D0=B0=D0=B5=D1=82=20=D0=BA=D0=BB=D0=B8?= =?UTF-8?q?=D0=B5=D0=BD=D1=82=20=D0=BD=D0=B0=D0=B2=D1=81=D0=B5=D0=B3=D0=B4?= =?UTF-8?q?=D0=B0=20(10.10.1.0)=20(#72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(client): исключение из OnConnected больше не убивает клиент навсегда (10.10.1.0) Connection.OnceOpen ловил любое исключение потребительского обработчика OnConnected и звал Disconnect() - пользовательский путь отключения: _permanentlyDisconnected = true плюс ClearReconnectState(). После этого клиент мёртв навсегда: реконнект-цикл не перезапускается, новых сокетов не открывается, OnConnected больше не поднимается, а любой запрос падает с NotConnectedException. Наружу при этом не выходило ничего - ни события, ни лога. Триггер - самый штатный сценарий: перезапуск ноды. OnConnected - типовое место восстановления подписок (SDK их после реконнекта не восстанавливает), а нода принимает TCP раньше, чем начинает отвечать на запросы, поэтому первый subscribe уходит в RequestTimeout и падает. В проде так встал флот ботов - по четыре часа тишины после обновления ноды. Теперь сбой обработчика трактуется как отказ СОЕДИНЕНИЯ, а не как воля пользователя: сокет сносится, дальше работает обычный реконнект с экспоненциальным бэкоффом, флаг постоянного отключения не ставится. Защита от вечного цикла: OnceOpen чистит реконнект-состояние до вызова обработчика, поэтому счётчик попыток самого цикла обнуляется на каждом успешном TCP-коннекте и сойтись не может. Подряд идущие сбои обработчика считаются отдельно (_connectHandlerFailures, сбрасывается при успешном прогоне обработчика, в Connect() и в ChangeServer()); при достижении MaxReconnectAttempts с StopAfterMaxAttempts клиент осознанно сдаётся - сразу понятный NotConnectedException вместо пятиминутного молчания. Причина теперь наблюдаема: исключение поднимается через OnError с errorMessage = "connectHandlerError" и через OnConnectionStatus. Попутно: WebSocketClient.SendMessageAsync (async void, вызывается без await) больше не глотает ошибку отправки молча - она трассируется. Поведение не меняется, запрос по-прежнему ограничен RequestTimeout, но причина перестала быть невидимой при диагностике. TestUOnConnectedHandlerFailure закрывает все четыре свойства против mock rippled: восстановление после разового сбоя, работоспособность клиента после восстановления, отчёт через OnError и остановка вечно падающего обработчика. * fix(review): замечания CodeRabbit к PR #72 1. Детальная причина отказа терялась. SetConnectionState после Disconnect() - no-op: Disconnect() уже перевёл состояние в Disconnected, а SetConnectionState уведомляет только при смене состояния. Подписчик видел "Disconnected by user request." вместо причины. Уведомление перенесено перед Disconnect(). 2. WaitForConnectionAsync проверял _permanentlyDisconnected один раз на входе и никогда - внутри цикла ожидания. Ждущий в Connect() вызывающий досиживал весь ConnectionAcquisitionTimeout (по умолчанию 5 минут) и получал общий TimeoutException вместо реальной причины. Проверка перенесена в цикл. 3. Личность сокета при разборе. WebSocketClient.Connect вызывает OnConnect без await, поэтому connect-лок отпускается, пока обработчик ещё выполняется, и к моменту разбора ws может указывать уже на новый сокет. Закрывается ровно тот сокет, для которого падал обработчик; ws обнуляется, только если всё ещё ссылается на него; если сокет уже не текущий - реконнект-состояние принадлежит новому соединению и не трогается. При этом тест на вечно падающий обработчик вскрыл гонку в самом фиксе: проверка "цикл реконнекта уже работает" гонится с выходом этого цикла - он прерывается сразу, как сокет отрапортовал Open, то есть ДО того, как обработчик успел упасть. Проигрыш гонки оставлял клиента без реконнекта - ровно тот клин, который правится. Вместо проверки путь теперь безусловно забирает владение: гасит текущий цикл и запускает новый, а пришедший позже OnceClose видит живой цикл и корректно отступает. Побочно: восстановление стало занимать сотни миллисекунд вместо десятков секунд. 4. Ошибка отправки: Debug.WriteLine вырезается без DEBUG, которого нет в релизной сборке. Мёртвый колбэк ошибки WebSocketClient (его никто не звал и не подписывал) теперь доносит исключение до Connection.OnError с errorMessage = "socketSendError". Только уведомление: неудачная отправка сама по себе не означает потерю соединения, реконнект не запускается, запрос по-прежнему ограничен RequestTimeout. Отклонено: подъём версий Xrpl.AddressCodec / Xrpl.BinaryCodec / Xrpl.Keypairs до 10.10.1.0. Эти пакеты в этом PR не менялись, версионируются независимо и уже расходятся с основным на NuGet осознанно (Xrpl 10.10.0 против base 10.9.0; так же было в 10.9.1.0 и 10.10.0.0). Публикация идёт с --skip-duplicate, так что неизменные пакеты просто пропускаются, а подъём выложил бы побайтово те же артефакты под новым номером. Тест TestPermanentlyFailingOnConnectedHandlerStops усилен: теперь проверяет, что ожидающий вызывающий разблокируется именно NotConnectedException. * fix(review): вторая порция замечаний CodeRabbit к PR #72 1. Реконнект-цикл писал в чужую сессию. StopReconnectLoop() отменяет токен, но не дожидается цикла, поэтому снятый цикл мог добраться до тела или хвоста уже после того, как установлена замена, и обнулить _reconnectMode живого цикла, сбросить его _reconnectAttempts или выбросить его _reconnectCts. Дефект существовал и раньше (RetireCurrentSessionAndReconnectAsync снимает циклы ровно так же), но путь сбоя OnConnected делает его гораздо более достижимым. ReconnectLoopAsync теперь принимает CancellationTokenSource, которым владеет, и трогает общее состояние, только пока этот источник остаётся активным. 2. Mock-сервер в тестах не останавливался: CreateMockRippled.Start() создавал Server (его конструктор поднимает слушателя) и терял ссылку. Добавлен Stop(), TestUOnConnectedHandlerFailure зовёт его в TestCleanup. * fix(client): ChangeServer на неподнятый сервер больше не убивает клиент Тот же класс клина, что и основной фикс PR, но другой путь. Найдено при прогоне Blazor-демо: переключение селектора сети на выключенную ноду. ChangeServer ставил ГЛОБАЛЬНЫЙ _isIntentionalDisconnect = true, чтобы отфильтровать поздние колбэки уходящего сокета, а сбрасывался этот флаг только в OnceOpen. Если новый сервер не поднимался, OnceOpen не выполнялся никогда: OnConnectionFailed читал отказ НОВОГО соединения как отключение по воле пользователя, писал "Connection closed permanently.", не запускал реконнект-цикл, и дальше всё - включая сам ChangeServer - падало с вводящим в заблуждение "No connection attempt in progress. Call Connect() first.". Поднятие сервера потом ничего не меняло: клиент был мёртв. Поздние колбэки теперь фильтруются исключительно по-сокетному отслеживанием, которое здесь и так уже было (_userInitiatedSockets плюс собственный флаг сокета, выставляемый в RetireOldSessionAsync) - ровно так же, как всегда делал путь ping-timeout / network-drop; в его коде даже висит комментарий, предостерегающий от глобального флага именно по этой причине. Дополнительно флаг явно гасится на входе, чтобы ChangeServer после пользовательского Disconnect() не подавлялся оставшимся от него значением. Дефект существовал до этого PR - проверено дважды: ни одна из строк этого пути им не менялась, и одинаковый диагностический тест на чистом dev (f17fc11) даёт байт-в-байт тот же вывод. TestUChangeServerFailure закрывает оба случая: клиент доходит до нового сервера, когда тот появляется, с предшествующим Disconnect() и без него. Оба теста падают на dev и проходят с фиксом. * fix(review): гонка старта и остановки mock-сервера в тестах Start() выполняется на фоновом потоке, поэтому Stop() из TestCleanup мог увидеть _server == null до присваивания и оставить живого слушателя. Останов теперь фиксируется под локом: старт, завершившийся после него, гасит собственный Server вместо того, чтобы его сохранить. --- CHANGES.md | 12 + .../Client/TestUChangeServerFailure.cs | 182 +++++++++++++ .../Client/TestUOnConnectedHandlerFailure.cs | 251 ++++++++++++++++++ Tests/Xrpl.Tests/CreateMockRippled.cs | 51 ++++ Xrpl/Client/WebSocketClient.cs | 7 +- Xrpl/Client/connection.cs | 193 +++++++++++++- Xrpl/Xrpl.csproj | 2 +- 7 files changed, 687 insertions(+), 11 deletions(-) create mode 100644 Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs create mode 100644 Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs diff --git a/CHANGES.md b/CHANGES.md index 19dab341..b9a410e8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,17 @@ # Changes +## 10.10.1.0 08/02/2026 +* **An exception from an `OnConnected` handler no longer kills the client forever** — `Connection.OnceOpen` caught anything thrown by a consumer `OnConnected` handler and called `Disconnect()`, i.e. the *user* disconnect path: it set `_permanentlyDisconnected = true` and called `ClearReconnectState()`. After that the client was dead — the reconnect loop was never restarted, no new socket was ever opened, `OnConnected` never fired again, and every later request threw `NotConnectedException("Client has been disconnected. Call Connect() to reconnect.")`. Nothing was logged and nothing was raised, so from the outside the client just went quiet: + * **The trigger is the most ordinary event there is — a node restart.** `OnConnected` is the natural place to restore subscriptions, because the SDK does not restore them after a reconnect. A restarting node accepts TCP seconds before it starts answering requests, so the first `subscribe` after the reconnect runs into `RequestTimeout` (40 s) and throws. A consumer that lets the exception out — the reasonable "fail loudly, let the SDK reconnect" reaction — got the opposite: a silent, permanent death. Observed in production on a fleet of bots, each wedged for four hours after a node upgrade, one of them dying 69 seconds before the node came back + * A failing handler is now treated as what it is — a **connection** failure, not a user disconnect. The socket is torn down and the regular reconnect loop takes over with its usual exponential backoff, exactly as for a transport failure. The permanent-disconnect flag is never set on this path + * **A permanently broken handler cannot spin forever.** `OnceOpen` clears the reconnect state before invoking the handler, so the loop's own attempt counter resets on every successful TCP connect and could never converge. Consecutive handler failures are therefore counted separately (`_connectHandlerFailures`, reset on a successful handler run, on `Connect()` and on `ChangeServer()`); once they reach `MaxReconnectAttempts` with `StopAfterMaxAttempts` set, the client gives up deliberately — an immediate, actionable `NotConnectedException` instead of a silent five-minute wait — and `Connect()` clears the counter so recovery stays possible. With `StopAfterMaxAttempts = false` it keeps retrying, which is what that option asks for + * **The cause is now observable.** The exception is surfaced through `OnError` with `errorMessage = "connectHandlerError"` (the same shape already used for stream-handler failures) and through `OnConnectionStatus` — previously the reason the client died was reported nowhere at all + * `TestUOnConnectedHandlerFailure` pins all four properties against the mock rippled: a transient failure recovers and the client is usable again, the failure is reported through `OnError`, and a permanently failing handler stops instead of looping +* **`ChangeServer` to a server that is not up leaves the client reconnecting instead of dead** — a second wedge of the same family, found while exercising the fix above through the Blazor demo (switch the network selector to a node that is down). `ChangeServer` set the *global* `_isIntentionalDisconnect` flag to filter late callbacks from the socket it was retiring, and that flag was only ever reset in `OnceOpen`. If the new server never came up, `OnceOpen` never ran: `OnConnectionFailed` then read the failure of the **new** connection as a user disconnect, reported `"Connection closed permanently."`, started no reconnect loop, and every later call — including `ChangeServer` itself — failed with the misleading `"No connection attempt in progress. Call Connect() first."` Starting the server afterwards changed nothing; the client was dead. Late callbacks are now filtered purely by the per-socket tracking that was already in place (`_userInitiatedSockets` plus the socket's own flag), exactly as the ping-timeout/network-drop path has always done — its code even carries a comment warning against setting the global flag for this reason. The flag is additionally cleared on entry, so a `ChangeServer` after a user `Disconnect()` is not suppressed by the leftover either. `TestUChangeServerFailure` pins both cases: the client reaches the new server once it appears, with and without a preceding `Disconnect()` +* **The reconnect loop no longer writes to a reconnect session it no longer owns** — `StopReconnectLoop()` cancels the loop's token without awaiting the loop, so a retired loop could still reach its body or its tail after a replacement had been installed and clear the *live* loop's `_reconnectMode`, reset its `_reconnectAttempts` or dispose its `_reconnectCts`. Pre-existing (`RetireCurrentSessionAndReconnectAsync` has always retired loops this way), but the handler-failure path above makes it far more reachable, so `ReconnectLoopAsync` now takes the `CancellationTokenSource` it owns and touches shared state only while that source is still the active one +* **`WaitForConnectionAsync` now rechecks the permanent-disconnect flag on every iteration**, not only once on entry. A caller already blocked there when the client is disconnected — by `Disconnect()` from another thread, or by the give-up path above — used to sit out the whole `ConnectionAcquisitionTimeout` (default five minutes) and then receive a generic `TimeoutException`. It now returns the actual reason immediately as a `NotConnectedException` +* **`WebSocketClient.SendMessageAsync` no longer swallows send failures silently** — it is `async void` and is invoked without `await` from `Connection.WebsocketSendAsync`, so a failed send could be reported to nobody: the pending request simply sat there until its 40-second `RequestTimeout` expired. The socket's error callback (previously dead code — nothing ever invoked or wired it) now carries the exception to `Connection.OnError` with `errorMessage = "socketSendError"`. Report-only: a failed send does not by itself mean the connection is gone, so this path never triggers a reconnect and the request is still bounded by `RequestTimeout` — but the cause is no longer invisible during diagnosis + ## 10.10.0.0 07/29/2026 * **`ConnectionOptions.authorization` did nothing — now it does** — the option was public on `XrplClient.ClientOptions` since the xrpl.js port, but `Connection.CreateWebSocket` was a block of commented-out JS pseudocode ending in `WebSocketClient.Create(url); // todo add options`, and `WebSocketClient` had no parameter to receive them. Nothing the caller set on `authorization`, `headers`, `proxy`, `trustedCertificates`, `key`, `passphrase` or `certificate` ever reached the socket: * `authorization` now produces `Authorization: Basic base64(value)` on the WebSocket upgrade handshake, matching xrpl.js `createWebSocket` — the value is the raw `user:password` pair, the SDK does the base64 diff --git a/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs b/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs new file mode 100644 index 00000000..1ab724b6 --- /dev/null +++ b/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs @@ -0,0 +1,182 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using Xrpl.Client; + +namespace Xrpl.Tests +{ + /// + /// Regression tests for ChangeServer pointed at a server that is not up yet. + /// + /// ChangeServer used to set the global _isIntentionalDisconnect flag, which was only ever + /// reset in OnceOpen. When the new server never came up, the flag stayed set, the failure of the + /// new connection was read as a user disconnect ("Connection closed permanently."), no reconnect loop + /// was started, and every later call failed with "No connection attempt in progress. Call Connect() + /// first." - the client was dead even after the server came up. + /// + /// + [TestClass] + public class TestUChangeServerFailure + { + private CreateMockRippled _mockedRippled; + private CreateMockRippled _secondRippled; + private XrplClient _client; + private int _port; + + private static Dictionary ServerInfoResponse() => new Dictionary + { + { "type", "response" }, + { "status", "success" }, + { "result", new Dictionary + { + { "info", new Dictionary + { + { "build_version", "test-mock" }, + { "complete_ledgers", "1-1" }, + { "server_state", "full" }, + } + }, + } + }, + }; + + private static CreateMockRippled StartMock(int port) + { + CreateMockRippled mock = new CreateMockRippled(port) { suppressOutput = true }; + mock.AddResponse("server_info", ServerInfoResponse()); + + Thread listenerThread = new Thread(() => mock.Start()) { IsBackground = true }; + listenerThread.Start(); + return mock; + } + + [TestInitialize] + public void MyTestInitialize() + { + _port = TestUtils.GetFreePort(); + _mockedRippled = StartMock(_port); + } + + [TestCleanup] + public async Task MyTestCleanup() + { + if (_client != null) + { + await _client.Disconnect(); + _client = null; + } + + _mockedRippled?.Stop(); + _secondRippled?.Stop(); + } + + /// + /// Switching to a server that is not listening yet must leave the client reconnecting, so it comes + /// up on its own once that server appears - not stranded in a permanent disconnect. + /// + [TestMethod] + public async Task TestChangeServerToUnreachableServerRecoversWhenItComesUp() + { + _client = new XrplClient($"ws://127.0.0.1:{_port}", new XrplClient.ClientOptions + { + RequestPolicy = RequestFailurePolicy.ImmediateFail, + ReconnectBaseDelay = TimeSpan.FromMilliseconds(200), + ReconnectMaxDelay = TimeSpan.FromSeconds(1), + MaxReconnectAttempts = 50, + StopAfterMaxAttempts = false, + // Short on purpose: ChangeServer gives up waiting quickly, but the reconnect loop it left + // behind is what this test is about. + ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(3), + ConnectionAttemptTimeout = TimeSpan.FromSeconds(3), + UseCustomPing = false, + }); + + await _client.Connect(); + Assert.IsTrue(_client.connection.IsConnected(), "Precondition: client must be connected to the first server."); + + int secondPort = TestUtils.GetFreePort(); // nothing is listening there yet + + try + { + await _client.connection.ChangeServer($"ws://127.0.0.1:{secondPort}"); + } + catch (Exception) + { + // Expected - the target is not up yet. What matters is the state it leaves behind. + } + + Assert.AreNotEqual( + XrpConnectionState.Disconnected, + _client.connection.CurrentConnectionState, + "A ChangeServer target that is down is a connection failure, not a permanent disconnect."); + + // The server appears afterwards - exactly the "start the node later" case. + _secondRippled = StartMock(secondPort); + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); + while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline) + { + await Task.Delay(TimeSpan.FromMilliseconds(200)); + } + + Assert.IsTrue( + _client.connection.IsConnected(), + $"Client never reached the new server after it came up (state: {_client.connection.CurrentConnectionState})."); + Assert.AreEqual($"ws://127.0.0.1:{secondPort}", _client.connection.GetUrl()); + + Dictionary response = + await _client.Request(new Dictionary { { "command", "server_info" } }); + Assert.IsNotNull(response, "Client must be usable on the new server."); + } + + /// + /// The same, after an explicit user Disconnect(): the global intentional-disconnect flag left + /// behind by it must not suppress reconnection for the server ChangeServer switches to. + /// + [TestMethod] + public async Task TestChangeServerAfterUserDisconnectStillReconnects() + { + _client = new XrplClient($"ws://127.0.0.1:{_port}", new XrplClient.ClientOptions + { + RequestPolicy = RequestFailurePolicy.ImmediateFail, + ReconnectBaseDelay = TimeSpan.FromMilliseconds(200), + ReconnectMaxDelay = TimeSpan.FromSeconds(1), + MaxReconnectAttempts = 50, + StopAfterMaxAttempts = false, + ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(3), + ConnectionAttemptTimeout = TimeSpan.FromSeconds(3), + UseCustomPing = false, + }); + + await _client.Connect(); + await _client.Disconnect(); + + int secondPort = TestUtils.GetFreePort(); + + try + { + await _client.connection.ChangeServer($"ws://127.0.0.1:{secondPort}"); + } + catch (Exception) + { + // Expected - the target is not up yet. + } + + _secondRippled = StartMock(secondPort); + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); + while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline) + { + await Task.Delay(TimeSpan.FromMilliseconds(200)); + } + + Assert.IsTrue( + _client.connection.IsConnected(), + $"Client never reached the new server after a user disconnect (state: {_client.connection.CurrentConnectionState})."); + } + } +} diff --git a/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs b/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs new file mode 100644 index 00000000..2869b432 --- /dev/null +++ b/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs @@ -0,0 +1,251 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using Xrpl.Client; +using Xrpl.Client.Exceptions; + +namespace Xrpl.Tests +{ + /// + /// Regression tests for the "silent wedge": an exception thrown by a consumer OnConnected + /// handler used to trigger the user-disconnect path (_permanentlyDisconnected = true), + /// which killed the client forever instead of reconnecting. + /// + [TestClass] + public class TestUOnConnectedHandlerFailure + { + private CreateMockRippled _mockedRippled; + private XrplClient _client; + private int _port; + + [TestInitialize] + public void MyTestInitialize() + { + _port = TestUtils.GetFreePort(); + _mockedRippled = new CreateMockRippled(_port) { suppressOutput = true }; + _mockedRippled.AddResponse("server_info", new Dictionary + { + { "type", "response" }, + { "status", "success" }, + { "result", new Dictionary + { + { "info", new Dictionary + { + { "build_version", "test-mock" }, + { "complete_ledgers", "1-1" }, + { "server_state", "full" }, + } + }, + } + }, + }); + + Thread tcpListenerThread = new Thread(() => _mockedRippled.Start()) { IsBackground = true }; + tcpListenerThread.Start(); + } + + [TestCleanup] + public async Task MyTestCleanup() + { + if (_client != null) + { + await _client.Disconnect(); + _client = null; + } + + _mockedRippled?.Stop(); + } + + private XrplClient CreateClient(int maxReconnectAttempts, bool stopAfterMaxAttempts) => + new XrplClient($"ws://127.0.0.1:{_port}", new XrplClient.ClientOptions + { + RequestPolicy = RequestFailurePolicy.ImmediateFail, + ReconnectBaseDelay = TimeSpan.FromMilliseconds(100), + ReconnectMaxDelay = TimeSpan.FromSeconds(1), + MaxReconnectAttempts = maxReconnectAttempts, + StopAfterMaxAttempts = stopAfterMaxAttempts, + ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(20), + ConnectionAttemptTimeout = TimeSpan.FromSeconds(10), + UseCustomPing = false, + }); + + /// + /// A transient failure inside OnConnected (e.g. a subscribe that timed out because the + /// node accepts TCP before it serves requests) must not strand the client: the socket is torn + /// down and the regular reconnect loop must bring it back. + /// + [TestMethod] + public async Task TestTransientOnConnectedFailureRecovers() + { + int invocations = 0; + TaskCompletionSource reconnected = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _client = CreateClient(maxReconnectAttempts: 10, stopAfterMaxAttempts: false); + _client.connection.OnConnected += () => + { + if (Interlocked.Increment(ref invocations) == 1) + { + throw new InvalidOperationException("subscribe failed after connect"); + } + + reconnected.TrySetResult(true); + return Task.CompletedTask; + }; + + Exception connectError = null; + try + { + await _client.Connect(); + } + catch (Exception error) + { + connectError = error; + } + + Task completed = await Task.WhenAny(reconnected.Task, Task.Delay(TimeSpan.FromSeconds(30))); + + Assert.AreSame( + reconnected.Task, + completed, + $"Client never reconnected after OnConnected threw (invocations: {Volatile.Read(ref invocations)}, connect error: {connectError?.Message ?? "none"})"); + Assert.IsTrue(_client.connection.IsConnected(), "Client must be connected again after recovery."); + } + + /// + /// After recovery the client must still be usable — the permanent-disconnect flag must not be set. + /// + [TestMethod] + public async Task TestClientIsUsableAfterOnConnectedFailure() + { + int invocations = 0; + TaskCompletionSource reconnected = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _client = CreateClient(maxReconnectAttempts: 10, stopAfterMaxAttempts: false); + _client.connection.OnConnected += () => + { + if (Interlocked.Increment(ref invocations) == 1) + { + throw new InvalidOperationException("subscribe failed after connect"); + } + + reconnected.TrySetResult(true); + return Task.CompletedTask; + }; + + try + { + await _client.Connect(); + } + catch (Exception) + { + // Recovery is asserted below - the initial Connect() may observe the failed attempt. + } + + Task completed = await Task.WhenAny(reconnected.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.AreSame(reconnected.Task, completed, "Client never reconnected after OnConnected threw."); + + Dictionary request = new Dictionary + { + { "command", "server_info" }, + }; + + Dictionary response = await _client.Request(request); + Assert.IsNotNull(response, "Request after recovery must succeed."); + } + + /// + /// A permanently broken handler must not spin forever: with StopAfterMaxAttempts the client + /// gives up after MaxReconnectAttempts consecutive handler failures. + /// + [TestMethod] + public async Task TestPermanentlyFailingOnConnectedHandlerStops() + { + const int maxAttempts = 3; + int invocations = 0; + + _client = CreateClient(maxReconnectAttempts: maxAttempts, stopAfterMaxAttempts: true); + _client.connection.OnConnected += () => + { + Interlocked.Increment(ref invocations); + throw new InvalidOperationException("handler is permanently broken"); + }; + + Exception connectError = null; + try + { + await _client.Connect(); + } + catch (Exception error) + { + connectError = error; + } + + Assert.IsInstanceOfType( + connectError, + $"Giving up must unblock the waiting caller with NotConnectedException, got: {connectError?.GetType().Name ?? "no exception"}."); + + await Task.Delay(TimeSpan.FromSeconds(10)); + int settled = Volatile.Read(ref invocations); + await Task.Delay(TimeSpan.FromSeconds(5)); + + Assert.AreEqual( + settled, + Volatile.Read(ref invocations), + "Client kept retrying a permanently failing OnConnected handler instead of giving up."); + Assert.IsTrue( + settled <= maxAttempts + 1, + $"Handler was retried {settled} times, expected at most {maxAttempts + 1}."); + Assert.IsFalse(_client.connection.IsConnected(), "Client must not report a live connection."); + } + + /// + /// The reason the connection was torn down must be observable through OnError. + /// + [TestMethod] + public async Task TestOnConnectedFailureIsReportedThroughOnError() + { + int invocations = 0; + TaskCompletionSource reported = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _client = CreateClient(maxReconnectAttempts: 10, stopAfterMaxAttempts: false); + _client.connection.OnError += (error, errorMessage, message, data) => + { + if (errorMessage == "connectHandlerError") + { + reported.TrySetResult(message); + } + + return Task.CompletedTask; + }; + _client.connection.OnConnected += () => + { + if (Interlocked.Increment(ref invocations) == 1) + { + throw new InvalidOperationException("subscribe failed after connect"); + } + + return Task.CompletedTask; + }; + + try + { + await _client.Connect(); + } + catch (Exception) + { + // The failure itself is asserted through OnError below. + } + + Task completed = await Task.WhenAny(reported.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.AreSame(completed, reported.Task, "OnConnected failure was never reported through OnError."); + StringAssert.Contains(reported.Task.Result, "subscribe failed after connect"); + } + } +} diff --git a/Tests/Xrpl.Tests/CreateMockRippled.cs b/Tests/Xrpl.Tests/CreateMockRippled.cs index 37cd57e1..fddc44a4 100644 --- a/Tests/Xrpl.Tests/CreateMockRippled.cs +++ b/Tests/Xrpl.Tests/CreateMockRippled.cs @@ -68,12 +68,51 @@ public class CreateMockRippled private Dictionary _responses = new Dictionary(); public bool suppressOutput = false; private Thread tcpListenerThread; + private readonly object _serverLock = new object(); + private Server _server; + private bool _stopped; public CreateMockRippled(int port) { this._port = port; } + /// + /// Stops the listen socket. Without this the server keeps accepting for the lifetime of the test + /// process, so every test that starts a mock leaks a listener. + /// Start() runs on a background thread, so shutdown is recorded here: a startup that finishes + /// afterwards stops its listener instead of leaving it behind. + /// + public void Stop() + { + Server server; + lock (_serverLock) + { + _stopped = true; + server = _server; + _server = null; + } + + StopServer(server); + } + + private static void StopServer(Server server) + { + if (server == null) + { + return; + } + + try + { + server.Stop(); + } + catch (Exception ex) + { + Debug.WriteLine($"MockRippled stop error: {ex.Message}"); + } + } + string CreateResponse(Dictionary request, Dictionary response) { var cloneResp = new Dictionary(response); @@ -218,6 +257,18 @@ public void Start() Server server = new Server(new IPEndPoint(IPAddress.Parse("127.0.0.1"), this._port)); + lock (_serverLock) + { + if (_stopped) + { + // Stop() already ran - do not leave this listener accepting behind the test's back. + StopServer(server); + return; + } + + _server = server; + } + // Bind the event for when a client connected server.OnClientConnected += (object sender, OnClientConnectedHandler e) => { diff --git a/Xrpl/Client/WebSocketClient.cs b/Xrpl/Client/WebSocketClient.cs index 97cf200a..dd8636e8 100644 --- a/Xrpl/Client/WebSocketClient.cs +++ b/Xrpl/Client/WebSocketClient.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Net.WebSockets; @@ -307,7 +308,11 @@ private async void SendMessageAsync(byte[] message) } catch (Exception e) { - //_onError?.Invoke(e, this); + // The send is fire-and-forget (async void), so nothing can observe this exception: + // the pending request just sits there until its RequestTimeout expires. Surface it + // through the error callback - report-only, the connection itself is left alone. + Debug.WriteLine($"{DateTime.Now}WebSocket send failed: {e.GetType().Name}: {e.Message}"); + CallOnError(e); return; } } diff --git a/Xrpl/Client/connection.cs b/Xrpl/Client/connection.cs index d443c3dd..8860add2 100644 --- a/Xrpl/Client/connection.cs +++ b/Xrpl/Client/connection.cs @@ -280,6 +280,11 @@ private static WebSocketClient CreateWebSocket(string url, ConnectionOptions con private int _reconnectAttempts = 0; + // Number of consecutive times the consumer OnConnected handler threw. + // Not part of the reconnect state: OnceOpen clears the reconnect state before invoking the handler, + // so this counter is the only thing that can bound an endlessly failing handler. + private int _connectHandlerFailures = 0; + private static readonly Random _random = new(); private CancellationTokenSource _reconnectCts; @@ -456,10 +461,16 @@ public async Task ChangeServer( ws = null; } - // 7. Mark socket for intentional disconnect + // 7. Mark old socket for intentional disconnect (per-socket tracking only) + // CRITICAL: Do NOT set global _isIntentionalDisconnect = true here - same rule as the ping/network + // recovery path. The global flag was only reset in OnceOpen, so if the NEW server never came up it + // stayed set forever: OnConnectionFailed then read the failure of the new socket as a user disconnect, + // reported "Connection closed permanently." and started no reconnect loop, leaving the client dead + // with the misleading "No connection attempt in progress. Call Connect() first." + // Per-socket tracking (_userInitiatedSockets + the socket's own flag, set in RetireOldSessionAsync) + // already filters late callbacks from the old socket, and keeps global state clean for the new one. if (oldSocket != null) { - _isIntentionalDisconnect = true; Interlocked.Exchange(ref _userInitiatedSocket, oldSocket); MarkSocketAsUserInitiated(oldSocket); @@ -476,11 +487,15 @@ public async Task ChangeServer( ValidateConfig(); _reconnectAttempts = 0; + Interlocked.Exchange(ref _connectHandlerFailures, value: 0); // 8. Reset permanentlyDisconnected for new connection _permanentlyDisconnected = false; - // _isIntentionalDisconnect stays true - reset in OnceOpen + // Clear the global intentional-disconnect flag explicitly: it may still be set from an earlier + // user Disconnect() (it is only ever reset in OnceOpen), and leaving it set would make a failure + // of the NEW connection look intentional and suppress reconnection. + _isIntentionalDisconnect = false; // 9. Immediately connect to new server (new session created in Connect) await Connect(cancellationToken); @@ -659,6 +674,15 @@ public async Task WaitForConnectionAsync(TimeSpan? timeout = null, CancellationT while (!IsConnected()) { + // Re-checked on every iteration, not only on entry: the client can be disconnected while a + // caller is already waiting here (user Disconnect(), or the client giving up on a permanently + // failing OnConnected handler). Without this the caller would sit out the whole acquisition + // timeout and get a generic TimeoutException instead of the actual reason. + if (_permanentlyDisconnected) + { + throw new NotConnectedException("Client has been disconnected. Call Connect() to reconnect."); + } + if (config.StopAfterMaxAttempts && _reconnectAttempts >= config.MaxReconnectAttempts && _reconnectCts == null) @@ -718,6 +742,7 @@ public async Task Connect(CancellationToken cancellationToken) } StopReconnectLoop(); + Interlocked.Exchange(ref _connectHandlerFailures, value: 0); SetConnectionState(XrpConnectionState.Connecting, message: $"Connecting to {url}..."); await ConnectInternalAsync(); await WaitForConnectionAsync(config.ConnectionAcquisitionTimeout, cancellationToken); @@ -838,6 +863,29 @@ await OnConnectionFailed( } }); + ws.OnError(async (e, errorSocket) => + { + try + { + // Report-only: a failed send does not by itself mean the connection is gone, so this + // path never triggers a reconnect. Without it a fire-and-forget send failure would be + // invisible and the request would simply sit until its RequestTimeout expires. + var errorHandler = OnError; + if (errorHandler is not null) + { + await errorHandler.Invoke( + error: "error", + errorMessage: "socketSendError", + e.Message, + data: e); + } + } + catch (Exception ex) + { + Debug.WriteLine($"{DateTime.Now}OnError callback error: {ex.Message}"); + } + }); + ws.OnMessageReceived(async (m, ws) => { try @@ -1623,15 +1671,16 @@ private async Task OnceOpen(WebSocketClient connectedSocket, long sessionId) await OnConnected?.Invoke(); } + Interlocked.Exchange(ref _connectHandlerFailures, value: 0); SetConnectionState(XrpConnectionState.Connected, message: $"Connected {url}"); } catch (Exception error) { connectionManager.RejectAllAwaiting(error); - await Disconnect(); + await OnConnectHandlerFailedAsync(connectedSocket, error); return; // Don't start ping timer if connection failed } - + // Start ping timer AFTER connection is fully established and all callbacks completed // This is outside try/catch to ensure it always runs on successful connection StartPingTimer(); @@ -1640,6 +1689,109 @@ private async Task OnceOpen(WebSocketClient connectedSocket, long sessionId) StartMessageProcessor(); } + /// + /// Handles an exception thrown by a consumer handler. + /// + /// A failing handler is a CONNECTION failure, not a user disconnect. Calling here + /// would set the permanent-disconnect flag and clear the reconnect state, stranding the client forever: + /// no reconnect loop is restarted, no new socket is ever opened and every later request fails with + /// . This is a very reachable scenario - restoring subscriptions in + /// fails whenever the node accepts TCP before it starts serving requests. + /// + /// + /// Instead the socket is torn down as a transport failure so the regular reconnect loop (with exponential + /// backoff) brings the client back. A handler that keeps failing is bounded by + /// when + /// is set, so a broken consumer cannot spin forever. + /// + /// + /// The socket whose handler threw. + /// The exception thrown by the handler. + private async Task OnConnectHandlerFailedAsync(WebSocketClient failedSocket, Exception error) + { + int failures = Interlocked.Increment(ref _connectHandlerFailures); + + Debug.WriteLine($"{DateTime.Now}OnConnected handler failed ({failures}): {error.Message}"); + + var errorHandler = OnError; + if (errorHandler is not null) + { + try + { + await errorHandler + .Invoke(error: "error", errorMessage: "connectHandlerError", error.Message, data: error) + .ConfigureAwait(false); + } + catch (Exception notifyError) + { + Debug.WriteLine($"{DateTime.Now}OnError handler threw while reporting OnConnected failure: {notifyError.Message}"); + } + } + + bool giveUp = config.StopAfterMaxAttempts && failures >= config.MaxReconnectAttempts; + if (giveUp) + { + // Terminal state on purpose: the handler is broken, not the connection. Disconnect() gives the + // consumer an immediate, actionable NotConnectedException instead of a silent 5-minute wait, + // and Connect() resets the counter so recovery stays possible. + // The detailed reason has to be notified BEFORE Disconnect(): Disconnect() moves the state to + // Disconnected itself, and SetConnectionState only notifies on a state change, so a call after it + // would be swallowed and the consumer would see "Disconnected by user request." instead. + SetConnectionState( + XrpConnectionState.Disconnected, + message: + $"OnConnected handler failed {failures} time(s) in a row: {error.Message}. Giving up after {config.MaxReconnectAttempts} attempts. Call Connect() to retry.", + ConnectionCloseSeverity.Error); + + await Disconnect(); + return; + } + + SetConnectionState( + XrpConnectionState.RestoringConnection, + message: $"OnConnected handler failed: {error.Message}. Reconnecting...", + ConnectionCloseSeverity.Warning, + reconnect: BuildReconnectInfo(failures)); + + StopPingTimerSync(); + requestManager.RejectAllWithCancellation(); + await WaitForPingToFinishAsync(); + + // Always tear down the socket the handler actually ran for. WebSocketClient.Connect invokes its + // OnConnect callback without awaiting it, so the connect lock can be released while this method is + // still running: by now `ws` may already point at a newer socket that must not be touched. + bool wasCurrentSocket; + lock (_disconnectLock) + { + wasCurrentSocket = ReferenceEquals(ws, failedSocket); + if (wasCurrentSocket) + { + ws = null; + } + } + + // The socket is deliberately NOT marked as user-initiated: OnceClose must treat this as a real + // close so the standard reconnect path runs instead of the "closed permanently" branch. + failedSocket.Cancel(); + failedSocket.Disconnect(); + + if (!wasCurrentSocket) + { + // A newer connection already replaced this socket - it owns the reconnect state now. + return; + } + + // Take ownership of the reconnect state instead of asking "is a loop already running?". + // This method can run inside the reconnect loop's own attempt: that loop breaks as soon as the + // socket reports Open, which happens before the handler has even finished failing. Both this check + // and the one in OnceClose would then race with the loop's exit, and losing the race leaves nobody + // reconnecting - the very wedge this path exists to prevent. Cancel whatever is there, start fresh; + // the later OnceClose sees a live loop and correctly stands down. + StopReconnectLoop(); + _reconnectLoop = null; + StartReconnectLoop(); + } + private async Task OnceClose(int? code, string? description, WebSocketClient closingSocket, long sessionId) { var (severity, userMessage) = DescribeClose(code, description); @@ -1837,11 +1989,16 @@ private void StartReconnectLoop() // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence // Note: _reconnectLoop was already cleared by RetireCurrentSessionAndReconnectAsync - _reconnectLoop = ReconnectLoopAsync(_reconnectCts.Token); + _reconnectLoop = ReconnectLoopAsync(_reconnectCts); } - private async Task ReconnectLoopAsync(CancellationToken ct) + private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) { + // The CTS this loop owns. StopReconnectLoop cancels without awaiting the loop, so a retired loop + // can still be running - or reach its tail - after a replacement has been installed. Everything + // this loop writes to shared reconnect state is therefore guarded by an ownership check. + CancellationToken ct = ownCts.Token; + // Don't reset _reconnectAttempts here - it may be pre-set to 1 by fast reconnect path // StartReconnectLoop() sets it to 0 when creating a new CTS @@ -1855,6 +2012,12 @@ private async Task ReconnectLoopAsync(CancellationToken ct) while (!ct.IsCancellationRequested) { + if (!ReferenceEquals(_reconnectCts, ownCts)) + { + // Retired: a newer loop owns the reconnect sequence now. + break; + } + _reconnectAttempts++; // Skip delay for first attempt if this is immediate reconnect (ping timeout or network drop) @@ -1938,7 +2101,11 @@ private async Task ReconnectLoopAsync(CancellationToken ct) if (IsConnected()) { - _reconnectAttempts = 0; + if (ReferenceEquals(_reconnectCts, ownCts)) + { + _reconnectAttempts = 0; + } + break; } } @@ -1969,13 +2136,21 @@ private async Task ReconnectLoopAsync(CancellationToken ct) // This ensures late callbacks from ping-timeout socket are still filtered // even if reconnect attempts fail + // A newer loop may already have taken over (this one was retired by StopReconnectLoop, which does + // not await it). Its state belongs to that loop: clearing the mode or disposing the CTS here would + // strand the live reconnect sequence. + if (!ReferenceEquals(_reconnectCts, ownCts)) + { + return; + } + // When loop exits (cancelled, max attempts, or success) and connection is not established, // clear the reconnect mode. If connected, OnceOpen already cleared it. if (!IsConnected()) { _reconnectMode = ReconnectMode.None; } - + if (config.StopAfterMaxAttempts && _reconnectAttempts >= config.MaxReconnectAttempts) { _reconnectCts?.Dispose(); diff --git a/Xrpl/Xrpl.csproj b/Xrpl/Xrpl.csproj index 745c024a..d49f6712 100644 --- a/Xrpl/Xrpl.csproj +++ b/Xrpl/Xrpl.csproj @@ -14,7 +14,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 10.10.0.0 + 10.10.1.0 From 1d9fa48a2309ab67c835c90e763c5bc8d795ce3b Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:04:38 -0300 Subject: [PATCH 3/7] =?UTF-8?q?feat(models):=20=D1=84=D0=BB=D0=B0=D0=B3?= =?UTF-8?q?=D0=B8=20ledger-=D0=BE=D0=B1=D1=8A=D0=B5=D0=BA=D1=82=D0=BE?= =?UTF-8?q?=D0=B2,=20=D0=B3=D0=B2=D0=B0=D1=80=D0=B4=20=D0=BF=D0=BE=20Ledge?= =?UTF-8?q?rFormats.h=20=D0=B8=20=D0=BF=D0=BE=D0=BA=D1=80=D1=8B=D1=82?= =?UTF-8?q?=D0=B8=D0=B5=20DynamicMPT=20(10.11.0.0)=20(#71)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(models): именованные lsf-флаги MPT и интеграционное покрытие DynamicMPT Модели MPT отставали от rippled по флагам ledger-объектов, а DynamicMPT (XLS-94) не имел ни гварда, ни интеграционных тестов: поля MutableFlags в моделях были, но против ноды никогда не проверялись. * MPTokenIssuanceFlags + MPTCanHoldConfidentialBalance (0x80) — вводит ConfidentialTransfer; остальная поддержка амендмента уже полная (транзакции 85-89, IssuerEncryptionKey/AuditorEncryptionKey, ConfidentialOutstandingAmount), не хватало только имени у флага * MPTokenFlags + lsfMPTAMM (0x4) — пробел старше: флаг есть уже в 3.2.1. AMMCreate ставит его вместе с lsfMPTAuthorized, неявно авторизуя MPT-актив для псевдо-аккаунта AMM (src/libxrpl/tx/transactors/dex/AMMCreate.cpp) * AmendmentGuard + DynamicMPT; id совпал с тем, что generate-amendments.sh уже кладёт в [amendments] nightly-стенда * TestIDynamicMPT — 4 теста, каждый читает результат из ledger-объекта: MutableFlags с Create доезжают в LOMPTokenIssuance; MPTokenIssuanceSet меняет TransferFee и MPTokenMetadata, не переписывая MutableFlags; tmfMPTSetCanLock поднимает lsfMPTCanLock на issuance, созданном без него; мутация без разрешения отбивается tecNO_PERMISSION, а метаданные в ledger остаются прежними Сценарии выверены по транзактору rippled 3.3.0-rc1 (src/libxrpl/tx/transactors/token/MPTokenIssuanceSet.cpp), а не по документации: отсюда tfMPTCanTransfer на создании — preclaim требует lsfMPTCanTransfer уже установленным, включение его той же транзакцией правило не удовлетворяет. Проверено: nightly-стенд (3.3.0-b1, DynamicMPT enabled) — 4/4 passed; CI-стенд (xrpld 3.2.0) — 4 skipped через гвард, exit 0; MPT-регрессия 28/28, юнит-тесты 867/867. Значение 0x80 подтверждено на живом объекте: MPTokenIssuanceSet с MutableFlags=0x40 даёт Flags=128. * feat(models): флаги ledger-объектов по LedgerFormats.h и гвард против дрейфа LedgerFormats.h — единственное место, где протокол объявляет, какие lsf-флаги принадлежат какому ledger-объекту (definitions.json несёт коды полей и типы объектов, но не значения флагов). Против него не стоял ни один тест, поэтому пропущенный флаг не давал симптома: безымянный бит всё равно приходит в модель числом, теряется только возможность проверить его по имени. Так lsfMPTAMM прожил несколько релизов. Пофайловый диф всех enum'ов против тега 3.3.0-rc1 нашёл четыре пробела. * MPTokenIssuanceFlags + MPTCanHoldConfidentialBalance (0x80) — от ConfidentialTransfer; значение подтверждено на живом объекте * MPTokenFlags + lsfMPTAMM (0x4) — пробел старше, флаг есть уже в 3.2.1 * LOLoan.Flags — у объекта Loan вообще не было свойства Flags (нет его и в BaseLedgerEntry), то есть lsfLoanDefault/lsfLoanImpaired/lsfLoanOverpayment не читались типизированной моделью никак: статус дефолта и impairment займа был ненаблюдаем. Добавлено как LoanFlags? * новые SignerListFlags и DirectoryNodeFlags — сами свойства Flags остаются raw uint (смена типа была бы breaking), enum'ы дают именованные константы вместо магических чисел; ложный комментарий у LODirectoryNode.Flags про «протокол не определяет флагов» исправлен TestULedgerFlagsConformance — ledger-аналог TestUTxFormatConformance: вендоренный Fixtures/LedgerFormats.h с пином по sha, парсер, падающий громко на незнакомом LSF_FLAG* и на тощем разборе, диф в обе стороны и требование регистрировать каждый флагованный объект — новый объект роняет сборку, а не пропускается молча. Проверен мутациями: неверное значение, удалённый флаг и незарегистрированный объект дают внятное падение. Интеграционное покрытие DynamicMPT (XLS-94): AmendmentGuard + TestIDynamicMPT, 4 теста, каждый читает результат из ledger-объекта. Сценарии выверены по транзактору rippled, а не по документации. Версия Xrpl поднята до 10.11.0.0 (minor: только добавления в публичном API), base-пакеты не менялись. CHANGES.md дополнен. Проверено: nightly-стенд — TestILoan/TestIDynamicMPT/TestIMPToken 41/41 passed; юнит-тесты 869/869; сборка решения без ошибок. * ci(protocol-watch): следить за LedgerFormats.h TestULedgerFlagsConformance сравнивает модели с ПРИПИНЕННОЙ копией LedgerFormats.h, поэтому сам по себе он не может заметить, что апстрим ушёл вперёд — этот сигнал даёт protocol-watch. Заголовка в его списке не было, и это вторая половина причины, по которой lsfMPTAMM прожил несколько релизов: не только никто не сверял флаги, но и никто не сообщал, что файл изменился. Заодно .ref фикстуры перестаёт врать: там написано «когда protocol-watch сообщит об изменении этого файла — замени копию целиком», а вотчер за ним не следил. Путь отсутствует в baseline, поэтому первый же прогон отметит заголовок как изменившийся один раз и внесёт его в baseline наравне с остальными — union ключей в сравнении это предусматривает. --- .github/workflows/protocol-watch.yml | 9 +- CHANGES.md | 23 ++ Tests/Xrpl.Tests/Fixtures/LedgerFormats.h | 322 ++++++++++++++++++ Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref | 22 ++ .../Xrpl.Tests/Integration/AmendmentGuard.cs | 3 + .../transactions/TestIDynamicMPT.cs | 211 ++++++++++++ Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs | 110 ++++++ .../Models/TestULedgerFlagsConformance.cs | 149 ++++++++ Tests/Xrpl.Tests/Xrpl.Tests.csproj | 3 + Xrpl/Models/Ledger/LODirectoryNode.cs | 25 +- Xrpl/Models/Ledger/LOLoan.cs | 28 ++ Xrpl/Models/Ledger/LOMPToken.cs | 5 + Xrpl/Models/Ledger/LOMPTokenIssuance.cs | 6 + Xrpl/Models/Ledger/LOSignerList.cs | 17 + Xrpl/Xrpl.csproj | 4 +- 15 files changed, 930 insertions(+), 7 deletions(-) create mode 100644 Tests/Xrpl.Tests/Fixtures/LedgerFormats.h create mode 100644 Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref create mode 100644 Tests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.cs create mode 100644 Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs create mode 100644 Tests/Xrpl.Tests/Models/TestULedgerFlagsConformance.cs diff --git a/.github/workflows/protocol-watch.yml b/.github/workflows/protocol-watch.yml index 2d46582c..f0295cde 100644 --- a/.github/workflows/protocol-watch.yml +++ b/.github/workflows/protocol-watch.yml @@ -7,9 +7,11 @@ name: Protocol Watch # # Coverage is an explicit list of full repo-relative paths (WATCH), so files # from any directory can be tracked: the five protocol .macro definition files -# plus TxFlags.h (transaction flags) and TER.h (result codes), which the SDK -# mirrors as flag enums and the EngineResult enum but which live outside the -# macros. +# plus three headers the SDK mirrors but which live outside the macros — +# TxFlags.h (transaction flags), TER.h (result codes) and LedgerFormats.h +# (lsf ledger-object flags, vendored as a test fixture and diffed by +# TestULedgerFlagsConformance, which compares against a pinned copy and so +# cannot notice upstream moving on its own). # # State lives in the tracking issue body (labeled protocol-watch), not in the # repo and not in actions cache: no commits through branch protection, no @@ -40,6 +42,7 @@ env: include/xrpl/protocol/detail/transactions.macro include/xrpl/protocol/TxFlags.h include/xrpl/protocol/TER.h + include/xrpl/protocol/LedgerFormats.h ISSUE_TITLE: 'Protocol watch: rippled develop' ISSUE_LABEL: protocol-watch diff --git a/CHANGES.md b/CHANGES.md index b9a410e8..4466a442 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,28 @@ # Changes +## 10.11.0.0 08/03/2026 +* **Ledger-object flags the protocol declares but the models never named** — an unnamed bit still arrives in the model as a number, so reading the object kept working and only the consumer's ability to test it by name was lost. That is why these went unnoticed; a field-by-field diff of rippled `LedgerFormats.h` (tag `3.3.0-rc1`) against every flag enum found four gaps: + * `MPTokenIssuanceFlags` + **`MPTCanHoldConfidentialBalance`** (0x80) — introduced by ConfidentialTransfer. The rest of the amendment was already complete (transactions 85–89, `IssuerEncryptionKey`/`AuditorEncryptionKey`, `ConfidentialOutstandingAmount`); only the flag had no name. Value confirmed against a live node: `MPTokenIssuanceSet` with `MutableFlags = tmfMPTSetCanHoldConfidentialBalance` moves the issuance from `Flags = 0` to `Flags = 128` + * `MPTokenFlags` + **`lsfMPTAMM`** (0x4) — a much older gap: the flag is present as far back as 3.2.1. `AMMCreate` sets it together with `lsfMPTAuthorized` to implicitly authorize an MPT asset for the AMM pseudo-account + * **`LOLoan.Flags`** — the Loan ledger object had no `Flags` property at all (and `BaseLedgerEntry` has none either), so `lsfLoanDefault`/`lsfLoanImpaired`/`lsfLoanOverpayment` were unreadable through the typed model: the default and impairment state of a loan could not be observed at all. Added as a typed `LoanFlags?` together with the enum + * new `SignerListFlags` (`lsfOneOwnerCount`) and `DirectoryNodeFlags` (`lsfNFTokenBuyOffers`/`lsfNFTokenSellOffers`) — both objects expose `Flags` as a raw `uint` and **keep doing so** (changing the property type would be breaking); the enums give consumers named constants to test bits against instead of magic numbers. The `LODirectoryNode.Flags` comment claiming "the protocol defines no flags for DirectoryNode objects" was false and is corrected + +* **`TestULedgerFlagsConformance` — the guard that would have caught all of the above** — `LedgerFormats.h` is the only place the protocol states which `lsf` flags belong to which ledger object (`definitions.json` carries field codes and entry types, but no flag values). Nothing checked it, which is how `lsfMPTAMM` survived several releases. The new test is the ledger-side counterpart of `TestUTxFormatConformance`: + * `Tests/Xrpl.Tests/Fixtures/LedgerFormats.h` is vendored byte-identical and pinned by sha in `LedgerFormats.h.ref`, verifiable with a plain `curl … | diff`. Pinned rather than live for the same reason as `transactions.macro`: upstream drift is protocol-watch's job, and a network-backed test would go red on Ripple's release schedule instead of ours + * `RippledLedgerFlags` parses the `LEDGER_OBJECT`/`LSF_FLAG` macro text and fails loudly — an unknown `LSF_FLAG*` variant or a parse yielding fewer than 10 objects / 50 flags throws rather than leaving the test green on an empty table + * the test diffs **both directions** (a flag rippled declares and the enum lacks, a flag the enum has and rippled does not) and requires every flagged object to be registered against a model enum, so a newly added ledger object fails the build instead of being skipped. `tf*` members sharing an enum with ledger flags (`OfferFlags.tfInnerBatchTxn`) and zero-valued members are excluded by rule + * name matching normalizes the `lsf`/`lsmf`/`tmf` prefixes, so `lsfMPTLocked` ≡ `MPTLocked` and rippled's `lsmfMPTCanEnableCanLock` ≡ the SDK's `tmfMPTCanEnableCanLock` (rippled itself aliases `tmfX = lsmfX` in `TxFlags.h`) + * verified by mutation, not just by passing: a wrong value, a removed flag and an unregistered object each make it fail with a readable message + * `protocol-watch` now watches `include/xrpl/protocol/LedgerFormats.h` as well. A pinned fixture cannot notice upstream moving — that signal is the watcher's job, and the header was missing from its list (which is the other half of why `lsfMPTAMM` went unnoticed for so long). The first run after this change reports the header as changed once, then carries it in the baseline like the rest + +* **DynamicMPT (XLS-94) integration coverage** — the `MutableFlags` fields existed on the models but had never been exercised against a node. `AmendmentGuard` gains the `DynamicMPT` id (it matches what `generate-amendments.sh` already writes into the nightly stand's `[amendments]`), and `TestIDynamicMPT` covers the amendment end to end, each test reading the result back from the ledger object rather than trusting `EngineResult`: + * `MutableFlags` set at `MPTokenIssuanceCreate` reach `LOMPTokenIssuance` unchanged + * `MPTokenIssuanceSet` mutates `TransferFee` and `MPTokenMetadata`, and leaves `MutableFlags` alone (rippled `doApply` only writes `sfFlags`/`sfTransferFee`/`sfMPTokenMetadata`) + * `tmfMPTSetCanLock` raises `lsfMPTCanLock` on an issuance created **without** that capability + * a mutation the issuance never permitted is rejected with `tecNO_PERMISSION` and leaves the metadata untouched + * scenarios were derived from the transactor (`src/libxrpl/tx/transactors/token/MPTokenIssuanceSet.cpp` @ `3.3.0-rc1`), not from the docs — hence `tfMPTCanTransfer` at creation in the fee test: `preclaim` requires `lsfMPTCanTransfer` to be **already** set, and enabling it in the same transaction does not satisfy the rule + * amendment-gated, so it skips on the CI stand (rippled 3.2.x has `DynamicMPT` as `Supported::No`) and runs for real on the nightly stand + ## 10.10.1.0 08/02/2026 * **An exception from an `OnConnected` handler no longer kills the client forever** — `Connection.OnceOpen` caught anything thrown by a consumer `OnConnected` handler and called `Disconnect()`, i.e. the *user* disconnect path: it set `_permanentlyDisconnected = true` and called `ClearReconnectState()`. After that the client was dead — the reconnect loop was never restarted, no new socket was ever opened, `OnConnected` never fired again, and every later request threw `NotConnectedException("Client has been disconnected. Call Connect() to reconnect.")`. Nothing was logged and nothing was raised, so from the outside the client just went quiet: * **The trigger is the most ordinary event there is — a node restart.** `OnConnected` is the natural place to restore subscriptions, because the SDK does not restore them after a reconnect. A restarting node accepts TCP seconds before it starts answering requests, so the first `subscribe` after the reconnect runs into `RequestTimeout` (40 s) and throws. A consumer that lets the exception out — the reasonable "fail loudly, let the SDK reconnect" reaction — got the opposite: a silent, permanent death. Observed in production on a fleet of bots, each wedged for four hours after a node upgrade, one of them dying 69 seconds before the node came back diff --git a/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h b/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h new file mode 100644 index 00000000..7c504f6b --- /dev/null +++ b/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h @@ -0,0 +1,322 @@ +#pragma once + +// NOLINTBEGIN(readability-identifier-naming) + +#include +#include + +#include +#include +#include +#include +#include + +namespace xrpl { +/** + * Identifiers for on-ledger objects. + * + * Each ledger object requires a unique type identifier, which is stored within the object itself; + * this makes it possible to iterate the entire ledger and determine each object's type and verify + * that the object you retrieved from a given hash matches the expected type. + * + * @warning Since these values are stored inside objects stored on the ledger they are part of the + * protocol. + * **Changing them should be avoided because without special handling, this will result in a hard + * fork.** + * + * @note Values outside this range may be used internally by the code for various purposes, but + * attempting to use such values to identify on-ledger objects will result in an invariant failure. + * + * @note When retiring types, the specific values should not be removed but should be marked as + * [[deprecated]]. This is to avoid accidental reuse of identifiers. + * + * @todo The C++ language does not enable checking for duplicate values here. + * If it becomes possible then we should do this. + * + * @ingroup protocol + */ +// Protocol-critical, hundreds of usages +// NOLINTNEXTLINE(cppcoreguidelines-use-enum-class) +enum LedgerEntryType : std::uint16_t { + +#pragma push_macro("LEDGER_ENTRY") +#undef LEDGER_ENTRY + +#define LEDGER_ENTRY(tag, value, ...) tag = value, + +#include + +#undef LEDGER_ENTRY +#pragma pop_macro("LEDGER_ENTRY") + + //--------------------------------------------------------------------------- + /** + * A special type, matching any ledger entry type. + * + * The value does not represent a concrete type, but rather is used in contexts where the + * specific type of a ledger object is unimportant, unknown or unavailable. + * + * Objects with this special type cannot be created or stored on the ledger. + * + * @see keylet::unchecked + */ + ltANY = 0, + + /** + * A special type, matching any ledger type except directory nodes. + * + * The value does not represent a concrete type, but rather is used in contexts where the + * ledger object must not be a directory node but its specific type is otherwise unimportant, + * unknown or unavailable. + * + * Objects with this special type cannot be created or stored on the ledger. + * + * @see keylet::child + */ + ltCHILD = 0x1CD2, + + //--------------------------------------------------------------------------- + /** + * A legacy, deprecated type. + * + * @deprecated **This object type is not supported and should not be used.** + * Support for this type of object was never implemented. + * No objects of this type were ever created. + */ + ltNICKNAME [[deprecated("This object type is not supported and should not be used.")]] = 0x006e, + + /** + * A legacy, deprecated type. + * + * @deprecated **This object type is not supported and should not be used.** + * Support for this type of object was never implemented. + * No objects of this type were ever created. + */ + ltCONTRACT [[deprecated("This object type is not supported and should not be used.")]] = 0x0063, + + /** + * A legacy, deprecated type. + * + * @deprecated **This object type is not supported and should not be used.** + * Support for this type of object was never implemented. + * No objects of this type were ever created. + */ + ltGENERATOR_MAP [[deprecated("This object type is not supported and should not be used.")]] = + 0x0067, +}; + +/** + * Ledger object flags. + * + * These flags are specified in ledger objects and modify their behavior. + * + * @warning Ledger object flags form part of the protocol. + * **Changing them should be avoided because without special handling, this will result in a hard + * fork.** + * + * @ingroup protocol + */ +#pragma push_macro("XMACRO") +#pragma push_macro("TO_VALUE") +#pragma push_macro("VALUE_TO_MAP") +#pragma push_macro("NULL_NAME") +#pragma push_macro("TO_MAP") +#pragma push_macro("ALL_LEDGER_FLAGS") + +#undef XMACRO +#undef TO_VALUE +#undef VALUE_TO_MAP +#undef NULL_NAME +#undef TO_MAP + +#undef ALL_LEDGER_FLAGS + +// clang-format off + +#define XMACRO(LEDGER_OBJECT, LSF_FLAG, LSF_FLAG2) \ + LEDGER_OBJECT(AccountRoot, \ + LSF_FLAG(lsfPasswordSpent, 0x00010000) /* True, if password set fee is spent. */ \ + LSF_FLAG(lsfRequireDestTag, 0x00020000) /* True, to require a DestinationTag for payments. */ \ + LSF_FLAG(lsfRequireAuth, 0x00040000) /* True, to require a authorization to hold IOUs. */ \ + LSF_FLAG(lsfDisallowXRP, 0x00080000) /* True, to disallow sending XRP. */ \ + LSF_FLAG(lsfDisableMaster, 0x00100000) /* True, force regular key */ \ + LSF_FLAG(lsfNoFreeze, 0x00200000) /* True, cannot freeze ripple states */ \ + LSF_FLAG(lsfGlobalFreeze, 0x00400000) /* True, all assets frozen */ \ + LSF_FLAG(lsfDefaultRipple, 0x00800000) /* True, incoming trust lines allow rippling by default */ \ + LSF_FLAG(lsfDepositAuth, 0x01000000) /* True, all deposits require authorization */ \ + LSF_FLAG(lsfDisallowIncomingNFTokenOffer, 0x04000000) /* True, reject new incoming NFT offers */ \ + LSF_FLAG(lsfDisallowIncomingCheck, 0x08000000) /* True, reject new checks */ \ + LSF_FLAG(lsfDisallowIncomingPayChan, 0x10000000) /* True, reject new paychans */ \ + LSF_FLAG(lsfDisallowIncomingTrustline, 0x20000000) /* True, reject new trustlines (only if no issued assets) */ \ + LSF_FLAG(lsfAllowTrustLineLocking, 0x40000000) /* True, enable trustline locking */ \ + LSF_FLAG(lsfAllowTrustLineClawback, 0x80000000)) /* True, enable clawback */ \ + \ + LEDGER_OBJECT(Offer, \ + LSF_FLAG(lsfPassive, 0x00010000) \ + LSF_FLAG(lsfSell, 0x00020000) /* True, offer was placed as a sell. */ \ + LSF_FLAG(lsfHybrid, 0x00040000)) /* True, offer is hybrid. */ \ + \ + LEDGER_OBJECT(RippleState, \ + LSF_FLAG(lsfLowReserve, 0x00010000) /* True, if entry counts toward reserve. */ \ + LSF_FLAG(lsfHighReserve, 0x00020000) \ + LSF_FLAG(lsfLowAuth, 0x00040000) \ + LSF_FLAG(lsfHighAuth, 0x00080000) \ + LSF_FLAG(lsfLowNoRipple, 0x00100000) \ + LSF_FLAG(lsfHighNoRipple, 0x00200000) \ + LSF_FLAG(lsfLowFreeze, 0x00400000) /* True, low side has set freeze flag */ \ + LSF_FLAG(lsfHighFreeze, 0x00800000) /* True, high side has set freeze flag */ \ + LSF_FLAG(lsfAMMNode, 0x01000000) /* True, trust line to AMM. */ \ + /* Used by client apps to identify payments via AMM. */ \ + LSF_FLAG(lsfLowDeepFreeze, 0x02000000) /* True, low side has set deep freeze flag */ \ + LSF_FLAG(lsfHighDeepFreeze, 0x04000000)) /* True, high side has set deep freeze flag */ \ + \ + LEDGER_OBJECT(SignerList, \ + LSF_FLAG(lsfOneOwnerCount, 0x00010000)) /* True, uses only one OwnerCount */ \ + \ + LEDGER_OBJECT(DirNode, \ + LSF_FLAG(lsfNFTokenBuyOffers, 0x00000001) \ + LSF_FLAG(lsfNFTokenSellOffers, 0x00000002)) \ + \ + LEDGER_OBJECT(NFTokenOffer, \ + LSF_FLAG(lsfSellNFToken, 0x00000001)) \ + \ + LEDGER_OBJECT(MPTokenIssuance, \ + LSF_FLAG(lsfMPTLocked, 0x00000001) /* Also used in ltMPTOKEN */ \ + LSF_FLAG(lsfMPTCanLock, 0x00000002) \ + LSF_FLAG(lsfMPTRequireAuth, 0x00000004) \ + LSF_FLAG(lsfMPTCanEscrow, 0x00000008) \ + LSF_FLAG(lsfMPTCanTrade, 0x00000010) \ + LSF_FLAG(lsfMPTCanTransfer, 0x00000020) \ + LSF_FLAG(lsfMPTCanClawback, 0x00000040) \ + LSF_FLAG(lsfMPTCanHoldConfidentialBalance, 0x00000080)) \ + \ + LEDGER_OBJECT(MPTokenIssuanceMutable, \ + LSF_FLAG(lsmfMPTCanEnableCanLock, 0x00000002) \ + LSF_FLAG(lsmfMPTCanEnableRequireAuth, 0x00000004) \ + LSF_FLAG(lsmfMPTCanEnableCanEscrow, 0x00000008) \ + LSF_FLAG(lsmfMPTCanEnableCanTrade, 0x00000010) \ + LSF_FLAG(lsmfMPTCanEnableCanTransfer, 0x00000020) \ + LSF_FLAG(lsmfMPTCanEnableCanClawback, 0x00000040) \ + LSF_FLAG(lsmfMPTCannotEnableCanHoldConfidentialBalance, 0x00000080) \ + LSF_FLAG(lsmfMPTCanMutateMetadata, 0x00010000) \ + LSF_FLAG(lsmfMPTCanMutateTransferFee, 0x00020000)) \ + \ + LEDGER_OBJECT(MPToken, \ + LSF_FLAG2(lsfMPTLocked, 0x00000001) \ + LSF_FLAG(lsfMPTAuthorized, 0x00000002) \ + LSF_FLAG(lsfMPTAMM, 0x00000004)) \ + \ + LEDGER_OBJECT(Credential, \ + LSF_FLAG(lsfAccepted, 0x00010000)) \ + \ + LEDGER_OBJECT(Vault, \ + LSF_FLAG(lsfVaultPrivate, 0x00010000)) \ + \ + LEDGER_OBJECT(Loan, \ + LSF_FLAG(lsfLoanDefault, 0x00010000) \ + LSF_FLAG(lsfLoanImpaired, 0x00020000) \ + LSF_FLAG(lsfLoanOverpayment, 0x00040000)) /* True, loan allows overpayments */ \ + \ + LEDGER_OBJECT(Sponsorship, \ + LSF_FLAG(lsfSponsorshipRequireSignForFee, 0x00010000) \ + LSF_FLAG(lsfSponsorshipRequireSignForReserve, 0x00020000)) + +// clang-format on + +// Create all the flag values as an enum. +// +// example: +// enum LedgerSpecificFlags { +// lsfPasswordSpent = 0x00010000, +// lsfRequireDestTag = 0x00020000, +// ... +// }; +#define TO_VALUE(name, value) name = (value), +#define NULL_NAME(name, values) values +#define NULL_OUTPUT(name, value) +// Bitwise flag enum +// NOLINTNEXTLINE(cppcoreguidelines-use-enum-class) +enum LedgerSpecificFlags : std::uint32_t { XMACRO(NULL_NAME, TO_VALUE, NULL_OUTPUT) }; + +// Create getter functions for each set of flags using Meyer's singleton pattern. +// This avoids static initialization order fiasco while still providing efficient access. +// This is used below in `getAllLedgerFlags()` to generate the server_definitions RPC output. +// +// example: +// inline LedgerFlagMap const& getAccountRootFlags() { +// static LedgerFlagMap const flags = { +// {"lsfPasswordSpent", 0x00010000}, +// {"lsfRequireDestTag", 0x00020000}, +// ...}; +// return flags; +// } +using LedgerFlagMap = std::map; +#define VALUE_TO_MAP(name, value) {#name, value}, +#define TO_MAP(name, values) \ + inline LedgerFlagMap const& get##name##Flags() \ + { \ + static LedgerFlagMap const flags = {values}; \ + return flags; \ + } +XMACRO(TO_MAP, VALUE_TO_MAP, VALUE_TO_MAP) + +// Create a getter function for all ledger flag maps using Meyer's singleton pattern. +// This is used to generate the server_definitions RPC output. +// +// example: +// inline std::vector> const& getAllLedgerFlags() { +// static std::vector> const flags = { +// {"AccountRoot", getAccountRootFlags()}, +// ...}; +// return flags; +// } +#define ALL_LEDGER_FLAGS(name, values) {#name, get##name##Flags()}, +inline std::vector> const& +getAllLedgerFlags() +{ + static std::vector> const flags = { + XMACRO(ALL_LEDGER_FLAGS, NULL_OUTPUT, NULL_OUTPUT)}; + return flags; +} + +#undef XMACRO +#undef TO_VALUE +#undef VALUE_TO_MAP +#undef NULL_NAME +#undef NULL_OUTPUT +#undef TO_MAP +#undef ALL_LEDGER_FLAGS + +#pragma pop_macro("XMACRO") +#pragma pop_macro("TO_VALUE") +#pragma pop_macro("VALUE_TO_MAP") +#pragma pop_macro("NULL_NAME") +#pragma pop_macro("TO_MAP") +#pragma pop_macro("ALL_LEDGER_FLAGS") + +//------------------------------------------------------------------------------ + +/** + * Holds the list of known ledger entry formats. + */ +class LedgerFormats : public KnownFormats +{ +private: + /** + * Create the object. + * This will load the object with all the known ledger formats. + */ + LedgerFormats(); + +public: + static LedgerFormats const& + getInstance(); + + // Fields shared by all ledger entry formats: + static std::vector const& + getCommonFields(); +}; + +} // namespace xrpl + +// NOLINTEND(readability-identifier-naming) diff --git a/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref b/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref new file mode 100644 index 00000000..f776654f --- /dev/null +++ b/Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref @@ -0,0 +1,22 @@ +https://github.com/XRPLF/rippled/blob/3.3.0-rc1/include/xrpl/protocol/LedgerFormats.h +sha 18e311e1e245bcc1813363bd1771b94931585d80 +date 2026-07-16T13:54:12Z +tag 3.3.0-rc1 + +LedgerFormats.h is vendored byte-identical to the ref above so that it can be +re-verified with a plain diff: + + curl -sSL https://raw.githubusercontent.com/XRPLF/rippled/18e311e1e245bcc1813363bd1771b94931585d80/include/xrpl/protocol/LedgerFormats.h \ + | diff - Tests/Xrpl.Tests/Fixtures/LedgerFormats.h + +This is the only place the protocol states which lsf flags belong to which ledger +object: definitions.json carries field codes and object types but no flag values, +so it cannot answer this question. + +Pinned to a tag rather than to develop on purpose — the same reason +transactions.macro is pinned: tracking upstream drift is protocol-watch's job, and +a network-backed test would go red on Ripple's release schedule instead of ours. + +Do not hand-edit it. When protocol-watch reports a change to this file upstream, +replace it wholesale, update the sha above, and let TestULedgerFlagsConformance +show which model enums have to follow. diff --git a/Tests/Xrpl.Tests/Integration/AmendmentGuard.cs b/Tests/Xrpl.Tests/Integration/AmendmentGuard.cs index 671f7c5f..e8135c9a 100644 --- a/Tests/Xrpl.Tests/Integration/AmendmentGuard.cs +++ b/Tests/Xrpl.Tests/Integration/AmendmentGuard.cs @@ -29,6 +29,9 @@ public static class AmendmentGuard /// Amendment id of ConfidentialTransfer (sha512half of the name). public const string ConfidentialTransfer = "2110E4A19966E2EF517C0A8C56A5F35099D7665B0BB89D7B126B30D50B86AAD5"; + /// Amendment id of DynamicMPT / XLS-94 (sha512half of the name). + public const string DynamicMPT = "58E92F338758479C06084E1B6BA366BAD8F75E5329A7F0EEAFFFDA51E5106B7F"; + /// Amendment id of PriceOracle / XLS-47 (sha512half of the name). public const string PriceOracle = "96FD2F293A519AE1DB6F8BED23E4AD9119342DA7CB6BAFD00953D16C54205D8B"; diff --git a/Tests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.cs b/Tests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.cs new file mode 100644 index 00000000..0256d7af --- /dev/null +++ b/Tests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.cs @@ -0,0 +1,211 @@ +using System.Threading.Tasks; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Xrpl.Client; +using Xrpl.Client.Exceptions; +using Xrpl.Models.Ledger; +using Xrpl.Models.Methods; +using Xrpl.Models.Transactions; +using Xrpl.Sugar; +using Xrpl.Wallet; + +namespace XrplTests.Xrpl.ClientLib.Integration; + +/// +/// DynamicMPT (XLS-94) end-to-end coverage: an issuance declares at creation +/// which capabilities and fields may change later (MutableFlags), and a +/// later MPTokenIssuanceSet performs the mutation. Both directions are checked +/// against the ledger object, plus the permission rule that makes the feature +/// meaningful — a mutation the issuance never allowed is rejected. +/// +/// Amendment-gated: DynamicMPT is Supported::No on rippled 3.2.x, so these +/// tests skip on the CI stand and run for real on the nightly stand, where +/// generate-amendments.sh puts DynamicMPT into [amendments]. +/// +[TestClass] +[TestCategory("DynamicMPT")] +public class TestIDynamicMPT : TestIMPTokenBase +{ + private static IXrplClient client; + private static bool dynamicMptActive; + + protected override IXrplClient GetClient() => client; + + /// "MPT-METADATA" in hex — the value the issuance is created with. + private const string InitialMetadata = "4D50542D4D45544144415441"; + + /// "MPT-UPDATED" in hex — the value a mutation writes over it. + private const string UpdatedMetadata = "4D50542D55504441544544"; + + [ClassInitialize] + public static async Task ClassInitializeAsync(TestContext testContext) + { + client = await CreateStandaloneClient(); + dynamicMptActive = await AmendmentGuard.IsEnabledAsync(client, AmendmentGuard.DynamicMPT); + } + + [TestInitialize] + public void CheckAmendment() + { + if (!dynamicMptActive) + { + Assert.Inconclusive("DynamicMPT amendment is not enabled on the test node; run the nightly stand (.ci-config/docker-compose.batchv11.yml)."); + } + } + + [ClassCleanup] + public static void ClassCleanup() => client?.Dispose(); + + [TestMethod] + public async Task TestDynamicMPT_MutableFlagsOnCreate_ReachTheLedgerObject() + { + XrplWallet issuer = XrplWallet.Generate(); + await IntegrationTestConfig.TryFundWalletAsync(client, issuer, nodeType); + + MPTokenIssuanceCreateMutableFlags mutable = + MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateMetadata | + MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateTransferFee | + MPTokenIssuanceCreateMutableFlags.tmfMPTCanEnableCanLock; + + string issuanceId = await CreateIssuance(issuer, MPTokenIssuanceCreateFlags.tfMPTCanTransfer, mutable, InitialMetadata); + + LOMPTokenIssuance issuance = await ReadIssuance(issuanceId); + + Assert.IsNotNull(issuance.MutableFlags, "MutableFlags should be present on the issuance"); + Assert.AreEqual((uint)mutable, issuance.MutableFlags.Value, "MutableFlags should round-trip unchanged"); + Assert.AreEqual(InitialMetadata, issuance.MPTokenMetadata, "MPTokenMetadata should round-trip unchanged"); + } + + [TestMethod] + public async Task TestDynamicMPT_MutateTransferFeeAndMetadata() + { + XrplWallet issuer = XrplWallet.Generate(); + await IntegrationTestConfig.TryFundWalletAsync(client, issuer, nodeType); + + // TransferFee > 0 requires lsfMPTCanTransfer to be already set on the + // issuance (rippled MPTokenIssuanceSet::preclaim), hence tfMPTCanTransfer here. + string issuanceId = await CreateIssuance( + issuer, + MPTokenIssuanceCreateFlags.tfMPTCanTransfer, + MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateMetadata | + MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateTransferFee, + InitialMetadata); + + MPTokenIssuanceSet mutation = new MPTokenIssuanceSet + { + Account = issuer.ClassicAddress, + MPTokenIssuanceID = issuanceId, + TransferFee = 500, + MPTokenMetadata = UpdatedMetadata, + }; + mutation = await client.Autofill(mutation); + TransactionSummary result = await client.SubmitAndWait(mutation, issuer, true); + ValidateResult(result); + + LOMPTokenIssuance issuance = await ReadIssuance(issuanceId); + + Assert.AreEqual((ushort)500, issuance.TransferFee, "TransferFee should be the mutated value"); + Assert.AreEqual(UpdatedMetadata, issuance.MPTokenMetadata, "MPTokenMetadata should be the mutated value"); + + // doApply writes lsf flags, TransferFee and metadata; it never rewrites + // sfMutableFlags, so the permission set survives the mutation + Assert.IsNotNull(issuance.MutableFlags, "MutableFlags should still be present after a mutation"); + Assert.AreEqual( + (uint)(MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateMetadata | + MPTokenIssuanceCreateMutableFlags.tmfMPTCanMutateTransferFee), + issuance.MutableFlags.Value, + "MutableFlags should be unchanged by a mutation"); + } + + [TestMethod] + public async Task TestDynamicMPT_SetMutableFlag_EnablesCapabilityOnTheIssuance() + { + XrplWallet issuer = XrplWallet.Generate(); + await IntegrationTestConfig.TryFundWalletAsync(client, issuer, nodeType); + + // Created WITHOUT tfMPTCanLock — only with the permission to enable it later + string issuanceId = await CreateIssuance( + issuer, + null, + MPTokenIssuanceCreateMutableFlags.tmfMPTCanEnableCanLock, + null); + + LOMPTokenIssuance before = await ReadIssuance(issuanceId); + Assert.IsTrue( + (before.Flags.GetValueOrDefault() & MPTokenIssuanceFlags.MPTCanLock) == 0, + "MPTCanLock should not be set before the mutation"); + + MPTokenIssuanceSet enable = new MPTokenIssuanceSet + { + Account = issuer.ClassicAddress, + MPTokenIssuanceID = issuanceId, + MutableFlags = MPTokenIssuanceSetMutableFlags.tmfMPTSetCanLock, + }; + enable = await client.Autofill(enable); + TransactionSummary result = await client.SubmitAndWait(enable, issuer, true); + ValidateResult(result); + + LOMPTokenIssuance after = await ReadIssuance(issuanceId); + Assert.IsTrue( + (after.Flags.GetValueOrDefault() & MPTokenIssuanceFlags.MPTCanLock) != 0, + "MPTCanLock should be set after tmfMPTSetCanLock"); + } + + [TestMethod] + public async Task TestDynamicMPT_MutationWithoutPermission_IsRejected() + { + XrplWallet issuer = XrplWallet.Generate(); + await IntegrationTestConfig.TryFundWalletAsync(client, issuer, nodeType); + + // No MutableFlags at all: nothing about this issuance may be changed later + string issuanceId = await CreateIssuance(issuer, null, null, InitialMetadata); + + MPTokenIssuanceSet mutation = new MPTokenIssuanceSet + { + Account = issuer.ClassicAddress, + MPTokenIssuanceID = issuanceId, + MPTokenMetadata = UpdatedMetadata, + }; + mutation = await client.Autofill(mutation); + + await Helper.ThrowsExceptionAsync( + () => client.SubmitAndWait(mutation, issuer, true), + "Final tx result is not success: tecNO_PERMISSION"); + + LOMPTokenIssuance issuance = await ReadIssuance(issuanceId); + Assert.AreEqual(InitialMetadata, issuance.MPTokenMetadata, "MPTokenMetadata should be untouched by the rejected mutation"); + } + + private static async Task CreateIssuance( + XrplWallet issuer, + MPTokenIssuanceCreateFlags? flags, + MPTokenIssuanceCreateMutableFlags? mutableFlags, + string metadata) + { + MPTokenIssuanceCreate create = new MPTokenIssuanceCreate + { + Account = issuer.ClassicAddress, + Flags = flags, + MutableFlags = mutableFlags, + MPTokenMetadata = metadata, + }; + create = await client.Autofill(create); + TransactionSummary created = await client.SubmitAndWait(create, issuer, true); + ValidateResult(created); + + string issuanceId = GetMPTokenIssuanceIdFromMeta(created); + Assert.IsNotNull(issuanceId, "MPTokenIssuanceID should be present in the metadata"); + return issuanceId; + } + + private static async Task ReadIssuance(string issuanceId) + { + LedgerEntryRequest request = new LedgerEntryRequest { MptIssuance = issuanceId }; + LedgerEntryResponse response = await client.LedgerEntry(request); + + Assert.IsNotNull(response?.Node, "ledger_entry should return the MPTokenIssuance node"); + Assert.IsInstanceOfType(response.Node, typeof(LOMPTokenIssuance), "Node should deserialize to LOMPTokenIssuance"); + return (LOMPTokenIssuance)response.Node; + } +} diff --git a/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs b/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs new file mode 100644 index 00000000..46a16d2a --- /dev/null +++ b/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.RegularExpressions; + +namespace Xrpl.Tests.Models.Tests +{ + /// + /// Reads the vendored rippled LedgerFormats.h — the only place the protocol states + /// which lsf flags belong to which ledger object. definitions.json and the + /// server_definitions RPC carry field codes and ledger entry types but no flag + /// values, so they cannot answer this question. + /// + /// + /// The source is C++ macro text, not a stability-guaranteed contract. Every parse step + /// fails loudly rather than yielding a thin or empty table — a silently empty result + /// would turn the conformance test green on nothing. + /// + internal static class RippledLedgerFlags + { + /// + /// LEDGER_ENTRY(ltNAME, 0x00, Name, ...) blocks are irrelevant here; the flags live in + /// LEDGER_OBJECT(Name, LSF_FLAG(lsfX, 0x…) …) blocks of the LEDGER_OBJECT_FLAGS list. + /// + private static readonly Regex ObjectBlock = new Regex( + @"LEDGER_OBJECT\(\s*(?\w+)\s*,(?(?:[^()]|\((?:[^()])*\))*)\)", + RegexOptions.Singleline | RegexOptions.Compiled); + + /// LSF_FLAG(lsfX, 0x00010000) / LSF_FLAG2(lsfX, 0x00000001) + private static readonly Regex FlagEntry = new Regex( + @"LSF_FLAG2?\(\s*(?ls[fm]\w+)\s*,\s*(?0x[0-9a-fA-F]+)\s*\)", + RegexOptions.Compiled); + + /// + /// Catches an LSF_FLAG variant the parser does not know yet — a new macro name would + /// otherwise drop its flags silently and leave the conformance test passing. + /// + private static readonly Regex AnyFlagMacro = new Regex( + @"(?LSF_FLAG\w*)\(", RegexOptions.Compiled); + + /// + /// Lower bounds on a healthy parse. Exposed so the guard test asserts against the same + /// numbers the parser enforces, instead of literals that would drift on re-pinning. + /// + internal const int MinimumExpectedObjects = 10; + + /// + internal const int MinimumExpectedFlags = 50; + + internal static string FixturePath => + Path.Combine(AppContext.BaseDirectory, "Fixtures", "LedgerFormats.h"); + + /// + /// Ledger object name -> flag name -> value, exactly as rippled declares it. + /// + internal static Dictionary> Parse() + { + if (!File.Exists(FixturePath)) + throw new InvalidOperationException($"Vendored LedgerFormats.h not found at {FixturePath}"); + + string header = File.ReadAllText(FixturePath); + if (string.IsNullOrWhiteSpace(header)) + throw new InvalidOperationException("Vendored LedgerFormats.h is empty"); + + foreach (Match macro in AnyFlagMacro.Matches(header)) + { + string name = macro.Groups["macro"].Value; + if (name is not ("LSF_FLAG" or "LSF_FLAG2")) + { + throw new InvalidOperationException( + $"Unknown flag macro '{name}' in LedgerFormats.h — the header layout changed, " + + "update the parser before trusting this test"); + } + } + + Dictionary> objects = new(); + int flagCount = 0; + + foreach (Match block in ObjectBlock.Matches(header)) + { + string name = block.Groups["name"].Value; + Dictionary flags = new(); + + foreach (Match flag in FlagEntry.Matches(block.Groups["body"].Value)) + { + flags[flag.Groups["flag"].Value] = + Convert.ToUInt32(flag.Groups["value"].Value.Substring(2), 16); + } + + // LEDGER_OBJECT is also used for objects that declare no flags at all; + // those carry nothing to conform to. + if (flags.Count == 0) + continue; + + objects[name] = flags; + flagCount += flags.Count; + } + + if (objects.Count < MinimumExpectedObjects || flagCount < MinimumExpectedFlags) + { + throw new InvalidOperationException( + $"Parsed only {objects.Count} flagged ledger objects / {flagCount} flags from " + + $"LedgerFormats.h (expected at least {MinimumExpectedObjects} / {MinimumExpectedFlags}) — " + + "the header layout changed and the parser silently stopped matching"); + } + + return objects; + } + } +} diff --git a/Tests/Xrpl.Tests/Models/TestULedgerFlagsConformance.cs b/Tests/Xrpl.Tests/Models/TestULedgerFlagsConformance.cs new file mode 100644 index 00000000..569b47ac --- /dev/null +++ b/Tests/Xrpl.Tests/Models/TestULedgerFlagsConformance.cs @@ -0,0 +1,149 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Xrpl.Models.Ledger; +using Xrpl.Models.Methods; +using Xrpl.Models.Transactions; + +namespace Xrpl.Tests.Models.Tests +{ + /// + /// Holds the ledger-object flag enums to the values rippled declares in the vendored + /// LedgerFormats.h. + /// + /// + /// The counterpart of for the other half of the + /// protocol surface. Nothing else in the suite notices a missing flag: an unnamed bit still + /// arrives in the model as a number, so reading the object keeps working and only the + /// consumer's ability to test it by name is lost. That is how lsfMPTAMM — present since at + /// least 3.2.1 — went unnoticed until a manual diff found it. + /// + /// Pinned copy, not the live develop branch, for the same reason as the TxFormat guard: + /// tracking upstream drift is protocol-watch's job, and a network-backed test would go red + /// on Ripple's release schedule instead of ours. + /// + [TestClass] + public class TestULedgerFlagsConformance + { + /// + /// rippled LEDGER_OBJECT name -> the enum that names its flags in the models. + /// Every flagged object in the fixture must appear here; a new one fails the test + /// rather than being skipped silently. + /// + private static readonly Dictionary FlagEnums = new(StringComparer.Ordinal) + { + ["AccountRoot"] = typeof(AccountRootFlags), + ["Offer"] = typeof(OfferFlags), + ["RippleState"] = typeof(RippleStateFlags), + ["SignerList"] = typeof(SignerListFlags), + ["DirNode"] = typeof(DirectoryNodeFlags), + ["NFTokenOffer"] = typeof(NFTokenOffer), + ["MPTokenIssuance"] = typeof(MPTokenIssuanceFlags), + // rippled declares these as lsmf* ledger flags; TxFlags.h then aliases tmfX = lsmfX, + // and the SDK names them after the transaction side (MPTokenIssuanceCreate.MutableFlags) + ["MPTokenIssuanceMutable"] = typeof(MPTokenIssuanceCreateMutableFlags), + ["MPToken"] = typeof(MPTokenFlags), + ["Credential"] = typeof(CredentialFlags), + ["Vault"] = typeof(VaultLedgerFlags), + ["Loan"] = typeof(LoanFlags), + ["Sponsorship"] = typeof(SponsorshipFlags), + }; + + /// + /// Strips the prefix rippled and the models use for the same bit, so + /// lsfMPTLocked, MPTLocked and tmfMPTCanEnableCanLock compare + /// against their upstream counterparts. + /// + private static string Normalize(string name) => + Regex.Replace(name, "^(lsmf|lsf|tmf)", string.Empty); + + [TestMethod] + public void TestULedgerFlags_MatchRippledLedgerFormats() + { + Dictionary> upstream = RippledLedgerFlags.Parse(); + StringBuilder report = new StringBuilder(); + + foreach (KeyValuePair> entry in upstream.OrderBy(o => o.Key, StringComparer.Ordinal)) + { + if (!FlagEnums.TryGetValue(entry.Key, out Type flagEnum)) + { + report.AppendLine( + $"{entry.Key}: declares {entry.Value.Count} flag(s) in LedgerFormats.h but no model enum " + + "is registered for it — add the enum and register it in FlagEnums"); + continue; + } + + Dictionary mine = Enum.GetNames(flagEnum) + .ToDictionary( + name => Normalize(name), + name => Convert.ToUInt32(Enum.Parse(flagEnum, name)), + StringComparer.Ordinal); + + foreach (KeyValuePair flag in entry.Value.OrderBy(f => f.Key, StringComparer.Ordinal)) + { + string key = Normalize(flag.Key); + if (!mine.TryGetValue(key, out uint value)) + { + report.AppendLine( + $"{entry.Key}.{flag.Key} (0x{flag.Value:X8}): declared by rippled, " + + $"missing from {flagEnum.Name}"); + } + else if (value != flag.Value) + { + report.AppendLine( + $"{entry.Key}.{flag.Key}: rippled 0x{flag.Value:X8}, {flagEnum.Name} 0x{value:X8}"); + } + } + + // The other direction: a bit the models claim the protocol does not have. + // Zero members (None) carry no bit, and tf* members are transaction flags + // that share an enum with the ledger ones (OfferFlags.tfInnerBatchTxn). + HashSet declared = entry.Value.Keys.Select(Normalize).ToHashSet(StringComparer.Ordinal); + foreach (string name in Enum.GetNames(flagEnum).OrderBy(n => n, StringComparer.Ordinal)) + { + uint value = Convert.ToUInt32(Enum.Parse(flagEnum, name)); + if (value == 0 || name.StartsWith("tf", StringComparison.Ordinal) && !name.StartsWith("tmf", StringComparison.Ordinal)) + continue; + + if (!declared.Contains(Normalize(name))) + { + report.AppendLine( + $"{flagEnum.Name}.{name} (0x{value:X8}): in the models, " + + $"not a flag of {entry.Key} in rippled"); + } + } + } + + Assert.AreEqual( + string.Empty, + report.ToString(), + $"Ledger flag enums diverge from rippled LedgerFormats.h ({RippledLedgerFlags.FixturePath}):\n" + report); + } + + [TestMethod] + public void TestULedgerFlags_FixtureParsesFully() + { + Dictionary> upstream = RippledLedgerFlags.Parse(); + + Assert.IsTrue( + upstream.Count >= RippledLedgerFlags.MinimumExpectedObjects, + $"Parsed {upstream.Count} flagged ledger objects, expected at least {RippledLedgerFlags.MinimumExpectedObjects}"); + + int flags = upstream.Sum(o => o.Value.Count); + Assert.IsTrue( + flags >= RippledLedgerFlags.MinimumExpectedFlags, + $"Parsed {flags} flags, expected at least {RippledLedgerFlags.MinimumExpectedFlags}"); + + // A parse that yields objects but drops their values would still satisfy the counts + Assert.AreEqual( + 0x00000080u, + upstream["MPTokenIssuance"]["lsfMPTCanHoldConfidentialBalance"], + "lsfMPTCanHoldConfidentialBalance should parse to 0x80"); + } + } +} diff --git a/Tests/Xrpl.Tests/Xrpl.Tests.csproj b/Tests/Xrpl.Tests/Xrpl.Tests.csproj index f076abe6..39091527 100644 --- a/Tests/Xrpl.Tests/Xrpl.Tests.csproj +++ b/Tests/Xrpl.Tests/Xrpl.Tests.csproj @@ -33,6 +33,9 @@ PreserveNewest + + PreserveNewest + diff --git a/Xrpl/Models/Ledger/LODirectoryNode.cs b/Xrpl/Models/Ledger/LODirectoryNode.cs index 3a60528a..e6e21c43 100644 --- a/Xrpl/Models/Ledger/LODirectoryNode.cs +++ b/Xrpl/Models/Ledger/LODirectoryNode.cs @@ -7,6 +7,27 @@ namespace Xrpl.Models.Ledger { + /// + /// Flags of a DirectoryNode ledger object. + /// + /// + /// stays a raw uint for backwards compatibility; + /// test a bit with (dir.Flags & (uint)DirectoryNodeFlags.lsfNFTokenBuyOffers) != 0. + /// + [System.Flags] + public enum DirectoryNodeFlags : uint + { + /// + /// The directory holds buy offers for an NFToken. + /// + lsfNFTokenBuyOffers = 0x00000001, + + /// + /// The directory holds sell offers for an NFToken. + /// + lsfNFTokenSellOffers = 0x00000002, + } + /// /// The DirectoryNode object type provides a list of links to other objects in the ledger's state tree. /// @@ -19,8 +40,8 @@ public LODirectoryNode() } /// - /// A bit-map of boolean flags enabled for this directory.Currently, - /// the protocol defines no flags for DirectoryNode objects. + /// A bit-map of boolean flags enabled for this directory. + /// See for the values the protocol defines. /// public uint Flags { get; set; } /// diff --git a/Xrpl/Models/Ledger/LOLoan.cs b/Xrpl/Models/Ledger/LOLoan.cs index 43be63bd..e4f1bf1d 100644 --- a/Xrpl/Models/Ledger/LOLoan.cs +++ b/Xrpl/Models/Ledger/LOLoan.cs @@ -8,6 +8,28 @@ namespace Xrpl.Models.Ledger; +/// +/// Flags of a Loan ledger object. +/// +[Flags] +public enum LoanFlags : uint +{ + /// + /// The loan is in default: the borrower missed a payment past the grace period. + /// + lsfLoanDefault = 0x00010000, + + /// + /// The loan is impaired: the broker expects it not to be repaid in full. + /// + lsfLoanImpaired = 0x00020000, + + /// + /// The loan allows overpayments. + /// + lsfLoanOverpayment = 0x00040000, +} + /// /// A Loan ledger object represents a loan between a borrower and a loan broker. /// @@ -19,6 +41,12 @@ public LOLoan() LedgerEntryType = LedgerEntryType.Loan; } + /// + /// A bit-map of boolean flags enabled for this loan. + /// + [JsonPropertyName("Flags")] + public LoanFlags? Flags { get; init; } + /// /// The account address of the Borrower. /// diff --git a/Xrpl/Models/Ledger/LOMPToken.cs b/Xrpl/Models/Ledger/LOMPToken.cs index 25e96920..e46a0449 100644 --- a/Xrpl/Models/Ledger/LOMPToken.cs +++ b/Xrpl/Models/Ledger/LOMPToken.cs @@ -21,6 +21,11 @@ public enum MPTokenFlags : uint /// it can also be "un-set" using a MPTokenAuthorize transaction specifying the tfMPTUnauthorize flag. /// lsfMPTAuthorized = 2, + /// + /// If set, indicates that this MPToken belongs to an AMM pseudo-account.
+ /// AMMCreate sets it together with lsfMPTAuthorized to implicitly authorize the MPT asset for the pool. + /// + lsfMPTAMM = 4, } /// /// The MPToken object represents an amount of an MPT held by an account that is not the issuer. diff --git a/Xrpl/Models/Ledger/LOMPTokenIssuance.cs b/Xrpl/Models/Ledger/LOMPTokenIssuance.cs index a819cf5d..75689ef3 100644 --- a/Xrpl/Models/Ledger/LOMPTokenIssuance.cs +++ b/Xrpl/Models/Ledger/LOMPTokenIssuance.cs @@ -50,6 +50,12 @@ public enum MPTokenIssuanceFlags : uint /// Issuer can claw back balances from holders. /// MPTCanClawback = 0x00000040, + + /// + /// Holders can hold confidential (encrypted) balances of this MPT. + /// Requires ConfidentialTransfer amendment. + /// + MPTCanHoldConfidentialBalance = 0x00000080, } /// diff --git a/Xrpl/Models/Ledger/LOSignerList.cs b/Xrpl/Models/Ledger/LOSignerList.cs index 3ea1194c..e608fd65 100644 --- a/Xrpl/Models/Ledger/LOSignerList.cs +++ b/Xrpl/Models/Ledger/LOSignerList.cs @@ -9,6 +9,23 @@ namespace Xrpl.Models.Ledger; +/// +/// Flags of a SignerList ledger object. +/// +/// +/// stays a raw uint for backwards compatibility; +/// test a bit with (list.Flags & (uint)SignerListFlags.lsfOneOwnerCount) != 0. +/// +[Flags] +public enum SignerListFlags : uint +{ + /// + /// The signer list counts as one item against the owner reserve + /// rather than one per signer entry (set on every list created since MultiSignReserve). + /// + lsfOneOwnerCount = 0x00010000, +} + /// /// The SignerList object type represents a list of parties that, as a group, /// are authorized to sign a transaction in place of an individual account.
diff --git a/Xrpl/Xrpl.csproj b/Xrpl/Xrpl.csproj index d49f6712..07e16dea 100644 --- a/Xrpl/Xrpl.csproj +++ b/Xrpl/Xrpl.csproj @@ -1,4 +1,4 @@ - + @@ -14,7 +14,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 10.10.1.0 + 10.11.0.0 From 76390415d317c4b42b93c8afd0b51a69b3dcf388 Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:19:00 -0300 Subject: [PATCH 4/7] =?UTF-8?q?feat(protocol):=20sfLEVersion=20=E2=80=94?= =?UTF-8?q?=20=D0=B2=D0=B5=D1=80=D1=81=D0=B8=D1=8F=20=D1=81=D1=85=D0=B5?= =?UTF-8?q?=D0=BC=D1=8B=20Vault=20(rippled=20#7817)=20(#74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(protocol): sfLEVersion — версия схемы Vault (rippled #7817) protocol-watch сообщил 03.08 об изменении sfields.macro и ledger_entries.macro на develop. Изменение одно: TYPED_SFIELD(sfLEVersion, UINT8, 6) и его появление в ltVAULT как SoeDefault (rippled #7817, cash-basis accounting для LoanBroker). Поле помечает, по какой схеме учёта живёт хранилище. Хранилища, созданные до активации cash-basis, не несут LEVersion вовсе, и rippled трактует его отсутствие как версию 0 (VaultVersion::Legacy), а не как ошибку — то есть отсутствие значения осмысленно, это не потеря данных. * definitions.json + сгенерированный Field.Uint8. Нужны ОБА: definitions.json в рантайме не читается, он вход для Tools/GenerateEnums, поэтому поле, добавленное только туда, не доезжает никуда. Файл перегенерирован инструментом, а не правился руками * LOVault.LEVersion (uint?, как остальные UInt8-поля этого объекта) и enum VaultVersion (Legacy = 0, CashBasis = 1) — имена для значений, которые протокол определяет на сегодня * TestULEVersion_BinaryRoundTrip — именно он показал, что одного definitions.json мало: до кодогенерации кодек поля не знал и тест падал * TestULOVault_LEVersion_Deserialize — обе формы: поле присутствует, и legacy- хранилище без него десериализуется в null Xrpl.BinaryCodec поднят до 10.10.0.0 (minor: добавлено поле). Остальные base-пакеты не тронуты и сохраняют версии. Xrpl не бампится: 10.11.0.0 ещё не выпущен и уже описывает модельные правки этой серии. Проверено: сборка решения без ошибок, юнит-тесты 877/877. * chore(release): выровнять версию Xrpl.BinaryCodec с Xrpl (10.11.0.0) Кодек везёт sfLEVersion, то есть меняется вместе с моделями, — держать его на собственной ветке нумерации незачем: потребитель читает один номер для обоих пакетов. Вместо следующего минора кодека (10.10.0.0) ставится 10.11.0.0. 10.10.x просто пропускается: последняя опубликованная версия Xrpl.BinaryCodec — 10.9.0, номер 10.10.0.0 на NuGet не выходил, так что ничего не переиспользуется и не переписывается. Xrpl.AddressCodec и Xrpl.Keypairs остаются на 10.9.0.0 — они не менялись. --- .../Enums/Field.Uint8.Generated.cs | 1 + Base/Xrpl.BinaryCodec/Enums/definitions.json | 10 +++++ Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj | 2 +- CHANGES.md | 6 +++ .../Models/TestUProtocolCompleteness.cs | 42 +++++++++++++++++++ Xrpl/Models/Ledger/LOVault.cs | 29 +++++++++++++ 6 files changed, 89 insertions(+), 1 deletion(-) diff --git a/Base/Xrpl.BinaryCodec/Enums/Field.Uint8.Generated.cs b/Base/Xrpl.BinaryCodec/Enums/Field.Uint8.Generated.cs index 28287e34..01a4ca2f 100644 --- a/Base/Xrpl.BinaryCodec/Enums/Field.Uint8.Generated.cs +++ b/Base/Xrpl.BinaryCodec/Enums/Field.Uint8.Generated.cs @@ -10,6 +10,7 @@ public partial class Field public static readonly Uint8Field Method = new Uint8Field(nameof(Method), 2); public static readonly Uint8Field Scale = new Uint8Field(nameof(Scale), 4); public static readonly Uint8Field AssetScale = new Uint8Field(nameof(AssetScale), 5); + public static readonly Uint8Field LEVersion = new Uint8Field(nameof(LEVersion), 6); public static readonly Uint8Field TickSize = new Uint8Field(nameof(TickSize), 16); public static readonly Uint8Field UNLModifyDisabling = new Uint8Field(nameof(UNLModifyDisabling), 17); public static readonly Uint8Field HookResult = new Uint8Field(nameof(HookResult), 18); diff --git a/Base/Xrpl.BinaryCodec/Enums/definitions.json b/Base/Xrpl.BinaryCodec/Enums/definitions.json index 0943aa48..14555c74 100644 --- a/Base/Xrpl.BinaryCodec/Enums/definitions.json +++ b/Base/Xrpl.BinaryCodec/Enums/definitions.json @@ -3150,6 +3150,16 @@ "type": "UInt8" } ], + [ + "LEVersion", + { + "isSerialized": true, + "isSigningField": true, + "isVLEncoded": false, + "nth": 6, + "type": "UInt8" + } + ], [ "TickSize", { diff --git a/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj b/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj index 770d5a8f..d38c8777 100644 --- a/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj +++ b/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj @@ -13,7 +13,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 10.9.0.0 + 10.11.0.0 diff --git a/CHANGES.md b/CHANGES.md index 4466a442..2cb96daa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,12 @@ # Changes ## 10.11.0.0 08/03/2026 +* **`sfLEVersion` — the Vault ledger entry's schema version** ([rippled #7817](https://github.com/XRPLF/rippled/pull/7817), merged into `develop` 07/30/2026, reported by protocol-watch). `UInt8` nth 6, `SoeDefault` on `ltVAULT`: it marks which accounting scheme a vault follows. Vaults created before cash-basis accounting was activated carry no `LEVersion` at all, and rippled resolves that absence as version 0 rather than an error — so an absent value is meaningful, not missing data: + * `definitions.json` + the generated `Field.Uint8` entry. **Both are required**: `definitions.json` is not read at runtime, it is the input to `Tools/GenerateEnums`, so a field added there alone travels nowhere. `TestULEVersion_BinaryRoundTrip` is what proves the round trip actually works rather than that the JSON was edited + * `LOVault.LEVersion` (`uint?`, matching the other UInt8 fields of that object) plus a `VaultVersion` enum naming the two values the protocol defines so far (`Legacy` = 0, `CashBasis` = 1) + * `TestULOVault_LEVersion_Deserialize` covers both shapes — the field present, and a legacy vault without it deserializing to `null` + * `Xrpl.BinaryCodec` bumped to **10.11.0.0**, aligned with `Xrpl` rather than to its own next minor (10.10.0.0): the codec ships the field, so the two move together and a consumer can read one version number off both. 10.10.x is simply skipped — the codec's last published version is 10.9.0, so no number is being reused. `Xrpl.AddressCodec` and `Xrpl.Keypairs` are untouched and keep 10.9.0.0 + * **Ledger-object flags the protocol declares but the models never named** — an unnamed bit still arrives in the model as a number, so reading the object kept working and only the consumer's ability to test it by name was lost. That is why these went unnoticed; a field-by-field diff of rippled `LedgerFormats.h` (tag `3.3.0-rc1`) against every flag enum found four gaps: * `MPTokenIssuanceFlags` + **`MPTCanHoldConfidentialBalance`** (0x80) — introduced by ConfidentialTransfer. The rest of the amendment was already complete (transactions 85–89, `IssuerEncryptionKey`/`AuditorEncryptionKey`, `ConfidentialOutstandingAmount`); only the flag had no name. Value confirmed against a live node: `MPTokenIssuanceSet` with `MutableFlags = tmfMPTSetCanHoldConfidentialBalance` moves the issuance from `Flags = 0` to `Flags = 128` * `MPTokenFlags` + **`lsfMPTAMM`** (0x4) — a much older gap: the flag is present as far back as 3.2.1. `AMMCreate` sets it together with `lsfMPTAuthorized` to implicitly authorize an MPT asset for the AMM pseudo-account diff --git a/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs b/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs index 367382b5..1a27dbcc 100644 --- a/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs +++ b/Tests/Xrpl.Tests/Models/TestUProtocolCompleteness.cs @@ -322,5 +322,47 @@ public void TestULORippleState_SponsorFields_Deserialize() Assert.AreEqual(Account1, state.HighSponsor); Assert.AreEqual(Account2, state.LowSponsor); } + + [TestMethod] + public void TestULOVault_LEVersion_Deserialize() + { + string json = JsonSerializer.Serialize(new Dictionary + { + ["LedgerEntryType"] = "Vault", + ["Account"] = Account1, + ["Owner"] = Account2, + ["ShareMPTID"] = "00000001A407AF5856CCF3C42619DAA925813FC955C72983", + ["WithdrawalPolicy"] = 1, + ["Scale"] = 6, + ["LEVersion"] = (uint)VaultVersion.CashBasis, + }); + LOVault vault = JsonSerializer.Deserialize(json, XrplJsonOptions.Default); + Assert.AreEqual((uint)VaultVersion.CashBasis, vault.LEVersion); + + // A vault created before cash-basis accounting carries no LEVersion at all; + // rippled resolves that absence as VaultVersion.Legacy rather than an error + string legacy = JsonSerializer.Serialize(new Dictionary + { + ["LedgerEntryType"] = "Vault", + ["Account"] = Account1, + ["Owner"] = Account2, + }); + Assert.IsNull(JsonSerializer.Deserialize(legacy, XrplJsonOptions.Default).LEVersion); + } + + [TestMethod] + public void TestULEVersion_BinaryRoundTrip() + { + // The field only travels if definitions.json knows it — this fails with an + // encoding error, not an assertion, when the entry is missing. + // Parsed from text rather than built from int literals: that is the shape a + // node response arrives in, and Uint8.FromJson takes a byte, not an Int32 + JsonObject json = JsonNode.Parse("""{"LEVersion":1,"Scale":6}""")!.AsObject(); + string blob = XrplBinaryCodec.Encode(json); + JsonObject decoded = XrplBinaryCodec.Decode(blob).AsObject(); + + Assert.AreEqual(1u, decoded["LEVersion"]!.GetValue()); + Assert.AreEqual(6u, decoded["Scale"]!.GetValue()); + } } } diff --git a/Xrpl/Models/Ledger/LOVault.cs b/Xrpl/Models/Ledger/LOVault.cs index f50c08f8..ae7b15be 100644 --- a/Xrpl/Models/Ledger/LOVault.cs +++ b/Xrpl/Models/Ledger/LOVault.cs @@ -23,6 +23,27 @@ public enum VaultLedgerFlags : uint lsfVaultPrivate = 0x00010000, } +/// +/// Values of the Vault ledger entry's LEVersion field (rippled VaultVersion). +/// +/// +/// stays a plain uint?, matching the other UInt8 +/// fields of this object; these constants name the values the protocol defines so far. +/// +public enum VaultVersion : uint +{ + /// + /// Accrual-basis accounting. Vaults created before cash-basis accounting was activated + /// carry no LEVersion at all and are treated as this version implicitly. + /// + Legacy = 0, + + /// + /// Cash-basis accounting (rippled #7817). + /// + CashBasis = 1, +} + /// /// Recommended structure for the Vault Data field. /// The JSON is whitespace-removed and hex-encoded (max 256 bytes). @@ -144,6 +165,14 @@ public LOVault() [JsonPropertyName("Scale")] public uint? Scale { get; init; } + /// + /// Schema version of this ledger entry (UInt8), see . + /// Absent on vaults created before cash-basis accounting was activated, which + /// rippled resolves as (0) rather than an error. + /// + [JsonPropertyName("LEVersion")] + public uint? LEVersion { get; init; } + /// /// Arbitrary hex-encoded data associated with the vault, limited to 256 bytes. /// Use for a human-readable representation. From 713da1a5a7ebcd6a5f0787eccd8b6089639349b5 Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:22:33 -0300 Subject: [PATCH 5/7] =?UTF-8?q?feat(models)!:=20=D0=B3=D0=B2=D0=B0=D1=80?= =?UTF-8?q?=D0=B4=20=D0=BF=D0=BE=D0=BB=D0=B5=D0=B9=20ledger-=D0=BE=D0=B1?= =?UTF-8?q?=D1=8A=D0=B5=D0=BA=D1=82=D0=BE=D0=B2=20=D0=B8=20=D1=87=D0=B8?= =?UTF-8?q?=D1=81=D1=82=D0=BA=D0=B0=20=D0=BC=D0=BE=D0=B4=D0=B5=D0=BB=D0=B5?= =?UTF-8?q?=D0=B9=20=D0=BF=D0=BE=20ledger=5Fentries.macro=20(10.11.0.0)=20?= =?UTF-8?q?(#75)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(models)!: гвард полей ledger-объектов и чистка моделей по ledger_entries.macro Третья и последняя поверхность конформанса: TestUTxFormatConformance держит поля транзакций, TestULedgerFlagsConformance — флаги ledger-объектов, а поля самих объектов не проверял никто. ledger_entries.macro — единственное место, где протокол объявляет состав объекта (definitions.json несёт коды полей и типы, но не списки), и пропущенное поле симптома не даёт: чтение объекта проходит, значение просто теряется. Гвард: * вендоренный Fixtures/ledger_entries.macro с пином по sha. Пин на develop, а не на тег (в отличие от LedgerFormats.h): модели по полям следуют develop, и sfLEVersion существует только после 30.07 — тег объявил бы его выдумкой SDK * диф в обе стороны + обязательная регистрация каждого объекта, поэтому новый ledger-объект роняет сборку, а не пропускается * четыре common-поля rippled (LedgerIndex, LedgerEntryType, Flags, Sponsor из LedgerFormats::getCommonFields()) исключаются с обеих сторон, как это уже сделано для commonFields в TxFormat-гварде; [JsonIgnore]-свойства не в счёт * проверен мутацией: переименование JsonPropertyName даёт обе половины отчёта Удалены свойства, которых нет в протоколе (BREAKING, без [Obsolete] — как при удалении инертных ConnectionOptions в 10.10.0.0). Значение они держать не могли: объект собирается по фиксированному SOTemplate. Проверено и на живой ноде, и по четырём версиям (3.2.1, 3.3.0-b1, 3.3.0-rc1, develop) — нет нигде: * LOVault.DomainID — positive control: VaultCreate с Data, AssetsMaximum и DomainID прошёл, первые два вернулись на объекте, DomainID — нет, он уехал на связанный share MPTokenIssuance * LOLoan.PrincipalRequested — поле транзакции LoanSet; реальный займ хранит сумму как PrincipalOutstanding * LOCredential.OwnerNode — у объекта IssuerNode/SubjectNode; нулевые hint'ы сериализуются ("OwnerNode":"0" у Loan), так что отсутствие настоящее * LONFTokenPage.NFTokenPage, LOAmm.LedgerCurrentIndex, LOAmm.Validated — последние два принадлежат конверту ответа amm_info (snake_case) Починено в LOAmm: * AMMAccount никогда не десериализовался — поле объекта называется Account, а атрибута не было; имя свойства не меняется, вызовы не ломаются * конструктор ставил LedgerEntryType.AccountRoot вместо AMM Добавлены PreviousTxnID/PreviousTxnLgrSeq в LOAmm, LOAmendments, LODirectoryNode, LOFeeSettings, LONegativeUNL. Xrpl 10.11.0.0 -> 10.12.0.0. Xrpl.BinaryCodec не менялся и остаётся 10.11.0.0. Проверено: юнит-тесты 879/879; на nightly-стенде TestILoan/TestIVault/ TestICredential/TestINFToken/TestIAmm — 64/64; сборка решения без ошибок. * chore(release): схлопнуть невыпущенные 10.10.1.0 / 10.11.0.0 / 10.12.0.0 в один 10.11.0.0 На NuGet последняя опубликованная версия Xrpl — 10.10.0. Значит 10.10.1.0 (фикс OnConnected), 10.11.0.0 (флаги ledger-объектов, гвард по LedgerFormats.h, DynamicMPT, sfLEVersion) и 10.12.0.0 (гвард полей ledger-объектов и чистка моделей) существуют только в dev и ни один из номеров наружу не уходил. Три раздела CHANGES.md слиты в один 10.11.0.0 от 08/04/2026, версия пакета Xrpl откачена с 10.12.0.0 на 10.11.0.0. Потребитель получит один релиз вместо трёх номеров, из которых два никогда не существовали как артефакты. Xrpl.BinaryCodec уже стоял на 10.11.0.0 — после схлопывания он совпадает с Xrpl ровно, как и задумывалось при выравнивании. AddressCodec и Keypairs остаются на 10.9.0.0: они не менялись. Проверено: сборка решения без ошибок, юнит-тесты 879/879. * refactor(review): нитпики CodeRabbit к PR #75 * LONegativeUNL: добавлен using System.Text.Json.Serialization и атрибуты сокращены до [JsonPropertyName] — как в LOAmendments, LODirectoryNode, LOFeeSettings и LOAmm. Полная форма стояла потому, что файл этот namespace не импортировал; теперь импортирует * RippledLedgerEntryFormats: парсер падает на повторно объявленном объекте вместо тихой перезаписи. Индексаторное присваивание позволяло второму объявлению вытеснить первое — объект молча уходил из таблицы конформанса, а fieldCount продолжал расти, так что порог минимального разбора потерю не замечал. Это ровно тот класс тихого отказа, против которого написан остальной парсер Дубликатов имён в текущей фикстуре нет (единственный LEDGER_ENTRY_DUPLICATE — DepositPreauth, и обычного объявления с этим именем не существует), так что гвард forward-looking. Проверен мутацией: дублирование блока Ticket даёт "Ticket: declared twice in ledger_entries.macro"; фикстура после проверки восстановлена и снова байт-в-байт совпадает с пином. Проверено: сборка без ошибок, юнит-тесты 879/879. --- CHANGES.md | 22 +- .../Xrpl.Tests/Fixtures/ledger_entries.macro | 641 ++++++++++++++++++ .../Fixtures/ledger_entries.macro.ref | 23 + .../Integration/transactions/TestILoan.cs | 8 +- .../Models/RippledLedgerEntryFormats.cs | 157 +++++ .../TestULedgerEntryFieldsConformance.cs | 181 +++++ Tests/Xrpl.Tests/Xrpl.Tests.csproj | 3 + Xrpl/Models/Ledger/LOAmendments.cs | 12 + Xrpl/Models/Ledger/LOAmm.cs | 29 +- Xrpl/Models/Ledger/LOCredential.cs | 6 - Xrpl/Models/Ledger/LODirectoryNode.cs | 12 + Xrpl/Models/Ledger/LOFeeSettings.cs | 12 + Xrpl/Models/Ledger/LOLoan.cs | 6 - Xrpl/Models/Ledger/LONFTokenPage.cs | 5 - Xrpl/Models/Ledger/LONegativeUNL.cs | 13 + Xrpl/Models/Ledger/LOVault.cs | 6 - 16 files changed, 1093 insertions(+), 43 deletions(-) create mode 100644 Tests/Xrpl.Tests/Fixtures/ledger_entries.macro create mode 100644 Tests/Xrpl.Tests/Fixtures/ledger_entries.macro.ref create mode 100644 Tests/Xrpl.Tests/Models/RippledLedgerEntryFormats.cs create mode 100644 Tests/Xrpl.Tests/Models/TestULedgerEntryFieldsConformance.cs diff --git a/CHANGES.md b/CHANGES.md index 2cb96daa..80ddaeee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,25 @@ # Changes -## 10.11.0.0 08/03/2026 +## 10.11.0.0 08/04/2026 + +* **`TestULedgerEntryFieldsConformance` — the third conformance surface**, completing the set next to `TestUTxFormatConformance` (transaction fields) and `TestULedgerFlagsConformance` (ledger flags). `ledger_entries.macro` is the only place the protocol states which fields belong to which ledger object — `definitions.json` carries field codes and object types but not the per-object lists — and nothing checked it. A missing field produces no symptom: reading the object still succeeds and the value is silently dropped, which is how `LOAccountRoot` went without `WalletLocator`/`WalletSize` until a manual pass, and how `sfLEVersion` had to arrive through a protocol-watch notification instead of a red test: + * `Tests/Xrpl.Tests/Fixtures/ledger_entries.macro`, vendored byte-identical and pinned by sha in the `.ref`. Pinned to a **develop** commit rather than a tag, unlike `LedgerFormats.h`: the models track develop for fields, and `sfLEVersion` exists only after 07/30/2026, so a tag would report it as a field the SDK invented + * both directions are diffed — a field rippled declares and the model lacks, and a property the model exposes that is not a field of that object — and every ledger object must be registered against a model, so a newly added one fails the build instead of being skipped + * rippled's four **common fields** (`LedgerIndex`, `LedgerEntryType`, `Flags`, `Sponsor` from `LedgerFormats::getCommonFields()`) are excluded on both sides, mirroring how the TxFormat guard treats `commonFields`; `[JsonIgnore]` properties (computed helpers like `DataParsed`, `MPTokenMetadataRow`) never reach the wire and are excluded too + * verified by mutation: renaming a field's `JsonPropertyName` makes it report both halves (`Loan.Borrower … missing from LOLoan` and `LOLoan.BorrowerX … not a field of Loan`) + +* **Ledger-object properties that are not protocol fields — removed** (**breaking**, no `[Obsolete]` grace period, consistent with the 10.10.0.0 removal of the inert `ConnectionOptions`). None of them could ever hold a value: rippled builds each object from a fixed `SOTemplate`, so a field outside the template cannot appear in it. Confirmed against a live node (nightly stand, 3.3.0-b1) *and* across four rippled versions — 3.2.1, 3.3.0-b1, 3.3.0-rc1 and develop — none of these exists in any of them, including the unreleased one: + * `LOVault.DomainID` — proven with a positive control: a `VaultCreate` carrying `Data`, `AssetsMaximum` **and** `DomainID` succeeded, the first two came back on the object, `DomainID` did not, and it turned up on the linked share `MPTokenIssuance` instead — exactly what the macro comment (`no PermissionedDomainID ever (use MPTIssuance.sfDomainID)`) and `VaultCreate.cpp` (`.domainId = tx[~sfDomainID]`) describe + * `LOLoan.PrincipalRequested` — a field of the **LoanSet transaction**, not of the object: a real loan created with `PrincipalRequested = 10000000` stores it as `PrincipalOutstanding`, and the object carries no such field + * `LOCredential.OwnerNode` — Credential hangs in two directories and uses `IssuerNode`/`SubjectNode`. Zero-valued directory hints *are* serialized (a Loan object returns `"OwnerNode":"0"`), so its absence is real, not a default being omitted + * `LONFTokenPage.NFTokenPage`, `LOAmm.LedgerCurrentIndex`, `LOAmm.Validated` — the last two are fields of the `amm_info` **response envelope** (`ledger_current_index`, `validated`, snake_case), not of the AMM object; `LOAmm` is only ever deserialized as a ledger object, and `amm_info` has its own `AMMInfo` model + +* **`LOAmm` fixes** — two bugs the guard surfaced: + * **`AMMAccount` never deserialized**: the AMM object's field is `Account`, and the property had no `[JsonPropertyName]`, so it silently stayed null on every AMM object ever read. Now mapped to `Account`; the property name is unchanged, so no call site breaks + * the constructor set `LedgerEntryType = LedgerEntryType.AccountRoot` — an AMM object identified itself as an AccountRoot. Now `LedgerEntryType.AMM` + +* **Fields declared by the protocol but missing from the models** — `PreviousTxnID`/`PreviousTxnLgrSeq` on `LOAmm`, `LOAmendments`, `LODirectoryNode`, `LOFeeSettings` and `LONegativeUNL`. Both are `SoeOptional` on these objects upstream; without them the transaction that last touched the object could not be read through the typed API + * **`sfLEVersion` — the Vault ledger entry's schema version** ([rippled #7817](https://github.com/XRPLF/rippled/pull/7817), merged into `develop` 07/30/2026, reported by protocol-watch). `UInt8` nth 6, `SoeDefault` on `ltVAULT`: it marks which accounting scheme a vault follows. Vaults created before cash-basis accounting was activated carry no `LEVersion` at all, and rippled resolves that absence as version 0 rather than an error — so an absent value is meaningful, not missing data: * `definitions.json` + the generated `Field.Uint8` entry. **Both are required**: `definitions.json` is not read at runtime, it is the input to `Tools/GenerateEnums`, so a field added there alone travels nowhere. `TestULEVersion_BinaryRoundTrip` is what proves the round trip actually works rather than that the JSON was edited * `LOVault.LEVersion` (`uint?`, matching the other UInt8 fields of that object) plus a `VaultVersion` enum naming the two values the protocol defines so far (`Legacy` = 0, `CashBasis` = 1) @@ -29,7 +48,6 @@ * scenarios were derived from the transactor (`src/libxrpl/tx/transactors/token/MPTokenIssuanceSet.cpp` @ `3.3.0-rc1`), not from the docs — hence `tfMPTCanTransfer` at creation in the fee test: `preclaim` requires `lsfMPTCanTransfer` to be **already** set, and enabling it in the same transaction does not satisfy the rule * amendment-gated, so it skips on the CI stand (rippled 3.2.x has `DynamicMPT` as `Supported::No`) and runs for real on the nightly stand -## 10.10.1.0 08/02/2026 * **An exception from an `OnConnected` handler no longer kills the client forever** — `Connection.OnceOpen` caught anything thrown by a consumer `OnConnected` handler and called `Disconnect()`, i.e. the *user* disconnect path: it set `_permanentlyDisconnected = true` and called `ClearReconnectState()`. After that the client was dead — the reconnect loop was never restarted, no new socket was ever opened, `OnConnected` never fired again, and every later request threw `NotConnectedException("Client has been disconnected. Call Connect() to reconnect.")`. Nothing was logged and nothing was raised, so from the outside the client just went quiet: * **The trigger is the most ordinary event there is — a node restart.** `OnConnected` is the natural place to restore subscriptions, because the SDK does not restore them after a reconnect. A restarting node accepts TCP seconds before it starts answering requests, so the first `subscribe` after the reconnect runs into `RequestTimeout` (40 s) and throws. A consumer that lets the exception out — the reasonable "fail loudly, let the SDK reconnect" reaction — got the opposite: a silent, permanent death. Observed in production on a fleet of bots, each wedged for four hours after a node upgrade, one of them dying 69 seconds before the node came back * A failing handler is now treated as what it is — a **connection** failure, not a user disconnect. The socket is torn down and the regular reconnect loop takes over with its usual exponential backoff, exactly as for a transport failure. The permanent-disconnect flag is never set on this path diff --git a/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro b/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro new file mode 100644 index 00000000..b6408581 --- /dev/null +++ b/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro @@ -0,0 +1,641 @@ +#if !defined(LEDGER_ENTRY) +#error "undefined macro: LEDGER_ENTRY" +#endif + +#ifndef LEDGER_ENTRY_DUPLICATE +// The EXPAND macro is needed for Windows +// https://stackoverflow.com/questions/5134523/msvc-doesnt-expand-va-args-correctly +#define EXPAND(x) x + +// The `LEDGER_ENTRY_DUPLICATE macro is needed to avoid JSS conflicts +// Since some transactions and ledger entries have the same name (like `DepositPreauth`) +// The compiler won't accept two instances of `JSS(DepositPreauth)` +#define LEDGER_ENTRY_DUPLICATE(...) EXPAND(LEDGER_ENTRY(__VA_ARGS__)) +#endif + +/** + * These objects are listed in order of increasing ledger type ID. + * There are many gaps between these IDs. + * You are welcome to fill them with new object types. + */ + +/** A ledger object which identifies an offer to buy or sell an NFT. + + \sa keylet::nftokenOffer + */ +LEDGER_ENTRY(ltNFTOKEN_OFFER, 0x0037, NFTokenOffer, nft_offer, ({ + {sfOwner, SoeRequired}, + {sfNFTokenID, SoeRequired}, + {sfAmount, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfNFTokenOfferNode, SoeRequired}, + {sfDestination, SoeOptional}, + {sfExpiration, SoeOptional}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which describes a check. + + \sa keylet::check + */ +LEDGER_ENTRY(ltCHECK, 0x0043, Check, check, ({ + {sfAccount, SoeRequired}, + {sfDestination, SoeRequired}, + {sfSendMax, SoeRequired}, + {sfSequence, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfDestinationNode, SoeRequired}, + {sfExpiration, SoeOptional}, + {sfInvoiceID, SoeOptional}, + {sfSourceTag, SoeOptional}, + {sfDestinationTag, SoeOptional}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** The ledger object which tracks the DID. + + \sa keylet::did +*/ +LEDGER_ENTRY(ltDID, 0x0049, DID, did, ({ + {sfAccount, SoeRequired}, + {sfDIDDocument, SoeOptional}, + {sfURI, SoeOptional}, + {sfData, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** The ledger object which tracks the current negative UNL state. + + \note This is a singleton: only one such object exists in the ledger. + + \sa keylet::negativeUNL + */ +LEDGER_ENTRY(ltNEGATIVE_UNL, 0x004e, NegativeUNL, nunl, ({ + {sfDisabledValidators, SoeOptional}, + {sfValidatorToDisable, SoeOptional}, + {sfValidatorToReEnable, SoeOptional}, + {sfPreviousTxnID, SoeOptional}, + {sfPreviousTxnLgrSeq, SoeOptional}, +})) + +/** A ledger object which contains a list of NFTs + + \sa keylet::nftokenPageMin, keylet::nftokenPageMax, keylet::nftokenPage + */ +LEDGER_ENTRY(ltNFTOKEN_PAGE, 0x0050, NFTokenPage, nft_page, ({ + {sfPreviousPageMin, SoeOptional}, + {sfNextPageMin, SoeOptional}, + {sfNFTokens, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which contains a signer list for an account. + + \sa keylet::signerList + */ +// All fields are SoeRequired because there is always a SignerEntries. +// If there are no SignerEntries the node is deleted. +LEDGER_ENTRY(ltSIGNER_LIST, 0x0053, SignerList, signer_list, ({ + {sfOwner, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfSignerQuorum, SoeRequired}, + {sfSignerEntries, SoeRequired}, + {sfSignerListID, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which describes a ticket. + + \sa keylet::ticket + */ +LEDGER_ENTRY(ltTICKET, 0x0054, Ticket, ticket, ({ + {sfAccount, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfTicketSequence, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which describes an account. + + \sa keylet::account + */ +LEDGER_ENTRY(ltACCOUNT_ROOT, 0x0061, AccountRoot, account, ({ + {sfAccount, SoeRequired}, + {sfSequence, SoeRequired}, + {sfBalance, SoeRequired}, + {sfOwnerCount, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfAccountTxnID, SoeOptional}, + {sfRegularKey, SoeOptional}, + {sfEmailHash, SoeOptional}, + {sfWalletLocator, SoeOptional}, + {sfWalletSize, SoeOptional}, + {sfMessageKey, SoeOptional}, + {sfTransferRate, SoeOptional}, + {sfDomain, SoeOptional}, + {sfTickSize, SoeOptional}, + {sfTicketCount, SoeOptional}, + {sfNFTokenMinter, SoeOptional}, + {sfMintedNFTokens, SoeDefault}, + {sfBurnedNFTokens, SoeDefault}, + {sfFirstNFTokenSequence, SoeOptional}, + {sfSponsoredOwnerCount, SoeDefault}, + {sfSponsoringOwnerCount, SoeDefault}, + {sfSponsoringAccountCount, SoeDefault}, + {sfAMMID, SoeOptional}, // pseudo-account designator + {sfVaultID, SoeOptional}, // pseudo-account designator + {sfLoanBrokerID, SoeOptional}, // pseudo-account designator +})) + +/** A ledger object which contains a list of object identifiers. + + \sa keylet::page, keylet::quality, keylet::book, keylet::next and + keylet::ownerDir + */ +LEDGER_ENTRY(ltDIR_NODE, 0x0064, DirectoryNode, directory, ({ + {sfOwner, SoeOptional}, // for owner directories + {sfTakerPaysCurrency, SoeOptional}, // order book directories + {sfTakerPaysIssuer, SoeOptional}, // order book directories + {sfTakerPaysMPT, SoeOptional}, // order book directories + {sfTakerGetsCurrency, SoeOptional}, // order book directories + {sfTakerGetsIssuer, SoeOptional}, // order book directories + {sfTakerGetsMPT, SoeOptional}, // order book directories + {sfExchangeRate, SoeOptional}, // order book directories + {sfIndexes, SoeRequired}, + {sfRootIndex, SoeRequired}, + {sfIndexNext, SoeOptional}, + {sfIndexPrevious, SoeOptional}, + {sfNFTokenID, SoeOptional}, + {sfPreviousTxnID, SoeOptional}, + {sfPreviousTxnLgrSeq, SoeOptional}, + {sfDomainID, SoeOptional} // order book directories +})) + +/** The ledger object which lists details about amendments on the network. + + \note This is a singleton: only one such object exists in the ledger. + + \sa keylet::amendments + */ +LEDGER_ENTRY(ltAMENDMENTS, 0x0066, Amendments, amendments, ({ + {sfAmendments, SoeOptional}, // Enabled + {sfMajorities, SoeOptional}, + {sfPreviousTxnID, SoeOptional}, + {sfPreviousTxnLgrSeq, SoeOptional}, +})) + +/** A ledger object that contains a list of ledger hashes. + + This type is used to store the ledger hashes which the protocol uses + to implement skip lists that allow for efficient backwards (and, in + theory, forward) forward iteration across large ledger ranges. + + \sa keylet::skip + */ +LEDGER_ENTRY(ltLEDGER_HASHES, 0x0068, LedgerHashes, hashes, ({ + {sfFirstLedgerSequence, SoeOptional}, + {sfLastLedgerSequence, SoeOptional}, + {sfHashes, SoeRequired}, +})) + +/** The ledger object which lists details about sidechains. + + \sa keylet::bridge +*/ +LEDGER_ENTRY(ltBRIDGE, 0x0069, Bridge, bridge, ({ + {sfAccount, SoeRequired}, + {sfSignatureReward, SoeRequired}, + {sfMinAccountCreateAmount, SoeOptional}, + {sfXChainBridge, SoeRequired}, + {sfXChainClaimID, SoeRequired}, + {sfXChainAccountCreateCount, SoeRequired}, + {sfXChainAccountClaimCount, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which describes an offer on the DEX. + + \sa keylet::offer + */ +LEDGER_ENTRY(ltOFFER, 0x006f, Offer, offer, ({ + {sfAccount, SoeRequired}, + {sfSequence, SoeRequired}, + {sfTakerPays, SoeRequired}, + {sfTakerGets, SoeRequired}, + {sfBookDirectory, SoeRequired}, + {sfBookNode, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfExpiration, SoeOptional}, + {sfDomainID, SoeOptional}, + {sfAdditionalBooks, SoeOptional}, +})) + +/** A ledger object which describes a deposit pre-authorization. + + \sa keylet::depositPreauth + */ +LEDGER_ENTRY_DUPLICATE(ltDEPOSIT_PREAUTH, 0x0070, DepositPreauth, deposit_preauth, ({ + {sfAccount, SoeRequired}, + {sfAuthorize, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfAuthorizeCredentials, SoeOptional}, +})) + +/** A claim id for a cross chain transaction. + + \sa keylet::xChainClaimID +*/ +LEDGER_ENTRY(ltXCHAIN_OWNED_CLAIM_ID, 0x0071, XChainOwnedClaimID, xchain_owned_claim_id, ({ + {sfAccount, SoeRequired}, + {sfXChainBridge, SoeRequired}, + {sfXChainClaimID, SoeRequired}, + {sfOtherChainSource, SoeRequired}, + {sfXChainClaimAttestations, SoeRequired}, + {sfSignatureReward, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which describes a bidirectional trust line. + + @note Per Vinnie Falco this should be renamed to ltTRUST_LINE + + \sa keylet::trustLine + */ +LEDGER_ENTRY(ltRIPPLE_STATE, 0x0072, RippleState, state, ({ + {sfBalance, SoeRequired}, + {sfLowLimit, SoeRequired}, + {sfHighLimit, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfLowNode, SoeOptional}, + {sfLowQualityIn, SoeOptional}, + {sfLowQualityOut, SoeOptional}, + {sfHighNode, SoeOptional}, + {sfHighQualityIn, SoeOptional}, + {sfHighQualityOut, SoeOptional}, + {sfHighSponsor, SoeOptional}, + {sfLowSponsor, SoeOptional}, +})) + +/** The ledger object which lists the network's fee settings. + + \note This is a singleton: only one such object exists in the ledger. + + \sa keylet::feeSettings + */ +LEDGER_ENTRY(ltFEE_SETTINGS, 0x0073, FeeSettings, fee, ({ + // Old version uses raw numbers + {sfBaseFee, SoeOptional}, + {sfReferenceFeeUnits, SoeOptional}, + {sfReserveBase, SoeOptional}, + {sfReserveIncrement, SoeOptional}, + // New version uses Amounts + {sfBaseFeeDrops, SoeOptional}, + {sfReserveBaseDrops, SoeOptional}, + {sfReserveIncrementDrops, SoeOptional}, + {sfPreviousTxnID, SoeOptional}, + {sfPreviousTxnLgrSeq, SoeOptional}, +})) + +/** A claim id for a cross chain create account transaction. + + \sa keylet::xChainCreateAccountClaimID +*/ +LEDGER_ENTRY(ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID, 0x0074, XChainOwnedCreateAccountClaimID, xchain_owned_create_account_claim_id, ({ + {sfAccount, SoeRequired}, + {sfXChainBridge, SoeRequired}, + {sfXChainAccountCreateCount, SoeRequired}, + {sfXChainCreateAccountAttestations, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object describing a single escrow. + + \sa keylet::escrow + */ +LEDGER_ENTRY(ltESCROW, 0x0075, Escrow, escrow, ({ + {sfAccount, SoeRequired}, + {sfSequence, SoeOptional}, + {sfDestination, SoeRequired}, + {sfAmount, SoeRequired}, + {sfCondition, SoeOptional}, + {sfCancelAfter, SoeOptional}, + {sfFinishAfter, SoeOptional}, + {sfSourceTag, SoeOptional}, + {sfDestinationTag, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfDestinationNode, SoeOptional}, + {sfTransferRate, SoeOptional}, + {sfIssuerNode, SoeOptional}, +})) + +/** A ledger object describing a single unidirectional XRP payment channel. + + \sa keylet::payChannel + */ +LEDGER_ENTRY(ltPAYCHAN, 0x0078, PayChannel, payment_channel, ({ + {sfAccount, SoeRequired}, + {sfDestination, SoeRequired}, + {sfSequence, SoeOptional}, + {sfAmount, SoeRequired}, + {sfBalance, SoeRequired}, + {sfPublicKey, SoeRequired}, + {sfSettleDelay, SoeRequired}, + {sfExpiration, SoeOptional}, + {sfCancelAfter, SoeOptional}, + {sfSourceTag, SoeOptional}, + {sfDestinationTag, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfDestinationNode, SoeOptional}, +})) + +/** The ledger object which tracks the AMM. + + \sa keylet::amm +*/ +LEDGER_ENTRY(ltAMM, 0x0079, AMM, amm, ({ + {sfAccount, SoeRequired}, + {sfTradingFee, SoeDefault}, + {sfVoteSlots, SoeOptional}, + {sfAuctionSlot, SoeOptional}, + {sfLPTokenBalance, SoeRequired}, + {sfAsset, SoeRequired}, + {sfAsset2, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeOptional}, + {sfPreviousTxnLgrSeq, SoeOptional}, +})) + +/** A ledger object which tracks MPTokenIssuance + \sa keylet::mptokenIssuance + */ +LEDGER_ENTRY(ltMPTOKEN_ISSUANCE, 0x007e, MPTokenIssuance, mpt_issuance, ({ + {sfIssuer, SoeRequired}, + {sfSequence, SoeRequired}, + {sfTransferFee, SoeDefault}, + {sfOwnerNode, SoeRequired}, + {sfAssetScale, SoeDefault}, + {sfMaximumAmount, SoeOptional}, + {sfOutstandingAmount, SoeRequired}, + {sfLockedAmount, SoeOptional}, + {sfMPTokenMetadata, SoeOptional}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfDomainID, SoeOptional}, + {sfMutableFlags, SoeDefault}, + {sfReferenceHolding, SoeOptional}, + {sfIssuerEncryptionKey, SoeOptional}, + {sfAuditorEncryptionKey, SoeOptional}, + {sfConfidentialOutstandingAmount, SoeDefault}, +})) + +/** A ledger object which tracks MPToken + \sa keylet::mptoken + */ +LEDGER_ENTRY(ltMPTOKEN, 0x007f, MPToken, mptoken, ({ + {sfAccount, SoeRequired}, + {sfMPTokenIssuanceID, SoeRequired}, + {sfMPTAmount, SoeDefault}, + {sfLockedAmount, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfConfidentialBalanceInbox, SoeOptional}, + {sfConfidentialBalanceSpending, SoeOptional}, + {sfConfidentialBalanceVersion, SoeDefault}, + {sfIssuerEncryptedBalance, SoeOptional}, + {sfAuditorEncryptedBalance, SoeOptional}, + {sfHolderEncryptionKey, SoeOptional}, +})) + +/** A ledger object which tracks Oracle + \sa keylet::oracle + */ +LEDGER_ENTRY(ltORACLE, 0x0080, Oracle, oracle, ({ + {sfOwner, SoeRequired}, + {sfOracleDocumentID, SoeOptional}, + {sfProvider, SoeRequired}, + {sfPriceDataSeries, SoeRequired}, + {sfAssetClass, SoeRequired}, + {sfLastUpdateTime, SoeRequired}, + {sfURI, SoeOptional}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which tracks Credential + \sa keylet::credential + */ +LEDGER_ENTRY(ltCREDENTIAL, 0x0081, Credential, credential, ({ + {sfSubject, SoeRequired}, + {sfIssuer, SoeRequired}, + {sfCredentialType, SoeRequired}, + {sfExpiration, SoeOptional}, + {sfURI, SoeOptional}, + {sfIssuerNode, SoeRequired}, + {sfSubjectNode, SoeOptional}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object which tracks PermissionedDomain + \sa keylet::permissionedDomain + */ +LEDGER_ENTRY(ltPERMISSIONED_DOMAIN, 0x0082, PermissionedDomain, permissioned_domain, ({ + {sfOwner, SoeRequired}, + {sfSequence, SoeRequired}, + {sfAcceptedCredentials, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object representing permissions an account has delegated to another account. + \sa keylet::delegate + */ +LEDGER_ENTRY(ltDELEGATE, 0x0083, Delegate, delegate, ({ + {sfAccount, SoeRequired}, + {sfAuthorize, SoeRequired}, + {sfPermissions, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfDestinationNode, SoeOptional}, + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, +})) + +/** A ledger object representing a single asset vault. + \sa keylet::vault + */ +LEDGER_ENTRY(ltVAULT, 0x0084, Vault, vault, ({ + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfSequence, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfOwner, SoeRequired}, + {sfAccount, SoeRequired}, + {sfData, SoeOptional}, + {sfAsset, SoeRequired}, + {sfAssetsTotal, SoeDefault}, + {sfAssetsAvailable, SoeDefault}, + {sfAssetsMaximum, SoeDefault}, + {sfLossUnrealized, SoeDefault}, + {sfShareMPTID, SoeRequired}, + {sfWithdrawalPolicy, SoeRequired}, + {sfScale, SoeDefault}, + {sfLEVersion, SoeDefault}, + // no SharesTotal ever (use MPTIssuance.sfOutstandingAmount) + // no PermissionedDomainID ever (use MPTIssuance.sfDomainID) +})) + +/** Reserve 0x0084-0x0087 for future Vault-related objects. */ + +/** A ledger object representing a loan broker + + \sa keylet::loanBroker + */ +LEDGER_ENTRY(ltLOAN_BROKER, 0x0088, LoanBroker, loan_broker, ({ + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfSequence, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfVaultNode, SoeRequired}, + {sfVaultID, SoeRequired}, + {sfAccount, SoeRequired}, + {sfOwner, SoeRequired}, + {sfLoanSequence, SoeRequired}, + {sfData, SoeDefault}, + {sfManagementFeeRate, SoeDefault}, + {sfOwnerCount, SoeDefault}, + {sfDebtTotal, SoeDefault}, + {sfDebtMaximum, SoeDefault}, + {sfCoverAvailable, SoeDefault}, + {sfCoverRateMinimum, SoeDefault}, + {sfCoverRateLiquidation, SoeDefault}, +})) + +/** A ledger object representing a loan between a Borrower and a Loan Broker + + \sa keylet::loan + */ +LEDGER_ENTRY(ltLOAN, 0x0089, Loan, loan, ({ + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfOwnerNode, SoeRequired}, + {sfLoanBrokerNode, SoeRequired}, + {sfLoanBrokerID, SoeRequired}, + {sfLoanSequence, SoeRequired}, + {sfBorrower, SoeRequired}, + {sfLoanOriginationFee, SoeDefault}, + {sfLoanServiceFee, SoeDefault}, + {sfLatePaymentFee, SoeDefault}, + {sfClosePaymentFee, SoeDefault}, + {sfOverpaymentFee, SoeDefault}, + {sfInterestRate, SoeDefault}, + {sfLateInterestRate, SoeDefault}, + {sfCloseInterestRate, SoeDefault}, + {sfOverpaymentInterestRate, SoeDefault}, + {sfStartDate, SoeRequired}, + {sfPaymentInterval, SoeRequired}, + {sfGracePeriod, SoeDefault}, + {sfPreviousPaymentDueDate, SoeDefault}, + {sfNextPaymentDueDate, SoeDefault}, + // The loan object tracks these values: + // + // - PaymentRemaining: The number of payments left in the loan. When it + // reaches 0, the loan is paid off, and all other relevant values + // must also be 0. + // + // - PeriodicPayment: The fixed, unrounded amount to be paid each + // interval. Stored with as much precision as possible. + // Payment transactions must round this value *UP*. + // + // - TotalValueOutstanding: The rounded total amount owed by the + // borrower to the lender / vault. + // + // - PrincipalOutstanding: The rounded portion of the + // TotalValueOutstanding that is from the principal borrowed. + // + // - ManagementFeeOutstanding: The rounded portion of the + // TotalValueOutstanding that represents management fees + // specifically owed to the broker based on the initial + // loan parameters. + // + // There are additional values that can be computed from these: + // + // - InterestOutstanding = TotalValueOutstanding - PrincipalOutstanding + // The total amount of interest still pending on the loan, + // independent of management fees. + // + // - InterestOwedToVault = InterestOutstanding - ManagementFeeOutstanding + // The amount of the total interest that is owed to the vault, and + // will be sent to it as part of a payment. + // + // - TrueTotalLoanValue = PaymentRemaining * PeriodicPayment + // The unrounded true total value of the loan. + // + // - TrueTotalPrincipalOutstanding can be computed using the algorithm + // in the xrpl::detail::loanPrincipalFromPeriodicPayment function. + // + // - TrueTotalInterestOutstanding = TrueTotalLoanValue - + // TrueTotalPrincipalOutstanding + // The unrounded true total interest remaining. + // + // - TrueTotalManagementFeeOutstanding = TrueTotalInterestOutstanding * + // LoanBroker.ManagementFeeRate + // The unrounded true total fee still owed to the broker. + // + // Note the "True" values may differ significantly from the tracked + // rounded values. + {sfPaymentRemaining, SoeDefault}, + {sfPeriodicPayment, SoeRequired}, + {sfPrincipalOutstanding, SoeDefault}, + {sfTotalValueOutstanding, SoeDefault}, + {sfManagementFeeOutstanding, SoeDefault}, + // Based on the computed total value at creation, used for + // rounding calculated values so they are all on a + // consistent scale - that is, they all have the same + // number of digits after the decimal point (excluding + // trailing zeros). + {sfLoanScale, SoeDefault}, +})) + +/** A ledger object representing a sponsorship. + \sa keylet::sponsorship + */ +LEDGER_ENTRY(ltSPONSORSHIP, 0x0090, Sponsorship, sponsorship, ({ + {sfPreviousTxnID, SoeRequired}, + {sfPreviousTxnLgrSeq, SoeRequired}, + {sfOwner, SoeRequired}, + {sfSponsee, SoeRequired}, + {sfFeeAmount, SoeOptional}, + {sfMaxFee, SoeOptional}, + {sfRemainingOwnerCount, SoeDefault}, + {sfOwnerNode, SoeRequired}, + {sfSponseeNode, SoeRequired}, +})) + +#undef EXPAND +#undef LEDGER_ENTRY_DUPLICATE diff --git a/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro.ref b/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro.ref new file mode 100644 index 00000000..c88bbc16 --- /dev/null +++ b/Tests/Xrpl.Tests/Fixtures/ledger_entries.macro.ref @@ -0,0 +1,23 @@ +https://github.com/XRPLF/rippled/blob/develop/include/xrpl/protocol/detail/ledger_entries.macro +sha ecdd457f3598c7286a9af4aff358fbd30039173f +date 2026-07-30T23:04:38Z + +ledger_entries.macro is vendored byte-identical to the ref above so that it can be +re-verified with a plain diff: + + curl -sSL https://raw.githubusercontent.com/XRPLF/rippled/ecdd457f3598c7286a9af4aff358fbd30039173f/include/xrpl/protocol/detail/ledger_entries.macro \ + | diff - Tests/Xrpl.Tests/Fixtures/ledger_entries.macro + +This is the only place the protocol states which fields belong to which ledger +object: definitions.json carries field codes and object types, but not the +per-object field lists. + +Pinned to a develop commit rather than to a release tag — unlike LedgerFormats.h, +which is pinned to 3.3.0-rc1 because the two are identical there. The models track +develop for fields: sfLEVersion (Vault) exists only after 07/30/2026 and is absent +from 3.3.0-rc1, so a tag would report it as a field the models invented. This sha +is the one protocol-watch recorded when it reported the change. + +Do not hand-edit it. When protocol-watch reports a change to this file upstream, +replace it wholesale, update the sha above, and let TestULedgerEntryFieldsConformance +show which models have to follow. diff --git a/Tests/Xrpl.Tests/Integration/transactions/TestILoan.cs b/Tests/Xrpl.Tests/Integration/transactions/TestILoan.cs index 8544dc8d..63f57c37 100644 --- a/Tests/Xrpl.Tests/Integration/transactions/TestILoan.cs +++ b/Tests/Xrpl.Tests/Integration/transactions/TestILoan.cs @@ -336,11 +336,9 @@ public async Task TestLoanLedgerEntry_VerifyFields() Assert.IsNotNull(loan.LoanBrokerID, "LoanBrokerID should be set"); Assert.IsNotNull(loan.LoanSequence, "LoanSequence should be set"); - // Number fields — PrincipalRequested was explicitly set to "10000000" in LoanSet, - // but rippled may omit zero-value Number fields. - // PrincipalOutstanding may be null if no payments have been made yet (depends on rippled behavior). - if (loan.PrincipalRequested != null) - Assert.IsTrue(loan.PrincipalRequested.Length > 0, "PrincipalRequested should be non-empty if present"); + // PrincipalRequested is a field of the LoanSet TRANSACTION, not of the Loan object: + // rippled records the amount as PrincipalOutstanding, so the object never carries it + // (confirmed against a live node — the created object holds PrincipalOutstanding only). if (loan.PrincipalOutstanding != null) Assert.IsTrue(loan.PrincipalOutstanding.Length > 0, "PrincipalOutstanding should be non-empty if present"); diff --git a/Tests/Xrpl.Tests/Models/RippledLedgerEntryFormats.cs b/Tests/Xrpl.Tests/Models/RippledLedgerEntryFormats.cs new file mode 100644 index 00000000..ec4b9ee8 --- /dev/null +++ b/Tests/Xrpl.Tests/Models/RippledLedgerEntryFormats.cs @@ -0,0 +1,157 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.RegularExpressions; + +namespace Xrpl.Tests.Models.Tests +{ + /// + /// Reads the vendored rippled ledger_entries.macro — the only place the protocol + /// states which fields belong to which ledger object. definitions.json carries field + /// codes and object types, but not the per-object field lists, so it cannot answer this. + /// + /// + /// The counterpart of for ledger objects. Same + /// contract: the source is C++ macro text, so every parse step fails loudly rather than + /// yielding a thin or empty table — a silently empty result would turn the conformance + /// test green on nothing. + /// + internal static class RippledLedgerEntryFormats + { + /// + /// LEDGER_ENTRY(ltTAG, 0x00NN, Name, rpcName, ({ {sfField, SoeX}, ... })) + /// LEDGER_ENTRY_DUPLICATE(...) has the same shape and declares an object that shares + /// a type code with another one, so it is parsed identically. + /// + private static readonly Regex EntryBlock = new Regex( + @"LEDGER_ENTRY(?:_DUPLICATE)?\(\s*lt\w+\s*,\s*0x[0-9a-fA-F]+\s*,\s*(?\w+)\s*,(?.*?)\}\)\)", + RegexOptions.Singleline | RegexOptions.Compiled); + + /// {sfField, SoeRequired} / {sfField, SoeOptional} / {sfField, SoeDefault} + private static readonly Regex FieldEntry = new Regex( + @"\{\s*sf(?\w+)\s*,\s*Soe(?Required|Optional|Default)\b", + RegexOptions.Compiled); + + /// Catches a requirement keyword the mapping below does not know yet. + private static readonly Regex AnyFieldEntry = new Regex( + @"\{\s*sf(?\w+)\s*,\s*Soe(?\w+)", + RegexOptions.Compiled); + + /// + /// Lower bounds on a healthy parse, asserted by the guard test as well so the two + /// cannot disagree about what "parsed enough" means. + /// + internal const int MinimumExpectedEntries = 25; + + /// + internal const int MinimumExpectedFields = 250; + + internal static string FixturePath => + Path.Combine(AppContext.BaseDirectory, "Fixtures", "ledger_entries.macro"); + + /// + /// Fields every ledger object carries, declared once in rippled's + /// LedgerFormats::getCommonFields() (src/libxrpl/protocol/LedgerFormats.cpp) + /// rather than per object in the macro — the ledger-side counterpart of + /// TxFormats' commonFields. Both directions of the conformance diff + /// exclude them: the macro never lists them, so requiring them of a model would be + /// wrong, and a model that does expose them is not inventing anything. + /// + /// + /// Hand-maintained: the list lives in a .cpp, which protocol-watch does not track + /// (it watches the headers and macros). It has held these four for releases — + /// sfSponsor was the last addition, with XLS-68 — so drift here is slow and + /// visible: a new common field would surface as the same name reported missing from + /// every single model at once. + /// + internal static HashSet CommonFields() => new(StringComparer.Ordinal) + { + "LedgerIndex", + "LedgerEntryType", + "Flags", + "Sponsor", + }; + + /// How rippled declares a field of a ledger object. + internal enum Requirement + { + /// Always present. + Required, + + /// May be absent. + Optional, + + /// Absent means the type's default value, not missing data. + Default, + } + + /// + /// Ledger object name -> field name -> requirement, exactly as rippled declares it. + /// + internal static Dictionary> Parse() + { + if (!File.Exists(FixturePath)) + throw new InvalidOperationException($"Vendored ledger_entries.macro not found at {FixturePath}"); + + string macro = File.ReadAllText(FixturePath); + if (string.IsNullOrWhiteSpace(macro)) + throw new InvalidOperationException("Vendored ledger_entries.macro is empty"); + + Dictionary> entries = new(); + int fieldCount = 0; + + foreach (Match block in EntryBlock.Matches(macro)) + { + string name = block.Groups["name"].Value; + string body = block.Groups["body"].Value; + + foreach (Match raw in AnyFieldEntry.Matches(body)) + { + string keyword = raw.Groups["requirement"].Value; + if (keyword is not ("Required" or "Optional" or "Default")) + { + throw new InvalidOperationException( + $"{name}.{raw.Groups["field"].Value}: unknown requirement keyword 'Soe{keyword}' — " + + "the macro format changed, update the parser before trusting this test"); + } + } + + Dictionary fields = new(); + foreach (Match field in FieldEntry.Matches(body)) + { + fields[field.Groups["field"].Value] = field.Groups["requirement"].Value switch + { + "Required" => Requirement.Required, + "Optional" => Requirement.Optional, + "Default" => Requirement.Default, + _ => throw new InvalidOperationException("unreachable"), + }; + } + + // Indexer assignment would let a second declaration of the same name replace + // the first, dropping that object from the conformance table while the field + // count below still grew — the minimum-count guard would not notice + if (entries.ContainsKey(name)) + { + throw new InvalidOperationException( + $"{name}: declared twice in ledger_entries.macro — the parser would drop one " + + "definition, update it before trusting this test"); + } + + entries.Add(name, fields); + fieldCount += fields.Count; + } + + if (entries.Count < MinimumExpectedEntries || fieldCount < MinimumExpectedFields) + { + throw new InvalidOperationException( + $"Parsed only {entries.Count} ledger entries / {fieldCount} fields from " + + $"ledger_entries.macro (expected at least {MinimumExpectedEntries} / " + + $"{MinimumExpectedFields}) — the macro layout changed and the parser " + + "silently stopped matching"); + } + + return entries; + } + } +} diff --git a/Tests/Xrpl.Tests/Models/TestULedgerEntryFieldsConformance.cs b/Tests/Xrpl.Tests/Models/TestULedgerEntryFieldsConformance.cs new file mode 100644 index 00000000..74e675d0 --- /dev/null +++ b/Tests/Xrpl.Tests/Models/TestULedgerEntryFieldsConformance.cs @@ -0,0 +1,181 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Text; +using System.Text.Json.Serialization; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Xrpl.Models.Ledger; + +using LONFTokenOffer = Xrpl.Models.Methods.LONFTokenOffer; +using LONFTokenPage = Xrpl.Models.Methods.LONFTokenPage; + +namespace Xrpl.Tests.Models.Tests +{ + /// + /// Holds the ledger-object models to the field sets rippled declares in the vendored + /// ledger_entries.macro. + /// + /// + /// The third conformance surface, next to (transaction + /// fields) and (ledger flags). A field the protocol + /// declares and the model lacks produces no symptom: reading the object still succeeds and the + /// value is simply dropped, so it stays invisible until someone needs it. That is how + /// LOAccountRoot went without WalletLocator/WalletSize until a manual completeness pass, and + /// how sfLEVersion had to be spotted through a protocol-watch notification instead of a red test. + /// + [TestClass] + public class TestULedgerEntryFieldsConformance + { + /// + /// rippled LEDGER_ENTRY name -> the model that carries its fields. Every entry in the + /// fixture must appear here; a newly added ledger object fails the test rather than + /// being skipped silently. + /// + private static readonly Dictionary Models = new(StringComparer.Ordinal) + { + ["AccountRoot"] = typeof(LOAccountRoot), + ["AMM"] = typeof(LOAmm), + ["Amendments"] = typeof(LOAmendments), + ["Bridge"] = typeof(LOBridge), + ["Check"] = typeof(LOCheck), + ["Credential"] = typeof(LOCredential), + ["Delegate"] = typeof(LODelegate), + ["DepositPreauth"] = typeof(LODepositPreauth), + ["DID"] = typeof(LODID), + ["DirectoryNode"] = typeof(LODirectoryNode), + ["Escrow"] = typeof(LOEscrow), + ["FeeSettings"] = typeof(LOFeeSettings), + ["LedgerHashes"] = typeof(LOLedgerHashes), + ["Loan"] = typeof(LOLoan), + ["LoanBroker"] = typeof(LOLoanBroker), + ["MPToken"] = typeof(LOMPToken), + ["MPTokenIssuance"] = typeof(LOMPTokenIssuance), + ["NegativeUNL"] = typeof(LONegativeUNL), + ["NFTokenOffer"] = typeof(LONFTokenOffer), + ["NFTokenPage"] = typeof(LONFTokenPage), + ["Offer"] = typeof(LOOffer), + ["Oracle"] = typeof(LOOracle), + ["PayChannel"] = typeof(LOPayChannel), + ["PermissionedDomain"] = typeof(LOPermissionedDomain), + ["RippleState"] = typeof(LORippleState), + ["SignerList"] = typeof(LOSignerList), + ["Sponsorship"] = typeof(LOSponsorship), + ["Ticket"] = typeof(LOTicket), + ["Vault"] = typeof(LOVault), + ["XChainOwnedClaimID"] = typeof(LOXChainOwnedClaimID), + ["XChainOwnedCreateAccountClaimID"] = typeof(LOXChainOwnedCreateAccountClaimID), + }; + + /// + /// Names that appear on a model but are not fields of that ledger object, with the + /// reason each is legitimate. Anything else the reverse check reports is a real finding. + /// Common fields are handled separately, via + /// . + /// + private static readonly Dictionary KnownExtras = new(StringComparer.Ordinal) + { + // BaseLedgerEntry.Index, serialized as "index" — the object's own key. rippled + // returns it alongside the object (account_objects, ledger_entry) and it is not + // part of any object's template + ["index"] = "the entry's key, returned beside the object rather than inside it", + }; + + /// + /// The JSON name a property maps to: when + /// present, the property name otherwise. Properties marked + /// never reach the wire and are excluded — that is where the computed helpers live + /// (DataParsed, MPTokenMetadataRow, Metadata, …). + /// + private static Dictionary WireProperties(Type model) + { + Dictionary map = new(StringComparer.Ordinal); + + foreach (PropertyInfo property in model.GetProperties(BindingFlags.Public | BindingFlags.Instance)) + { + if (property.GetCustomAttribute() != null) + continue; + + string name = property.GetCustomAttribute()?.Name ?? property.Name; + map[name] = property; + } + + return map; + } + + [TestMethod] + public void TestULedgerEntryModels_MatchRippledLedgerEntriesMacro() + { + Dictionary> upstream = + RippledLedgerEntryFormats.Parse(); + HashSet common = RippledLedgerEntryFormats.CommonFields(); + StringBuilder report = new StringBuilder(); + + foreach (KeyValuePair> entry + in upstream.OrderBy(e => e.Key, StringComparer.Ordinal)) + { + if (!Models.TryGetValue(entry.Key, out Type model)) + { + report.AppendLine( + $"{entry.Key}: declared in ledger_entries.macro but no model is registered for it — " + + "add the LO type and register it in Models"); + continue; + } + + Dictionary mine = WireProperties(model); + + foreach (string field in entry.Value.Keys.OrderBy(f => f, StringComparer.Ordinal)) + { + if (!mine.ContainsKey(field)) + { + report.AppendLine( + $"{entry.Key}.{field} ({entry.Value[field]}): declared by rippled, " + + $"missing from {model.Name}"); + } + } + + foreach (string name in mine.Keys.OrderBy(n => n, StringComparer.Ordinal)) + { + if (entry.Value.ContainsKey(name) || common.Contains(name) || KnownExtras.ContainsKey(name)) + continue; + + report.AppendLine( + $"{model.Name}.{name}: on the model, not a field of {entry.Key} in rippled"); + } + } + + Assert.AreEqual( + string.Empty, + report.ToString(), + $"Ledger-object models diverge from rippled ledger_entries.macro ({RippledLedgerEntryFormats.FixturePath}):\n" + report); + } + + [TestMethod] + public void TestULedgerEntryFixture_ParsesFully() + { + Dictionary> upstream = + RippledLedgerEntryFormats.Parse(); + + Assert.IsTrue( + upstream.Count >= RippledLedgerEntryFormats.MinimumExpectedEntries, + $"Parsed {upstream.Count} ledger entries, expected at least {RippledLedgerEntryFormats.MinimumExpectedEntries}"); + + int fields = upstream.Sum(e => e.Value.Count); + Assert.IsTrue( + fields >= RippledLedgerEntryFormats.MinimumExpectedFields, + $"Parsed {fields} fields, expected at least {RippledLedgerEntryFormats.MinimumExpectedFields}"); + + // Counts alone would still pass on a parse that dropped requirements + Assert.AreEqual( + RippledLedgerEntryFormats.Requirement.Default, + upstream["Vault"]["LEVersion"], + "Vault.LEVersion should parse as SoeDefault"); + Assert.AreEqual( + RippledLedgerEntryFormats.Requirement.Required, + upstream["Vault"]["Owner"], + "Vault.Owner should parse as SoeRequired"); + } + } +} diff --git a/Tests/Xrpl.Tests/Xrpl.Tests.csproj b/Tests/Xrpl.Tests/Xrpl.Tests.csproj index 39091527..72fd5fa9 100644 --- a/Tests/Xrpl.Tests/Xrpl.Tests.csproj +++ b/Tests/Xrpl.Tests/Xrpl.Tests.csproj @@ -36,6 +36,9 @@ PreserveNewest + + PreserveNewest + diff --git a/Xrpl/Models/Ledger/LOAmendments.cs b/Xrpl/Models/Ledger/LOAmendments.cs index 688c3f7a..7ee5ad8e 100644 --- a/Xrpl/Models/Ledger/LOAmendments.cs +++ b/Xrpl/Models/Ledger/LOAmendments.cs @@ -43,6 +43,18 @@ public LOAmendments() /// No flags are defined for the Amendments object type, so this value is always 0. /// public uint Flags { get; set; } + + /// + /// The identifying hash of the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnID")] + public string PreviousTxnID { get; set; } + + /// + /// The index of the ledger that contains the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnLgrSeq")] + public uint? PreviousTxnLgrSeq { get; set; } } public class Majority diff --git a/Xrpl/Models/Ledger/LOAmm.cs b/Xrpl/Models/Ledger/LOAmm.cs index bcb7ead1..67d3b284 100644 --- a/Xrpl/Models/Ledger/LOAmm.cs +++ b/Xrpl/Models/Ledger/LOAmm.cs @@ -12,11 +12,13 @@ public class LOAmm : BaseLedgerEntry { public LOAmm() { - LedgerEntryType = LedgerEntryType.AccountRoot; + LedgerEntryType = LedgerEntryType.AMM; } /// - /// The account that tracks the balance of LPTokens between the AMM instance via Trustline. + /// The special account that holds the AMM's assets and issues its LPTokens. + /// Serialized as Account, which is the name rippled gives this field. /// + [JsonPropertyName("Account")] public string AMMAccount { get; set; } /// /// Specifies one of the pool assets (XRP or token) of the AMM instance. @@ -53,21 +55,22 @@ public LOAmm() /// A list of vote objects, representing votes on the pool's trading fee.. /// public List VoteSlots { get; set; } - /// - /// The ledger index of the current in-progress ledger, which was used when - /// retrieving this information. - /// - public int? LedgerCurrentIndex { get; set; } - /// - /// True if this data is from a validated ledger version;
- /// if omitted or set to false, this data is not final. - /// - public bool? Validated { get; set; } - /// Owner directory page hint (hex UInt64). [JsonPropertyName("OwnerNode")] public string OwnerNode { get; set; } + + /// + /// The identifying hash of the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnID")] + public string PreviousTxnID { get; set; } + + /// + /// The index of the ledger that contains the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnLgrSeq")] + public uint? PreviousTxnLgrSeq { get; set; } } public interface IAuthAccount diff --git a/Xrpl/Models/Ledger/LOCredential.cs b/Xrpl/Models/Ledger/LOCredential.cs index c7325f5c..1360741e 100644 --- a/Xrpl/Models/Ledger/LOCredential.cs +++ b/Xrpl/Models/Ledger/LOCredential.cs @@ -101,12 +101,6 @@ public string URI [JsonPropertyName("Flags")] public new uint Flags { get; set; } - /// - /// A hint indicating which page of the owner directory links to this entry. - /// - [JsonPropertyName("OwnerNode")] - public string OwnerNode { get; set; } - /// /// A hint indicating which page of the subject's owner directory links to this entry. /// diff --git a/Xrpl/Models/Ledger/LODirectoryNode.cs b/Xrpl/Models/Ledger/LODirectoryNode.cs index e6e21c43..4999a9fd 100644 --- a/Xrpl/Models/Ledger/LODirectoryNode.cs +++ b/Xrpl/Models/Ledger/LODirectoryNode.cs @@ -102,5 +102,17 @@ public LODirectoryNode() /// MPT order books: MPT issuance id on the TakerGets side. [JsonPropertyName("TakerGetsMPT")] public string TakerGetsMPT { get; set; } + + /// + /// The identifying hash of the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnID")] + public string PreviousTxnID { get; set; } + + /// + /// The index of the ledger that contains the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnLgrSeq")] + public uint? PreviousTxnLgrSeq { get; set; } } } diff --git a/Xrpl/Models/Ledger/LOFeeSettings.cs b/Xrpl/Models/Ledger/LOFeeSettings.cs index de6998aa..4b2fc02b 100644 --- a/Xrpl/Models/Ledger/LOFeeSettings.cs +++ b/Xrpl/Models/Ledger/LOFeeSettings.cs @@ -47,5 +47,17 @@ public LOFeeSettings() /// XRPFees: owner reserve increment in drops. [JsonPropertyName("ReserveIncrementDrops")] public string ReserveIncrementDrops { get; set; } + + /// + /// The identifying hash of the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnID")] + public string PreviousTxnID { get; set; } + + /// + /// The index of the ledger that contains the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnLgrSeq")] + public uint? PreviousTxnLgrSeq { get; set; } } } diff --git a/Xrpl/Models/Ledger/LOLoan.cs b/Xrpl/Models/Ledger/LOLoan.cs index e4f1bf1d..3b3dafe8 100644 --- a/Xrpl/Models/Ledger/LOLoan.cs +++ b/Xrpl/Models/Ledger/LOLoan.cs @@ -101,12 +101,6 @@ public LOLoan() [JsonPropertyName("PrincipalOutstanding")] public string PrincipalOutstanding { get; init; } - /// - /// The principal amount originally requested (Number type, string representation). - /// - [JsonPropertyName("PrincipalRequested")] - public string PrincipalRequested { get; init; } - /// /// The total amount owed including fees (Number type, string representation). /// diff --git a/Xrpl/Models/Ledger/LONFTokenPage.cs b/Xrpl/Models/Ledger/LONFTokenPage.cs index c6cea0b0..31a555aa 100644 --- a/Xrpl/Models/Ledger/LONFTokenPage.cs +++ b/Xrpl/Models/Ledger/LONFTokenPage.cs @@ -17,11 +17,6 @@ public LONFTokenPage() } [JsonConverter(typeof(NumberOrStringConverter))] public string Flags { get; set; } - /// - /// The locator of the next page, if any. Details about this field and how it should be used are outlined below. - /// - public string NFTokenPage { get; set; } - /// /// The collection of NFToken objects contained in this NFTokenPage object. /// This specification places an upper bound of 32 NFToken objects per page. diff --git a/Xrpl/Models/Ledger/LONegativeUNL.cs b/Xrpl/Models/Ledger/LONegativeUNL.cs index 96441a3b..bc7f8c92 100644 --- a/Xrpl/Models/Ledger/LONegativeUNL.cs +++ b/Xrpl/Models/Ledger/LONegativeUNL.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Text.Json.Serialization; namespace Xrpl.Models.Ledger { @@ -25,6 +26,18 @@ public LONegativeUNL() /// The public key of a trusted validator in the Negative UNL that is scheduled to be re-enabled in the next flag ledger. /// public string ValidatorToReEnable { get; set; } + + /// + /// The identifying hash of the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnID")] + public string PreviousTxnID { get; set; } + + /// + /// The index of the ledger that contains the transaction that most recently modified this object. + /// + [JsonPropertyName("PreviousTxnLgrSeq")] + public uint? PreviousTxnLgrSeq { get; set; } } public interface IDisabledValidator { diff --git a/Xrpl/Models/Ledger/LOVault.cs b/Xrpl/Models/Ledger/LOVault.cs index ae7b15be..d8594488 100644 --- a/Xrpl/Models/Ledger/LOVault.cs +++ b/Xrpl/Models/Ledger/LOVault.cs @@ -202,12 +202,6 @@ public string DataRaw } } - /// - /// The ID of a permissioned domain associated with the vault. - /// - [JsonPropertyName("DomainID")] - public string DomainID { get; init; } - /// /// The transaction sequence number that created the vault. /// From 3d35665e8d3e86566528032fafc65b891a971d26 Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Wed, 5 Aug 2026 15:31:37 -0300 Subject: [PATCH 6/7] =?UTF-8?q?fix(client):=20=D0=B1=D1=8D=D0=BA=D0=BE?= =?UTF-8?q?=D1=84=D1=84=20=D0=BF=D1=80=D0=B8=20=D0=BF=D0=BE=D0=B2=D1=82?= =?UTF-8?q?=D0=BE=D1=80=D0=BD=D1=8B=D1=85=20=D1=81=D0=B1=D0=BE=D1=8F=D1=85?= =?UTF-8?q?=20OnConnected=20+=20=D0=B7=D0=B0=D0=BC=D0=B5=D1=87=D0=B0=D0=BD?= =?UTF-8?q?=D0=B8=D1=8F=20CodeRabbit=20=D0=BA=20=D1=80=D0=B5=D0=BB=D0=B8?= =?UTF-8?q?=D0=B7=D0=BD=D0=BE=D0=BC=D1=83=20PR=20#76=20(#77)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(client): бэкофф при повторных сбоях OnConnected + замечания CodeRabbit к PR #76 * Бэкофф не рос при повторяющихся сбоях OnConnected-хендлера. Путь OnConnectHandlerFailedAsync сносит цикл переподключения и запускает заново на каждом сбое; StopReconnectLoop обнуляет _reconnectAttempts, свежая последовательность обнуляет его ещё раз, а CalcBackoff считает задержку только по этому счётчику. При StopAfterMaxAttempts = false ветки сдачи нет вовсе, и клиент бесконечно повторял connect -> сбой хендлера -> teardown с постоянной ReconnectBaseDelay — устойчивая нагрузка ровно на ту ноду, которая ещё не умеет обслуживать запросы. StartReconnectLoop получил параметр начального значения счётчика, путь сбоя хендлера засевает его своим числом последовательных сбоев. TestRepeatedOnConnectedFailuresBackOff это пинит: с откаченным фиксом тест показывает ~100 переподключений за 20 с с ровным интервалом ~200 мс * _reconnectCts объявлен volatile: цикл сравнивает его по ссылке, решая, владеет ли он ещё состоянием переподключения, а пишут его три метода из других потоков. Устаревшее чтение дало бы retired-циклу лишнюю итерацию либо увело бы владеющий цикл раньше времени. Соседние кросс-потоковые поля уже volatile * Гайд по кредитованию (обе языковые версии): таблица Loan Fields перечисляла четыре имени, которых у ledger-объекта нет — Account (заёмщик лежит в Borrower), а также Counterparty, PrincipalRequested и PaymentTotal, которые являются полями транзакции LoanSet. После удаления PrincipalRequested из LOLoan гайд обещал бы несуществующее свойство * TestUtils.GetFreePort больше не выдаёт один порт дважды в пределах процесса: ОС вправе вернуть только что освобождённый порт, классы тестов идут параллельно, и второй mock падал бы при бинде в фоновом потоке — это выглядело как таймаут, а не как конфликт. TestUChangeServerFailure дополнительно проверяет порт прямо перед стартом второго mock, чтобы остаточная внешняя гонка падала внятно * RippledLedgerFlags.Parse падает на повторно объявленном объекте, как это уже делает RippledLedgerEntryFormats.Parse. Проверено мутацией (дублирование блока Offer), фикстура после проверки восстановлена * Фикстуры в тестовом .csproj подключены через None Update вместо None Include — дефолтный glob SDK их уже включает Версия не бампится: 10.11.0.0 ещё не выпущен, правки дописаны в его раздел. Проверено: сборка решения без ошибок, юнит-тесты 880/880; обе вендоренные фикстуры сверены с пинами через curl | diff. * fix(tests): ловить дубликат ledger-объекта до пропуска бесфлаговых Замечание CodeRabbit к PR #77: проверка дубликата стояла ПОСЛЕ `if (flags.Count == 0) continue`, поэтому имя, объявленное дважды, проскакивало, если одно из объявлений разбиралось без флагов. Имена теперь отслеживаются отдельным HashSet до этой ветки, а `objects` по-прежнему хранит только флагованные записи. Проверено мутацией именно этого сценария: вставка второго `LEDGER_OBJECT(Offer, )` с пустым телом теперь даёт "Offer: declared twice in LedgerFormats.h", а до правки проходила молча. Фикстура после проверки восстановлена и сверена с пином через curl | diff. Побочно всплыло, что PreserveNewest не обновляет копию фикстуры в bin, когда исходник возвращают из git (у восстановленного файла время правки старше копии): после мутационных проверок каталог Fixtures в bin нужно удалять, иначе тесты идут против подделанного файла. На этом и попались два прогона. Проверено: юнит-тесты 880/880. --- CHANGES.md | 5 ++ DocFx/LendingProtocol-Guide.md | 10 +-- DocFx/LendingProtocol-Guide.ru.md | 10 +-- .../Client/TestUChangeServerFailure.cs | 8 +++ .../Client/TestUOnConnectedHandlerFailure.cs | 62 ++++++++++++++++++ Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs | 16 ++++- Tests/Xrpl.Tests/TestUtils.cs | 64 +++++++++++++++++-- Tests/Xrpl.Tests/Xrpl.Tests.csproj | 9 ++- Xrpl/Client/connection.cs | 27 ++++++-- 9 files changed, 187 insertions(+), 24 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 80ddaeee..e4603159 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,11 @@ ## 10.11.0.0 08/04/2026 +* **Repeated `OnConnected` handler failures now back off** — the give-up branch added earlier is bounded by `MaxReconnectAttempts`, but only when `StopAfterMaxAttempts` is set. With it turned off there is no give-up at all, and the delay between retries never grew: this path tears the reconnect loop down and starts it again on every failure, `StopReconnectLoop` zeroes `_reconnectAttempts`, a fresh sequence zeroes it again, and `CalcBackoff` derives the delay from that counter alone. The client therefore repeated connect → handler failure → teardown at a constant `ReconnectBaseDelay` forever — a sustained connection load on exactly the node that cannot serve requests yet. `StartReconnectLoop` now takes the value to seed the counter with, and the handler-failure path seeds it from its own consecutive-failure count so the sequence keeps growing across failures. `TestRepeatedOnConnectedFailuresBackOff` pins it; reverting the fix makes that test show ~100 reconnects in 20s at a flat ~200ms interval +* **`_reconnectCts` is `volatile`** — the reconnect loop compares it by reference to decide whether it still owns the reconnect state, while `StopReconnectLoop`, `StartReconnectLoop` and `RetireCurrentSessionAndReconnectAsync` write it from other threads. A stale read could let a retired loop run one more iteration or make the owning loop stand down early. The other cross-thread fields in that class were already `volatile` +* **Lending guide corrected** — the `Loan Fields` table in `LendingProtocol-Guide` (both languages) listed four names the ledger object does not have: `Account` (the borrower is in `Borrower`), plus `Counterparty`, `PrincipalRequested` and `PaymentTotal`, which are fields of the **`LoanSet` transaction**. After `PrincipalRequested` was removed from `LOLoan` in this release the guide would have promised a property that no longer exists. Fixed, with a note pointing the three transaction fields at `LoanSet` +* **Test-side fixes** — `TestUtils.GetFreePort` never handed out a port twice within the process (the OS is free to return a just-released port, and test classes run in parallel, so two callers could get the same one and the second mock would fail to bind on its background thread, surfacing as a timeout rather than an error); `TestUChangeServerFailure` checks the port is still free right before starting the second mock, so the remaining external race fails fast with a clear message; `RippledLedgerFlags.Parse` throws on a ledger object declared twice, matching `RippledLedgerEntryFormats.Parse`; the fixture entries in the test `.csproj` use `None Update` instead of `None Include`, since the SDK's default glob already includes them + * **`TestULedgerEntryFieldsConformance` — the third conformance surface**, completing the set next to `TestUTxFormatConformance` (transaction fields) and `TestULedgerFlagsConformance` (ledger flags). `ledger_entries.macro` is the only place the protocol states which fields belong to which ledger object — `definitions.json` carries field codes and object types but not the per-object lists — and nothing checked it. A missing field produces no symptom: reading the object still succeeds and the value is silently dropped, which is how `LOAccountRoot` went without `WalletLocator`/`WalletSize` until a manual pass, and how `sfLEVersion` had to arrive through a protocol-watch notification instead of a red test: * `Tests/Xrpl.Tests/Fixtures/ledger_entries.macro`, vendored byte-identical and pinned by sha in the `.ref`. Pinned to a **develop** commit rather than a tag, unlike `LedgerFormats.h`: the models track develop for fields, and `sfLEVersion` exists only after 07/30/2026, so a tag would report it as a field the SDK invented * both directions are diffed — a field rippled declares and the model lacks, and a property the model exposes that is not a field of that object — and every ledger object must be registered against a model, so a newly added one fails the build instead of being skipped diff --git a/DocFx/LendingProtocol-Guide.md b/DocFx/LendingProtocol-Guide.md index 9808c087..f3db8a8f 100644 --- a/DocFx/LendingProtocol-Guide.md +++ b/DocFx/LendingProtocol-Guide.md @@ -365,18 +365,20 @@ The lending protocol creates the following ledger objects: | Field | Type | Description | |-------|------|-------------| -| `Account` | AccountID | Borrower account | -| `Counterparty` | AccountID | Broker account | +| `Borrower` | AccountID | Borrower account | | `LoanBrokerID` | Hash256 | Reference to loan broker | -| `PrincipalRequested` | Number | Original loan amount | +| `LoanSequence` | UInt32 | Sequence number within the broker | | `PrincipalOutstanding` | Number | Remaining principal | | `TotalValueOutstanding` | Number | Total amount owed | +| `PeriodicPayment` | Number | Amount due per interval | | `InterestRate` | UInt32 | Annual interest rate | | `PaymentInterval` | UInt32 | Seconds between payments | -| `PaymentTotal` | UInt32 | Total number of payments | +| `GracePeriod` | UInt32 | Seconds before late fees apply | | `PaymentRemaining` | UInt32 | Remaining payments | | `StartDate` | UInt32 | Loan start (Ripple epoch) | +> `Counterparty`, `PrincipalRequested` and `PaymentTotal` are fields of the **`LoanSet` transaction**, not of the `Loan` object. rippled records the requested principal as `PrincipalOutstanding`, so the ledger entry carries no `PrincipalRequested` — and neither does `LOLoan`. + ### Querying Loan State Use `account_objects` to retrieve loans owned by an account: diff --git a/DocFx/LendingProtocol-Guide.ru.md b/DocFx/LendingProtocol-Guide.ru.md index 3d886c06..d808c471 100644 --- a/DocFx/LendingProtocol-Guide.ru.md +++ b/DocFx/LendingProtocol-Guide.ru.md @@ -365,18 +365,20 @@ await client.SubmitRequest(fullySigned.TxBlob); | Поле | Тип | Описание | |------|-----|----------| -| `Account` | AccountID | Аккаунт заёмщика | -| `Counterparty` | AccountID | Аккаунт брокера | +| `Borrower` | AccountID | Аккаунт заёмщика | | `LoanBrokerID` | Hash256 | Ссылка на кредитного брокера | -| `PrincipalRequested` | Number | Исходная сумма кредита | +| `LoanSequence` | UInt32 | Порядковый номер в рамках брокера | | `PrincipalOutstanding` | Number | Остаток основной суммы | | `TotalValueOutstanding` | Number | Общая задолженность | +| `PeriodicPayment` | Number | Сумма платежа за интервал | | `InterestRate` | UInt32 | Годовая процентная ставка | | `PaymentInterval` | UInt32 | Интервал между платежами (секунды) | -| `PaymentTotal` | UInt32 | Общее количество платежей | +| `GracePeriod` | UInt32 | Отсрочка до начисления пеней (секунды) | | `PaymentRemaining` | UInt32 | Оставшиеся платежи | | `StartDate` | UInt32 | Начало кредита (Ripple epoch) | +> `Counterparty`, `PrincipalRequested` и `PaymentTotal` — поля **транзакции `LoanSet`**, а не объекта `Loan`. Запрошенную сумму rippled записывает в `PrincipalOutstanding`, поэтому у ledger-объекта поля `PrincipalRequested` нет — как нет его и у `LOLoan`. + ### Запрос состояния кредита Используйте `account_objects` для получения кредитов аккаунта: diff --git a/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs b/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs index 1ab724b6..916ea34c 100644 --- a/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs +++ b/Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs @@ -115,6 +115,11 @@ public async Task TestChangeServerToUnreachableServerRecoversWhenItComesUp() "A ChangeServer target that is down is a connection failure, not a permanent disconnect."); // The server appears afterwards - exactly the "start the node later" case. + // The mock binds on a background thread, so a port taken in the meantime would + // surface as a 30s timeout below rather than as a bind error; check first. + Assert.IsTrue( + TestUtils.IsPortStillFree(secondPort), + $"Port {secondPort} was taken by another process while the test held it — rerun."); _secondRippled = StartMock(secondPort); DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); @@ -166,6 +171,9 @@ public async Task TestChangeServerAfterUserDisconnectStillReconnects() // Expected - the target is not up yet. } + Assert.IsTrue( + TestUtils.IsPortStillFree(secondPort), + $"Port {secondPort} was taken by another process while the test held it — rerun."); _secondRippled = StartMock(secondPort); DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); diff --git a/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs b/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs index 2869b432..c8c372e5 100644 --- a/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs +++ b/Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs @@ -247,5 +247,67 @@ public async Task TestOnConnectedFailureIsReportedThroughOnError() Assert.AreSame(completed, reported.Task, "OnConnected failure was never reported through OnError."); StringAssert.Contains(reported.Task.Result, "subscribe failed after connect"); } + + /// + /// With StopAfterMaxAttempts = false there is no give-up branch, so a permanently + /// failing handler reconnects forever. The delay between attempts must still grow: this + /// path tears the reconnect loop down and starts it again on every failure, and the loop + /// derives its delay from the attempt counter alone — seeded from zero it would hammer a + /// node that accepts TCP but cannot serve requests at a constant ReconnectBaseDelay. + /// + [TestMethod] + public async Task TestRepeatedOnConnectedFailuresBackOff() + { + List attempts = new List(); + TaskCompletionSource enough = + new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _client = CreateClient(maxReconnectAttempts: 50, stopAfterMaxAttempts: false); + _client.connection.OnConnected += () => + { + lock (attempts) + { + attempts.Add(DateTime.UtcNow); + if (attempts.Count >= 4) + { + enough.TrySetResult(true); + } + } + + throw new InvalidOperationException("subscribe failed after connect"); + }; + + try + { + await _client.Connect(); + } + catch (Exception) + { + // Expected: the first handler invocation throws. + } + + Task completed = await Task.WhenAny(enough.Task, Task.Delay(TimeSpan.FromSeconds(30))); + Assert.AreSame(completed, enough.Task, "The client stopped retrying a failing handler."); + + List gaps = new List(); + lock (attempts) + { + for (int i = 1; i < attempts.Count; i++) + { + gaps.Add(attempts[i] - attempts[i - 1]); + } + } + + // ReconnectBaseDelay is 100ms and CalcBackoff doubles per attempt with 25% jitter, + // so the third gap is ~4x the first even at the extremes of the jitter range. + // Comparing first vs last rather than each consecutive pair keeps the assertion + // robust: what regressed before was a flat sequence, not the exact multiplier. + Assert.IsTrue( + gaps.Count >= 3, + $"Expected at least 3 gaps between handler invocations, got {gaps.Count}."); + Assert.IsTrue( + gaps[gaps.Count - 1] > gaps[0], + $"Backoff did not grow across consecutive handler failures: {string.Join(", ", gaps)}"); + } } } diff --git a/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs b/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs index 46a16d2a..d7614637 100644 --- a/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs +++ b/Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs @@ -76,9 +76,23 @@ internal static Dictionary> Parse() Dictionary> objects = new(); int flagCount = 0; + // Tracked separately from `objects`, which only holds flagged entries: a name declared + // twice must be caught even when one of the two declarations parses to no flags at all, + // otherwise the flagless-skip below would let the duplicate through unnoticed. + HashSet seenNames = new(StringComparer.Ordinal); + foreach (Match block in ObjectBlock.Matches(header)) { string name = block.Groups["name"].Value; + + // Same rule as RippledLedgerEntryFormats.Parse, so the two parsers stay consistent + if (!seenNames.Add(name)) + { + throw new InvalidOperationException( + $"{name}: declared twice in LedgerFormats.h — the parser would drop one " + + "definition, update it before trusting this test"); + } + Dictionary flags = new(); foreach (Match flag in FlagEntry.Matches(block.Groups["body"].Value)) @@ -92,7 +106,7 @@ internal static Dictionary> Parse() if (flags.Count == 0) continue; - objects[name] = flags; + objects.Add(name, flags); flagCount += flags.Count; } diff --git a/Tests/Xrpl.Tests/TestUtils.cs b/Tests/Xrpl.Tests/TestUtils.cs index 1420cb87..746e7d2c 100644 --- a/Tests/Xrpl.Tests/TestUtils.cs +++ b/Tests/Xrpl.Tests/TestUtils.cs @@ -1,6 +1,8 @@ - + // https://github.com/XRPLF/xrpl.js/blob/main/packages/xrpl/test/testUtils.ts +using System; +using System.Collections.Concurrent; using System.Net; using System.Net.Sockets; @@ -8,14 +10,62 @@ namespace Xrpl.Tests { public class TestUtils { + /// + /// Ports this process has already handed out. The OS is free to return a just-released + /// port to the next caller, and test classes run in parallel (see test.runsettings), so + /// two callers could otherwise receive the same port and the second server would fail to + /// bind — silently, because the mock listens on a background thread, leaving the test to + /// time out instead of reporting a conflict. + /// + private static readonly ConcurrentDictionary ClaimedPorts = new(); + + /// + /// A loopback port free at the moment of the call and not handed out before. + /// + /// + /// The listener is stopped before returning, so the port is closed when the caller gets + /// it — several tests need exactly that (connect to a server that is not up yet, start it + /// later). The gap that leaves cannot be closed while callers need a closed port; what + /// this does remove is the collision between concurrent callers inside this process, + /// which is the reachable half of the race. + /// static public int GetFreePort() { - TcpListener l = new TcpListener(IPAddress.Loopback, 0); - l.Start(); - int port = ((IPEndPoint)l.LocalEndpoint).Port; - l.Stop(); - return port; + for (int attempt = 0; attempt < 50; attempt++) + { + TcpListener listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + int port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + + if (ClaimedPorts.TryAdd(port, 0)) + { + return port; + } + } + + throw new InvalidOperationException( + "GetFreePort: could not obtain an unclaimed loopback port after 50 attempts"); + } + + /// + /// Whether can still be bound on loopback right now. Tests that + /// hold a port across an await use this to fail fast with a clear reason instead of + /// waiting out a connection timeout when something else took it. + /// + static public bool IsPortStillFree(int port) + { + try + { + TcpListener listener = new TcpListener(IPAddress.Loopback, port); + listener.Start(); + listener.Stop(); + return true; + } + catch (SocketException) + { + return false; + } } } } - diff --git a/Tests/Xrpl.Tests/Xrpl.Tests.csproj b/Tests/Xrpl.Tests/Xrpl.Tests.csproj index 72fd5fa9..95f801ec 100644 --- a/Tests/Xrpl.Tests/Xrpl.Tests.csproj +++ b/Tests/Xrpl.Tests/Xrpl.Tests.csproj @@ -30,13 +30,16 @@ PreserveNewest - + + PreserveNewest - + PreserveNewest - + PreserveNewest diff --git a/Xrpl/Client/connection.cs b/Xrpl/Client/connection.cs index 8860add2..2362bc6d 100644 --- a/Xrpl/Client/connection.cs +++ b/Xrpl/Client/connection.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -287,7 +287,11 @@ private static WebSocketClient CreateWebSocket(string url, ConnectionOptions con private static readonly Random _random = new(); - private CancellationTokenSource _reconnectCts; + // Volatile: StopReconnectLoop, StartReconnectLoop and RetireCurrentSessionAndReconnectAsync + // write this from other threads, and the loop compares it by reference to decide whether it + // still owns the reconnect state. A stale read would let a retired loop run one more + // iteration, or make the owning loop stand down. Matches the other cross-thread fields here. + private volatile CancellationTokenSource _reconnectCts; private Task _reconnectLoop; @@ -1787,9 +1791,15 @@ await errorHandler // and the one in OnceClose would then race with the loop's exit, and losing the race leaves nobody // reconnecting - the very wedge this path exists to prevent. Cancel whatever is there, start fresh; // the later OnceClose sees a live loop and correctly stands down. + // Seed the attempt counter with the consecutive-failure count. StopReconnectLoop zeroes + // _reconnectAttempts and a fresh sequence would zero it again, and CalcBackoff derives the + // delay from that counter alone — so without the seed every handler failure would restart + // the backoff at ReconnectBaseDelay. With StopAfterMaxAttempts = false (no give-up branch) + // that means connect -> handler failure -> teardown forever at a constant 2s, a sustained + // connection load on a node that accepts TCP but cannot serve requests yet. StopReconnectLoop(); _reconnectLoop = null; - StartReconnectLoop(); + StartReconnectLoop(initialAttempts: failures); } private async Task OnceClose(int? code, string? description, WebSocketClient closingSocket, long sessionId) @@ -1955,7 +1965,14 @@ private void ClearReconnectState() _reconnectMode = ReconnectMode.None; } - private void StartReconnectLoop() + /// + /// Value to seed _reconnectAttempts with when a fresh reconnect sequence starts. + /// Defaults to 0 — a genuine new sequence begins at the base delay. The OnConnected-handler + /// path passes its own consecutive-failure count instead: that path tears the loop down and + /// starts it again on every failure, so with a 0 seed the backoff would restart at + /// ReconnectBaseDelay each time and never grow. + /// + private void StartReconnectLoop(int initialAttempts = 0) { // Set reconnect mode to LoopReconnect (upgrades from FastReconnect or sets from None) _reconnectMode = ReconnectMode.LoopReconnect; @@ -1983,7 +2000,7 @@ private void StartReconnectLoop() existingCts?.Cancel(); existingCts?.Dispose(); _reconnectCts = new CancellationTokenSource(); - _reconnectAttempts = 0; + _reconnectAttempts = initialAttempts; } // else: Reuse existing valid CTS (pre-created for fast reconnect) // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence From f3d5d61c09c1e06a46fa4eb274a168d6125749d8 Mon Sep 17 00:00:00 2001 From: Aleksandr <44946855+Platonenkov@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:44:56 -0300 Subject: [PATCH 7/7] =?UTF-8?q?fix(client):=20=D1=81=D0=B8=D0=BD=D1=85?= =?UTF-8?q?=D1=80=D0=BE=D0=BD=D0=B8=D0=B7=D0=B8=D1=80=D0=BE=D0=B2=D0=B0?= =?UTF-8?q?=D1=82=D1=8C=20=D0=B6=D0=B8=D0=B7=D0=BD=D0=B5=D0=BD=D0=BD=D1=8B?= =?UTF-8?q?=D0=B9=20=D1=86=D0=B8=D0=BA=D0=BB=20=D1=81=D0=B5=D1=81=D1=81?= =?UTF-8?q?=D0=B8=D0=B8=20=D0=BF=D0=B5=D1=80=D0=B5=D0=BF=D0=BE=D0=B4=D0=BA?= =?UTF-8?q?=D0=BB=D1=8E=D1=87=D0=B5=D0=BD=D0=B8=D1=8F=20(#78)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(client): синхронизировать жизненный цикл сессии переподключения volatile делает атомарным каждое отдельное обращение, но не их последовательность. StopReconnectLoop читал _reconnectCts трижды подряд — Cancel, Dispose, = null — и старт, попавший между этими обращениями, получал свой свежий источник уничтоженным уходящим стопом. Цикл оставался с мёртвым источником, отступал по проверке владения, а замену никто не запускал: клиент вставал навсегда — ровно тот исход, против которого писалась вся эта область. Введён _reconnectStateLock, накрывающий каждое чтение-с-изменением тройки _reconnectCts / _reconnectLoop / _reconnectAttempts во всех четырёх точках записи: RetireCurrentSessionAndReconnectAsync (замена сессии и её же успешное завершение), StopReconnectLoop, StartReconnectLoop и хвост ReconnectLoopAsync. Решение «цикл уже идёт / источник переиспользуем / ставим новый / отдаём его циклу» стало одной транзакцией — иначе гонка давала два живых цикла либо цикл с уже уничтоженным источником. Под локом не выполняется ничего, что может позвать потребительский код: отмена и освобождение отставленного источника вынесены за лок, а ReconnectLoopAsync начинается с await Task.Yield(), поэтому старт цикла под локом только планирует его и возвращается — уведомление не выполняется на стеке вызывающего. Без этого Disconnect из обработчика мог бы войти в тот же лок и заклинить. volatile у поля сохранён: проверки владения в цикле читают его вне лока, а одиночное чтение ссылки атомарно и ничего не изменяет. TestUReconnectSessionRaces — и сразу оговорка: эти тесты НЕ воспроизводят исходную гонку. Окно шириной в несколько инструкций из публичного API, где между вызовами лежат целые await'ы, не достаётся: с убранным локом тесты по-прежнему проходят (проверено мутацией, три прогона). Ценность у них обратная — сам лок создаёт риск взаимной блокировки, и они гоняют конкурентные ChangeServer и Disconnect, требуя, чтобы клиент затем дошёл до живого сервера. Дедлок или потерянная сессия проявятся здесь, а не в бою. Проверено: юнит-тесты 882/882; TestIConnectionStates 7/7 на CI-стенде; сборка решения без ошибок. * fix(client): не читать Token после yield и перезапускать цикл одной транзакцией Селф-ревью предыдущего коммита нашло два дефекта, один из которых внесён им же. * ObjectDisposedException в ReconnectLoopAsync. Добавленный await Task.Yield() стоял ПЕРЕД чтением ownCts.Token, а Cancel/Dispose отставленного источника вынесены за лок — значит за время ожидания продолжения другой поток успевал уничтожить именно этот источник, и обращение к .Token бросало ObjectDisposedException вне всякого try. Таск падал, не сделав ни одной попытки переподключения, а исключение терялось как unobserved. До прошлого коммита Token читался синхронно, так что окна не было вовсе — оно возникло ровно из-за yield. Теперь Token читается ДО yield, пока вызывающий ещё держит лок и источник заведомо жив; Task.Delay(delay, ct) дополнительно ловит ObjectDisposedException и трактует его как отставку, а не как сбой * RestartReconnectLoop. Последовательность StopReconnectLoop(); _reconnectLoop = null; StartReconnectLoop(seed) брала лок дважды, а запись между вызовами шла без лока. Конкурентный старт из OnceClose или OnConnectionFailed успевал поставить свой цикл, после чего засеянный старт видел живой цикл и молча выходил, не применив seed — тихий откат роста бэкоффа, ради которого seed и вводился. Retire, установка нового источника, счётчика и запуск цикла делаются одной транзакцией под локом * XML-док у _reconnectStateLock больше не утверждает, что лок покрывает «every read-modify-write»: перечислены места, которые он реально охватывает, и явно названы оставшиеся вне его (per-iteration инкремент, сбросы в ChangeServer и OnceClose) Проверено мутацией: если в RestartReconnectLoop не применять seed, TestRepeatedOnConnectedFailuresBackOff падает и показывает ~90 переподключений подряд с ровным интервалом ~200 мс. Юнит-тесты 882/882. * fix(demo): показывать фактический сервер после неудачной смены Blazor-демо обновляло CurrentServerUrl только при успешном ChangeServer: присваивание стояло в try сразу после await. ChangeServer переключает цель клиента ДО попытки подключения, поэтому при переходе на недоступный сервер он бросает по таймауту получения соединения — а клиент к этому моменту уже переподключается к НОВОМУ адресу. Метка же продолжала показывать прежний сервер. Дальше ломалась и кнопка: гвард "Already connected to this server" сравнивал выбранный адрес с этим устаревшим значением и отказывался переключаться обратно на сервер, к которому клиент на самом деле подключён не был. Присваивание перенесено в finally, то есть выполняется независимо от исхода. SDK тут ни при чём: connection.GetUrl()/client.Url() возвращает новый адрес сразу после ChangeServer. Проверено вручную в демо против локальной ноды 3.2.1: переключение на ws://127.0.0.1:19997 (порт мёртв) — метка показывает 19997, а не прежний mainnet; последующее переключение на ws://localhost:6006 проходит и подключается за 0.3 с, тогда как раньше отбивалось сообщением "Already connected to this server". * fix(client): снимать ссылку на цикл внутри стоп-транзакции Замечание CodeRabbit к PR #78 (Major). StopReconnectLoop обнулял _reconnectCts, но оставлял _reconnectLoop. Отставленный цикл выходит асинхронно — он узнаёт о потере владения только на следующей проверке, — поэтому оставшаяся ссылка даёт такую последовательность: StartReconnectLoop видит !IsCompleted и возвращается, ничего не запустив, а отставленный цикл тут же отступает по проверке владения. Не остаётся никого, кто бы переподключался. Достижимо, когда Connect или ChangeServer гасит живой цикл, а новое подключение падает. Ссылка снимается в той же транзакции под локом. Нитпики оттуда же: * TestUReconnectSessionRaces: финальный Connect обёрнут, чтобы падал ассерт с внятным текстом, а не сырое исключение от проигравшего гонку переключения * StartMock вызывает mock.Start() напрямую: он биндит, слушает и уходит в BeginAccept, не блокируя, так что возврат из него уже означает готовность порта. Обёртка в поток лишь открывала окно, где тест успевал подключиться раньше mock'а. В соседнем TestUChangeServerFailure обёртка осталась — отдельная уборка, этот PR его не трогает TestFailedConnectDuringReconnectLeavesLoopRunning закрывает функциональный путь (Connect во время реконнекта, сервер появляется позже). Гонку он НЕ пинит: нужно, чтобы отставленная задача была ещё жива в момент проверки IsCompleted, а к приходу неудачного Connect она обычно уже вышла — с откаченным фиксом тест проходит. Это записано в его XML-доке, чтобы его не приняли за регрессионный. Проверено: юнит-тесты 883/883. --- .../Blazor-WebAssembly/Pages/Index.razor | 9 +- .../Client/TestUReconnectSessionRaces.cs | 295 ++++++++++++++++++ Xrpl/Client/connection.cs | 243 +++++++++++---- 3 files changed, 490 insertions(+), 57 deletions(-) create mode 100644 Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs diff --git a/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor b/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor index 81c4a731..e2eee217 100644 --- a/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor +++ b/Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor @@ -719,8 +719,6 @@ AddStatusMessage($"Changing server to: {newServerUrl}...", MessageType.Info); await client.ChangeServer(newServerUrl); - - CurrentServerUrl = client.Url(); } catch (Exception ex) { @@ -730,6 +728,13 @@ } finally { + // Read the address back whether or not the connection succeeded. ChangeServer switches + // the client's target before it starts connecting, so a switch to a server that is down + // throws (the acquisition timeout) while the client is already reconnecting to the NEW + // address. Updating this only on success left the label showing the previous server, and + // the "Already connected to this server" guard above then compared against a stale value + // and refused a legitimate switch back. + CurrentServerUrl = client.Url(); IsChangingServer = false; StateHasChanged(); } diff --git a/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs b/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs new file mode 100644 index 00000000..7d434a45 --- /dev/null +++ b/Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs @@ -0,0 +1,295 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using Xrpl.Client; + +namespace Xrpl.Tests +{ + /// + /// Concurrency smoke tests for the reconnect session — the _reconnectCts / + /// _reconnectLoop / _reconnectAttempts triple, which is now updated under a + /// shared lock. + /// + /// + /// + /// These do not reproduce the race the lock fixes. That window is a few instructions + /// wide — a start landing between the stop path's cancel, dispose and null — and driving it + /// from public API calls, which are separated by whole awaits, does not hit it: with the lock + /// removed again these tests still pass. Claiming them as regression coverage would be false. + /// + /// + /// What they do earn their place for is the other direction. Introducing a lock around the + /// session creates a deadlock risk of its own: the loop is now started while the lock is held, + /// and anything that called back into consumer code from there could re-enter a path that takes + /// the same lock. These tests hammer ChangeServer and Disconnect concurrently and require the + /// client to still reach a live server afterwards, so a deadlock or a lost session shows up as + /// a hang or a failure here rather than in production. + /// + /// + [TestClass] + public class TestUReconnectSessionRaces + { + private CreateMockRippled _mockedRippled; + private CreateMockRippled _secondRippled; + private XrplClient _client; + private int _port; + + private static Dictionary ServerInfoResponse() => new Dictionary + { + { "type", "response" }, + { "status", "success" }, + { "result", new Dictionary + { + { "info", new Dictionary + { + { "build_version", "test-mock" }, + { "complete_ledgers", "1-1" }, + { "server_state", "full" }, + } + }, + } + }, + }; + + private static CreateMockRippled StartMock(int port) + { + CreateMockRippled mock = new CreateMockRippled(port) { suppressOutput = true }; + mock.AddResponse("server_info", ServerInfoResponse()); + + // Called directly rather than on a background thread: Start() binds, listens and hands + // off to BeginAccept without blocking, so returning from it means the port is already + // accepting. Handing it to a thread only opened a window where a test could connect + // before the mock was up. + mock.Start(); + return mock; + } + + [TestInitialize] + public void MyTestInitialize() + { + _port = TestUtils.GetFreePort(); + _mockedRippled = StartMock(_port); + } + + [TestCleanup] + public async Task MyTestCleanup() + { + if (_client != null) + { + try + { + await _client.Disconnect(); + } + catch (Exception) + { + // The client may already be down; cleanup must not mask the test result. + } + + _client = null; + } + + _mockedRippled?.Stop(); + _secondRippled?.Stop(); + } + + private XrplClient CreateClient(string url) => + new XrplClient(url, new XrplClient.ClientOptions + { + RequestPolicy = RequestFailurePolicy.ImmediateFail, + ReconnectBaseDelay = TimeSpan.FromMilliseconds(50), + ReconnectMaxDelay = TimeSpan.FromMilliseconds(400), + MaxReconnectAttempts = 100, + StopAfterMaxAttempts = false, + ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(20), + ConnectionAttemptTimeout = TimeSpan.FromSeconds(5), + UseCustomPing = false, + }); + + /// + /// Concurrent ChangeServer calls tear down and install reconnect sessions from several + /// threads at once. Whatever interleaving wins, the client must end up able to connect to the + /// live server — not stranded with a disposed or orphaned session. + /// + [TestMethod] + public async Task TestConcurrentChangeServerKeepsClientRecoverable() + { + int deadPortA = TestUtils.GetFreePort(); + int deadPortB = TestUtils.GetFreePort(); + + _client = CreateClient($"ws://127.0.0.1:{_port}"); + await _client.Connect(); + Assert.IsTrue(_client.connection.IsConnected(), "Precondition: connected to the live mock."); + + // Writers of the reconnect session running at once: two pointed at ports where nothing + // listens (each starts a reconnect sequence), one pointed back at the live server, plus a + // Disconnect taking the session down underneath them. + for (int round = 0; round < 5; round++) + { + Task[] racers = + { + SwitchTo($"ws://127.0.0.1:{deadPortA}"), + SwitchTo($"ws://127.0.0.1:{deadPortB}"), + SwitchTo($"ws://127.0.0.1:{_port}"), + Task.Run(async () => + { + // Disconnect takes the same session down while the switches install new + // ones — the stop-vs-start interleaving the lock has to make safe. + try + { + await _client.Disconnect(); + } + catch (Exception) + { + } + }), + }; + + await Task.WhenAll(racers); + } + + // Whoever won, point the client at the live server and require it to get there. + await SwitchTo($"ws://127.0.0.1:{_port}"); + try { await _client.Connect(); } catch (Exception) { } + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); + while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline) + { + await Task.Delay(TimeSpan.FromMilliseconds(100)); + } + + Assert.IsTrue( + _client.connection.IsConnected(), + "After concurrent ChangeServer calls the client could not reach a server that is up — " + + "the reconnect session was left disposed or orphaned."); + } + + /// + /// Disconnect racing a reconnect sequence must leave the client cleanly stopped and + /// still able to reconnect afterwards — a stop that tore down someone else's session would + /// either strand a live loop or leave a stale one running. + /// + [TestMethod] + public async Task TestDisconnectRacingReconnectLeavesClientReconnectable() + { + int deadPort = TestUtils.GetFreePort(); + + _client = CreateClient($"ws://127.0.0.1:{_port}"); + await _client.Connect(); + + for (int round = 0; round < 5; round++) + { + // Start a reconnect sequence against a dead port and disconnect while it runs. + Task switching = SwitchTo($"ws://127.0.0.1:{deadPort}"); + Task disconnecting = Task.Run(async () => + { + await Task.Delay(TimeSpan.FromMilliseconds(20)); + await _client.Disconnect(); + }); + + await Task.WhenAll(switching, disconnecting); + } + + // The client must still be usable: point it back at the live server and connect. + // Both calls are tolerated so the assertion below reports the failure, rather than the + // test dying on a raw exception from a switch that lost a race. + await SwitchTo($"ws://127.0.0.1:{_port}"); + try + { + await _client.Connect(); + } + catch (Exception) + { + } + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(30); + while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline) + { + await Task.Delay(TimeSpan.FromMilliseconds(100)); + } + + Assert.IsTrue( + _client.connection.IsConnected(), + "Disconnect racing a reconnect sequence left the client unable to connect again."); + } + + /// + /// A failed Connect issued while a reconnect loop is already running must leave a + /// live loop behind, so the client still comes back on its own once the server returns. + /// + /// + /// Covers the functional path end to end. It does not pin the narrow race that made + /// StopReconnectLoop drop the loop reference: that needs the retired task to still be + /// running when the restart checks IsCompleted, and by the time a failed Connect gets + /// there the task has normally already exited, so the loop is restarted either way — with + /// the fix reverted this test still passes. Kept because the path itself (Connect while + /// reconnecting, server appears later) is worth guarding. + /// + [TestMethod] + public async Task TestFailedConnectDuringReconnectLeavesLoopRunning() + { + int laterPort = TestUtils.GetFreePort(); + + // Short acquisition timeout: the Connect below is expected to fail, and waiting out the + // class default would add 20s of nothing to the run. + _client = new XrplClient($"ws://127.0.0.1:{_port}", new XrplClient.ClientOptions + { + RequestPolicy = RequestFailurePolicy.ImmediateFail, + ReconnectBaseDelay = TimeSpan.FromMilliseconds(50), + ReconnectMaxDelay = TimeSpan.FromMilliseconds(400), + MaxReconnectAttempts = 100, + StopAfterMaxAttempts = false, + ConnectionAcquisitionTimeout = TimeSpan.FromSeconds(3), + ConnectionAttemptTimeout = TimeSpan.FromSeconds(2), + UseCustomPing = false, + }); + await _client.Connect(); + Assert.IsTrue(_client.connection.IsConnected(), "Precondition: connected to the live mock."); + + // Point the client at a port where nothing listens: a reconnect loop starts and retries. + await SwitchTo($"ws://127.0.0.1:{laterPort}"); + Assert.IsFalse(_client.connection.IsConnected(), "Precondition: the target port is closed."); + + // A Connect while that loop is running: it stops the loop, then fails because nothing + // is listening yet. Something must still be reconnecting afterwards. + try + { + await _client.Connect(); + } + catch (Exception) + { + // Expected - nothing is listening on that port yet. + } + + // The server appears. Nobody touches the client from here on. + _secondRippled = StartMock(laterPort); + + DateTime deadline = DateTime.UtcNow + TimeSpan.FromSeconds(40); + while (!_client.connection.IsConnected() && DateTime.UtcNow < deadline) + { + await Task.Delay(TimeSpan.FromMilliseconds(200)); + } + + Assert.IsTrue( + _client.connection.IsConnected(), + "The client never reconnected after the server returned - a failed Connect during " + + "an active reconnect sequence left no loop running."); + } + + private async Task SwitchTo(string url) + { + try + { + await _client.connection.ChangeServer(url); + } + catch (Exception) + { + // Failing to reach a dead port is the point of the race; the invariant is asserted + // by the caller once the dust settles. + } + } + } +} diff --git a/Xrpl/Client/connection.cs b/Xrpl/Client/connection.cs index 2362bc6d..261bfdf0 100644 --- a/Xrpl/Client/connection.cs +++ b/Xrpl/Client/connection.cs @@ -287,10 +287,37 @@ private static WebSocketClient CreateWebSocket(string url, ConnectionOptions con private static readonly Random _random = new(); - // Volatile: StopReconnectLoop, StartReconnectLoop and RetireCurrentSessionAndReconnectAsync - // write this from other threads, and the loop compares it by reference to decide whether it - // still owns the reconnect state. A stale read would let a retired loop run one more - // iteration, or make the owning loop stand down. Matches the other cross-thread fields here. + /// + /// Guards the reconnect session — , and + /// — wherever one is read and another written as a unit: + /// , , + /// , and + /// the ownership-guarded writes in . + /// + /// + /// Not every touch of these fields is covered: the per-iteration _reconnectAttempts++ in + /// , and the plain resets in ChangeServer and + /// OnceClose, still run outside it. Those predate this lock; do not read the list above + /// as "all three fields are always synchronized". + /// + /// + /// + /// volatile alone was not enough: it makes each individual access atomic, not the + /// sequence of them. The stop path used to read the field three times in a row (Cancel, + /// Dispose, null it), so a start running in between could have its brand-new source disposed + /// and cleared by the retiring stop — leaving the loop with a dead source and nobody + /// reconnecting, which is exactly the permanent wedge this whole area exists to prevent. + /// + /// + /// Nothing that can call back into consumer code runs while the lock is held: cancellation and + /// disposal of a retired source happen after the lock is released, and the loop body starts + /// with a yield so that starting it under the lock never runs a notification inline. + /// + /// + private readonly object _reconnectStateLock = new object(); + + // Volatile so the ownership checks in ReconnectLoopAsync can read it outside the lock: + // a single reference read is atomic, and those checks only ever compare, never mutate. private volatile CancellationTokenSource _reconnectCts; private Task _reconnectLoop; @@ -541,15 +568,20 @@ private async Task RetireCurrentSessionAndReconnectAsync(string reason) _reconnectMode = ReconnectMode.FastReconnect; _isFastReconnectActive = true; // Keep for backward compatibility - // 2. Stop any existing reconnect loop - var oldCts = _reconnectCts; + // 2-3. Retire the previous reconnect session and install this one as a single transaction, + // so a concurrent stop/start cannot dispose the source created here. Cancellation and + // disposal of the old source happen after the lock is released. + CancellationTokenSource oldCts; + lock (_reconnectStateLock) + { + oldCts = _reconnectCts; + _reconnectLoop = null; // Clear old loop reference so StartReconnectLoop can start a new one + _reconnectAttempts = 1; + _reconnectCts = new CancellationTokenSource(); + } + oldCts?.Cancel(); oldCts?.Dispose(); - _reconnectLoop = null; // Clear old loop reference so StartReconnectLoop can start a new one - - // 3. Initialize reconnect state BEFORE any notifications - _reconnectAttempts = 1; - _reconnectCts = new CancellationTokenSource(); // 4. Now send first notification - IsReconnectActive() will return true SetConnectionState( @@ -631,10 +663,17 @@ private async Task RetireCurrentSessionAndReconnectAsync(string reason) // Connect succeeded - cleanup reconnect state // Note: _reconnectMode will be cleared in OnceOpen when connection is fully established _isFastReconnectActive = false; - _reconnectCts?.Cancel(); - _reconnectCts?.Dispose(); - _reconnectCts = null; - _reconnectAttempts = 0; + + CancellationTokenSource settled; + lock (_reconnectStateLock) + { + settled = _reconnectCts; + _reconnectCts = null; + _reconnectAttempts = 0; + } + + settled?.Cancel(); + settled?.Dispose(); } catch (Exception ex) { @@ -1797,9 +1836,38 @@ await errorHandler // the backoff at ReconnectBaseDelay. With StopAfterMaxAttempts = false (no give-up branch) // that means connect -> handler failure -> teardown forever at a constant 2s, a sustained // connection load on a node that accepts TCP but cannot serve requests yet. - StopReconnectLoop(); - _reconnectLoop = null; - StartReconnectLoop(initialAttempts: failures); + RestartReconnectLoop(initialAttempts: failures); + } + + /// + /// Retires the current reconnect session and installs a fresh one in a single transaction, + /// seeding the attempt counter with . + /// + /// + /// Doing this as StopReconnectLoop(); _reconnectLoop = null; StartReconnectLoop(seed); + /// took the lock twice with a bare write in between, so a concurrent start (from OnceClose or + /// OnConnectionFailed) could slip in and install its own loop; the seeded start would then see + /// a live loop, return without applying the seed, and the backoff would silently stop growing + /// across consecutive handler failures — the very regression the seed exists to prevent. + /// + private void RestartReconnectLoop(int initialAttempts) + { + CancellationTokenSource retired; + lock (_reconnectStateLock) + { + retired = _reconnectCts; + _reconnectMode = ReconnectMode.LoopReconnect; + _isFastReconnectActive = false; + _reconnectAttempts = initialAttempts; + _reconnectCts = new CancellationTokenSource(); + + // Safe to start under the lock: ReconnectLoopAsync reads its token and yields before + // anything else, so this only schedules the loop - no consumer notification runs inline. + _reconnectLoop = ReconnectLoopAsync(_reconnectCts); + } + + retired?.Cancel(); + retired?.Dispose(); } private async Task OnceClose(int? code, string? description, WebSocketClient closingSocket, long sessionId) @@ -1942,10 +2010,27 @@ private async Task OnceClose(int? code, string? description, WebSocketClient clo private void StopReconnectLoop() { - _reconnectCts?.Cancel(); - _reconnectCts?.Dispose(); - _reconnectCts = null; - _reconnectAttempts = 0; + // Detach under the lock, then cancel/dispose outside it: a start racing with this stop can + // no longer have its fresh source torn down, and cancellation callbacks never run while the + // lock is held. + CancellationTokenSource retired; + lock (_reconnectStateLock) + { + retired = _reconnectCts; + _reconnectCts = null; + _reconnectAttempts = 0; + + // Drop the task reference too, in the same transaction. The retired loop exits + // asynchronously - it only notices it lost ownership on its next check - so leaving the + // reference behind makes StartReconnectLoop see `!IsCompleted` and return without + // starting anything, while the retired loop then stands down on its ownership check. + // Nobody would be reconnecting. Reachable whenever Connect or ChangeServer stops a live + // loop and the new connection fails. + _reconnectLoop = null; + } + + retired?.Cancel(); + retired?.Dispose(); // Note: Do NOT clear _reconnectMode here! // _reconnectMode is cleared only by: // - OnceOpen (connection succeeded) @@ -1976,37 +2061,49 @@ private void StartReconnectLoop(int initialAttempts = 0) { // Set reconnect mode to LoopReconnect (upgrades from FastReconnect or sets from None) _reconnectMode = ReconnectMode.LoopReconnect; - - // CRITICAL: If a loop is already running, don't start another or reset the counter - // This prevents _reconnectAttempts from being reset mid-loop when callbacks trigger - // reconnect logic (OnceClose, OnConnectionFailed, etc.) - var loopIsRunning = _reconnectLoop != null && !_reconnectLoop.IsCompleted; - if (loopIsRunning) - { - // Loop is already running - let it continue, don't reset _reconnectAttempts - return; - } - - // If we have a valid pre-created CTS (from RetireCurrentSessionAndReconnectAsync), - // we should reuse it. Check for this case first. - var existingCts = _reconnectCts; - var hasValidPreCreatedCts = existingCts != null && !existingCts.IsCancellationRequested; - - // If no valid pre-created CTS, create a new one - // Only reset _reconnectAttempts when creating a FRESH CTS (new reconnect sequence) - if (!hasValidPreCreatedCts) + + // The whole decision — is a loop already running, is the current source reusable, install a + // fresh one, hand it to the new loop — is one transaction. Split across the lock it would + // race with StopReconnectLoop and with another start: two loops could end up running, or a + // loop could be handed a source that a concurrent stop has already disposed. + CancellationTokenSource retired = null; + lock (_reconnectStateLock) { - // Cancel/dispose old CTS if any - existingCts?.Cancel(); - existingCts?.Dispose(); - _reconnectCts = new CancellationTokenSource(); - _reconnectAttempts = initialAttempts; + // CRITICAL: If a loop is already running, don't start another or reset the counter + // This prevents _reconnectAttempts from being reset mid-loop when callbacks trigger + // reconnect logic (OnceClose, OnConnectionFailed, etc.) + var loopIsRunning = _reconnectLoop != null && !_reconnectLoop.IsCompleted; + if (loopIsRunning) + { + // Loop is already running - let it continue, don't reset _reconnectAttempts + return; + } + + // If we have a valid pre-created CTS (from RetireCurrentSessionAndReconnectAsync), + // we should reuse it. Check for this case first. + var existingCts = _reconnectCts; + var hasValidPreCreatedCts = existingCts != null && !existingCts.IsCancellationRequested; + + // If no valid pre-created CTS, create a new one + // Only reset _reconnectAttempts when creating a FRESH CTS (new reconnect sequence) + if (!hasValidPreCreatedCts) + { + // Retire the old CTS after the lock is released - see _reconnectStateLock + retired = existingCts; + _reconnectCts = new CancellationTokenSource(); + _reconnectAttempts = initialAttempts; + } + // else: Reuse existing valid CTS (pre-created for fast reconnect) + // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence + // Note: _reconnectLoop was already cleared by RetireCurrentSessionAndReconnectAsync + + // Safe to start under the lock: ReconnectLoopAsync yields before touching anything, so + // this call only schedules the loop and returns - no consumer notification runs inline. + _reconnectLoop = ReconnectLoopAsync(_reconnectCts); } - // else: Reuse existing valid CTS (pre-created for fast reconnect) - // Don't reset _reconnectAttempts - this is continuation of existing reconnect sequence - // Note: _reconnectLoop was already cleared by RetireCurrentSessionAndReconnectAsync - - _reconnectLoop = ReconnectLoopAsync(_reconnectCts); + + retired?.Cancel(); + retired?.Dispose(); } private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) @@ -2014,8 +2111,20 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) // The CTS this loop owns. StopReconnectLoop cancels without awaiting the loop, so a retired loop // can still be running - or reach its tail - after a replacement has been installed. Everything // this loop writes to shared reconnect state is therefore guarded by an ownership check. + // + // Read BEFORE the yield below, and deliberately so: the caller still holds + // _reconnectStateLock here, so this source cannot yet have been retired. After the yield a + // concurrent stop may already have disposed it - Cancel/Dispose of a retired source run + // outside the lock - and CancellationTokenSource.Token throws ObjectDisposedException once + // disposed. Taken after the yield, that throw would land outside every try below, faulting + // the loop before its first attempt and vanishing as an unobserved task exception. CancellationToken ct = ownCts.Token; + // Yield so nothing beyond that read runs inline on the caller: StartReconnectLoop starts the + // loop while holding _reconnectStateLock, and a consumer notification executing under that + // lock could deadlock against any path that takes it (Disconnect from a handler, say). + await Task.Yield(); + // Don't reset _reconnectAttempts here - it may be pre-set to 1 by fast reconnect path // StartReconnectLoop() sets it to 0 when creating a new CTS @@ -2078,6 +2187,14 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) { break; } + catch (ObjectDisposedException) + { + // The source this loop owns was retired and disposed while the delay was being + // set up: registering a callback on a token whose source is gone throws instead + // of cancelling. Same meaning as cancellation - a newer sequence owns the + // reconnect state now - so leave quietly rather than fault the task. + break; + } } if (ct.IsCancellationRequested) @@ -2118,9 +2235,14 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) if (IsConnected()) { - if (ReferenceEquals(_reconnectCts, ownCts)) + // Ownership check and the write it guards belong together: checked outside the + // lock, this loop could be retired in between and reset a live sequence's counter. + lock (_reconnectStateLock) { - _reconnectAttempts = 0; + if (ReferenceEquals(_reconnectCts, ownCts)) + { + _reconnectAttempts = 0; + } } break; @@ -2170,8 +2292,19 @@ private async Task ReconnectLoopAsync(CancellationTokenSource ownCts) if (config.StopAfterMaxAttempts && _reconnectAttempts >= config.MaxReconnectAttempts) { - _reconnectCts?.Dispose(); - _reconnectCts = null; + // Re-check ownership inside the lock: between the check above and here a new sequence + // could have installed its own source, and disposing that one would strand it. + CancellationTokenSource finished = null; + lock (_reconnectStateLock) + { + if (ReferenceEquals(_reconnectCts, ownCts)) + { + finished = _reconnectCts; + _reconnectCts = null; + } + } + + finished?.Dispose(); } }