fix(client): бэкофф при повторных сбоях OnConnected + замечания CodeRabbit к релизному PR #76

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

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:

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` для получения кредитов аккаунта:

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);

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");
         }
+
+        /// <summary>
+        /// With <c>StopAfterMaxAttempts = false</c> 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.
+        /// </summary>
+        [TestMethod]
+        public async Task TestRepeatedOnConnectedFailuresBackOff()
+        {
+            List<DateTime> attempts = new List<DateTime>();
+            TaskCompletionSource<bool> enough =
+                new TaskCompletionSource<bool>(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<TimeSpan> gaps = new List<TimeSpan>();
+            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)}");
+        }
     }
 }

Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs

@@ -76,9 +76,23 @@ internal static Dictionary<string, Dictionary<string, uint>> Parse()
             Dictionary<string, Dictionary<string, uint>> 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<string> 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<string, uint> flags = new();
 
                 foreach (Match flag in FlagEntry.Matches(block.Groups["body"].Value))
@@ -92,7 +106,7 @@ internal static Dictionary<string, Dictionary<string, uint>> Parse()
                 if (flags.Count == 0)
                     continue;
 
-                objects[name] = flags;
+                objects.Add(name, flags);
                 flagCount += flags.Count;
             }
 

Tests/Xrpl.Tests/TestUtils.cs

@@ -1,21 +1,71 @@
-
+
 // 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;
 
 namespace Xrpl.Tests
 {
     public class TestUtils
     {
+        /// <summary>
+        /// 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.
+        /// </summary>
+        private static readonly ConcurrentDictionary<int, byte> ClaimedPorts = new();
+
+        /// <summary>
+        /// A loopback port free at the moment of the call and not handed out before.
+        /// </summary>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
         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");
+        }
+
+        /// <summary>
+        /// Whether <paramref name="port"/> 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.
+        /// </summary>
+        static public bool IsPortStillFree(int port)
+        {
+            try
+            {
+                TcpListener listener = new TcpListener(IPAddress.Loopback, port);
+                listener.Start();
+                listener.Stop();
+                return true;
+            }
+            catch (SocketException)
+            {
+                return false;
+            }
         }
     }
 }
-

Tests/Xrpl.Tests/Xrpl.Tests.csproj

@@ -30,13 +30,16 @@
                 <None Include="..\..\Base\Xrpl.BinaryCodec\Enums\definitions.json" Link="definitions.json">
                         <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
                 </None>
-                <None Include="Fixtures\transactions.macro">
+                <!-- Update, not Include: the SDK's default None glob already picks these up,
+                     so Include would declare them twice. Only the fixtures themselves are
+                     copied — the .ref sidecars document the pin and are never read by tests. -->
+                <None Update="Fixtures\transactions.macro">
                         <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
                 </None>
-                <None Include="Fixtures\LedgerFormats.h">
+                <None Update="Fixtures\LedgerFormats.h">
                         <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
                 </None>
-                <None Include="Fixtures\ledger_entries.macro">
+                <None Update="Fixtures\ledger_entries.macro">
                         <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
                 </None>
         </ItemGroup>