feat(protocol): поля транзакций, потерянные моделями, и приведение TxFormat к rippled (10.10.0.0) - #66
Conversation
…Format к rippled (10.10.0.0)
Поля, объявленные протоколом транзакционными, но отсутствовавшие в типизированных
моделях: TxFormat их перечислял и кодек знал, поэтому через Dictionary<string, object>
значения доезжали, но у моделей не было свойства — чтение молча теряло их, а задать
через типизированный API было нельзя вообще.
* TransactionRequest/TransactionResponse + Delegate и OperationLimit — оба поля входят
в commonFields rippled (TxFormats.cpp), то есть допустимы на любом типе транзакции,
поэтому размещены на общей базе, а не на отдельных транзакциях. Delegate помечает
транзакцию, отправленную по правам DelegateSet (BatchUtils уже учитывал его при сборе
обязательных подписантов батча). OperationLimit инертен на XRPL, но именно его читает
Burn-2-Mint на Xahau — потребителям больше не нужен обходной путь через словарь.
* AccountSet/AccountSetResponse + WalletLocator и WalletSize — оба стоят в формате
AccountSet у rippled. WalletSize легаси и транзактором не обрабатывается, добавлен
ради целостности чтения.
* ValidateBaseTransaction типизированно проверяет два новых common-поля.
* Target намеренно не добавлен: sfTarget выведен из обращения (AccountID nth 7 помечен
unused в sfields.macro, имени нет в definitions.json), а TicketCreate после амендмента
TicketBatch несёт только sfTicketCount. Протухшие Target/Expiration убраны из
TicketCreate в TxFormat; Field.Target оставлен в кодеке для декодирования старых blob.
TxFormat приведён к rippled полностью. Таблица инертна в рантайме (TxFormat.Validate не
на пути подписи, кодек сериализует по definitions.json), поэтому неверные записи не давали
симптомов и жили годами. Диff по transactions.macro нашёл 6 неверных форматов из 82:
* CheckCreate/CheckCash/CheckCancel — все три были дословной копией записи
PaymentChannelClaim, стоящей выше по файлу.
* NFTokenMint — не хватало Amount/Destination/Expiration: поля NFTokenMintOffer попали
в модель ещё в 10.7.0, а формат за ними не последовал.
* OracleSet (BaseAsset/QuoteAsset/AssetPrice/Scale) и SignerListSet (WalletLocator) —
поля вложенных объектов, поднятые на верхний уровень.
* VaultCreate — лишнее Amount.
TestUTxFormatConformance диффает все 82 формата с вендоренным ref-пиннутым макросом и
называет расхождения поимённо. Пиннинг вместо живого develop намеренный: дрейф вверху уже
ловит protocol-watch, а сетевой тест краснел бы по расписанию Ripple. Парсер падает громко
на неизвестном Soe*-ключевом слове и на коротком разборе, чтобы смена раскладки макроса не
сделала проверку зелёной на пустой таблице.
Попутно: CheckCreate.InvoiceID был uint?, хотя sfInvoiceID это Hash256, у Payment он string,
а ValidateCheckCreate уже требовал строку. Любое непустое значение падало при подписи
("Can't decode InvoiceID from 123") — поле было нерабочим во всех релизах. Тип изменён на
string: формально ломающее изменение сигнатуры, фактически ломать нечего.
Тесты:
* TestUTransactionProtocolFields — полный цикл новых полей: десериализация, round-trip
ToJson/ToDictionary, побайтовый паритет типизированной подписи со словарным путём и, как
защита от регрессии при правке общей базы, blob транзакций без новых полей против подписей,
снятых с 10.9.1.0.
* TestIProtocolFieldSets — исправленные наборы полей против живой ноды: CheckCreate с
Expiration/DestinationTag/InvoiceID, CheckCash через ветку DeliverMin, NFTokenMint с
полями оффера, и AccountSet с WalletLocator/WalletSize/OperationLimit, пережившим полный
цикл через реестр обратно в типизированный AccountSetResponse.
Прогон: юнит 999/0, интеграция 217 пройдено / 39 пропущено (amendment-gated) / 0 падений.
|
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 release adds typed support for protocol-declared transaction fields, aligns transaction formats with a pinned rippled macro fixture, expands conformance and codec tests, and verifies corrected field sets through real-node ledger integration tests. ChangesProtocol field and format conformance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TestIProtocolFieldSets
participant XRPLNode
participant LedgerObjects
TestIProtocolFieldSets->>XRPLNode: Submit CheckCreate, CheckCash, NFTokenMint, or AccountSet
XRPLNode->>LedgerObjects: Accept transaction and update ledger state
TestIProtocolFieldSets->>LedgerObjects: Query typed ledger objects
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Xrpl/Models/Transactions/AccountSet.cs (1)
142-145: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider type-validating the new
WalletLocator/WalletSizefields.
ValidateAccountSetdoesn't type-checkWalletLocator/WalletSizethe wayValidateCheckCreatedoes forInvoiceID(is not string/IsUInt32). Not a regression, but since these are newly-exposed protocol fields, adding the same lightweight type guard would catch malformed input earlier and keep this transaction's validation consistent withCheckCreate.♻️ Proposed validation addition
if (tx.TryGetValue("TickSize", out var TickSize) && TickSize is not null) { if (!Common.TryGetUInt32(TickSize, out uint size)) throw new ValidationException("AccountSet: out of TickSize"); } + + if (tx.TryGetValue("WalletLocator", out var WalletLocator) && WalletLocator is not string { }) + throw new ValidationException("AccountSet: invalid WalletLocator"); + + if (tx.TryGetValue("WalletSize", out var WalletSize) && !Common.IsUInt32(WalletSize)) + throw new ValidationException("AccountSet: invalid WalletSize");Also applies to: 194-205, 228-233
🤖 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/AccountSet.cs` around lines 142 - 145, Update ValidateAccountSet to type-validate WalletLocator as a string and WalletSize as a UInt32, matching the lightweight guards used by ValidateCheckCreate for protocol fields. Apply the checks wherever AccountSet validation handles these properties, while preserving existing validation behavior for valid values.
🤖 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 `@CHANGES.md`:
- Around line 11-15: Reconcile the “six wrong formats” statement in the TxFormat
changelog entry with the seven named formats: CheckCreate, CheckCash,
CheckCancel, NFTokenMint, OracleSet, SignerListSet, and VaultCreate. Update the
count or enumeration so both accurately describe the changes.
- Line 3: Update the 10.10.0.0 release heading in CHANGES.md so its level
follows the document’s preceding heading hierarchy without skipping from h2 to
h3. Preserve the release title and date while using the appropriate incremental
Markdown heading level.
In `@Xrpl/Xrpl.csproj`:
- Line 17: Update the package version across the Xrpl, Xrpl.AddressCodec,
Xrpl.BinaryCodec, and Xrpl.Keypairs project files so they consistently use
10.10.0.0, or confirm and document the approved release policy if the base
packages intentionally remain at 10.9.0.0.
---
Nitpick comments:
In `@Xrpl/Models/Transactions/AccountSet.cs`:
- Around line 142-145: Update ValidateAccountSet to type-validate WalletLocator
as a string and WalletSize as a UInt32, matching the lightweight guards used by
ValidateCheckCreate for protocol fields. Apply the checks wherever AccountSet
validation handles these properties, while preserving existing validation
behavior for valid values.
🪄 Autofix (Beta)
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: cbca45bd-84e0-4501-b999-25a0bb62caef
📒 Files selected for processing (15)
CHANGES.mdTests/Xrpl.Tests/Fixtures/transactions.macroTests/Xrpl.Tests/Fixtures/transactions.macro.refTests/Xrpl.Tests/Integration/transactions/TestIProtocolFieldSets.csTests/Xrpl.Tests/Models/RippledTransactionFormats.csTests/Xrpl.Tests/Models/TestCheckCreate.csTests/Xrpl.Tests/Models/TestUProtocolCompleteness.csTests/Xrpl.Tests/Models/TestUTransactionProtocolFields.csTests/Xrpl.Tests/Models/TestUTxFormatConformance.csTests/Xrpl.Tests/Xrpl.Tests.csprojXrpl/Models/Transactions/AccountSet.csXrpl/Models/Transactions/CheckCreate.csXrpl/Models/Transactions/Common.csXrpl/Models/Transactions/TxFormat.csXrpl/Xrpl.csproj
| <PackageProjectUrl>https://github.com/StaticBit-io/XrplCSharp</PackageProjectUrl> | ||
| <Title>XrplCSharp</Title> | ||
| <PackageVersion>10.9.1.0</PackageVersion> | ||
| <PackageVersion>10.10.0.0</PackageVersion> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Base package versions left at 10.9.0.0 while Xrpl moves to 10.10.0.0.
Per the repo's release guideline, package versions should be updated consistently across Xrpl, Xrpl.AddressCodec, Xrpl.BinaryCodec, and Xrpl.Keypairs. This release only bumps Xrpl. If the base packages truly have no changes this cycle, this may be intentional — please confirm that's the accepted release policy here rather than an oversight.
As per coding guidelines, `**/*.csproj`: When releasing, update the package version consistently in the `Xrpl`, `Xrpl.AddressCodec`, `Xrpl.BinaryCodec`, and `Xrpl.Keypairs` project files.
🤖 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/Xrpl.csproj` at line 17, Update the package version across the Xrpl,
Xrpl.AddressCodec, Xrpl.BinaryCodec, and Xrpl.Keypairs project files so they
consistently use 10.10.0.0, or confirm and document the approved release policy
if the base packages intentionally remain at 10.9.0.0.
Source: Coding guidelines
Delegate/OperationLimit (commonFields rippled) и Sponsor/SponsorFlags (XLS-68) объявлены общими для всех транзакций, но жили только на классах TransactionRequest и TransactionResponse. Всё, что типизировано интерфейсом, добиралось до них лишь через каст: Batch.RawTransaction, SimulateRequest.Transaction/TxJson, HashOrTransaction.Transaction. Sponsor/SponsorFlags подняты тем же движением намеренно: иначе часть common-полей в интерфейсе есть, а часть нет, и правило размещения становится непонятным. Реализации в классах не изменились — только сняты дублирующие XML-докблоки в пользу <inheritdoc />, документация теперь одна на интерфейсе. Сериализация не затронута: она идёт по конкретному рантайм-типу, и golden-blob тест это подтверждает. Source-breaking для внешних реализаторов ITransactionCommon. Внутри репозитория интерфейс реализуют только два базовых класса, которые уже несут эти свойства. Тесты: чтение всех четырёх полей через ITransactionCommon на request и response, плюс Delegate у внутренней транзакции Batch без каста. chore(coderabbit): выключить автоматическое ревью auto_review.enabled: false — ревью запрашивается по требованию комментарием "@coderabbitai review". Остальные настройки (base_branches, incremental) оставлены на месте и снова заработают, если автоматику вернут. Прогон: юнит 1001/0, интеграция 217 пройдено / 39 пропущено / 0 падений.
|
@coderabbitai review |
✅ Action performedReview finished.
|
* «six wrong formats» -> «seven»: перечислено семь (CheckCreate, CheckCash, CheckCancel, NFTokenMint, OracleSet, SignerListSet, VaultCreate). SignerListSet был упомянут внутри пункта про OracleSet и потерялся при подсчёте. Ошибка только в тексте release notes — в TxFormat исправлены все семь, conformance-тест это подтверждает. * Заголовки версий переведены с h3 на h2. markdownlint MD001 ругался на скачок h1 -> h3, но это сквозная конвенция файла, а не свойство новой секции: правка только своего заголовка оставила бы её единственной h2 среди 58 h3 и не убрала бы предупреждение с остальных. Переведены все 59 разом — файл стал и консистентным, и чистым по линтеру. Текст записей не тронут.
…ountSet Новые поля AccountSet не проверялись по типу, хотя все соседние проверяются — несогласованность, внесённая вместе с самими полями. WalletLocator проверяется как 256-битное hex-значение, а не просто как строка: sfWalletLocator объявлен Hash256, и то же правило уже применяет валидатор SignerListSet к WalletLocator внутри SignerEntry. Форма проверки взята из Common.ValidateDomainId, где 256-битные поля валидируются так же. WalletSize проверяется через Common.IsUInt32. На путь подписи это не влияет: валидаторы вызываются явно, из Submit не дёргаются (единственный внутренний вызов Validation.Validate — в BatchUtils). Найдено ревью CodeRabbit (nitpick). Предложенная им проверка WalletLocator была слабее нужной — "is not string" пропустил бы любую строку, включая ту, что не пройдёт кодек.
|
Разобран nitpick по Замечание по сути верное: Однако предложенный вариант для Реализовано: if (tx.TryGetValue("WalletLocator", out var WalletLocator) && WalletLocator is not null)
{
if (WalletLocator is not string walletLocator ||
walletLocator.Length != 64 || !walletLocator.All(Uri.IsHexDigit))
throw new ValidationException("AccountSet: invalid WalletLocator");
}
if (tx.TryGetValue("WalletSize", out var WalletSize) && !Common.IsUInt32(WalletSize))
throw new ValidationException("AccountSet: invalid WalletSize");Форма 256-битной проверки взята из Покрыто На путь подписи это не влияет — валидаторы вызываются явно, из |
…закции Delegate был единственным из новых полей без интеграционного покрытия: оно существовало только на юнит-уровне. TestIDelegateSet проверял выдачу прав (DelegateSet -> ledger-объект), но транзакции, отправленной ОТ ИМЕНИ делегата, не собирал ни один тест. TestDelegatedPayment_DelegateFieldSurvivesTheLedgerRoundTrip закрывает вторую половину: владелец выдаёт право на Payment, делегат подписывает Payment, у которого Account — владелец, а Delegate — он сам, и транзакция читается обратно в типизированную модель с заполненным Delegate (напрямую и через ITransactionCommon). Тест не вырожденный: без sfDelegate rippled отверг бы подпись как чужую (подписывает делегат, Account — владельца), так что зелёный результат означает, что поле реально доехало и было учтено, а не просто отражено в ответе. Гейтится PermissionDelegationV1_1, то есть выполняется на nightly-стенде; на CI уходит в Inconclusive, как и остальные тесты этого класса. Дополнительно TestICheckCreate_OptionalFieldsLandOnTheLedger читает InvoiceID не только из ledger-объекта Check, но и из модели транзакции через client.Tx() — поле было uint? и не могло пережить такой цикл вообще.
Два места в наборе ходили в интернет, из-за чего зелёный CI зависел от доступности
чужих сервисов.
TestIConnectionStates (7 тестов) работал против публичного testnet и devnet. Ничего
специфичного для публичной сети в них нет — все проверки про машину состояний самого
клиента, — поэтому переведены на локальный стенд. Кейс с несуществующим хостом,
проверяющий исчерпание реконнектов, резолвил DNS; заменён на закрытый порт loopback:
отказ мгновенный и резолвер не участвует. ChangeServer между двумя эндпоинтами
сохранён через другое написание того же адреса (localhost / 127.0.0.1), второй
контейнер для этого не нужен.
Вторая половина флейка — фиксированные Task.Delay. Один из этих тестов упал в полном
прогоне и прошёл при перезапуске. Ожидание состояния заменено на опрос с таймаутом:
класс с ~40 секунд сна ушёл на доли секунды (12-370 мс там, где раньше стояло 3 с).
Live-тесты x402 (t54) требуют публичного faucet И стороннего хостед-фасилитатора.
Помечены TestCategory("Live") и исключены из прогона CI фильтром
"TestI&TestCategory!=Live"; шесть герметичных x402 E2E остаются. Запуск вручную:
--filter "TestCategory=Live".
Прогон: юнит 1002/0; интеграция Xrpl.Tests 217 пройдено / 40 пропущено (amendment-gated)
/ 0 падений; x402 герметичные 6/6.
Что и зачем
Две связанные проблемы одного класса: SDK объявляет поле на уровне протокола, но типизированный слой его не выражает — и это ничем не проявляется.
1. Поля протокола, отсутствовавшие в моделях транзакций
TxFormatих перечислял, кодек знал поdefinitions.json, поэтому черезDictionary<string, object>значения доезжали до сети. Но свойства у моделей не было: чтение молча теряло значение, а задать через типизированный API было нельзя вообще.DelegateTransactionRequest/TransactionResponsecommonFieldsrippled — допустимо на любом типе транзакцииOperationLimitcommonFields; инертен на XRPL, но именно его читает Burn-2-Mint на XahauWalletLocatorAccountSet(+Response, +интерфейс)WalletSizeTargetsfTargetвыведен из обращения (AccountID nth 7 помеченunusedвsfields.macro, имени нет вdefinitions.json)Поля объявлены в
ITransactionCommon— там же, где остальные общие поля. Заодно туда поднятыSponsor/SponsorFlags(XLS-68), которые раньше жили только на классах: иначе часть common-полей в интерфейсе есть, а часть нет, и правило размещения становится непонятным.Это важно для всего, что типизировано интерфейсом, — раньше такие места добирались до полей только через каст:
Batch.RawTransaction(аBatchUtilsименно поDelegateвнутренней транзакции определяет обязательных подписантов),SimulateRequest.Transaction/TxJson,HashOrTransaction.Transaction.Реализации в классах не менялись — только сняты дублирующие докблоки в пользу
<inheritdoc />. Сериализация не затронута: она идёт по конкретному рантайм-типу, golden-blob тест это подтверждает.ValidateBaseTransactionтипизированно проверяет два новых common-поля, как и все остальные.2.
TxFormatприведён к rippled и удерживается тамТаблица инертна в рантайме:
TxFormat.Validateне вызывается нигде в продакшн-коде, кодек сериализует поdefinitions.json. Поэтому неверные записи не давали симптомов и жили годами. Диff поtransactions.macroнашёл 6 неверных форматов из 82:CheckCreate/CheckCash/CheckCancel— все три были дословной копией записиPaymentChannelClaim, стоящей выше по файлуNFTokenMint— не хваталоAmount/Destination/Expiration: поля NFTokenMintOffer попали в модель ещё в 10.7.0, а формат за ними не последовалOracleSet(BaseAsset/QuoteAsset/AssetPrice/Scale) иSignerListSet(WalletLocator) — поля вложенных объектов, поднятые на верхний уровеньVaultCreate— лишнееAmountСейчас все 82 формата совпадают с rippled поле в поле.
3.
TestUTxFormatConformance— чтобы это не повторилосьДиффает все 82 формата с вендоренным
transactions.macroи называет расхождения поимённо.Макрос пиннится по ref, а не тянется с
develop, намеренно: дрейф вверху уже ловит protocol-watch (transactions.macroв его списке), а сетевой тест краснел бы по расписанию Ripple вместо нашего. Порядок при срабатывании protocol-watch: заменить вендоренный файл целиком, обновить sha в.ref, тест покажет, какие записиTxFormatобязаны последовать.Парсер падает громко на неизвестном
Soe*-ключевом слове и на коротком разборе — чтобы смена раскладки макроса не сделала проверку зелёной на пустой таблице. Отдельный тест сторожит сторожа.definitions.jsonиserver_definitionsдля этой сверки непригодны: в них есть коды полей, но нет пер-транзакционных форматов. Единственный источник —transactions.macro.Ломающие изменения
1.
ITransactionCommonполучил четыре новых члена —Delegate,OperationLimit,Sponsor,SponsorFlags. Source-breaking для внешних реализаторов интерфейса (моки, обёртки). Внутри репозитория интерфейс реализуют толькоTransactionRequestиTransactionResponse, которые уже несут эти свойства.**2.
CheckCreate.InvoiceID:uint?→string.sfInvoiceID— этоHash256. УPaymentполе уже былоstring, аValidateCheckCreateв том же файле уже требовал строку — модель противоречила собственной валидации. Любое непустое значение падало при подписи:То есть поле было нерабочим во всех релизах, где существует. Сигнатура формально ломается, но рабочего кода, который его выставляет, существовать не может. Найдено при написании интеграционного покрытия для исправленного формата
CheckCreate.Тесты
TestUTransactionProtocolFields— полный цикл новых полей: десериализация, round-tripToJson/ToDictionary, побайтовый паритет типизированной подписи со словарным путём и, как защита от регрессии при правке общей базы, blob'ы транзакций без новых полей против подписей, снятых с 10.9.1.0TestUTxFormatConformance— сверка всех 82 форматов + защита парсераTestUProtocolCompleteness— точные наборы полей исправленных Check-транзакций иSignerListSetTestIProtocolFieldSets(standalone) — исправленные наборы против живой ноды:CheckCreateсExpiration/DestinationTag/InvoiceID,CheckCashчерез ранее непокрытую веткуDeliverMin,NFTokenMintс полями оффера, иAccountSetсWalletLocator/WalletSize/OperationLimit, пережившим полный цикл через реестр обратно в типизированныйAccountSetResponseСам
TxFormatend-to-end не проверяется — он инертен; интеграционные тесты пинят утверждение под ним: нода принимает ровно те наборы полей, которые SDK теперь объявляет.Прогоны (локально)
На CI-стенде 39 тестов уходят в
Inconclusive: четырёх амендментов (Sponsor,BatchV1_1,PermissionDelegationV1_1,ConfidentialTransfer) в релизной сборке 3.2.0 нет вообще — этоTestIBatch(19),TestISponsorship(7),TestISponsorshipSigningMatrix(7),TestIBatchSponsorship(3),TestIDelegateSet(2),TestIConfidentialMPT(1). Штатное поведение, а не сбой.Поэтому набор прогнан ещё и на nightly-стенде (
docker-compose.batchv11.yml, запиннутыйxrpld 3.3.0~b1+202607110018.8306ac77), где все восемь релевантных амендментов активны на генезисе: 0 пропусков, 0 падений.Это важно именно для этого PR:
BatchUtilsопределяет обязательных подписантов батча по полюDelegateвнутренней транзакции, аDelegateвместе сSponsor/SponsorFlagsподнят вITransactionCommon. На CI-стенде вся эта область пропускалась и живьём не проверялась; на nightly она отработала — включая матрицу со-подписания XLS-68 иDelegateSet.Версии
Xrplподнят до10.10.0.0. Базовые пакеты (Xrpl.AddressCodec,Xrpl.BinaryCodec,Xrpl.Keypairs) оставлены на10.9.0.0— их код не менялся, вBase/нет ни одной правки. Это соответствует принципу из 10.8.0 («Xrpl.BinaryCodecbumped — the codec changed») и текущему состоянию репозитория, где версии уже не в локстепе.Summary by CodeRabbit
Delegate,OperationLimit, andAccountSetwallet settings (WalletLocator,WalletSize).CheckCreate.InvoiceIDis now astring(wasuint?).10.10.0.0.Прочее
.coderabbit.yaml—auto_review.enabled: false. Автоматическое ревью выключено, ревью запрашивается по требованию комментарием@coderabbitai review. Остальные настройки (base_branches, инкрементальное ревью) оставлены на месте и снова заработают, если автоматику вернут. Изменение общерепозиторное — действует на все будущие PR, не только на этот.