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