From 82c000ca18ff5054b7e3fcb67749aaed307c4a23 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonenkov Date: Wed, 26 Aug 2026 12:58:11 -0300 Subject: [PATCH 1/5] fix(models): refuse an amount decimal cannot hold instead of guessing at it Currency.ValueAsNumber answered an out-of-range amount three different ways: a positive one clamped to decimal.MaxValue, a negative one threw FormatException, and a very small one quietly became zero. XRPL issued currency runs from 1e-81 to roughly 1e96 - a 16-digit mantissa with an exponent in [-96, 80], per rippled's STAmount - while decimal stops near 7.9e28. No parsing changes that. The only thing available is how to fail. The clamp is gone. Above the range this now throws AmountOutOfRangeException, carrying the value as the node sent it. Answering 1e96 with 7.9e28 is wrong by 67 orders of magnitude, and it did not stay contained: GetBalanceChanges subtracts two balances, so the clamped value went on to throw OverflowException from the arithmetic instead - one silent lie turning into a second, unrelated exception a caller could not diagnose. The negative case was a plain bug. The fallback's NumberStyles expression came to AllowExponent | AllowDecimalPoint, missing AllowLeadingSign, so no negative value could reach the branch written to handle it. The primary parse was correct throughout, despite six & terms that all evaluate to zero. An amount below 1e-28 still returns zero, and that asymmetry is deliberate: a balance of 1e-81 rounded to zero is zero at any scale a caller can act on, so failing over it would cost more than it protects. Offer.AmountEach reads the same property on both sides of an order and divides them, on values anyone may place in the book. It used to return a plausible-looking exchange rate that was wrong by 67 orders of magnitude without throwing at all. It and GetBalanceChanges now document what they do on untrusted amounts rather than leaving it to be found. Console.WriteLine(exception) is out of the parse path. Six tests, including that a negative amount inside the range still parses - without it the same tests would pass on an implementation that refused every negative value, and negative balances are ordinary. Restoring the clamp fails three of them. Minor rather than patch: code that read an out-of-range amount used to get a number and now gets an exception, which is a contract change even though no signature moved. Representing the full range rather than refusing it is #150. --- CHANGES.md | 11 ++ Tests/Xrpl.Tests/Models/TestCurrency.cs | 133 ++++++++++++++++++ .../Exceptions/AmountOutOfRangeException.cs | 48 +++++++ Xrpl/Models/Common/Currency.cs | 79 ++++++----- Xrpl/Models/Transactions/BookOffers.cs | 9 ++ Xrpl/Utils/GetBalanceChanges.cs | 17 +++ Xrpl/Xrpl.csproj | 2 +- 7 files changed, 260 insertions(+), 39 deletions(-) create mode 100644 Xrpl/Client/Exceptions/AmountOutOfRangeException.cs diff --git a/CHANGES.md b/CHANGES.md index c5cf7a5a..842487d8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,16 @@ # Changes +## Unreleased + +* **An amount the ledger allows but `decimal` cannot hold is refused, not guessed at** (#148). `Currency.ValueAsNumber` answered such values three different ways: a positive one clamped to `decimal.MaxValue`, a negative one threw `FormatException`, and a very small one quietly became zero. XRPL issued currency runs from `1e-81` to roughly `1e96` - a 16-digit mantissa with an exponent in `[-96, 80]`, per rippled's `STAmount` - while `decimal` stops near `7.9e28`, so this cannot be parsed away; the only choice is how to fail. + * the clamp is gone. An amount above the range now throws `AmountOutOfRangeException`, which carries the value as the node sent it. Returning `7.9e28` for `1e96` is wrong by 67 orders of magnitude, and it did not stay contained: `GetBalanceChanges` subtracts two balances, so the clamped value went on to throw `OverflowException` from arithmetic instead + * the negative case was a parse bug. The fallback's `NumberStyles` expression came to `AllowExponent | AllowDecimalPoint` - `AllowLeadingSign` was missing, so no negative value could reach the branch meant to handle it. The primary parse was correct all along, despite six `&` terms that all evaluate to zero + * **an amount below `1e-28` still returns zero, and the asymmetry is deliberate.** A balance of `1e-81` rounded to zero is zero at any scale a caller can act on; failing over it would cost more than it protects. An amount of `1e96` reported as `7.9e28` is not in that category + * the threshold is nowhere near the protocol's ceiling: `1e29` is barely above `decimal.MaxValue` and was already unreachable. A token with a large supply meets this without going anywhere near the ledger's limits + * `Offer.AmountEach` reads the same property on both sides of an order and divides them. Anyone may place an offer in their own token at any value the protocol allows, so it fails the same way - and used to return a plausible-looking exchange rate that was wrong by 67 orders of magnitude, without throwing. Both it and `GetBalanceChanges` now say so in their own documentation rather than leaving it to be discovered + * `Console.WriteLine(exception)` is out of the parse path. A library does not write to the console + * **breaking in effect, if not in signature**: code that read an out-of-range amount used to get a number and now gets an exception. Representing the full range instead of refusing it is #150 + ## 11.0.0.0 08/26/2026 ### Migration at a glance diff --git a/Tests/Xrpl.Tests/Models/TestCurrency.cs b/Tests/Xrpl.Tests/Models/TestCurrency.cs index c0aaabe0..822eede8 100644 --- a/Tests/Xrpl.Tests/Models/TestCurrency.cs +++ b/Tests/Xrpl.Tests/Models/TestCurrency.cs @@ -2,13 +2,146 @@ using System.Globalization; +using System; + +using Xrpl.Client.Exceptions; using Xrpl.Models.Common; +using Xrpl.Models.Transactions; namespace XrplTests.Xrpl.Models { [TestClass] public class TestUCurrency { + #region Amounts outside decimal's range - issue #148 + + private static Currency Iou(string value) => + new Currency { CurrencyCode = "USD", Issuer = "rIssuer", Value = value }; + + /// + /// An amount too large for decimal is refused, whichever sign it carries. + /// + /// + /// + /// These used to do two different things. A positive one clamped to decimal.MaxValue + /// and said nothing, so a balance of 1e96 was answered with 7.9e28 - wrong by 67 orders of + /// magnitude, and wrong in a way that flowed onward into the caller's arithmetic. A + /// negative one threw FormatException, because the fallback parse was missing + /// AllowLeadingSign and could not read the minus. + /// + /// + /// The threshold is not near the ledger's ceiling: 1e29 is barely above + /// decimal.MaxValue and already unreachable. + /// + /// + [TestMethod] + public void ValueAsNumber_OutsideDecimalRange_Throws() + { + foreach (string value in new[] + { + "1e29", "-1e29", + "9e80", "-9e80", + "9999999999999999e80", "-9999999999999999e80", + }) + { + Assert.ThrowsExactly( + () => _ = Iou(value).ValueAsNumber, + $"'{value}' is a legitimate ledger amount that decimal cannot hold."); + } + } + + /// + /// The exception carries the amount as the node sent it. + /// + /// + /// The point of refusing rather than clamping is that the real figure is still available; + /// an exception that only said "too big" would trade one lost value for another. + /// + [TestMethod] + public void ValueAsNumber_OutOfRangeException_CarriesTheOriginalValue() + { + AmountOutOfRangeException error = Assert.ThrowsExactly( + () => _ = Iou("-9999999999999999e80").ValueAsNumber); + + Assert.AreEqual("-9999999999999999e80", error.Value); + Assert.Contains("-9999999999999999e80", error.Message, StringComparison.Ordinal); + } + + /// + /// Negative amounts inside the range still parse, which is what makes the bound the bug. + /// + /// + /// Without this the test above would pass on an implementation that simply refused every + /// negative amount - and negative balances are ordinary: a RippleState balance is + /// negative from the low account's side. + /// + [TestMethod] + public void ValueAsNumber_NegativeInsideRange_StillParses() + { + Assert.AreEqual(-100m, Iou("-100").ValueAsNumber); + Assert.AreEqual(-0.00000000015m, Iou("-1.5e-10").ValueAsNumber); + Assert.AreEqual(-79228162514264337593543950335m, Iou("-79228162514264337593543950335").ValueAsNumber); + } + + /// + /// An amount too small for decimal becomes zero rather than throwing. + /// + /// + /// The asymmetry with overflow is deliberate. The ledger goes down to 1e-81 and decimal + /// stops near 1e-28, but a balance that small is zero at any scale a caller can act on, so + /// failing over it would cost more than it protects. Overflow is the opposite: the number + /// that would be returned is wrong by orders of magnitude and unsafe to use. + /// + [TestMethod] + public void ValueAsNumber_BelowDecimalPrecision_IsZeroNotAnError() + { + Assert.AreEqual(0m, Iou("1e-96").ValueAsNumber); + Assert.AreEqual(0m, Iou("-9999999999999999e-96").ValueAsNumber); + } + + /// + /// Something that is not a number is still a format error, not an out-of-range one. + /// + /// + /// The two are told apart by whether double can read the string: it spans the whole + /// ledger range, so it succeeds exactly when the value is real and decimal merely cannot + /// hold it. Reporting both the same way would hide a malformed response behind a message + /// about magnitude. + /// + [TestMethod] + public void ValueAsNumber_NotANumber_IsAFormatError() + { + Assert.ThrowsExactly(() => _ = Iou("abc").ValueAsNumber); + Assert.ThrowsExactly(() => _ = Iou("1.2.3").ValueAsNumber); + Assert.ThrowsExactly(() => _ = Iou(" 100 ").ValueAsNumber); + } + + /// + /// The two properties that compute rather than read fail the same single way. + /// + /// + /// GetBalanceChanges subtracts two balances and Offer.AmountEach divides two + /// amounts, and both used to have a second failure behind the first: a clamped + /// decimal.MaxValue would go on to throw OverflowException from the + /// arithmetic, or - worse for the order book - return a plausible exchange rate that was + /// wrong by 67 orders of magnitude without throwing at all. + /// + [TestMethod] + public void ValueAsNumber_ArithmeticOnOutOfRangeAmounts_FailsAtTheSource() + { + Assert.ThrowsExactly( + () => _ = Iou("9e80").ValueAsNumber - Iou("-100").ValueAsNumber, + "This used to be an OverflowException from subtracting a clamped MaxValue."); + + Offer offer = new Offer { TakerGets = Iou("9e80"), TakerPays = Iou("1") }; + + Assert.ThrowsExactly( + () => _ = offer.AmountEach, + "This used to return decimal.MaxValue as an exchange rate, silently."); + } + + #endregion + #region Round-trip ValueAsNumber (G16 fix verification) [TestMethod] diff --git a/Xrpl/Client/Exceptions/AmountOutOfRangeException.cs b/Xrpl/Client/Exceptions/AmountOutOfRangeException.cs new file mode 100644 index 00000000..824c8620 --- /dev/null +++ b/Xrpl/Client/Exceptions/AmountOutOfRangeException.cs @@ -0,0 +1,48 @@ +using System; +using System.Globalization; + +namespace Xrpl.Client.Exceptions +{ + /// + /// An issued-currency amount the node sent is outside the range can hold. + /// + /// + /// + /// XRPL issued currency runs from 1e-81 to roughly 1e96 - rippled's + /// STAmount allows a 16-digit mantissa with an exponent in [-96, 80] - while + /// stops at about 7.9e28. The two do not fit inside one another, + /// and no parsing of the string can change that. + /// + /// + /// This used to be answered by clamping to , which is a number + /// that is wrong by up to 67 orders of magnitude and does not say so - and by a bare + /// for negative amounts, which named the string rather than the + /// problem. Both are gone; the amount that does not fit is reported as such. + /// + /// + /// carries what the node actually sent, so a caller who needs the real + /// figure still has it. Representing it rather than reporting it is issue #150. + /// + /// + public class AmountOutOfRangeException : RippleException + { + /// + /// The amount as the node sent it, in the ledger's own string form. + /// + public string Value { get; } + + /// + public AmountOutOfRangeException(string value) + : base(BuildMessage(value)) + { + Value = value; + } + + private static string BuildMessage(string value) => string.Format( + CultureInfo.InvariantCulture, + "The amount '{0}' is outside the range System.Decimal can represent (about ±7.9e28). " + + "XRPL issued currency reaches roughly 1e96, so this is a legitimate ledger value that " + + "this property cannot return. Read Currency.Value for the amount as sent.", + value); + } +} diff --git a/Xrpl/Models/Common/Currency.cs b/Xrpl/Models/Common/Currency.cs index 2ddc5d1f..0d22642e 100644 --- a/Xrpl/Models/Common/Currency.cs +++ b/Xrpl/Models/Common/Currency.cs @@ -2,6 +2,7 @@ using System; using System.Globalization; +using Xrpl.Client.Exceptions; using System.Linq; using System.Text; using System.Text.RegularExpressions; @@ -78,57 +79,59 @@ public string MPTokenIssuanceID [JsonIgnore] public string CurrencyValidName => CurrencyCode.CurrencyReadableName(); + /// + /// What the ledger's amount string allows: a sign, a decimal point and an exponent, and + /// nothing else. Surrounding whitespace is not accepted, because the node never sends it. + /// + private const NumberStyles AmountStyles = + NumberStyles.AllowLeadingSign | NumberStyles.AllowDecimalPoint | NumberStyles.AllowExponent; + /// /// decimal currency amount (drops for XRP) /// + /// + /// + /// does not cover the range XRPL allows an issued currency: the ledger + /// reaches roughly 1e96 and down to 1e-81, this type stops at about + /// 7.9e28. Amounts above that throw rather than + /// being answered with a number that is not the one the node sent. + /// + /// + /// Amounts below 1e-28 return zero instead of throwing, and the asymmetry is deliberate. + /// A balance of 1e-81 rounded to zero is zero at any scale a caller can act on, so + /// failing over it would cost more than it protects; an amount of 1e96 reported as + /// 7.9e28 is wrong by 67 orders of magnitude and is worth stopping for. + /// + /// + /// The amount exceeds what can hold. + /// The amount is not a number at all. [JsonIgnore] public decimal ValueAsNumber { get { - try + if (string.IsNullOrWhiteSpace(Value)) { - return string.IsNullOrWhiteSpace(Value) - ? 0 - : decimal.Parse( - Value, - NumberStyles.AllowLeadingSign - | (NumberStyles.AllowLeadingSign & NumberStyles.AllowDecimalPoint) - | (NumberStyles.AllowLeadingSign & NumberStyles.AllowExponent) - | (NumberStyles.AllowLeadingSign & NumberStyles.AllowExponent & NumberStyles.AllowDecimalPoint) - | (NumberStyles.AllowExponent & NumberStyles.AllowDecimalPoint) - | NumberStyles.AllowExponent - | NumberStyles.AllowDecimalPoint, - CultureInfo.InvariantCulture); + return 0; } - catch (Exception e) + + if (decimal.TryParse(Value, AmountStyles, CultureInfo.InvariantCulture, out decimal amount)) { - try - { - var num = double.Parse( - Value, - (NumberStyles.Float & NumberStyles.AllowExponent) | NumberStyles.AllowExponent | NumberStyles.AllowDecimalPoint, - CultureInfo.InvariantCulture); - var valid = $"{num:#########e00}"; - if (valid.Contains(value: "e-")) - { - return 0; - } - - if (valid.Contains(value: '-')) - { - return decimal.MinValue; - } - - return decimal.MaxValue; - } - catch (Exception exception) - { - Console.WriteLine(exception); - throw; - } + return amount; } + + // Tell the two failures apart rather than reporting both as a bad format. double + // spans the whole ledger range, so parsing there succeeds exactly when the string is + // a real number that decimal simply cannot hold. + if (double.TryParse(Value, AmountStyles, CultureInfo.InvariantCulture, out _)) + { + throw new AmountOutOfRangeException(Value); + } + + throw new FormatException( + $"The amount '{Value}' is not a number in the form the XRP Ledger uses."); } + set => Value = value.ToString( CurrencyCode == "XRP" ? "G0" diff --git a/Xrpl/Models/Transactions/BookOffers.cs b/Xrpl/Models/Transactions/BookOffers.cs index f05645da..d14088ee 100644 --- a/Xrpl/Models/Transactions/BookOffers.cs +++ b/Xrpl/Models/Transactions/BookOffers.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using Xrpl.Client.Json.Converters; +using Xrpl.Client.Exceptions; using Xrpl.Models.Common; using Xrpl.Models.Enums; using Xrpl.Models.Methods; @@ -98,6 +99,14 @@ public Offer() /// /// The exchange rate, as the ratio taker_gets divided by taker_pays. /// + /// + /// Anyone may place an offer in their own token at any value the protocol allows, so both + /// sides of this ratio are untrusted input. An amount beyond what + /// holds throws rather than yielding a rate that + /// looks usable and is not - see issue #148. Guard this when walking an order book that is + /// not your own. + /// + /// Either side exceeds what can hold. public decimal AmountEach { get diff --git a/Xrpl/Utils/GetBalanceChanges.cs b/Xrpl/Utils/GetBalanceChanges.cs index bd91c94b..ebac24ea 100644 --- a/Xrpl/Utils/GetBalanceChanges.cs +++ b/Xrpl/Utils/GetBalanceChanges.cs @@ -2,6 +2,7 @@ using System.Linq; using Xrpl.Models; +using Xrpl.Client.Exceptions; using Xrpl.Models.Common; using Xrpl.Models.Ledger; using Xrpl.Models.Transactions; @@ -153,8 +154,24 @@ private static Dictionary> GroupByAccount(IEnumerable /// Computes balance changes per account from transaction metadata. /// + /// + /// + /// This walks every affected node and computes a delta for every account on the payment path, + /// not only the one the caller has in mind. The amounts it reads are therefore untrusted: it + /// is enough for a payment to route through an offer in somebody's own token for a value + /// beyond to reach this code, and then + /// comes out - see issue #148. + /// + /// + /// That matters most to anything that re-reads history. A monitor catching up over a ledger + /// range, an indexer or a reconciler meets the same transaction on every pass, so an unguarded + /// call does not fail once, it stops there permanently. Catch it and decide what an + /// unrepresentable balance means for you; issue #150 tracks representing it instead. + /// + /// /// Transaction metadata including affected nodes. /// Dictionary mapping account addresses to balance changes (XRP string or IssuedCurrencyAmount). + /// An amount in the metadata exceeds what can hold. public static Dictionary> GetBalanceChanges(ITransactionMetadata metadata) { var list = new List(); diff --git a/Xrpl/Xrpl.csproj b/Xrpl/Xrpl.csproj index cf08d66b..6ac2e1d8 100644 --- a/Xrpl/Xrpl.csproj +++ b/Xrpl/Xrpl.csproj @@ -14,7 +14,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 11.0.0.0 + 11.1.0.0 From 532dce40a67d89da4007216c69b655432c0016c2 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonenkov Date: Wed, 26 Aug 2026 13:51:22 -0300 Subject: [PATCH 2/5] fix(models): a non-finite string is not an amount that is too large Review findings, both accepted. double.TryParse accepts "NaN", "Infinity" and "-Infinity" whatever NumberStyles it is handed - those symbols are matched separately from the numeric ones. Since the test that separates "will not fit" from "is not a number" runs through double, all three came back as AmountOutOfRangeException: a confident statement about magnitude for a string that has none. Checked against the runtime rather than taken on the reviewer's word, then guarded with double.IsFinite. Offer.AmountEach read its two sides lazily, so the early return for a zero TakerPays skipped TakerGets entirely - an unrepresentable numerator went unnoticed whenever the denominator happened to be zero. The exception this branch documents was therefore not one a caller could rely on: whether it appeared depended on the value of an unrelated field. Both sides are read first. It also parsed TakerPays twice, and ValueAsNumber parses on every read. The second is the one worth noting. The documentation added in the previous commit claimed something the code did not do, in a change whose whole subject is not saying false things about values. Four tests. Removing IsFinite fails one, restoring the lazy read fails another. --- Tests/Xrpl.Tests/Models/TestCurrency.cs | 50 +++++++++++++++++++++++++ Xrpl/Models/Common/Currency.cs | 9 ++++- Xrpl/Models/Transactions/BookOffers.cs | 15 +++++--- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/Tests/Xrpl.Tests/Models/TestCurrency.cs b/Tests/Xrpl.Tests/Models/TestCurrency.cs index 822eede8..6f1cdf83 100644 --- a/Tests/Xrpl.Tests/Models/TestCurrency.cs +++ b/Tests/Xrpl.Tests/Models/TestCurrency.cs @@ -116,6 +116,56 @@ public void ValueAsNumber_NotANumber_IsAFormatError() Assert.ThrowsExactly(() => _ = Iou(" 100 ").ValueAsNumber); } + /// + /// A non-finite string is not a quantity too large, and must not be reported as one. + /// + /// + /// double.TryParse accepts NaN, Infinity and -Infinity whatever + /// NumberStyles it is handed, because those symbols are matched separately from the + /// numeric ones. Since the check that separates "will not fit" from "is not a number" runs + /// through double, without a finiteness test these would come back as + /// - a confident answer about magnitude for a + /// string that has none. + /// + [TestMethod] + public void ValueAsNumber_NonFiniteStrings_AreFormatErrorsNotRangeErrors() + { + foreach (string value in new[] { "NaN", "Infinity", "-Infinity" }) + { + Assert.ThrowsExactly( + () => _ = Iou(value).ValueAsNumber, + $"'{value}' is not a quantity at all, let alone one that is too large."); + } + } + + /// + /// An out-of-range numerator is refused even when the denominator is zero. + /// + /// + /// Offer.AmountEach returns zero when TakerPays is zero. While it read the + /// two sides lazily, that early return meant an unrepresentable TakerGets slipped + /// through unnoticed - so whether the documented exception appeared depended on the value + /// of an unrelated field, which is not a contract anyone can hold you to. + /// + [TestMethod] + public void AmountEach_OutOfRangeNumerator_ThrowsEvenWithAZeroDenominator() + { + Offer offer = new Offer { TakerGets = Iou("9e80"), TakerPays = Iou("0") }; + + Assert.ThrowsExactly(() => _ = offer.AmountEach); + } + + /// + /// A zero denominator on its own still yields zero rather than dividing. + /// + [TestMethod] + public void AmountEach_ZeroDenominator_IsZero() + { + Offer offer = new Offer { TakerGets = Iou("100"), TakerPays = Iou("0") }; + + Assert.AreEqual(0m, offer.AmountEach); + } + /// /// The two properties that compute rather than read fail the same single way. /// diff --git a/Xrpl/Models/Common/Currency.cs b/Xrpl/Models/Common/Currency.cs index 0d22642e..59a48f4c 100644 --- a/Xrpl/Models/Common/Currency.cs +++ b/Xrpl/Models/Common/Currency.cs @@ -123,7 +123,14 @@ public decimal ValueAsNumber // Tell the two failures apart rather than reporting both as a bad format. double // spans the whole ledger range, so parsing there succeeds exactly when the string is // a real number that decimal simply cannot hold. - if (double.TryParse(Value, AmountStyles, CultureInfo.InvariantCulture, out _)) + // + // IsFinite matters: double.TryParse accepts "NaN", "Infinity" and "-Infinity" whatever + // NumberStyles it is given, because those symbols are matched separately from the + // numeric ones. Without the check, a string that is not a quantity at all would be + // reported as a quantity too large - which is the sort of confident wrong answer this + // property is being changed to stop giving. + if (double.TryParse(Value, AmountStyles, CultureInfo.InvariantCulture, out double asDouble) + && double.IsFinite(asDouble)) { throw new AmountOutOfRangeException(Value); } diff --git a/Xrpl/Models/Transactions/BookOffers.cs b/Xrpl/Models/Transactions/BookOffers.cs index d14088ee..4add340e 100644 --- a/Xrpl/Models/Transactions/BookOffers.cs +++ b/Xrpl/Models/Transactions/BookOffers.cs @@ -111,12 +111,15 @@ public decimal AmountEach { get { - if ((TakerPays.ValueAsXrp ?? TakerPays.ValueAsNumber) != 0) - { - return (TakerGets.ValueAsXrp ?? TakerGets.ValueAsNumber) / - (TakerPays.ValueAsXrp ?? TakerPays.ValueAsNumber); - } - return 0; + // Both sides are read before the denominator is tested. Reading them lazily made + // whether this threw depend on an unrelated field: an out-of-range TakerGets went + // unnoticed whenever TakerPays happened to be zero, so the exception documented + // above was not one a caller could rely on. It also parsed TakerPays twice, and + // ValueAsNumber parses the string on every read. + decimal takerPays = TakerPays.ValueAsXrp ?? TakerPays.ValueAsNumber; + decimal takerGets = TakerGets.ValueAsXrp ?? TakerGets.ValueAsNumber; + + return takerPays != 0 ? takerGets / takerPays : 0; } } /// From 0fd55801a9a606ba267072365b38f1414c1e7970 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonenkov Date: Wed, 26 Aug 2026 16:48:32 -0300 Subject: [PATCH 3/5] fix(models): ToString shows an out-of-range amount instead of failing on it Review findings. ToString interpolates ValueAsNumber, so making the getter throw made ToString throw with it - for positives, which used to print a clamped number, as well as for negatives, which already threw. By convention ToString does not throw, and the places it is reached from are logging, string interpolation and a debugger's watch window: exactly where someone would be while working out why an amount is unusual. Failing there hides the value at the moment it is most wanted. It now falls back to the raw string, which is what the node sent. Two tests were missing behind claims already made. GetBalanceChanges documents that it throws on an out-of-range amount, and nothing exercised that through GetBalanceChanges - only a hand-written subtraction imitating what it does. Imitating the arithmetic proves the arithmetic; it does not prove the method reaches it, which is what the documentation promises. Now driven through the method, on a negative balance in a RippleState node - the ordinary shape from the low account's side, and the case that used to fail as FormatException. And one edge is documented rather than fixed: writing decimal.MaxValue through the setter formats with G16, which rounds the mantissa up past what decimal holds, so the SDK can write a string the ledger would accept and then refuse to read it. The window is the last ~7e12 below decimal.MaxValue, reachable only by assigning a number no token amount would be, and changing how the setter rounds would touch every round trip in the type to rescue a value nobody writes. The test states the decision so the next person meets one rather than a surprise. Restoring the clamp now fails seven tests. --- CHANGES.md | 2 + Tests/Xrpl.Tests/Models/TestCurrency.cs | 52 ++++++++++++++ .../Utils/GetBalanceChangesTests.cs | 68 +++++++++++++++++++ Xrpl/Models/Common/Currency.cs | 21 +++++- 4 files changed, 142 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 842487d8..0fe2ea8f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,8 @@ * **an amount below `1e-28` still returns zero, and the asymmetry is deliberate.** A balance of `1e-81` rounded to zero is zero at any scale a caller can act on; failing over it would cost more than it protects. An amount of `1e96` reported as `7.9e28` is not in that category * the threshold is nowhere near the protocol's ceiling: `1e29` is barely above `decimal.MaxValue` and was already unreachable. A token with a large supply meets this without going anywhere near the ledger's limits * `Offer.AmountEach` reads the same property on both sides of an order and divides them. Anyone may place an offer in their own token at any value the protocol allows, so it fails the same way - and used to return a plausible-looking exchange rate that was wrong by 67 orders of magnitude, without throwing. Both it and `GetBalanceChanges` now say so in their own documentation rather than leaving it to be discovered + * `Currency.ToString()` falls back to the raw value rather than letting the getter throw through it. By convention `ToString` does not throw, and logging, string interpolation and a debugger's watch window are exactly where someone would be while working out why an amount is unusual - failing there hides the value at the moment it is most wanted + * one edge is documented rather than fixed, in a test that pins it: writing `decimal.MaxValue` through the setter formats with `G16`, which rounds the mantissa **up** past what `decimal` holds, so the SDK can write a string the ledger would accept and then refuse to read it. The window is the last ~7e12 below `decimal.MaxValue`, reachable only by assigning a number no token amount would be * `Console.WriteLine(exception)` is out of the parse path. A library does not write to the console * **breaking in effect, if not in signature**: code that read an out-of-range amount used to get a number and now gets an exception. Representing the full range instead of refusing it is #150 diff --git a/Tests/Xrpl.Tests/Models/TestCurrency.cs b/Tests/Xrpl.Tests/Models/TestCurrency.cs index 6f1cdf83..a04a6fa3 100644 --- a/Tests/Xrpl.Tests/Models/TestCurrency.cs +++ b/Tests/Xrpl.Tests/Models/TestCurrency.cs @@ -190,6 +190,58 @@ public void ValueAsNumber_ArithmeticOnOutOfRangeAmounts_FailsAtTheSource() "This used to return decimal.MaxValue as an exchange rate, silently."); } + /// + /// ToString shows the amount instead of failing on it. + /// + /// + /// By convention does not throw, and the places it is reached + /// from - logging, string interpolation, a debugger's watch window - are exactly where + /// someone would be while working out why an amount is unusual. Letting the getter throw + /// through it would hide the value at the moment it is most wanted. + /// + [TestMethod] + public void ToString_OutOfRangeAmount_ShowsTheRawValue() + { + Assert.AreEqual("USD: 9e80", Iou("9e80").ToString()); + Assert.AreEqual("USD: -9e80", Iou("-9e80").ToString()); + Assert.AreEqual("USD: NaN", Iou("NaN").ToString()); + + // Anything it can render, it still renders the same way. + Assert.AreEqual("USD: 100", Iou("100").ToString()); + } + + /// + /// Writing produces an amount that cannot be read back. + /// + /// + /// + /// Documented rather than fixed. The setter formats with G16, which rounds the + /// mantissa to nearest - and near the ceiling that rounds up, past what + /// holds. So the SDK can write a string the ledger would accept + /// (16-digit mantissa, exponent 13) and then refuse to read it. + /// + /// + /// The window is the last ~7e12 below , reachable only by + /// assigning a number no token amount would be. Changing how the setter rounds would touch + /// every round trip in the type to rescue a value nobody writes. Pinned here so the next + /// person meets a decision rather than a surprise. + /// + /// + [TestMethod] + public void ValueAsNumber_WritingDecimalMaxValue_RoundsUpBeyondWhatCanBeRead() + { + Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer" }; + currency.ValueAsNumber = decimal.MaxValue; + + Assert.AreEqual("7.922816251426434E+28", currency.Value); + Assert.ThrowsExactly(() => _ = currency.ValueAsNumber); + + // Just below the rounding boundary the round trip is intact, which is what makes the + // line above an edge rather than a broken setter. + currency.ValueAsNumber = 79228162514264330000000000000m; + Assert.AreEqual(79228162514264330000000000000m, currency.ValueAsNumber); + } + #endregion #region Round-trip ValueAsNumber (G16 fix verification) diff --git a/Tests/Xrpl.Tests/Utils/GetBalanceChangesTests.cs b/Tests/Xrpl.Tests/Utils/GetBalanceChangesTests.cs index 0ca19e49..d6aebacf 100644 --- a/Tests/Xrpl.Tests/Utils/GetBalanceChangesTests.cs +++ b/Tests/Xrpl.Tests/Utils/GetBalanceChangesTests.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Text.Json; +using Xrpl.Client.Exceptions; using Xrpl.Client.Json; using Xrpl.Models.Transactions; @@ -66,6 +67,73 @@ public void TestBalanceChanges_Metadata1() Assert.AreEqual("-639.11146416", issuerTokenBuyerChanges.Value); Assert.AreEqual(buyer, issuerTokenBuyerChanges.Issuer); } + /// + /// An amount the ledger allows but decimal cannot hold stops this, and says so. + /// + /// + /// + /// The exception is documented on GetBalanceChanges itself, so it is exercised through + /// GetBalanceChanges rather than by imitating the subtraction it performs. A test that + /// mimics the arithmetic proves the arithmetic; it does not prove that this method reaches it, + /// which is what the documentation promises a caller. + /// + /// + /// The balance below is negative, which is the ordinary shape for a RippleState node + /// from the low account's side, and it is out of range - so it exercises the case that used to + /// fail with FormatException before the parse fix, and now names the real problem. + /// + /// + [TestMethod] + public void TestUGetBalanceChanges_AmountBeyondDecimal_ThrowsRatherThanReportingSomethingElse() + { + Meta metadata = JsonSerializer.Deserialize(metaDataOutOfRange, XrplJsonOptions.Default); + + AmountOutOfRangeException error = Assert.ThrowsExactly( + () => BalanceChanges.GetBalanceChanges(metadata)); + + Assert.AreEqual("-9999999999999999e80", error.Value); + } + + private const string metaDataOutOfRange = @"{ + ""AffectedNodes"": [ + { + ""ModifiedNode"": { + ""FinalFields"": { + ""Balance"": { + ""currency"": ""USD"", + ""issuer"": ""rrrrrrrrrrrrrrrrrrrrBZbvji"", + ""value"": ""-9999999999999999e80"" + }, + ""Flags"": 1114112, + ""HighLimit"": { + ""currency"": ""USD"", + ""issuer"": ""rXPMxBeefHGxx2K7g5qmmWq3gFsgawkoa"", + ""value"": ""0"" + }, + ""HighNode"": ""0"", + ""LowLimit"": { + ""currency"": ""USD"", + ""issuer"": ""rLiooJRSKeiNfRJcDBUhu4rcjQjGLWqa4p"", + ""value"": ""1000000000"" + }, + ""LowNode"": ""0"" + }, + ""LedgerEntryType"": ""RippleState"", + ""LedgerIndex"": ""1BC0B4F0B0C0B0F0B0C0B0F0B0C0B0F0B0C0B0F0B0C0B0F0B0C0B0F0B0C0B0F0"", + ""PreviousFields"": { + ""Balance"": { + ""currency"": ""USD"", + ""issuer"": ""rrrrrrrrrrrrrrrrrrrrBZbvji"", + ""value"": ""-100"" + } + } + } + } + ], + ""TransactionIndex"": 0, + ""TransactionResult"": ""tesSUCCESS"" + }"; + private const string metaData_1 = @"{ ""AffectedNodes"": [ { diff --git a/Xrpl/Models/Common/Currency.cs b/Xrpl/Models/Common/Currency.cs index 59a48f4c..fba89646 100644 --- a/Xrpl/Models/Common/Currency.cs +++ b/Xrpl/Models/Common/Currency.cs @@ -178,9 +178,28 @@ public decimal? ValueAsXrp #region Overrides of Object + /// + /// A readable form of the amount. + /// + /// + /// Falls back to the raw for an amount outside what + /// can hold, rather than letting throw through it. By convention + /// does not throw, and the places it is called from - logging, + /// string interpolation, a debugger's watch window - are exactly where someone would be while + /// working out why an amount is unusual. Failing there hides the value instead of showing it. + /// public override string ToString() { - return CurrencyValidName == "XRP" ? $"XRP: {ValueAsXrp:0.######}" : $"{CurrencyValidName}: {ValueAsNumber:0.###############}"; + try + { + return CurrencyValidName == "XRP" + ? $"XRP: {ValueAsXrp:0.######}" + : $"{CurrencyValidName}: {ValueAsNumber:0.###############}"; + } + catch (Exception exception) when (exception is AmountOutOfRangeException or FormatException) + { + return $"{CurrencyValidName}: {Value}"; + } } public override bool Equals(object o) { return o is Currency model && model.Issuer == Issuer && model.CurrencyCode == CurrencyCode; } From 9352add63fec47d21b2a5250d7319f2340694ca8 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonenkov Date: Wed, 26 Aug 2026 22:38:13 -0300 Subject: [PATCH 4/5] fix(models): the setter stops writing an amount it cannot read back Checked against rippled first, which changed what this should be. I had proposed replacing G16 with truncation, on the belief that rippled truncates a mantissa when normalising. It does not: Number.cpp sets RoundingMode::ToNearest as the default, which is what G16 already does. Making the SDK truncate would have moved it away from the protocol, not toward it. The rounding stays. What is left is narrow. At the top of decimal's own range, rounding to nearest rounds up past what the type holds, so the setter wrote a string it then refused to read - a valid ledger amount the SDK produced and could not consume. There, and only there, the sixteenth digit is truncated instead; truncating cannot overflow, because dropping digits only moves a number toward zero. Dust is pinned by a test. Balances like 0.000000000000000001 arrive from the network and must go back out, and they are safe because the ledger's limit is sixteen significant digits while dust carries one. The test exists because the obvious way to bound precision - truncating to sixteen decimal places rather than significant digits - turns 1e-18 into zero, and a remainder would vanish in silence. That mutation fails it. ValueAsNumber_16Digits_NeverRoundsUp asserted that a round trip must not increase a value. The protocol makes no such promise, and the test could not have caught a violation anyway: its input has exactly sixteen significant digits, so there was nothing to round. Replaced by the property that does hold, and by one stating the rounding outright so the next reader does not repeat the mistake I nearly shipped. Also written down: why the setter rounds while the codec refuses more than sixteen digits. They see different inputs. Seventeen digits cannot arrive from the network - rippled normalises the mantissa into [1e15, 1e16) before serialising - so the codec only ever meets a hand-written string, while the setter meets computed decimals that routinely carry 28. AmmMath returns them. --- CHANGES.md | 5 +- Tests/Xrpl.Tests/Models/TestCurrency.cs | 146 ++++++++++++++++++++---- Xrpl/Models/Common/Currency.cs | 73 +++++++++++- 3 files changed, 193 insertions(+), 31 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 0fe2ea8f..750926f9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,7 +9,10 @@ * the threshold is nowhere near the protocol's ceiling: `1e29` is barely above `decimal.MaxValue` and was already unreachable. A token with a large supply meets this without going anywhere near the ledger's limits * `Offer.AmountEach` reads the same property on both sides of an order and divides them. Anyone may place an offer in their own token at any value the protocol allows, so it fails the same way - and used to return a plausible-looking exchange rate that was wrong by 67 orders of magnitude, without throwing. Both it and `GetBalanceChanges` now say so in their own documentation rather than leaving it to be discovered * `Currency.ToString()` falls back to the raw value rather than letting the getter throw through it. By convention `ToString` does not throw, and logging, string interpolation and a debugger's watch window are exactly where someone would be while working out why an amount is unusual - failing there hides the value at the moment it is most wanted - * one edge is documented rather than fixed, in a test that pins it: writing `decimal.MaxValue` through the setter formats with `G16`, which rounds the mantissa **up** past what `decimal` holds, so the SDK can write a string the ledger would accept and then refuse to read it. The window is the last ~7e12 below `decimal.MaxValue`, reachable only by assigning a number no token amount would be + * the setter no longer writes a string it cannot read back. `G16` keeps the ledger's sixteen significant digits and rounds to nearest - which is what rippled does, so it stays - but at the top of `decimal`'s own range rounding to nearest rounds **up**, past what the type holds. Only there is the sixteenth digit truncated instead, which cannot overflow because dropping digits only moves a number toward zero + * **dust survives.** Balances like `0.000000000000000001` do arrive from the network and the SDK must be able to send them back; they are safe because the ledger's limit is sixteen *significant* digits and dust carries one. Pinned by a test, because the obvious way to bound precision - truncating to sixteen decimal *places* - turns `1e-18` into zero and would make a remainder disappear silently + * a test that claimed a round trip can never increase a value was asserting something the protocol does not promise, on a value where it could not fail. rippled's `Number` defaults to `ToNearest`, so an amount beyond sixteen digits can legitimately come back larger. Replaced with the property that does hold - an amount already at ledger precision goes out unchanged - and with one stating the rounding, so the next reader does not reach for truncation and move the SDK away from rippled + * why the setter rounds while the binary codec refuses more than sixteen digits outright is now written down. The two see different inputs: seventeen digits cannot arrive from the network, so the codec only ever meets a hand-written string, while the setter meets computed `decimal`s that routinely carry 28 - `AmmMath` returns them * `Console.WriteLine(exception)` is out of the parse path. A library does not write to the console * **breaking in effect, if not in signature**: code that read an out-of-range amount used to get a number and now gets an exception. Representing the full range instead of refusing it is #150 diff --git a/Tests/Xrpl.Tests/Models/TestCurrency.cs b/Tests/Xrpl.Tests/Models/TestCurrency.cs index a04a6fa3..4d8b9d06 100644 --- a/Tests/Xrpl.Tests/Models/TestCurrency.cs +++ b/Tests/Xrpl.Tests/Models/TestCurrency.cs @@ -211,35 +211,78 @@ public void ToString_OutOfRangeAmount_ShowsTheRawValue() } /// - /// Writing produces an amount that cannot be read back. + /// Writing the largest produces an amount that can be read back. /// /// /// - /// Documented rather than fixed. The setter formats with G16, which rounds the - /// mantissa to nearest - and near the ceiling that rounds up, past what - /// holds. So the SDK can write a string the ledger would accept - /// (16-digit mantissa, exponent 13) and then refuse to read it. + /// The setter formats with G16 to keep the ledger's sixteen significant digits, + /// rounding to nearest - which is what rippled does, so it is not a place to be clever. + /// The one input that could not serve is the top of 's own range, + /// where rounding to nearest rounds up, past what the type holds: the SDK wrote a + /// string it then refused to read. /// /// - /// The window is the last ~7e12 below , reachable only by - /// assigning a number no token amount would be. Changing how the setter rounds would touch - /// every round trip in the type to rescue a value nobody writes. Pinned here so the next - /// person meets a decision rather than a surprise. + /// There the sixteenth digit is truncated instead. Truncating cannot overflow, because + /// dropping digits only ever moves a number toward zero. /// /// [TestMethod] - public void ValueAsNumber_WritingDecimalMaxValue_RoundsUpBeyondWhatCanBeRead() + public void ValueAsNumber_WritingTheLargestDecimal_CanBeReadBack() { - Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer" }; - currency.ValueAsNumber = decimal.MaxValue; + foreach (decimal edge in new[] { decimal.MaxValue, decimal.MinValue }) + { + Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer" }; + currency.ValueAsNumber = edge; - Assert.AreEqual("7.922816251426434E+28", currency.Value); - Assert.ThrowsExactly(() => _ = currency.ValueAsNumber); + decimal readBack = currency.ValueAsNumber; - // Just below the rounding boundary the round trip is intact, which is what makes the - // line above an edge rather than a broken setter. - currency.ValueAsNumber = 79228162514264330000000000000m; - Assert.AreEqual(79228162514264330000000000000m, currency.ValueAsNumber); + Assert.IsTrue( + Math.Abs(readBack) <= Math.Abs(edge), + $"Truncation moves toward zero; {currency.Value} came back as {readBack}."); + } + } + + /// + /// A dust remainder survives being read, written and encoded for the ledger. + /// + /// + /// + /// Balances like 0.000000000000000001 do arrive from the network, and the SDK has to + /// be able to send one back. They are safe here because smallness is not the constraint - + /// the ledger's limit is sixteen significant digits, and dust carries one. + /// + /// + /// Pinned because the obvious way to bound precision destroys exactly these. Truncating to + /// sixteen decimal places rather than significant digits turns 1e-18 into + /// zero: a remainder would silently disappear, and the SDK would send nothing where a + /// balance stood. This test fails if anyone reaches for that. + /// + /// + [TestMethod] + public void ValueAsNumber_DustRemainders_SurviveTheWholeRoundTrip() + { + foreach (string fromTheWire in new[] + { + "0.000000000000000001", // 1e-18 + "1e-18", + "0.0000000000000000000000001", // 1e-25 + "1e-28", // the smallest decimal holds + "-0.000000000000000001", + }) + { + Currency incoming = new Currency { CurrencyCode = "USD", Issuer = "rIssuer", Value = fromTheWire }; + decimal amount = incoming.ValueAsNumber; + + Assert.AreNotEqual(0m, amount, $"'{fromTheWire}' must not read as nothing."); + + Currency outgoing = new Currency { CurrencyCode = "USD", Issuer = "rIssuer" }; + outgoing.ValueAsNumber = amount; + + Assert.AreEqual( + amount, + outgoing.ValueAsNumber, + $"'{fromTheWire}' must survive being written back; it became '{outgoing.Value}'."); + } } #endregion @@ -346,15 +389,68 @@ public void ValueAsNumber_Setter_UsesG0ForXrp() Assert.AreEqual("1500000", currency.Value); } + /// + /// An amount already at the ledger's precision survives a round trip untouched. + /// + /// + /// + /// This was called NeverRoundsUp and asserted that a round trip must not increase a + /// value. That is not a property of the ledger: G16 rounds to nearest, and so does + /// rippled - Number's default mode is ToNearest - so an amount carrying more + /// than sixteen significant digits can legitimately come back larger. The old name + /// promised an invariant the protocol does not have. + /// + /// + /// It also could not have caught a violation. The value it used has exactly sixteen + /// significant digits, so there is nothing for G16 to round in either direction; the + /// assertion passed for a value where it could not fail. + /// + /// + /// What is worth pinning is the property that does hold: an amount the ledger could have + /// sent goes out again unchanged. + /// + /// [TestMethod] - public void ValueAsNumber_16Digits_NeverRoundsUp() + public void ValueAsNumber_AtLedgerPrecision_RoundTripsUnchanged() { - Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer", Value = "316227.7660168379" }; - decimal original = currency.ValueAsNumber; - currency.ValueAsNumber = original; - decimal afterRoundTrip = decimal.Parse(currency.Value, CultureInfo.InvariantCulture); - Assert.IsTrue(afterRoundTrip <= original, - $"Round-trip must not increase value: original={original}, afterRoundTrip={afterRoundTrip}"); + foreach (string atPrecision in new[] + { + "316227.7660168379", // an AMM LP token amount + "9999999999999999", // the largest mantissa + "1000000000000000", // the smallest + "0.1234567890123457", + }) + { + Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer", Value = atPrecision }; + decimal original = currency.ValueAsNumber; + + currency.ValueAsNumber = original; + + Assert.AreEqual( + original, + currency.ValueAsNumber, + $"'{atPrecision}' is already at ledger precision and must survive intact."); + } + } + + /// + /// Beyond sixteen digits the value rounds to nearest, and may grow. That is the ledger's + /// own behaviour, not a defect. + /// + /// + /// Stated as a test because the opposite was previously asserted, and because a reader who + /// finds a value that grew will otherwise reach for the same wrong fix: rippled's + /// Number defaults to RoundingMode::ToNearest, so truncating here would move + /// the SDK away from the protocol rather than toward it. + /// + [TestMethod] + public void ValueAsNumber_BeyondLedgerPrecision_RoundsToNearestAsTheLedgerDoes() + { + Currency currency = new Currency { CurrencyCode = "USD", Issuer = "rIssuer" }; + currency.ValueAsNumber = 1.2345678901234565m; + + Assert.AreEqual("1.234567890123457", currency.Value); + Assert.IsTrue(currency.ValueAsNumber > 1.2345678901234565m, "Rounding to nearest went up here, as it should."); } #endregion diff --git a/Xrpl/Models/Common/Currency.cs b/Xrpl/Models/Common/Currency.cs index fba89646..cc889636 100644 --- a/Xrpl/Models/Common/Currency.cs +++ b/Xrpl/Models/Common/Currency.cs @@ -102,6 +102,15 @@ public string MPTokenIssuanceID /// failing over it would cost more than it protects; an amount of 1e96 reported as /// 7.9e28 is wrong by 67 orders of magnitude and is worth stopping for. /// + /// + /// Writing rounds to the ledger's sixteen significant digits; the binary codec, given a string + /// with more than sixteen, refuses it outright. That looks inconsistent and is not: the two see + /// different inputs. A string with seventeen digits cannot arrive from the network - rippled + /// normalises the mantissa into [1e15, 1e16) before serialising - so the codec only ever + /// meets one a caller wrote by hand, where refusing is right. This setter meets a computed + /// , which routinely carries 28 digits: + /// returns such values. Rounding them is the service, not a loss. + /// /// /// The amount exceeds what can hold. /// The amount is not a number at all. @@ -139,13 +148,67 @@ public decimal ValueAsNumber $"The amount '{Value}' is not a number in the form the XRP Ledger uses."); } - set => Value = value.ToString( - CurrencyCode == "XRP" - ? "G0" - : "G16", - CultureInfo.InvariantCulture); + set + { + if (CurrencyCode == "XRP") + { + Value = value.ToString("G0", CultureInfo.InvariantCulture); + return; + } + + // G16 keeps the sixteen significant digits the ledger allows, rounding to nearest - + // which is what rippled does too, so this is not a place to be clever. The one input + // it cannot serve is the top of decimal's own range: there, rounding to nearest rounds + // *up*, past what decimal holds, and the SDK would write a string it could not read + // back. Only then is the sixteenth digit truncated instead. + string formatted = value.ToString("G16", CultureInfo.InvariantCulture); + + if (!decimal.TryParse(formatted, AmountStyles, CultureInfo.InvariantCulture, out _)) + { + formatted = TruncateToLedgerPrecision(value).ToString("G16", CultureInfo.InvariantCulture); + } + + Value = formatted; + } + } + + /// + /// The same number with its digits beyond the ledger's sixteen dropped rather than rounded. + /// + /// + /// Reached only when rounding would carry the value past . + /// Truncating cannot: dropping digits only ever moves a number toward zero. + /// + private static decimal TruncateToLedgerPrecision(decimal value) + { + if (value == 0m) + { + return 0m; + } + + int magnitude = (int)Math.Floor(Math.Log10((double)Math.Abs(value))); + int digitsToDrop = magnitude - (LedgerSignificantDigits - 1); + + if (digitsToDrop <= 0) + { + return value; + } + + decimal scale = 1m; + for (int i = 0; i < digitsToDrop; i++) + { + scale *= 10m; + } + + return Math.Truncate(value / scale) * scale; } + /// + /// How many significant digits an issued-currency amount carries on the ledger. + /// + /// rippled's STAmount normalises the mantissa into [1e15, 1e16). + private const int LedgerSignificantDigits = 16; + /// /// XRP token amount (non drops value) /// From aa108eeed96433e5e3221c924eb286f2a10f2991 Mon Sep 17 00:00:00 2001 From: Aleksandr Platonenkov Date: Wed, 26 Aug 2026 22:59:46 -0300 Subject: [PATCH 5/5] chore(release): bump Xrpl.BinaryCodec, and record the change that moved it Release preparation for 27/08, found by checking what actually changed since 11.0.0.0 rather than by looking at this branch alone. Xrpl.BinaryCodec/XrplBinaryCodec.cs changed in #147 and the package version did not. Promoting that way publishes nothing: dotnet nuget push runs with --skip-duplicate, so a package whose version already exists on the feed is passed over in silence, and the fix reaches no consumer while the run stays green. Moved to 11.0.1.0 - a performance fix with no contract change, so patch. The same PR left no CHANGES.md entry. A 1.73x change on the path every signing operation takes is not a silent one, so it has one now, with the measurement and with why the usual telling of that bug oversells it. Xrpl stays at 11.1.0.0: this release carries a contract change, since code that read an out-of-range amount used to get a number and now gets an exception. AddressCodec, Keypairs and both X402 packages are untouched and keep their versions - they are consumed by ProjectReference, so a package built at a newer version keeps depending on the published ones. CHANGES.md still opens with "## Unreleased". Stamping it belongs to the promotion, when the date is known. --- Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj | 2 +- CHANGES.md | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj b/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj index 3f3af843..e6711263 100644 --- a/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj +++ b/Base/Xrpl.BinaryCodec/Xrpl.BinaryCodec.csproj @@ -13,7 +13,7 @@ Apache-2.0 https://github.com/StaticBit-io/XrplCSharp XrplCSharp - 11.0.0.0 + 11.0.1.0 diff --git a/CHANGES.md b/CHANGES.md index 750926f9..f20e80ee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,11 @@ ## Unreleased +* **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. + * not the catastrophe this bug is usually described as: since .NET 7 System.Text.Json shares a caching context between structurally equal options instances, so type metadata was not being rebuilt per call - had it been, the gap would be orders of magnitude rather than 1.7x. What was paid is an allocation and a structural-equality lookup in a pool capped at 64 contexts + * `LOVault.ToHex` had the same pattern on a colder path + * `Xrpl.BinaryCodec` moves to **11.0.1.0** for it. The other base packages are untouched and stay where they are - they are consumed by `ProjectReference`, so a package built at a newer version keeps depending on the published ones + * **An amount the ledger allows but `decimal` cannot hold is refused, not guessed at** (#148). `Currency.ValueAsNumber` answered such values three different ways: a positive one clamped to `decimal.MaxValue`, a negative one threw `FormatException`, and a very small one quietly became zero. XRPL issued currency runs from `1e-81` to roughly `1e96` - a 16-digit mantissa with an exponent in `[-96, 80]`, per rippled's `STAmount` - while `decimal` stops near `7.9e28`, so this cannot be parsed away; the only choice is how to fail. * the clamp is gone. An amount above the range now throws `AmountOutOfRangeException`, which carries the value as the node sent it. Returning `7.9e28` for `1e96` is wrong by 67 orders of magnitude, and it did not stay contained: `GetBalanceChanges` subtracts two balances, so the clamped value went on to throw `OverflowException` from arithmetic instead * the negative case was a parse bug. The fallback's `NumberStyles` expression came to `AllowExponent | AllowDecimalPoint` - `AllowLeadingSign` was missing, so no negative value could reach the branch meant to handle it. The primary parse was correct all along, despite six `&` terms that all evaluate to zero