rippled 3.3.0 schema, three conformance guards, autofill fees and reconnect hardening (10.11.0.0) - #76
Merged
Conversation
Co-authored-by: github-merge-queue <github-merge-queue@users.noreply.github.com>
…гда (10.10.1.0) (#72) * 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 вместо того, чтобы его сохранить.
…тие DynamicMPT (10.11.0.0) (#71) * 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 ключей в сравнении это предусматривает.
* 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 — они не менялись.
…_entries.macro (10.11.0.0) (#75) * 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.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change aligns XRPL 3.3.0 protocol definitions, ledger models, path encoding, sponsorship, DynamicMPT, autofill fees, connection recovery, JSON conversion, and nightly CI maintenance. ChangesProtocol alignment and conformance
Connection recovery
Nightly automation and JSON conversion
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Xrpl/Models/Ledger/LOLoan.cs (1)
104-104: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCorrect the lending guides.
PrincipalRequestedis aLoanSettransaction field, not aLOLoanfield. Remove it from theLoan Fieldstables in both language versions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Xrpl/Models/Ledger/LOLoan.cs` at line 104, Remove PrincipalRequested from the Loan Fields documentation tables in both language versions, keeping it documented only under LoanSet transaction fields. Update the relevant tables near the LOLoan documentation without changing other field entries.
🧹 Nitpick comments (2)
Xrpl/Client/connection.cs (1)
2015-2019: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRead
_reconnectCtswith volatile semantics for the ownership checks.
_reconnectCtsis a plain field.StopReconnectLoop,StartReconnectLoop, andRetireCurrentSessionAndReconnectAsyncwrite it from other threads. The three newReferenceEquals(_reconnectCts, ownCts)checks (Lines 2015, 2104, 2142) now carry ownership semantics, so a stale read can make a retired loop run one extra iteration or make the owning loop exit early.The other cross-thread flags in this class (
_permanentlyDisconnected,_reconnectMode,_isIntentionalDisconnect) arevolatile. Apply the same treatment here.♻️ Proposed change
- private CancellationTokenSource _reconnectCts; + private volatile CancellationTokenSource _reconnectCts;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Xrpl/Client/connection.cs` around lines 2015 - 2019, Declare _reconnectCts with volatile semantics, matching the existing cross-thread state fields such as _permanentlyDisconnected and _reconnectMode. Ensure all ownership checks in StopReconnectLoop, StartReconnectLoop, and RetireCurrentSessionAndReconnectAsync read the updated value so retired loops cannot continue or owning loops exit prematurely.Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs (1)
79-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard against a duplicate
LEDGER_OBJECTname, as the entry parser does.
objects[name] = flagslets a second block with the same name replace the first, whileflagCountstill grows. The object then disappears from the conformance table and the minimum-count guard at Line 99 stays satisfied.RippledLedgerEntryFormats.Parsethrows for this case; keep the two parsers consistent.♻️ Proposed fix
+ if (objects.ContainsKey(name)) + { + throw new InvalidOperationException( + $"{name}: declared twice in LedgerFormats.h — the parser would drop one " + + "definition, update it before trusting this test"); + } + objects[name] = flags; flagCount += flags.Count;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs` around lines 79 - 97, Update the object parsing loop around objects and flagCount to reject duplicate LEDGER_OBJECT names before assigning objects[name] or incrementing flagCount, matching the duplicate-handling behavior of RippledLedgerEntryFormats.Parse. Preserve the existing skip for objects with no flags and ensure duplicate names cannot overwrite the first entry or inflate the count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs`:
- Around line 101-118: Update both free-port allocation sites in
TestUChangeServerFailure so the port remains reserved until StartMock binds it,
or retry with a newly allocated port when binding fails. Preserve the test’s
intended “server starts later” behavior and apply the same race-safe handling at
both locations.
In `@Tests/Xrpl.Tests/Xrpl.Tests.csproj`:
- Around line 36-41: Change the project-local fixture entries for
LedgerFormats.h and ledger_entries.macro from None Include to None Update,
preserving their existing CopyToOutputDirectory metadata. Apply the same Update
form to all nearby project-local fixture entries and do not add entries for .ref
sidecars.
In `@Xrpl/Client/connection.cs`:
- Around line 1784-1792: Preserve the accumulated backoff when the
handler-failure path in the reconnect flow calls StopReconnectLoop and
StartReconnectLoop. Update the relevant reconnect state handling so
_reconnectAttempts is not reset for this restart, or derive CalcBackoff from
_connectHandlerFailures, while retaining normal reset behavior after a
successful connection and existing StopAfterMaxAttempts handling.
---
Outside diff comments:
In `@Xrpl/Models/Ledger/LOLoan.cs`:
- Line 104: Remove PrincipalRequested from the Loan Fields documentation tables
in both language versions, keeping it documented only under LoanSet transaction
fields. Update the relevant tables near the LOLoan documentation without
changing other field entries.
---
Nitpick comments:
In `@Tests/Xrpl.Tests/Models/RippledLedgerFlags.cs`:
- Around line 79-97: Update the object parsing loop around objects and flagCount
to reject duplicate LEDGER_OBJECT names before assigning objects[name] or
incrementing flagCount, matching the duplicate-handling behavior of
RippledLedgerEntryFormats.Parse. Preserve the existing skip for objects with no
flags and ensure duplicate names cannot overwrite the first entry or inflate the
count.
In `@Xrpl/Client/connection.cs`:
- Around line 2015-2019: Declare _reconnectCts with volatile semantics, matching
the existing cross-thread state fields such as _permanentlyDisconnected and
_reconnectMode. Ensure all ownership checks in StopReconnectLoop,
StartReconnectLoop, and RetireCurrentSessionAndReconnectAsync read the updated
value so retired loops cannot continue or owning loops exit prematurely.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 14e721fa-0e54-4587-875e-52fc0c0cff4c
⛔ Files ignored due to path filters (1)
Base/Xrpl.BinaryCodec/Enums/Field.Uint8.Generated.csis excluded by!**/*.generated.*
📒 Files selected for processing (36)
.ci-config/docker-compose.ci.yml.github/workflows/protocol-watch.ymlBase/Xrpl.BinaryCodec/Enums/definitions.jsonBase/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csprojCHANGES.mdTests/Xrpl.Tests/Client/TestUChangeServerFailure.csTests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.csTests/Xrpl.Tests/CreateMockRippled.csTests/Xrpl.Tests/Fixtures/LedgerFormats.hTests/Xrpl.Tests/Fixtures/LedgerFormats.h.refTests/Xrpl.Tests/Fixtures/ledger_entries.macroTests/Xrpl.Tests/Fixtures/ledger_entries.macro.refTests/Xrpl.Tests/Integration/AmendmentGuard.csTests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.csTests/Xrpl.Tests/Integration/transactions/TestILoan.csTests/Xrpl.Tests/Models/RippledLedgerEntryFormats.csTests/Xrpl.Tests/Models/RippledLedgerFlags.csTests/Xrpl.Tests/Models/TestULedgerEntryFieldsConformance.csTests/Xrpl.Tests/Models/TestULedgerFlagsConformance.csTests/Xrpl.Tests/Models/TestUProtocolCompleteness.csTests/Xrpl.Tests/Xrpl.Tests.csprojXrpl/Client/WebSocketClient.csXrpl/Client/connection.csXrpl/Models/Ledger/LOAmendments.csXrpl/Models/Ledger/LOAmm.csXrpl/Models/Ledger/LOCredential.csXrpl/Models/Ledger/LODirectoryNode.csXrpl/Models/Ledger/LOFeeSettings.csXrpl/Models/Ledger/LOLoan.csXrpl/Models/Ledger/LOMPToken.csXrpl/Models/Ledger/LOMPTokenIssuance.csXrpl/Models/Ledger/LONFTokenPage.csXrpl/Models/Ledger/LONegativeUNL.csXrpl/Models/Ledger/LOSignerList.csXrpl/Models/Ledger/LOVault.csXrpl/Xrpl.csproj
💤 Files with no reviewable changes (2)
- Xrpl/Models/Ledger/LONFTokenPage.cs
- Xrpl/Models/Ledger/LOCredential.cs
Comment on lines
+101
to
+118
| 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); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate and inspect the free-port helper used by these tests.
fd -i 'TestUtils.cs' --exec cat -n {}
rg -nP -C4 '\bGetFreePort\s*\(' --type=csRepository: StaticBit-io/XrplCSharp
Length of output: 2573
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- target test file ---'
fd -i 'TestUChangeServerFailure.cs' --exec sh -c 'cat -n "$1"' sh {}
printf '%s\n' '--- mock startup definitions and calls ---'
rg -n -P -C8 '\b(StartMock|GetFreePort|ChangeServer|TestChangeServerAfterUserDisconnectStillReconnects)\b' --type=cs TestsRepository: StaticBit-io/XrplCSharp
Length of output: 40908
Prevent the free-port race.
TestUtils.GetFreePort stops its listener before returning, so another process can claim secondPort before StartMock binds it. Preserve the listener through startup or retry allocation when binding fails. Apply this at Lines 101 and 158.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs` around lines 101 - 118,
Update both free-port allocation sites in TestUChangeServerFailure so the port
remains reserved until StartMock binds it, or retry with a newly allocated port
when binding fails. Preserve the test’s intended “server starts later” behavior
and apply the same race-safe handling at both locations.
…abbit к релизному PR #76 (#77) * 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.
) * 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.
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs (1)
234-268: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
laterPortis held across several seconds before it is bound; use the newIsPortStillFreeguard.
TestUtils.GetFreePort()at Line 234 stops its listener before it returns, so the port is closed.StartMock(laterPort)at Line 268 binds it only after aChangeServerand aConnectthat runs to a 3s acquisition timeout. In that window another process can take the port.StartMockthen throwsSocketExceptionand the test reports a bind failure instead of the reconnect assertion it is written for.
TestUtils.IsPortStillFreewas added in this PR for exactly this case. This test does not call it.♻️ Proposed guard
// The server appears. Nobody touches the client from here on. + Assert.IsTrue( + TestUtils.IsPortStillFree(laterPort), + $"Port {laterPort} was taken by another process while the test held it; " + + "the reconnect assertion below cannot be evaluated."); _secondRippled = StartMock(laterPort);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs` around lines 234 - 268, Use TestUtils.IsPortStillFree to guard laterPort after the delayed connection attempt and before StartMock(laterPort), asserting that the port remains available. Keep the existing reconnect scenario and StartMock flow unchanged otherwise.Xrpl/Client/connection.cs (1)
2053-2098: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
initialAttemptsparameter is no longer used, and its documentation now describes a path that calls a different method.
OnConnectHandlerFailedAsynccallsRestartReconnectLoop(initialAttempts: failures)at Line 1839. The threeStartReconnectLoop()call sites (Line 689, Line 1537, Line 1999) all use the default0. The doc comment at Line 2053-2059 still attributes the parameter to "The OnConnected-handler path", which is now inaccurate.The seed also has a silent failure mode for any future caller: on the
hasValidPreCreatedCtsbranch at Line 2096 the value is discarded without notice.Remove the parameter, or keep it and document that it applies only when a fresh source is created.
♻️ Proposed simplification
- /// <param name="initialAttempts"> - /// Value to seed <c>_reconnectAttempts</c> 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. - /// </param> - private void StartReconnectLoop(int initialAttempts = 0) + /// <summary> + /// Starts a reconnect loop if none is running. A seeded restart is done by + /// <see cref="RestartReconnectLoop"/> instead. + /// </summary> + private void StartReconnectLoop() {retired = existingCts; _reconnectCts = new CancellationTokenSource(); - _reconnectAttempts = initialAttempts; + _reconnectAttempts = 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Xrpl/Client/connection.cs` around lines 2053 - 2098, Remove the unused initialAttempts parameter and its associated documentation from StartReconnectLoop, then update its callers to invoke the parameterless method. Ensure reconnect attempt initialization remains handled by the fresh-CTS path and no seed value is silently discarded when reusing a pre-created CTS.Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor (1)
731-737: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
client.Url()consistently for the server URL.
IXrplClient.Url()delegates toconnection.GetUrl(). Replace the directclient.connection.GetUrl()calls at lines 400, 620, and 630 withclient.Url().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor` around lines 731 - 737, Replace the direct client.connection.GetUrl() calls in the server URL handling paths around Index.razor with client.Url(), including the usages near lines 400, 620, and 630. Use IXrplClient.Url() consistently while preserving the existing server-switch and comparison behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Tests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.cs`:
- Around line 301-310: Update the comments above the gap assertions to account
for CreateClient’s 1-second ReconnectMaxDelay cap and the handler-failure
attempts used by CalcBackoff: describe the first-to-last range as approximately
2.5x at jitter extremes rather than 4x, and explicitly note that the assertion
depends on the configured cap remaining above the earlier backoff values.
In `@Xrpl/Client/connection.cs`:
- Around line 666-676: Update the fast-reconnect success cleanup in OnceOpen to
retain the CancellationTokenSource created or installed by this method and,
under _reconnectStateLock, clear and reset _reconnectAttempts only when
_reconnectCts still references that owned source. If a newer source was
installed by RestartReconnectLoop, leave it untouched and do not cancel or
dispose it.
---
Nitpick comments:
In `@Tests/TestsClients/Blazor-WebAssembly/Pages/Index.razor`:
- Around line 731-737: Replace the direct client.connection.GetUrl() calls in
the server URL handling paths around Index.razor with client.Url(), including
the usages near lines 400, 620, and 630. Use IXrplClient.Url() consistently
while preserving the existing server-switch and comparison behavior.
In `@Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs`:
- Around line 234-268: Use TestUtils.IsPortStillFree to guard laterPort after
the delayed connection attempt and before StartMock(laterPort), asserting that
the port remains available. Keep the existing reconnect scenario and StartMock
flow unchanged otherwise.
In `@Xrpl/Client/connection.cs`:
- Around line 2053-2098: Remove the unused initialAttempts parameter and its
associated documentation from StartReconnectLoop, then update its callers to
invoke the parameterless method. Ensure reconnect attempt initialization remains
handled by the fresh-CTS path and no seed value is silently discarded when
reusing a pre-created CTS.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 188c3bcc-f2b3-428a-9adb-ab30182b5d2b
📒 Files selected for processing (11)
CHANGES.mdDocFx/LendingProtocol-Guide.mdDocFx/LendingProtocol-Guide.ru.mdTests/TestsClients/Blazor-WebAssembly/Pages/Index.razorTests/Xrpl.Tests/Client/TestUChangeServerFailure.csTests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.csTests/Xrpl.Tests/Client/TestUReconnectSessionRaces.csTests/Xrpl.Tests/Models/RippledLedgerFlags.csTests/Xrpl.Tests/TestUtils.csTests/Xrpl.Tests/Xrpl.Tests.csprojXrpl/Client/connection.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/Xrpl.Tests/Client/TestUChangeServerFailure.cs
…ии (#79) * fix(client): не трогать чужой источник отмены в быстром переподключении Замечания CodeRabbit к релизному мержу. Все пять проверены по актуальному коду, все оказались в силе. * Ветка успеха RetireCurrentSessionAndReconnectAsync безусловно забирала _reconnectCts и диспозила его. За время await ConnectInternalAsync и WaitForConnectionAsync конкурентный путь (RestartReconnectLoop из падающего OnConnected-хендлера) успевает установить НОВЫЙ источник — и его отменяли вместе с обнулением счётчика, оставляя новую последовательность без источника. Метод теперь запоминает созданный им самим источник и снимает состояние только если _reconnectCts всё ещё ссылается на него — так же, как проверки владения в ReconnectLoopAsync * StartReconnectLoop: параметр initialAttempts стал мёртвым после появления RestartReconnectLoop (все три вызова шли без аргумента), удалён вместе с документацией; свежая последовательность начинается с нуля явно * TestUOnConnectedHandlerFailure: комментарий к ассерту роста бэкоффа врал. С ReconnectBaseDelay 100 мс и ReconnectMaxDelay 1 с задержки идут 400, 800, 1000 мс — третья упирается в cap, то есть первый-к-последнему это ~2.5x, а не 4x. Добавлено, что ассерт держится лишь пока cap выше более ранних значений: при cap 400 мс все интервалы легли бы на него и сравнялись * TestUReconnectSessionRaces: laterPort выдаётся до await'ов, поэтому перед StartMock добавлена проверка IsPortStillFree — иначе занятый порт проявился бы таймаутом финального ассерта, а не внятным сообщением * Blazor-демо: client.connection.GetUrl() заменён на client.Url() во всех трёх оставшихся местах — интерфейсный метод, и он же уже использовался в правке смены сервера Гонку из первого пункта тестом не воспроизвести: нужно попасть в окно между await'ами, а через публичный API оно недостижимо — с убранной проверкой владения все тесты переподключения проходят (проверено мутацией). Правка обоснована тем же инвариантом, что и остальные проверки владения в этом файле. Проверено: сборка решения без ошибок, юнит-тесты 883/883. * fix(client): снимать ссылку на цикл и в ветке успеха быстрого переподключения Селф-ревью PR #79 тремя независимыми проходами (Opus) — нашлось одно блокирующее и четыре неточности в моих же комментариях. Блокирующее: ветка успеха RetireCurrentSessionAndReconnectAsync забирала _reconnectCts, но оставляла _reconnectLoop — та же дыра, которую PR #78 закрыл в StopReconnectLoop, и здесь я её не заметил. Метод обнуляет _reconnectLoop на входе, поэтому за время await'ов конкурентный OnConnectionFailed видит «цикла нет», а StartReconnectLoop переиспользует ещё валидный ownCts и ставит на него новый цикл. Дальше ветка успеха отменяла источник, но ссылку на задачу оставляла: все проверки loopIsRunning видели выходящую задачу и отступали, а сам цикл вставал по проверке владения. Переподключаться было некому. Ссылка теперь снимается в той же транзакции. Неточности комментариев, все мои: * обоснование guard'а порта было скопировано из TestUChangeServerFailure, где StartMock уходит в фоновый поток. В этом файле он теперь синхронный (я сам так сделал по прошлому нитпику), поэтому занятый порт даёт SocketException сразу, а не таймаут ассерта. Плюс отмечено, что guard — диагностика: он сам биндит и отпускает, TOCTOU остаётся * XML-док StartReconnectLoop обещал «always begins at the base delay». Счётчик свежей последовательности равен нулю, инкремент идёт до расчёта, значит первая задержка — CalcBackoff(1), то есть вдвое больше базовой; а на путях ping-timeout и network-drop первая попытка вообще без задержки * remarks у _reconnectStateLock перечисляли известные исключения, но пропускали предпроверки «цикл уже идёт?» в OnConnectionFailed и OnceClose — они читают невалатильный _reconnectLoop вне лока * комментарий в catch-ветке утверждал, что источник заведомо на месте и будет переиспользован, — ровно то допущение, которое ветка успеха теперь объявляет неверным. Переписан с разбором обоих исходов * добавлена явная фраза, что при потере владения ownCts освобождать не нужно: его отменил и задиспозил тот, кто вытеснил Проверено: сборка решения без ошибок, юнит-тесты 883/883. Живой прогон Blazor-демо против локальной ноды 3.2.1 — обрыв соединения (нода убита при активном клиенте, код 1005) даёт восстановление следующей же попыткой после подъёма ноды, отключение/подключение, переключения на мёртвые порты и возврат на живой проходят, стрим ledger-ов идёт, необработанных исключений в консоли нет. * fix(client): не терять цикл на исключении потребителя и не оживлять после Disconnect Два предсуществующих дефекта в RetireCurrentSessionAndReconnectAsync, найденные селф-ревью PR #79. Оба в том же методе, чей жизненный цикл этот PR и приводит в порядок, поэтому закрываются здесь же. 1. Исключение из потребительского обработчика уносило сессию с собой. SetConnectionState синхронно зовёт OnConnectionStatus, а метод исполняется только на ping-путях, где вызывающие глушат всё. Бросок из обработчика вылетал из метода, оставляя установленный источник отмены без цикла и без владельца: клиент замирал в RestoringConnection, а CheckIfNotConnected по непустому _reconnectCts считал, что попытка идёт, из-за чего WaitForConnectionAsync выжигал весь ConnectionAcquisitionTimeout. Обе нотификации обёрнуты. В catch-ветке вдобавок изменён порядок: StartReconnectLoop вызывается ДО уведомления — без цикла клиент не вернётся, а сообщение вторично. 2. Метод оживлял клиент после пользовательского Disconnect(). Disconnect ставит _permanentlyDisconnected, чистит состояние переподключения и ждёт ping-задачу — ту самую, внутри которой крутится этот метод, то есть он ещё не завершился. Метод же безусловно сбрасывал флаг и поднимал новую сессию: Disconnect возвращался с отчётом об успехе, пока за ним собиралось живое соединение. Теперь флаг проверяется перед сбросом: если пользователь отключился, метод освобождает свой источник (по проверке владения) и уходит. Та же проверка добавлена в catch-ветку перед запуском цикла. Тестами эти пути не покрыты, и это не оговорка: все три вызова метода — на ping-путях, а во всех юнит-тестах клиента стоит UseCustomPing = false, то есть метод в тестах не исполняется вовсе. Тест возможен (UseCustomPing = true плюс мок, не отвечающий на ping), но интервал пинга захардкожен 20 с в трёх местах и конфигом не управляется, так что прогон вышел бы на 25-40 с. Дешёвый тест появится, если интервал сделать настраиваемым — отдельная задача. Проверено: сборка решения без ошибок, юнит-тесты 883/883. * refactor(client): вынести интервал и порог health-check в конфиг Проверка живости соединения жила на двух захардкоженных числах: интервал таймера 20 с (в трёх местах — обычный Timer и WASM-таймер) и порог неактивности 60 с. Теперь это HealthCheckInterval и InactivityTimeout в ConnectionOptions, значения по умолчанию прежние. Мотивация — тестируемость fast-reconnect. Все вызовы RetireCurrentSessionAndReconnectAsync стоят за этим таймером, поэтому путь, правленный в трёх PR подряд (#72, #78, #79), ни разу не исполнялся ни в одном тесте: во всех тестах клиента UseCustomPing = false. С настраиваемыми порогами он достижим за миллисекунды вместо минуты. Тесты этим коммитом НЕ добавлены, и вот почему: воспроизвести обрыв на CreateMockRippled не удалось. mock.Stop() гасит listener, но клиентский ClientWebSocket остаётся в состоянии Open — он узнаёт о смерти пира только на следующем вводе-выводе, а ошибка keepalive-пинга состояние не меняет. Пробовал паузы до 8 с: клиент так и не входит в RestoringConnection, то есть тест зелёный на несработавшем сценарии — хуже, чем никакого. Ветка порога неактивности тоже недостижима: мок отвечает даже на неизвестные команды, поэтому активность обновляется каждый тик и порог не переступается. Чтобы покрыть путь по-настоящему, нужно одно из двух: научить мок обрывать соединение (сейчас у него только Start/Stop) или поставить в тестах TCP-прокси между клиентом и сервером и рвать его. Обе опции — отдельная задача; эти два свойства делают её выполнимой и попутно убирают магические числа. Проверено: сборка решения без ошибок, юнит-тесты 883/883. * fix(client): гасить исключения нотификации в одной точке и валидировать health-check Три замечания CodeRabbit к PR #79. 1. (Major) Исключение обработчика внутри ReconnectLoopAsync. В прошлом коммите я обернул нотификации в fast-reconnect, но сам цикл переподключения тоже зовёт SetConnectionState — перед первой попыткой соединения. Бросок из OnConnectionStatus ронял задачу цикла, оставляя _reconnectCts установленным без живого цикла: восстанавливать соединение становилось некому. Латать по местам бессмысленно — защита перенесена в саму SetConnectionState, через которую проходят все уведомления класса. Две локальные обёртки убраны как избыточные; обработчик потребителя больше не может уронить машину состояний ниоткуда. 2. (Major, частично) Пользовательский Disconnect против летящего fast reconnect. ConnectInternalAsync и WaitForConnectionAsync теперь получают токен сессии, которой владеет метод: Disconnect отменяет её, и попытка прекращается вместо открытия сокета за спиной у отключённого клиента. Проверки флага для этого недостаточно — Disconnect ждёт ping-задачу считанные секунды, а получение соединения может длиться дольше. Полностью замечание не закрыто: остаётся принципиальный вопрос, вправе ли автоматические пути вообще сбрасывать _permanentlyDisconnected, — это отдельная задача, здесь взято то, что устраняет главное окно. 3. (Minor) Валидация новых опций. HealthCheckInterval на WASM-пути кастится в int миллисекунд: ноль выстреливает один раз и больше не повторяется, значения вне диапазона таймер отвергает сам. Оба свойства проверяются в ValidateConfig, то есть в конструкторе Connection, с указанием имени опции. TestUHealthCheckOptions покрывает границы: ноль, отрицательное, за пределами int.MaxValue, неположительный InactivityTimeout, и приёмку 1 мс и значений по умолчанию. Проверено: сборка решения без ошибок, юнит-тесты 888/888.
* feat(sugar): комиссии LoanSet/LoanPay/ConfidentialMPT в Autofill Три транзактора со специальной базовой комиссией считались как обычные, и все три — в сторону НЕДОплаты: комиссия ниже требуемого минимума отклоняется с telINSUF_FEE_P, а не дотягивается сетью. LoanSet: плоское baseFee*2 верно только при одиночной подписи контрагента. LoanSet::calculateBaseFee берёт по одной базовой комиссии за каждую запись CounterpartySignature.Signers. Если подпись уже проставлена — считаем её записи напрямую; если нет (обычный порядок в autofill) — запрашиваем signer list контрагента, как это делает xrpl.js. LoanPay: LoanPay::calculateBaseFee умножает ВЕСЬ Transactor-костыль, включая подписи, на один инкремент за каждые kLoanPaymentsPerFeeIncrement (5) платежей, с потолком kLoanMaximumPaymentsPerTransaction/5 (20). Оценка читает объект Loan, считает платёж как roundPeriodicPayment(PeriodicPayment, LoanScale) + LoanServiceFee и делит на него Amount транзакции. Все ветки, где rippled возвращает normalCost, воспроизведены: tfLoanFullPayment/tfLoanLatePayment, PaymentRemaining <= 5 и недоступный объект Loan. ConfidentialMPT (5 типов): Transactor::calculateBaseFee вызывается с kConfidentialFeeMultiplier = 9 — десять базовых комиссий за одиночно подписанную транзакцию. Автозаполнялись одной, т.е. в 10 раз меньше. 12 юнит-тестов на новые формулы; весь набор TestU (895) зелёный. * fix(sugar): current-леджер и защита от переполнения в комиссиях займов Замечания CodeRabbit к PR #81. account_info контрагента LoanSet и ledger_entry объекта Loan запрашивались на validated-леджере. SignerListSet или LoanSet, попавшие в последний леджер, там ещё не видны: в первом случае комиссия считается по одной подписи вместо списка, во втором объект Loan выглядит недоступным и множитель платежей теряется — обе ошибки в сторону недоплаты. Оба запроса переведены на current, как это уже делает SetNextValidSequenceNumber. Проверка потолка платежей масштабировала regularPayment вверх: PeriodicPayment у верхней границы decimal ронял autofill с OverflowException. Сравнение переставлено на деление amount на константу, которое переполниться не может. Кламп оценки платежей по PaymentRemaining, предложенный в том же ревью, не применён: LoanPay::calculateBaseFee читает sfPaymentRemaining только как short-circuit <= 5 и не ограничивает им оценку. Кламп дал бы 2 инкремента там, где сеть требует 20, то есть недоплату и telINSUF_FEE_P. Зафиксировано тестом FewPaymentsRemainingWithLargeAmount.
…ина nightly (#83) * ci: bump standalone stand to rippled 3.3.0 * feat(models)!: ImmutableFlags и дельты SponsorshipSet по схеме rippled 3.3.0 Стенд перешёл на релиз 3.3.0, что включает Sponsor, DynamicMPT, BatchV1_1, PermissionDelegationV1_1 и ConfidentialTransfer на генезисе — 44 теста, ранее скипавшихся через AmendmentGuard, начали выполняться, и 17 из них упали: обе фичи были реализованы против nightly-сборки 3.3.0~b1 от 11.07.2026, а до релиза upstream поменял их схему. DynamicMPT: - sfMutableFlags переименовано в sfImmutableFlags (тот же UInt32 nth 53, те же значения битов) с инверсией смысла: установленный бит означает «заморожено», а не «разрешено менять». Выпуск без поля теперь полностью изменяемый — противоположность прежнему поведению - MPTokenIssuanceCreateMutableFlags и MPTokenIssuanceSetMutableFlags заменены общим MPTokenIssuanceImmutableFlags (tif* = lsif*) - включение возможности переехало из отдельного поля в флаги транзакции: MPTokenIssuanceSetFlags пополнен tfMPTSet* (0x04–0x100) Sponsor: - SponsorshipSet принимает FeeAmountDelta (Amount 34) и RemainingOwnerCountDelta (Int32 2) вместо абсолютных FeeAmount/RemainingOwnerCount, которые остались полями только у объекта Sponsorship. Старые поля нода отвергает на разборе: STObject::applyTemplate → «Field 'FeeAmount' found in disallowed location» - клиентская валидация повторяет preflight: дельты ненулевые, FeeAmountDelta в XRP, при tfDeleteObject модификационные поля запрещены - добавлен Common.TryGetInt32 для знаковой дельты Фикстуры гвардов перепиныны на тег 3.3.0 (transactions.macro, LedgerFormats.h); ledger_entries.macro остаётся на develop из-за sfLEVersion. RippledLedgerFlags научился читать lsif*-константы, которые в 3.3.0 объявлены вне LEDGER_OBJECT, и падает, если не нашёл ни одной. Пин nightly-стенда сдвинут на 3.4.0~b0+202608111815.26cc683e, конфиг перегенерирован с того же ref: Definitions Watch поднимает стенд именно из него, поэтому устаревший пин делал еженедельный мониторинг слепым к этим двум переименованиям. * ci: еженедельный сторож пина nightly-стенда definitions-watch поднимает стенд из ARG XRPLD_VERSION, поэтому устаревший пин сужает еженедельную проверку до состояния rippled на момент последнего касания пина: оба переименования 3.3.0 месяц просидели за пином от 11.07. Снять пин нельзя — формат таймстампа nightly-сборок сократился с 14 до 12 цифр, и порядок версий Debian ставит старые сборки выше новых. - .ci-config/bump-nightly-pin.sh: берёт свежую сборку xrpld из nightly-канала, переписывает ARG XRPLD_VERSION и перегенерирует rippled.batchv11.cfg по develop-коммиту, зашитому в саму версию — конфиг и бинарник не могут разойтись. Режим --check только отчитывается: текущий пин, свежая сборка, возраст пина - .github/workflows/nightly-pin-watch.yml: раз в неделю; бампает, когда пин старше MAX_PIN_AGE_DAYS (21), поднимает стенд на новом пине, требует включённого на генезисе AMM-сентинела и прикладывает к PR дифф definitions против новой сборки. Учётки, идемпотентность и запасной путь через tracking issue — как в release-watch * fix(ci): валидное имя ветки в nightly-pin-watch и правка changelog Замечания CodeRabbit к PR #83. - имя ветки собиралось из версии xrpld как есть, а `~` git в ref не пускает: `git checkout -b` упал бы и PR с бампом не создавался. Версия чистится по классу допустимых символов, а не по одному символу текущего формата, плюс проверка через git check-ref-format перед использованием - в секции 10.11.0.0 CHANGES.md остались пункты, описывающие MutableFlags и tmfMPTSet* как актуальный API, хотя тот же релиз их и заменяет. Приведены к ImmutableFlags/tfMPTSet*; более ранние секции не тронуты — они описывают состояние API на момент своего релиза --------- Co-authored-by: github-merge-queue <github-merge-queue@users.noreply.github.com>
Platonenkov
changed the title
…ге пути (#82) * feat(codec): MPT-хопы (0x40) в PathSet, mpt_issuance_id в шаге пути PathSet знал только три классических бита типа хопа — 0x01 account, 0x10 currency, 0x20 issuer. rippled добавил STPathElement::TypeMpt = 0x40 в 3.2.0: хоп может нести 24-байтный MPTokenIssuanceID вместо валюты. Пробел был тихим в обе стороны. FromParser не совпадал ни с одной маской на байте 0x40, создавал пустой хоп и не считывал 24 байта MPTID — дальше весь блоб разбирался со сдвигом, без единого исключения. SynthesizeType не умел выставить бит вовсе, то есть собрать MPT-путь было нечем. Что сделано в кодеке (Base/Xrpl.BinaryCodec/Types/PathSet.cs): * PathHop.MptIssuanceId (Hash192), второй конструктор, HasMpt(), константы TypeMpt и TypeAll (0x71). currency и mpt_issuance_id в одном шаге — InvalidJsonException, как rippled бросает "bad path element: MPT and Currency" * порядок записи повторяет STPathSet::add(): байт типа, account(20), MPTID(24), currency(20), issuer(20) * FromParser отвергает то же, что и rippled: байт типа с битами вне TypeAll, currency вместе с MPT и пустой путь (ведущий или сдвоенный разделитель 0xFF). Раньше любой мусорный байт принимался, а пустой путь переживал декодирование и исчезал при обратной сборке — блоб и хеш транзакции не совпадали с исходными * не-строковый mpt_issuance_id даёт InvalidJsonException, а не сырой InvalidOperationException из JsonNode — так же, как это делают Amount и Issue Модель (Xrpl/Models/Methods/Path.cs): * добавлено MPTokenIssuanceID (mpt_issuance_id) — шаг с MPT из ripple_path_find/path_find было нечем представить * TypeHex помечен [Obsolete]: rippled убрал type_hex из STPath::getJson в 1.7.0 (коммит f0724694), в jss.h осталось только неиспользуемое объявление. Проверено на mainnet: 19 транзакций с Paths в трёх подряд идущих леджерах, 21 шаг пути, type присутствует во всех 21, type_hex — ни в одном; то же в ripple_path_find на s1/s2.ripple.com * у Type уточнена документация: документация XRPL помечает поле устаревшим, но rippled по-прежнему отдаёт его в каждом шаге каждого ответа, поэтому поле остаётся. На исходящем пути оно игнорируется — STParsedJSON читает только account/currency/mpt_issuance_id/issuer, а кодек синтезирует байт из фактически заданных полей * Payment.IsPathStep принимает mpt_issuance_id как актив шага и отвергает его в паре с currency Тесты (Tests/Xrpl.BinaryCodec.Test/Types/TestUPathSet.cs, 9 штук): раскладка классического хопа (0x30), MPT-хопа (0x40) и MPT+issuer (0x60) против rippled, round-trip декодирования, три пути отказа и тест, пиннящий синтез байта типа: удаление type или заведомо неверный type/type_hex не меняют блоб. Опережает сеть осознанно: MPTokensV2 на mainnet не включена и сейчас не голосуется, xrpl.js и xrpl-py бит 0x40 тоже не знают. Проверено: юнит-тесты 1036/1036. Отдельно — регрессия на реальной транзакции 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43: блоб, собранный кодеком из её tx_json, совпадает с mainnet-блобом побайтово. * refactor(models)!: тип шага пути — [Flags] enum PathStepType вместо int Байт типа хопа — битовая маска, но оба слоя писали её голым числом: Path.Type был int?, PathHop.Type — int, и в коде вызывающего это превращалось в сравнения с магическим 48. Ledger-объекты давно живут иначе: AccountRootFlags и ещё восемь [Flags]-энумов стоят прямо в свойстве. Приводим шаг пути к тому же виду. PathStepType (Base/Xrpl.BinaryCodec/Enums/PathStepType.cs) — один enum на оба слоя: Account 0x01, Currency 0x10, Issuer 0x20, MPTokenIssuanceID 0x40, All 0x71. Живёт в кодеке, потому что участвует в бинарной сериализации; модель ссылается на него так же, как TxFormat и MPTokenIssuanceCreate уже ссылаются на типы BinaryCodec. Ломающее: Path.Type теперь PathStepType?, PathHop.Type — PathStepType, а константы PathHop.TypeAccount/TypeCurrency/TypeIssuer/TypeMpt/TypeAll удалены в пользу enum. Код вида step.Type == 48 перестанет собираться, чинится как PathStepType.Currency | PathStepType.Issuer. Формат на проводе не меняется: XrplJsonOptions намеренно не регистрирует глобальный JsonStringEnumConverter — протокольные enum числовые, — поэтому type по-прежнему пишется и читается как число. Значение с битом, которого enum не объявляет, переживает десериализацию без потерь (проверено на 176). Единственная потеря терпимости: "type":"48" строкой больше не разбирается, так как NumberHandling.AllowReadingFromString на enum не распространяется; rippled всегда шлёт число. TestUPathStep (5 тестов) пиннит разбор 48 и 96 во флаги, числовую форму на проводе, сохранение необъявленного бита и null при отсутствии поля. Проверено: юнит-тесты 1060/1060, блоб реальной mainnet-транзакции 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43 по-прежнему собирается побайтово идентично. * fix(models): замечания CodeRabbit к PR #82 — шаги пути по правилам toStrand Три находки, все приняты. 1. Payment.IsPathStep пропускал account вместе с активом (Major). Метод был портом xrpl.js isPathStep, который принимает {account, currency} и {account, issuer}; после добавления MPT туда же попало {account, mpt_issuance_id}. rippled такие пути отвергает до исполнения — toStrand() в PaySteps.cpp: if (hasAccount && (hasIssuer || hasCurrency)) return {temBAD_PATH, Strand{}}; if (hasMPT && (hasCurrency || hasAccount)) return {temBAD_PATH, Strand{}}; То есть SDK формировал транзакцию с гарантированным temBAD_PATH. Валидация приведена к правилам toStrand; расхождение с xrpl.js осознанное — там пробел, а не контракт. Проверено эмпирически: в 40 подряд идущих mainnet-леджерах 166 шагов пути, типы только 48 (currency+issuer), 1 (account) и 16 (currency) — ни одной комбинации account с активом, так что ужесточение не отвергает то, что сеть реально использует. 2. Пустые пути отвергались только на одной границе кодека (Major). Прошлый коммит закрыл ведущий и сдвоенный разделитель при разборе, но мимо прошли ещё два случая: ToBytes молча выбрасывал пустой Path (на выходе получался блоб без этого пути), а разбор принимал терминатор сразу после разделителя — хвостовой пустой путь. Оба места теперь бросают BinaryCodecException, как rippled в STPathSet(SerialIter&). 3. CHANGES.md ссылался на удалённые константы (Minor). TypeMpt/TypeAll исчезли вместе с переходом на PathStepType — текст исправлен на PathStepType.MPTokenIssuanceID и PathStepType.All. Тесты: TestUPathSetEmptyPathThrowsOnEncode, TestUPathSetTrailingSeparatorThrowsOnDecode, TestUPathStepValidationMatchesRippledToStrand. Проверено: юнит-тесты 1063/1063, блоб mainnet-транзакции 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43 по-прежнему собирается побайтово идентично. * refactor(models): PathStepType переезжает в модели, кодек остаётся байтовым Кодек — низкоуровневый слой, ему незачем экспортировать перечисление, которое становится частью публичной поверхности моделей, а моделям незачем тянуть namespace кодека ради имени типа. В репозитории эта граница уже проведена: Xrpl.Models.Enums.TransactionType и LedgerEntryType живут отдельно от BinaryCodec.Types.TransactionType. PathStepType переехал в Xrpl/Models/Enums/PathStepType.cs и используется только в Path.Type. В кодеке возвращены байтовые константы TypeAccount/TypeCurrency/TypeIssuer/TypeMpt/TypeAll, PathHop.Type снова byte, маски в FromParser считаются по ним. Base/Xrpl.BinaryCodec/Enums/ PathStepType.cs удалён; Xrpl/Models/Methods/Path.cs больше не ссылается на BinaryCodec. Отдельно: ToJson пишет байт типа как int, а не как byte. JsonValue<byte> отказывается отдавать GetValue<int>(), то есть потребитель Decode, читавший type как int, получил бы InvalidOperationException — это поймал TestUPathSetMptHopRoundTrips после перевода поля в byte. Проверено: юнит-тесты 1063/1063, блоб mainnet-транзакции 1D813B78FC55ABF9054AEBD2AF9DD7C90361F9985B7897E8E9A592D63BF0CC43 собирается побайтово идентично, decode возвращает type: 48. * refactor(models)!: удалить Path.TypeHex вместо пометки [Obsolete] Помечать устаревшим имеет смысл то, что работает и однажды перестанет. type_hex не работает с февраля 2021: rippled убрал его из STPath::getJson в 1.7.0 (коммит f0724694), в jss.h осталось только неиспользуемое объявление. Свойство не могло принять ничего, кроме null, ни на одной существующей ноде — это не устаревающий контракт, а мёртвая поверхность. Удалено сразу, без периода [Obsolete], как в 10.11.0.0 удалялись свойства ledger-объектов, не являющиеся полями протокола. Ответ ноды старее 1.7.0 по-прежнему разбирается: неизвестный ключ System.Text.Json игнорирует, UnmappedMemberHandling.Disallow в XrplJsonOptions не выставлен. Закреплено тестом TestUPathStepIgnoresLegacyTypeHex. Тест кодека, доказывающий что лишний type_hex во входном JSON не влияет на блоб, оставлен как есть — он о поведении кодека, а не о модели. Проверено: юнит-тесты 1064/1064.
…брыва (#84) * fix(tests): мок-сервер переставал принимать соединения после одного обрыва Причина флейка TestConcurrentChangeServerKeepsClientRecoverable: unit упал на push-прогоне dev, тогда как тот же коммит в PR-прогоне прошёл. Server.connectionCallback вызывал BeginAccept последней строкой try-блока, поэтому любое исключение выше по телу навсегда обрывало приём соединений. Пир, рвущий соединение во время handshake — ровно то, что порождают конкурентные ChangeServer/Disconnect — глушил мок именно так. Со стороны всё выглядело исправно: слушающий сокет оставался связан, порт числился занятым, TCP-соединения устанавливались ядром, и клиент видел сервер, который принял подключение и молчит. Отсюда 2m06s у упавшего прогона против 10s у обычного и сообщение, обвиняющее клиент в том, что он «не дошёл до живого сервера». - BeginAccept перезапускается безусловно после колбэка, чем бы тот ни кончился; ObjectDisposedException обрабатывается отдельно и без перезапуска — он означает, что Stop() закрыл listener - сокет, чей handshake упал, закрывается, а не течёт до конца процесса - TestUMockRippledAcceptLoop фиксирует поведение: обрыв на handshake (RST через LingerOption(true, 0)) поодиночке и десять подряд, затем обычный клиент, который обязан подключиться. На старом колбэке оба падают, после фикса проходят за 0.6s - TestUtils.MockCompletesHandshake: проба реальным WS-upgrade. Упавший reconnect-тест теперь сообщает, отвечает ли мок, — глухой сервер больше не будет прочитан как баг клиента. Проба покрыта на живом моке, свободном порту и слушающем сокете, который никогда не принимает * docs(changes): убрать запись о фиксе мок-сервера из changelog Правка чисто в тестовой инфраструктуре: код SDK не менялся, потребителю пакета изменение не видно, поэтому в changelog релиза ему места нет. Разбор причины остаётся в описании PR и в комментариях к самому коду.
…с неизвестного LedgerEntryType (#85) * perf(json): кэш производных JsonSerializerOptions + фикс неизвестного LedgerEntryType Полиморфные конвертеры (LOConverter, оба конвертера транзакций, MetaBinaryConverter, LedgerBinaryConverter, LONFTokenConverter, GenericStringConverter<T>) заново входят в сериализатор с удалённым собственным конвертером, и каждый вызов строил производные JsonSerializerOptions с нуля. System.Text.Json кэширует метаданные типов per-instance, а копирующий конструктор кэш не переносит — страница account_objects на 200 записей перестраивала контракты 200 раз. JsonSerializerOptionsCache строит их один раз на пару (исходные options, тип конвертера), слабо ключуясь по исходному экземпляру. Безопасно: STJ замораживает options при первом использовании, поэтому то, что конвертер получает на вход, измениться уже не может. Попутно — LOConverter.DetermineType разрешал неизвестный LedgerEntryType в LOAccountRoot: Enum.TryParse при неудаче пишет в out default(TEnum), а AccountRoot — нулевое значение, и инициализация Unknown затиралась. Тип объекта новее SDK десериализовался как account root с молча потерянными полями вместо отката к BaseLedgerEntry. Снят //todo change from class to interface на AccountObjects.AccountObjectList: парсинг работает как в transactionResponse с момента глобальной регистрации LOConverter, а BaseLedgerEntry обязан оставаться конкретным классом именно как fallback для Unknown. Тесты: TestUJsonSerializerOptionsCache (переиспользование экземпляра, изоляция по источнику и типу конвертера), TestUAccountObjectsPolymorphism (разнотипная страница account_objects, round-trip, неизвестный тип, страница на 200 записей), два теста в TestULOConverter. * docs(json): уточнить механизм выигрыша от кэша и усилить его тесты Селф-ревью показал, что заявленное обоснование неверно для .NET 8+. Пробник-конвертер, записывающий полученный экземпляр options при разборе списка, видит один и тот же экземпляр на всех элементах даже при копии-на-вызов: с .NET 8 STJ разделяет caching-context между структурно равными options, поэтому метаданные типов не перестраивались. Настоящая цена копии — аллокация, копирование списка конвертеров и структурный поиск в пуле контекстов на каждое конвертируемое значение. Оптимизация остаётся: 200 страниц account_objects по 200 записей — 456 мс / 47 МБ до, 217 мс / 29 МБ после; плюс снимается зависимость от пула контекстов, ограниченного 64 записями. Формулировки в XML-doc и CHANGES.md приведены к измеренному. Deserialize_ReusesTheCachedOptionsInstance не проверял заявленного: обе стороны сравнения читали кэш напрямую, тест оставался зелёным при откате LOConverter.Read на копию-на-вызов. Заменён на Deserialize_GoesThroughTheCache — приватный экземпляр options, которого не касается ни один другой тест, и проверка появления записи в кэше через internal HasCachedEntry. Откат конвертера теперь роняет тест (проверено). Добавлен WithoutConverter_LeavesTheSourceOptionsIntact: если Build когда-нибудь начнёт снимать конвертер с исходного экземпляра вместо копии, XrplJsonOptions.Default потеряет LOConverter на весь процесс, и раньше этого не поймал бы ни один тест класса. * docs(json): убрать устаревший механизм из summary тестов кэша options
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Xrpl/Client/connection.cs (1)
2538-2546: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: derive the keepalive/ping split from
InactivityTimeouttoo.The inactivity limit is now configurable, but the keepalive threshold at Line 2548 stays a hard-coded 30 seconds. If a consumer sets
InactivityTimeoutbelow 30 seconds, the full ping-request branch becomes unreachable, because this branch returns first. The defaults keep the previous behavior, so this is a clarity concern only.Consider expressing the keepalive threshold as a fraction of
InactivityTimeout, or document the coupling next to the option.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Xrpl/Client/connection.cs` around lines 2538 - 2546, Align the keepalive threshold in the surrounding connection-monitoring logic with configurable config.InactivityTimeout instead of leaving it hard-coded at 30 seconds, so the ping-request branch remains reachable for shorter timeout values. Prefer deriving the threshold as a documented fraction of InactivityTimeout while preserving the existing default behavior.Xrpl/Models/Transactions/MPTokenIssuanceSet.cs (1)
244-255: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParse
FlagswithCommon.TryGetUInt32for consistent validation errors.
Convert.ToUInt32(flags)throwsFormatExceptionorInvalidCastExceptionwhenFlagsis a non-numeric value. The adjacentImmutableFlagscheck usesCommon.TryGetUInt32and reports aValidationException. Callers that catchValidationExceptiondo not catch the conversion exceptions.♻️ Proposed change
uint flagValue = 0; if (tx.TryGetValue("Flags", out var flags) && flags is not null) { - flagValue = Convert.ToUInt32(flags); + if (!Common.TryGetUInt32(flags, out flagValue)) + { + throw new ValidationException("MPTokenIssuanceSet: Flags must be a number"); + } bool hasLock = (flagValue & (uint)MPTokenIssuanceSetFlags.tfMPTLock) != 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Xrpl/Models/Transactions/MPTokenIssuanceSet.cs` around lines 244 - 255, In the Flags parsing block of MPTokenIssuanceSet, replace Convert.ToUInt32(flags) with Common.TryGetUInt32 so invalid values produce the same ValidationException-based validation behavior as the adjacent ImmutableFlags check. Preserve the existing tfMPTLock/tfMPTUnlock conflict validation after successful parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Xrpl/Sugar/Autofill.cs`:
- Around line 340-351: Update the catch clauses in FetchCounterpartySignerCount
at Xrpl/Sugar/Autofill.cs lines 340-351 and FetchLoan at lines 413-431 to apply
the cancellationToken exception filter when
(!cancellationToken.IsCancellationRequested). This must let cancellation
propagate while preserving the existing fallback values for other exceptions.
---
Nitpick comments:
In `@Xrpl/Client/connection.cs`:
- Around line 2538-2546: Align the keepalive threshold in the surrounding
connection-monitoring logic with configurable config.InactivityTimeout instead
of leaving it hard-coded at 30 seconds, so the ping-request branch remains
reachable for shorter timeout values. Prefer deriving the threshold as a
documented fraction of InactivityTimeout while preserving the existing default
behavior.
In `@Xrpl/Models/Transactions/MPTokenIssuanceSet.cs`:
- Around line 244-255: In the Flags parsing block of MPTokenIssuanceSet, replace
Convert.ToUInt32(flags) with Common.TryGetUInt32 so invalid values produce the
same ValidationException-based validation behavior as the adjacent
ImmutableFlags check. Preserve the existing tfMPTLock/tfMPTUnlock conflict
validation after successful parsing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 562ff2b7-88b1-4d34-b9f1-7593104c3a0b
⛔ Files ignored due to path filters (3)
Base/Xrpl.BinaryCodec/Enums/Field.Amount.Generated.csis excluded by!**/*.generated.*Base/Xrpl.BinaryCodec/Enums/Field.Int32.Generated.csis excluded by!**/*.generated.*Base/Xrpl.BinaryCodec/Enums/Field.Uint32.Generated.csis excluded by!**/*.generated.*
📒 Files selected for processing (65)
.ci-config/Dockerfile.nightly.ci-config/bump-nightly-pin.sh.ci-config/docker-compose.ci.yml.ci-config/rippled.batchv11.cfg.ci-config/rippled.cfg.github/workflows/definitions-watch.yml.github/workflows/nightly-pin-watch.yml.github/workflows/release-watch.ymlBase/Xrpl.BinaryCodec/Enums/definitions.jsonBase/Xrpl.BinaryCodec/Types/PathSet.csCHANGES.mdCLAUDE.mdDocFx/ConfidentialMPT-Guide.mdDocFx/ConfidentialMPT-Guide.ru.mdDocFx/Sponsorship-Guide.mdDocFx/Sponsorship-Guide.ru.mdTests/TestsClients/Blazor-WebAssembly/Pages/Index.razorTests/Xrpl.BinaryCodec.Test/Types/TestUPathSet.csTests/Xrpl.Tests/Client/Json/Converters/AccountObjectsPolymorphismTests.csTests/Xrpl.Tests/Client/Json/Converters/LOConverterTests.csTests/Xrpl.Tests/Client/Json/JsonSerializerOptionsCacheTests.csTests/Xrpl.Tests/Client/TestUHealthCheckOptions.csTests/Xrpl.Tests/Client/TestUMockRippledAcceptLoop.csTests/Xrpl.Tests/Client/TestUOnConnectedHandlerFailure.csTests/Xrpl.Tests/Client/TestUReconnectSessionRaces.csTests/Xrpl.Tests/Fixtures/LedgerFormats.hTests/Xrpl.Tests/Fixtures/LedgerFormats.h.refTests/Xrpl.Tests/Fixtures/ledger_entries.macroTests/Xrpl.Tests/Fixtures/ledger_entries.macro.refTests/Xrpl.Tests/Fixtures/transactions.macroTests/Xrpl.Tests/Fixtures/transactions.macro.refTests/Xrpl.Tests/Integration/transactions/TestIBatchSponsorship.csTests/Xrpl.Tests/Integration/transactions/TestIDynamicMPT.csTests/Xrpl.Tests/Integration/transactions/TestISponsorship.csTests/Xrpl.Tests/Integration/transactions/TestISponsorshipSigningMatrix.csTests/Xrpl.Tests/MockRippled/Server.csTests/Xrpl.Tests/Models/RippledLedgerFlags.csTests/Xrpl.Tests/Models/TestUConfidentialMPT.csTests/Xrpl.Tests/Models/TestULedgerFlagsConformance.csTests/Xrpl.Tests/Models/TestUPathStep.csTests/Xrpl.Tests/Models/TestUProtocolCompleteness.csTests/Xrpl.Tests/Sugar/TestUAutofillFees.csTests/Xrpl.Tests/TestUtils.csXrpl/Client/Json/Converters/GenericStringConverter.csXrpl/Client/Json/Converters/LONFTokenConverter.csXrpl/Client/Json/Converters/LedgerBinaryConverter.csXrpl/Client/Json/Converters/LedgerObjectConverter.csXrpl/Client/Json/Converters/MetaBinaryConverter.csXrpl/Client/Json/Converters/TransactionRequestConverter.csXrpl/Client/Json/Converters/TransactionResponseConverter.csXrpl/Client/Json/JsonSerializerOptionsCache.csXrpl/Client/connection.csXrpl/Models/Enums/PathStepType.csXrpl/Models/Ledger/LOMPTokenIssuance.csXrpl/Models/Methods/AccountObjects.csXrpl/Models/Methods/Path.csXrpl/Models/Transactions/Common.csXrpl/Models/Transactions/MPTokenIssuanceCreate.csXrpl/Models/Transactions/MPTokenIssuanceImmutableFlags.csXrpl/Models/Transactions/MPTokenIssuanceSet.csXrpl/Models/Transactions/Payment.csXrpl/Models/Transactions/SponsorshipSet.csXrpl/Models/Transactions/TxFormat.csXrpl/Sugar/Autofill.csXrpl/Xrpl.csproj
🚧 Files skipped from review as they are similar to previous changes (6)
- Tests/Xrpl.Tests/Fixtures/LedgerFormats.h.ref
- .ci-config/docker-compose.ci.yml
- Tests/Xrpl.Tests/Fixtures/ledger_entries.macro.ref
- Tests/Xrpl.Tests/Client/TestUReconnectSessionRaces.cs
- Tests/Xrpl.Tests/Fixtures/ledger_entries.macro
- Tests/Xrpl.Tests/Models/TestULedgerFlagsConformance.cs
Comment on lines
+340
to
+351
| try | ||
| { | ||
| AccountInfo data = await client.AccountInfo(request, cancellationToken); | ||
| int? entries = data?.SignerLists?.Length > 0 ? data.SignerLists[0].SignerEntries?.Count : null; | ||
| return entries is > 0 ? entries.Value : 1; | ||
| } | ||
| catch (Exception) | ||
| { | ||
| // The counterparty account may not exist yet; preclaim rejects the transaction anyway. | ||
| return 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unfiltered catch (Exception) hides cancellation in both fee lookups. Both helpers wrap a client call in a bare catch, so an OperationCanceledException from cancellationToken becomes a silent fee fallback instead of stopping Autofill. Add an exception filter at each site.
Xrpl/Sugar/Autofill.cs#L340-L351: addwhen (!cancellationToken.IsCancellationRequested)to the catch inFetchCounterpartySignerCountso cancellation propagates instead of returning 1.Xrpl/Sugar/Autofill.cs#L413-L431: addwhen (!cancellationToken.IsCancellationRequested)to the catch inFetchLoanso cancellation propagates instead of returningnull.
📍 Affects 1 file
Xrpl/Sugar/Autofill.cs#L340-L351(this comment)Xrpl/Sugar/Autofill.cs#L413-L431
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Xrpl/Sugar/Autofill.cs` around lines 340 - 351, Update the catch clauses in
FetchCounterpartySignerCount at Xrpl/Sugar/Autofill.cs lines 340-351 and
FetchLoan at lines 413-431 to apply the cancellationToken exception filter when
(!cancellationToken.IsCancellationRequested). This must let cancellation
propagate while preserving the existing fallback values for other exceptions.
…ption для Flags (#86) Замечания CodeRabbit к релизному PR #76. - FetchCounterpartySignerCount и FetchLoan оборачивают вызов клиента широким catch — это верно для случая, ради которого он существует (аккаунт контрагента или объект Loan ещё не созданы, дальше отработает preclaim), но туда же уходил OperationCanceledException от токена вызывающего. Autofill продолжал работу и записывал комиссию, посчитанную по fallback — один подписант, отсутствующий заём, — для запроса, который вызывающий уже отменил. Добавлен фильтр when (!cancellationToken.IsCancellationRequested): отмена вызывающего всплывает, а таймаут внутри клиента, который этот токен не отменяет, по-прежнему уходит в fallback - MPTokenIssuanceSet: Flags разбирался через Convert.ToUInt32, бросающий FormatException/InvalidCastException на нечисловом значении, тогда как соседняя проверка ImmutableFlags и остальные валидаторы сообщают ValidationException — её и ловят вызывающие Тесты: отменённый токен обязан бросить и не оставить Fee (LoanSet и LoanPay), нечитаемый объект Loan обязан по-прежнему уходить в fallback, нечисловой Flags — давать ValidationException. Первые два падают, если убрать фильтры.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Релиз 10.11.0.0:
XrplиXrpl.BinaryCodecидут на 10.11.0.0,Xrpl.AddressCodecиXrpl.Keypairsостаются на 10.9.0.0 — они не менялись и продолжают подтягиваться потребителями в опубликованной версии.Полное описание — в секции 10.11.0.0 CHANGES.md. Ниже — что именно едет и почему.
DynamicMPT:
MutableFlags→ImmutableFlags, смысл инвертирован. Поле то же —UInt32, nth 53, те же значения битов — но установленный бит теперь означает «заморожено навсегда», а не «разрешено менять позже». Выпуск, созданный без поля, стал полностью изменяемым: противоположность прежнему поведению. Так как код поля не изменился, старые модели собирали блоб, который нода принимала и читала наоборот. ЗатронутыMPTokenIssuanceCreate,MPTokenIssuanceSet,LOMPTokenIssuance; два enum'а*MutableFlagsзаменены общимMPTokenIssuanceImmutableFlags.DynamicMPT: включение возможности переехало в флаги транзакции. Вместо
MPTokenIssuanceSet.MutableFlags = tmfMPTSet*—Flags = tfMPTSet*(0x04–0x100) вMPTokenIssuanceSetFlags.Sponsor:
SponsorshipSetпринимает дельты.FeeAmountDelta(Amount34) иRemainingOwnerCountDelta(Int322, знаковый) вместо абсолютныхFeeAmount/RemainingOwnerCount— те остались полями только объектаSponsorship. Отправка старых полей отвергается нодой на разборе:Field 'FeeAmount' found in disallowed location.Удалены свойства ledger-объектов, которые не являются полями протокола —
LOVault.DomainID,LOLoan.PrincipalRequested,LOCredential.OwnerNode,LONFTokenPage.NFTokenPage,LOAmm.LedgerCurrentIndex,LOAmm.Validated. Ни одно из них никогда не могло содержать значение: rippled строит объект по фиксированномуSOTemplate. Проверено на живой ноде и по четырём версиям rippled.Миграция для всех пунктов — с примерами до/после — в
CHANGES.md.Протокол и модели
Sponsor,DynamicMPT,BatchV1_1,PermissionDelegationV1_1,ConfidentialTransferна генезисе: 44 теста, прежде скипавшихся черезAmendmentGuard, теперь выполняются на каждом прогонеsfLEVersion— версия схемы Vault (rippled #7817):definitions.json, генерируемыйField.Uint8,LOVault.LEVersionи enumVaultVersion. Отсутствие значения у старых vault'ов осмысленно, а не «нет данных»MPTCanHoldConfidentialBalance,lsfMPTAMMи другие: безымянный бит всё равно приходил числом, поэтому пропажа ничем себя не проявлялаPreviousTxnID/PreviousTxnLgrSeqнаLOAmm,LOAmendments,LODirectoryNode,LOFeeSettings,LONegativeUNLLOAmm—AMMAccountникогда не десериализовался (поле объекта называетсяAccount, атрибута не было), а конструктор объявлял объект какAccountRootТри поверхности конформности
Модели теперь проверяются против вендорённых копий rippled по всем трём осям, а не выборочно:
TestUTxFormatConformancetransactions.macroTestULedgerEntryFieldsConformanceledger_entries.macroTestULedgerFlagsConformanceLedgerFormats.hФикстуры пиньтся по sha и байт-идентичны upstream (перепроверяются
curl … | diffиз.ref).transactions.macroиLedgerFormats.h— на теге 3.3.0,ledger_entries.macro— на develop, потому чтоsfLEVersionесть только там. Парсер флагов дополнительно читаетlsif*-константы, которые в 3.3.0 вынесены изLEDGER_OBJECT.Autofill
Покрыты три транзактора со special base fee, которые недоплачивали — а это отказ
telINSUF_FEE_P, а не переплата:LoanSet— одна базовая комиссия на каждого подписантаCounterpartySignature.Signers, а не фиксированный×2LoanPay— инкремент на каждые 5 обрабатываемых платежей, до 20 инкрементовКлиент: переподключение
OnConnectedбольше не убивало клиент навсегдаOnConnectedуходят в бэкофф вместо бесконечного цикла connect → падение → разрыв на фиксированной задержке_reconnectCtsобъявленvolatileИнфраструктура
nightly-pin-watch.yml— новый еженедельный сторож пина nightly-стенда. Пин определяет горизонтdefinitions-watch: пока он устаревший, еженедельная сверкаdefinitions.jsonидёт против сборки старее той, что гоняет CI, и рапортует «в порядке» про прошлое. Именно так оба переименования 3.3.0 месяц оставались незамеченными. Бампает.ci-config/bump-nightly-pin.sh, конфиг перегенерируется по develop-коммиту, зашитому в саму строку версии, поэтому конфиг и бинарник не могут разойтисьXrpl.BinaryCodecсинхронизирован по версии сXrpl(10.11.0.0): кодек везёт новые поля, поэтому двигаются вместеПроверка
devSummary by CodeRabbit
New Features
Bug Fixes
Documentation