From 0c76b5cabb629cdefaddaa094d402114baf819ac Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Sun, 30 Jun 2024 15:28:43 -0300 Subject: [PATCH] [#60] Amount(value: Double) false positive tooManyFractionalDigits This commit adds tests to reproduce the described case and several checks and changes on how `Double` inputs are used to initialize an `Amount`. This branches off the rounding of inputs to let the caller chose whether to round the input when creating an Amount and also removes the rounding from the `toString()` method to avoid rouding twice when printing the number to a String Closes #60 swiftlint fixes --- Sources/ZcashPaymentURI/Amount.swift | 43 ++++++++++++-------- Sources/ZcashPaymentURI/Model.swift | 2 +- Tests/ZcashPaymentURITests/AmountTests.swift | 36 ++++++++++++++++ 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/Sources/ZcashPaymentURI/Amount.swift b/Sources/ZcashPaymentURI/Amount.swift index 2472115..6ce40eb 100644 --- a/Sources/ZcashPaymentURI/Amount.swift +++ b/Sources/ZcashPaymentURI/Amount.swift @@ -19,7 +19,7 @@ public struct Amount: Equatable { static let maxFractionalDecimalDigits: Int = 8 - static let zecRounding = Rounding(.toNearestOrEven, maxFractionalDecimalDigits) + static let zecRounding = Rounding(.toNearestOrEven, 16) static let decimalHandler = NSDecimalNumberHandler( roundingMode: NSDecimalNumber.RoundingMode.bankers, @@ -36,16 +36,20 @@ public struct Amount: Equatable { let value: BigDecimal - /// Initializes an Amount from a `Decimal` number - /// - parameter value: decimal representation of the desired amount. **Important:** `Decimal` values with more than 8 fractional digits ** will be rounded** using bankers rounding. + /// Initializes an Amount from a `Double` number + /// - parameter value: double representation of the desired amount. **Important:** `Double` values with more than 8 fractional digits ** will be rounded** using bankers rounding. /// - returns A valid ZEC amount /// - throws `Amount.AmountError` then the provided value can't represent or can't be rounded to a non-negative ZEC decimal amount. + /// - important: Apparently sound `Double` values like `0.02` will result into invalid ZEC amounts if not rounded properly. Therefore all `Double` inputs are rounded to prevent further errors or undesired values. + /// - note: this is a convenience initializer. when possible favor the use of other initializer with safer input values public init(value: Double) throws { guard value >= 0 else { throw AmountError.negativeAmount } guard value <= Self.maxSupply.asDouble() else { throw AmountError.greaterThanSupply } - try self.init(decimal: BigDecimal(value).round(Self.zecRounding)) + let rounded = Decimal(value).zecBankersRounding() + + try self.init(decimal: rounded) } /// Initializes an Amount from a `BigDecimal` number @@ -63,23 +67,28 @@ public struct Amount: Equatable { guard decimal <= Self.maxSupply else { throw AmountError.greaterThanSupply } - self.value = decimal + self.value = decimal.trim } /// Initializes an Amount from a `BigDecimal` number /// - parameter decimal: decimal representation of the desired amount. **Important:** `Decimal` values with more than 8 fractional digits ** will be rounded** using bankers rounding. + /// - parameter rounding: whether this initializer should eagerly perform a bankers rounding to /// - returns A valid ZEC amount /// - throws `Amount.AmountError` then the provided value can't represent or can't be rounded to a non-negative ZEC decimal amount. - public init(decimal: Decimal) throws { + public init(decimal: Decimal, rounding: Bool = false) throws { guard decimal >= 0 else { throw AmountError.negativeAmount } + guard decimal <= Self.maxSupply.asDecimal() else { throw AmountError.greaterThanSupply } + guard decimal.significantFractionalDecimalDigits <= Self.maxFractionalDecimalDigits else { throw AmountError.tooManyFractionalDigits } - guard decimal <= Self.maxSupply.asDecimal() else { throw AmountError.greaterThanSupply } - - self.value = BigDecimal(decimal).round(Self.zecRounding).trim + if rounding { + self.value = BigDecimal(decimal).round(Self.zecRounding).trim + } else { + self.value = BigDecimal(decimal).trim + } } public init(string: String) throws { @@ -89,10 +98,6 @@ public struct Amount: Equatable { throw AmountError.invalidTextInput } - guard decimalAmount.significantFractionalDecimalDigits <= Self.maxFractionalDecimalDigits else { - throw AmountError.tooManyFractionalDigits - } - try self.init(decimal: decimalAmount) } @@ -101,9 +106,7 @@ public struct Amount: Equatable { } public func toString() -> String { - let decimal = value.round(Rounding(.toNearestOrEven, Self.maxFractionalDecimalDigits)).trim - - return decimal.asString(.plain) // this value is already validated. + return self.value.asString(.plain) // this value is already validated. } } @@ -117,4 +120,12 @@ extension Decimal { var significantFractionalDecimalDigits: Int { return max(-exponent, 0) } + + func zecBankersRounding() -> Decimal { + var result = Decimal() + var number = self + + NSDecimalRound(&result, &number, Amount.maxFractionalDecimalDigits, .bankers) + return result + } } diff --git a/Sources/ZcashPaymentURI/Model.swift b/Sources/ZcashPaymentURI/Model.swift index 106a5ae..7d046b2 100644 --- a/Sources/ZcashPaymentURI/Model.swift +++ b/Sources/ZcashPaymentURI/Model.swift @@ -40,7 +40,7 @@ public struct Payment: Equatable { public init( recipientAddress: RecipientAddress, amount: Amount, - memo: MemoBytes?, + memo: MemoBytes?, label: String?, message: String?, otherParams: [OtherParam]? diff --git a/Tests/ZcashPaymentURITests/AmountTests.swift b/Tests/ZcashPaymentURITests/AmountTests.swift index eeec226..1ce70be 100644 --- a/Tests/ZcashPaymentURITests/AmountTests.swift +++ b/Tests/ZcashPaymentURITests/AmountTests.swift @@ -54,4 +54,40 @@ final class AmountTests: XCTestCase { func testAmountParsesMaxAmount() throws { XCTAssertEqual(try Amount(string: "21000000").toString(), try Amount(value: 21_000_000).toString()) } + + func testDoubleToDecimal() throws { + + var result = Decimal() + var number = Decimal(10_000.00002) + + NSDecimalRound(&result, &number, Amount.maxFractionalDecimalDigits, .bankers) + + let amount = try Amount(value: 10_000.00002) + XCTAssertEqual(amount.toString(), "10000.00002") + } + + func testFractionsOfZecFromDouble() throws { + // XCTAssertEqual(try Amount(value: 0.2).toString(), "0.2") + XCTAssertEqual(try Amount(value: 0.02).toString(), "0.02") + XCTAssertEqual(try Amount(value: 0.002).toString(), "0.002") + XCTAssertEqual(try Amount(value: 0.0002).toString(), "0.0002") + XCTAssertEqual(try Amount(value: 0.00002).toString(), "0.00002") + XCTAssertEqual(try Amount(value: 0.000002).toString(), "0.000002") + XCTAssertEqual(try Amount(value: 0.0000002).toString(), "0.0000002") + XCTAssertEqual(try Amount(value: 0.00000002).toString(), "0.00000002") + XCTAssertEqual(try Amount(value: 0.2).toString(), "0.2") + XCTAssertEqual(try Amount(value: 10.02).toString(), "10.02") + XCTAssertEqual(try Amount(value: 100.002).toString(), "100.002") + XCTAssertEqual(try Amount(value: 1_000.0002).toString(), "1000.0002") + XCTAssertEqual(try Amount(value: 10_000.00002).toString(), "10000.00002") + XCTAssertEqual(try Amount(value: 100_000.000002).toString(), "100000.000002") + XCTAssertEqual(try Amount(value: 1_000_000.0000002).toString(), "1000000.0000002") + XCTAssertEqual(try Amount(value: 10_000_000.00000002).toString(), "10000000.00000002") + } + + func testTooManyFractionsThrows() throws { + //more digits than supposed to + XCTAssertThrowsError(try Amount(decimal: Decimal(0.000000002)).toString()) + XCTAssertThrowsError(try Amount(decimal: Decimal(10_000_000.000000002))) + } }