fix(batch)!: NormalizeInnerTransaction returns a copy instead of rewriting its argument

[Platonenkov]

Second of the two steps agreed in #157. Stacked on #158 — until that merges, the diff here also shows its test commit.

The change

NormalizeInnerTransaction normalises a copy and returns it, leaving the argument alone. The call site in SignAsBatchPart writes the normalised transaction back:

JsonObject normalizedInner = innerTx.NormalizeInnerTransaction();
item["RawTransaction"] = normalizedInner;
normalizedInners.Add(normalizedInner);

That line is the point of the change. The blob is encoded from outer further down, and the batch preimage commits to the txIDs computed here — the old code got the normalised transactions into the blob purely because normalisation rewrote the objects inside outer, which nothing at the call site said.

Why

The public surface. This is an extension method on JsonObject in a published package. It strips TxnSignature, Signers and LastLedgerSequence and overwrites Fee, SigningPubKey and Flags — and did so to whatever a consumer handed over, from a signature that reads as though it produces something new.

The overloads 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.

The old behaviour was a trap. Adding a defensive clone — the obvious tidy-up — silently emitted blobs the signature had not committed to, and left all 1215 unit tests green. #158 closed that hole; this removes the trap itself.

Verification

Both contracts are pinned, and both pins were checked by mutation:

Test	Mutation applied	Result
the argument survives the call untouched	assign instead of DeepClone	fails on the JSON comparison
the blob carries normalised inners, and the signature covers them	compute the preimage from the originals	fails only the new signature assertion

All seven batch tests pass with the pure version. Full unit suite: 1217 passed, 0 failed.

The defensive DeepClone() the batch tests performed before calling is gone — that ceremony is exactly what this removes. The remaining clone in BuildOuterBatch stays for an unrelated reason: a JsonNode cannot be attached to two parents.

Release

Xrpl moves to 11.2.0.0 with a CHANGES.md entry: the observable behaviour of a public method has changed. The base packages are untouched and stay where they are.

Summary by CodeRabbit

• Bug Fixes

  • Improved batch transaction signing so normalized transactions match their serialized payloads and signature preimages.
  • Prevented transaction normalization from modifying caller-provided transaction data.
  • Improved handling of transaction flags across cultures.

• Tests

  • Added coverage for normalized batch contents, transaction IDs, signatures, and input immutability.

• Chores

  • Updated the package version to 11.2.0.0.
  • Added release notes documenting the signing and normalization improvements.

[Platonenkov]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5cc8eefd-a5da-45e8-9640-e4547fd92286

📥 Commits

Reviewing files that changed from the base of the PR and between b3a7432 and f6bb638.

📒 Files selected for processing (5)

• CHANGES.md
• Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs
• Xrpl/Models/Utils/BatchNormalizer.cs
• Xrpl/Wallet/XrplWallet.cs
• Xrpl/Xrpl.csproj

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

📝 Walkthrough

Walkthrough

The change makes inner-transaction normalization non-mutating. Batch signing stores normalized transactions in the serialized payload before calculating IDs and signatures. Tests validate normalized blobs, signature preimages, and unchanged inputs. The package version is 11.2.0.0.

Changes

Batch signing consistency

Layer / File(s)	Summary
Non-mutating inner transaction normalization Xrpl/Models/Utils/BatchNormalizer.cs, Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs	NormalizeInnerTransaction deep-clones the source before applying normalization fields. Tests verify the source remains unchanged and the clone receives the required values.
Normalized batch signing payload Xrpl/Wallet/XrplWallet.cs, Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs	SignAsBatchPart stores normalized inner transactions before calculating IDs and encoding the signed blob. Tests verify normalized payload fields and signature preimages.
Release metadata Xrpl/Xrpl.csproj, CHANGES.md	The package version changes to 11.2.0.0. The changelog documents the normalization and signing changes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f6bb6

The change makes inner-transaction normalization explicit at the call site while preserving the expected batch signing behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: NormalizeInnerTransaction now returns a copy without mutating its argument.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/normalize-inner-pure-8fd79c

---

_{Comment @coderabbitai help to get the list of available commands.}