diff --git a/CHANGES.md b/CHANGES.md index 9ee5d183..792fd22b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,6 +13,10 @@ * `SignAsBatchPart` no longer filters its inner-transaction loop on `n is JsonObject` either. That guard cannot fire - the gate above refuses such an element first, shown by mutation - but a silent skip is the wrong thing to leave behind, and every other malformed input in that loop is refused rather than dropped * `Batch.Validate` called such an element null in its message when it was, for instance, a string. It now says what is actually wrong +* **`GetBatchSignerAccounts` no longer rewrites the batch it is asked to report on** (#161). The method returns the root account and the accounts required to sign, and 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. It is the gate every batch-signing path reaches through `VerifyBatchSubmitter`, so this happened on every signature. + * the conversion is still made, for reading; only the store-back is gone. No consumer needed it: every reader of `RawTransaction` works from a `JsonNode` built by re-serializing the transaction, not from the dictionary that was passed in + * that the two representations sign identically is now pinned by a test of its own, since it is the property that makes dropping the store-back safe rather than merely tidy + ## 11.1.0.0 08/27/2026 * **The signing path builds its `JsonSerializerOptions` once** (#147). `XrplBinaryCodec.ObjectToJsonNode` constructed a fresh instance on every call, and every signing operation goes through it - `Encode`, `EncodeForSigning`, `EncodeForSigningClaim` and `EncodeForMultiSigning` all route there. Measured end to end on `EncodeForSigning`, 50 000 calls: **1075.8 ms and 14458 B/op before, 621.8 ms and 13601 B/op after** - 1.73x, and 857 fewer bytes each call. The encoded blob is unchanged, hashing identically either way. diff --git a/Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs b/Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs index e0f414a2..a8424462 100644 --- a/Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs +++ b/Tests/Xrpl.Tests/Wallet/TestUBatchSigningV11.cs @@ -298,6 +298,71 @@ public void TestUSignAsBatchPart_NonObjectRawTransactionsEntry_Throws() Assert.AreEqual("RawTransactions[1] must be an object.", error.Message); } + private static Dictionary BatchWithWrapperValue(string outerAccount, object rawTransactionValue) => + new Dictionary + { + ["TransactionType"] = "Batch", + ["Account"] = outerAccount, + ["Sequence"] = 3u, + ["Flags"] = TfAllOrNothing, + ["Fee"] = "40", + ["RawTransactions"] = new List + { + new Dictionary { ["RawTransaction"] = rawTransactionValue } + } + }; + + [TestMethod] + public void TestUSignAsBatchPart_InnerRepresentationDoesNotChangeTheBlob() + { + XrplWallet submitter = XrplWallet.Generate(); + XrplWallet participant = XrplWallet.Generate(); + XrplWallet destination = XrplWallet.Generate(); + + JsonObject inner = InnerPayment(participant.ClassicAddress, destination.ClassicAddress, 20); + + // The same batch twice: one wrapper holds a JsonObject, the other an equivalent + // Dictionary. They describe the same transaction, so they must sign to the same blob. + Dictionary asNode = BatchWithWrapperValue(submitter.ClassicAddress, inner.DeepClone()); + Dictionary asDictionary = BatchWithWrapperValue( + submitter.ClassicAddress, + JsonSerializer.Deserialize>(inner.ToJsonString(), XrplJsonOptions.Default)!); + + SignatureResult fromNode = participant.SignAsBatchPart(asNode, multisign: false, signingFor: participant.ClassicAddress); + SignatureResult fromDictionary = participant.SignAsBatchPart(asDictionary, multisign: false, signingFor: participant.ClassicAddress); + + Assert.AreEqual(fromDictionary.TxBlob, fromNode.TxBlob, + "The representation the caller happened to use must not change what gets signed."); + } + + [TestMethod] + public void TestUGetBatchSignerAccounts_LeavesTheCallersDictionaryAlone() + { + XrplWallet submitter = XrplWallet.Generate(); + XrplWallet participant = XrplWallet.Generate(); + XrplWallet destination = XrplWallet.Generate(); + + JsonObject inner = InnerPayment(participant.ClassicAddress, destination.ClassicAddress, 20); + Dictionary wrapper = new Dictionary { ["RawTransaction"] = inner }; + + Dictionary tx = new Dictionary + { + ["TransactionType"] = "Batch", + ["Account"] = submitter.ClassicAddress, + ["RawTransactions"] = new List { wrapper } + }; + + BatchSignerAccounts accounts = tx.GetBatchSignerAccounts(); + + Assert.AreSame(inner, wrapper["RawTransaction"], + "The method reports the accounts of a batch; it must not rewrite the batch it was given."); + + // And the guarantee is not bought by the method having stopped doing its work. + Assert.AreEqual(submitter.ClassicAddress, accounts.Root); + Assert.AreEqual(1, accounts.Raw.Count); + Assert.AreEqual(participant.ClassicAddress, accounts.Raw[0]); + } + [TestMethod] public void TestUGetBatchSignerAccounts_JsonArrayOfSerializableWrappers_Accepted() { diff --git a/Xrpl/Models/Utils/BatchUtils.cs b/Xrpl/Models/Utils/BatchUtils.cs index e37749e3..ba0c4a3c 100644 --- a/Xrpl/Models/Utils/BatchUtils.cs +++ b/Xrpl/Models/Utils/BatchUtils.cs @@ -112,12 +112,14 @@ public static BatchSignerAccounts GetBatchSignerAccounts(this Dictionary ?? ToObjectDictionary(rawTxObj, $"RawTransactions[{i}].RawTransaction"); - wrapper["RawTransaction"] = rawTx; - if (!rawTx.TryGetValue("Account", out object accObj) || accObj is null) throw new ValidationException("Each RawTransaction must contain Account.");