Release 11.2.0.0 - #164
Merged
Merged
Conversation
* docs(ci): the amendment the comment warns about is no longer an example of one protocol-watch fired on issue #46: features.macro changed on rippled develop. The change is one line - LendingProtocolV1_1 moved from Supported::No to Supported::Yes in "feat: Enable LendingProtocolV1_1 amendment" (#8125), 2026-08-26. That line is cited in this script as the example of a name that must not go into EXTRA_FEATURES, on two grounds: Supported::No on develop, and absent from the release macro. Both have stopped being true. develop marked it supported, and the CI stand moved to 3.3.0, which declares it - the comment still said 3.2.0. So the example now argues the opposite of what it was written to show: by the stated rule, both binaries know the name, which is exactly what makes an entry admissible. Rewritten as history rather than replaced, because no amendment fits that shape today: checked every Supported::No name on develop against 3.3.0 and all of them are declared there. Saying so is more useful than picking a new example that does not hold. No config change, and none is possible: the generator selects Supported::Yes names, and the nightly pin is 3.4.0~b0+202608111815.26cc683e - built 15 days before the flip, so its binary still has the amendment as Supported::No. Regenerating from that ref, which is the only ref allowed to match the pin, produces what is already committed. Verified rather than assumed. Regenerating both stands from their own refs - 26cc683e for nightly, 3.3.0 for CI - reproduces the committed files byte for byte, 47 amendments each. Generating into a throwaway copy from develop yields 48, with LendingProtocolV1_1 in [features] and in [amendments] with its hash: the entry arrives on its own when the pin moves, which nightly-pin-watch will propose within the day - the pin is 20 days old against its 21-day allowance. * ci(protocol-watch): show what changed, not just which files The watch tells the operator that a protocol file moved and links to its history. Naming the file still leaves the reading of the change to a human who must fetch two revisions and compare them by hand - which is the work the watch exists to save. The last firing was one line of features.macro flipping LendingProtocolV1_1 from Supported::No to Supported::Yes, and seeing that took several minutes of manual blob fetching. Both blob shas are already in hand at that point: the baseline in the tracking issue holds the old one, the fresh snapshot the new. So fetch the two blobs by sha and render a unified diff inline, per file, folded into a <details> block so a wide change does not bury the comment. Rendering the diff is best-effort while the notification is not. A blob fetch that fails leaves the file listed with its links and a note rather than aborting a run whose only job is to tell somebody that something changed. Long diffs are cut at 80 lines per file with the true total stated, keeping the comment far below GitHub's 65536-character limit.
* docs: comments in production code are English, all 153 of them CLAUDE.md asks for English comments in two places - the global rules, and the Development Notes line added in #149. Eleven files did not comply. The count is the finding. The task began as one line in Batch.cs, and the sweep it prescribed - grep -rnP over the Cyrillic block - reported zero. That zero was false: this build of grep rejects \x{0400} as "character value too large", the error went to /dev/null, and the count of nothing came back as clean. Swept again by reading the files, and found 153 lines across eleven, 73 of them in XrplWallet.cs alone. Classified before touching anything, because the distinction decides whether a translation is cosmetic or behavioural: 30 XML doc comments, 115 line comments, and 8 trailing comments on lines of code. No string literals - so nothing here can reach a user or an assertion. The diff is 153 insertions against 153 deletions, one line for one line, and every changed line carries a comment: verified by walking the diff and looking for a changed line without one. None. The build succeeds and the unit suite is 1370 green. Meaning preserved rather than paraphrased. The dense ones are protocol knowledge - XLS-56 batch preimages, why SigningPubKey is empty for a multi-signed primary but kept for a sponsored single signature, why Sequence must be 0 with tickets in both the preimage and the blob - and they are worth more in English than they were in Russian, since the audience for a public XRPL SDK is not a Russian-speaking one. * docs: three comments that described something the code does not do Review of the translation found three comments whose English is now a faithful rendering of a claim that was never true. BatchSignStatus.InnerRequired said the set was RawTransaction.Account. GetBatchSignerAccounts builds rippled's Batch::preflight requiredSigners: an inner's Delegate stands in for its Account, a Counterparty joins, and so does a Sponsor that carries a SponsorSignature - while the root is excluded throughout. CombineBatchSigners said only BatchSigners are merged, and the outer signature only when BatchSigners are absent. Canonicalize strips root Signers too, so they belong in the identical-body exemption list; step 5 merges them; and the outer signature is carried over whether or not BatchSigners exist - as the comment inside that very branch already said, contradicting the summary above it. The third is not prose at all: a ':contentReference[oaicite:0]{index=0}' citation artifact left in a trailing comment by whatever produced it. * docs: three more comments at odds with the code around them ExtractWarnings called itself a safe stub to be adapted to a BaseResponse that carries Warnings. ErrorResponse already carries them, and the method reads the list, drops the Clio informational id 2001 and returns the rest. It was scaffolding prose that outlived the scaffolding. NormalizeInnerTransaction promised a new JsonObject with the original left alone. It strips TxnSignature, Signers and LastLedgerSequence from source, overwrites Fee and SigningPubKey, and returns source itself. A caller trusting the summary keeps a reference it believes is untouched. The nested-Signers note said each Signer signs over the same preimage as the single BatchSigner form. It signs over twenty bytes more - its own account ID, after the BatchSigner's - which the comment further down the same method states correctly. * docs: four comments measured against the code and against rippled The secp256k1 branch called the 00 byte a sign byte that BigInteger will not take. It is the XRPL key-type prefix; BouncyCastle takes the sign as its first argument and the magnitude after it unsigned, so the reason to strip the byte is what it is, not what BigInteger wants. BatchSignStatus.InnerSigned said it held those of InnerRequired that had signed. GetBatchSignStatus adds every signed BatchSigner account without filtering, so an account that signed without being required lands there. CombineBatchSigners said the outer signature is kept if it is the same across every blob. Blobs carrying neither TxnSignature nor SigningPubKey take no part at all; the first pair found is kept and a later differing pair is a conflict. The Batch validation comment claimed temBAD_SIGNER for signing by an account outside the batch, above two checks that test for a duplicate and for the outer account. The claim is true of rippled and false of the code beneath it: Batch::preflight rejects a duplicate, the outer account among the signers, an unsorted array, and a signer that is not the next required one. The comment now states rippled's set and says which two of them these checks cover.
…actions (#158) * test(batch): pin that the emitted blob carries normalised inner transactions Batch signing normalises each inner transaction, hashes the results into the batch preimage, and encodes the outer transaction into the blob. The first and third steps are joined by nothing but aliasing: normalisation rewrites the objects that live inside the object being encoded, and the call site reads as though it only collects a list for the txIDs. Nothing pinned that. The existing fixture hands in inner transactions that already carry Fee = "0", SigningPubKey = "" and tfInnerBatchTxn, so normalisation is a no-op on them and every batch test passes whether the normaliser mutates its argument or returns a copy. Making it return a copy leaves all 1215 unit tests green while the blob goes out carrying inner transactions the signature never committed to. This signs a batch whose inner transaction carries none of those three fields, decodes the resulting blob, and asserts they are there - then checks that the decoded transaction hashes to the txID the signature was made over, which is what the three fields are for. Verified by mutation: making the normaliser clone its argument fails this test on the emitted Fee, and reverting makes it pass. The Flags reading goes through the JSON text on purpose. The node came back as JsonValuePrimitive, for which TryGetValue<long> simply returns false - a reader that treated that as zero reported an absent flag on a blob that carried it. * test(batch): pin the other half - the signature covers the normalised txIDs The assertions so far show the blob carries normalised inner transactions and that they hash to a txID computed here. They say nothing about which txIDs went into the preimage the signature was made over. That leaves a regression uncovered: hash the originals into the preimage while still writing the normalised form into the blob, and every existing assertion holds. Verified by mutation - computing the txIDs from a clone taken before normalisation fails only the new assertion and leaves the other four green.
…iting its argument (#159) The method strips TxnSignature, Signers and LastLedgerSequence and overwrites Fee, SigningPubKey and Flags. It did that to the caller's own JsonObject and returned that same instance, so anything a consumer held and handed over came back altered - from a public extension method whose signature reads as though it produces something new. The two overloads also disagreed. NormalizeInnerTransaction(object) rewrote its argument when the runtime type happened to be a JsonObject and did not when it was anything else: the same call, with aliasing decided by a type test the caller cannot see. SignAsBatchPart depended on that mutation, and not visibly. It normalises each inner transaction, hashes the results into the batch preimage, and encodes outer into the blob - and the normalised fields reached the blob only because normalisation rewrote the objects living inside outer. The call site read as though it collected a list for the txIDs and nothing more. It now writes the normalised transaction back explicitly, saying what the old code accomplished by side effect. Two tests hold the contract: the argument survives the call untouched, and the emitted blob still carries normalised inner transactions covered by the signature. Both were checked by mutation. The defensive DeepClone the batch tests used before the call is gone - it is exactly the ceremony this change removes. Xrpl moves to 11.2.0.0: the observable behaviour of a public method has changed.
…ng inside a converter (#160) * fix(batch): name the malformed RawTransactions entry instead of failing inside a converter A RawTransactions element that is not an object was refused - but by System.Text.Json, as "Expected StartObject token" thrown from DictionaryObjectConverter. That is the first thing a caller sees about a malformed batch and it names neither the field nor the position, while every other malformed input on this path answers with a ValidationException that says what is wrong. The refusal happens in GetBatchSignerAccounts, which every batch-signing path reaches through VerifyBatchSubmitter before any of the XLS-56 checks run. It now rejects such an element with the field and index. The IEnumerable branch serializes the element and inspects the result rather than guessing at its type, so a typed RawTransactionWrapper, an IDictionary and a JsonObject all keep working exactly as before. SignAsBatchPart's own loop no longer filters on "n is JsonObject" either. That guard cannot fire - the gate above refuses such an element first, proven by mutation: restoring the filter leaves the new test green, while reverting the gate fails it. It is replaced rather than left because a silent skip is the wrong thing to leave behind if that order ever changes, and because every other malformed input in that loop is refused rather than dropped. The test asserts the exception type and that the message carries the offending index. * fix(batch): judge a RawTransactions entry by what it serializes to, not by its type Review found the first version of this change had made things worse in one place. Testing "n is JsonObject" in the JsonArray branch refused input the old code accepted: JsonArray.Add<T> wraps a value in a JsonValue rather than a JsonObject, and it still writes a JSON object. Worse, the same object was accepted or refused depending on the container it arrived in - the IEnumerable branch judged by the serialized form, the JsonArray branch by the runtime type. Both now go through one helper that serializes first and looks at the result, so every shape that legitimately turns up here is accepted and only a JSON value that is not an object is named. The converter error was also only half gone: RawTransactions[i] no longer produced "Expected StartObject token", but RawTransactions[i].RawTransaction still did, from the next statement of the same method. It is now named the same way, with the field and the index. Batch.Validate reported such an element as null in its message when it was a string. It says what is actually wrong. Three tests, each checked by mutation: a JsonArray of serializable wrappers is accepted (restoring the type test fails it), a non-object RawTransaction is refused by name (restoring the old parse fails it with JsonException), and the existing entry test now compares the whole message rather than searching it for the index - both layers that could refuse the input name the same index, and only the full text says which one did.
…ts on (#162) The method returns the root account and the accounts required to sign a batch. It also replaced RawTransactions[i].RawTransaction in the caller's own dictionary with a converted copy: the IEnumerable branch aliases an element that is already a Dictionary, so the assignment landed in the caller's object. Being the gate every batch-signing path reaches through VerifyBatchSubmitter, it did this on every signature. The conversion stays, for reading; only the store-back is gone, and the reason is written next to it so it does not come back. Nothing needed the stored form: Autofill and Submit read RawTransaction from a JsonNode, SignAsBatchPart reads it from the tree it parsed itself, and BatchNormalizer writes its own on a separate path. Two tests, because they answer different questions. One asserts the caller's wrapper still holds the object it was given - restoring the assignment fails it. The other signs the same batch twice, once with the wrapper holding a JsonObject and once an equivalent Dictionary, and requires the same blob; it passes either way, which is the point. It does not detect this change, it pins the property that makes the change safe instead of merely tidy, and it would fail first if the two representations ever stopped serializing alike. The unit suite does not otherwise notice: all 1220 tests pass with the assignment removed, exactly as they did in #157 for the mutation there.
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.
Promotes
devtorelease, publishingXrpl11.2.0.0 to nuget.org.What ships
Four changes to batch signing, three of which are the same defect found in three places: a method that rewrites the object it was handed.
NormalizeInnerTransactionreturns a copy (#159, #157). It strippedTxnSignature,SignersandLastLedgerSequencefrom the caller's ownJsonObjectand overwroteFee,SigningPubKeyandFlags, then returned that same instance — from a public extension method whose signature reads as though it produces something new. The two overloads also disagreed: theobjectone rewrote its argument only when the runtime type happened to be aJsonObject.SignAsBatchPartdepended on that mutation invisibly — the normalised fields reached the emitted blob only because normalisation rewrote the objects inside the transaction being encoded. It now writes the normalised transaction back explicitly.A malformed
RawTransactionsentry is named (#160). An element that is not a JSON object was refused bySystem.Text.JsonasExpected StartObject tokenfrom a converter, naming neither the field nor the position, while every other malformed input on that path answers with aValidationException. It is now refused by name, atRawTransactions[i]and atRawTransactions[i].RawTransaction, and judged by what it serializes to rather than by its runtime type.GetBatchSignerAccountsstops rewriting the batch it reports on (#162, #161). It replacedRawTransactions[i].RawTransactionin the caller's dictionary with a converted copy, on every signature, from a method whose job is to report.The invariant is pinned (#158). Signing a batch whose inner transaction lacks
Fee,SigningPubKeyandtfInnerBatchTxnmust emit a blob carrying all three, and the signature must cover a preimage built from them. Nothing pinned this before: the batch fixture supplied inner transactions that were already normalised, so making the normaliser return a copy left all 1215 unit tests green while the blob went out carrying transactions the signature had never committed to.Also on this promotion, not part of the package: protocol-watch now renders the actual diff of a changed rippled file rather than only naming it (#155), and the 153 Russian comment lines left in production code are English (#156).
Versions
Only
Xrplmoves, 11.1.0.0 → 11.2.0.0.Xrpl.AddressCodec(10.9.0.0),Xrpl.BinaryCodec(11.0.1.0) andXrpl.Keypairs(10.9.0.0) stay where they are.Xrpl.BinaryCodecandXrpl.Keypairsdo appear in the diff againstrelease, but only in trailing comments — the code is byte-identical, so there is nothing to publish. They are consumed byProjectReference, soXrplbuilt at 11.2.0.0 keeps depending on the published versions.On minor rather than patch, since these are all fixes: the repository's rule is patch for a bugfix with no contract change, minor otherwise, and there are three contract changes here — two methods stop mutating their argument, and a malformed entry now throws
ValidationExceptionwhere it threwSystem.Text.Json.JsonException, which acatchclause distinguishes. Worth noting that strict SemVer would call these breaking and ask for a major; the repository's rule collapses "new feature" and "contract change" into minor, so 11.2.0.0 errs small rather than large. Say the word before merging if you would rather ship this as 12.0.0.Verification
Do not merge without deciding the version question above — pushing to
releasepublishes to nuget.org irreversibly.