Skip to content

Commit

Permalink
Merge pull request #61 from pacu/fix-double-initializer
Browse files Browse the repository at this point in the history
[#60] Amount(value: Double) false positive tooManyFractionalDigits
  • Loading branch information
pacu authored Jun 30, 2024
2 parents 6e336ae + 0c76b5c commit bd32863
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 17 deletions.
43 changes: 27 additions & 16 deletions Sources/ZcashPaymentURI/Amount.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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)
}

Expand All @@ -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.
}
}

Expand All @@ -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
}
}
2 changes: 1 addition & 1 deletion Sources/ZcashPaymentURI/Model.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public struct Payment: Equatable {
public init(
recipientAddress: RecipientAddress,
amount: Amount,
memo: MemoBytes?,
memo: MemoBytes?,
label: String?,
message: String?,
otherParams: [OtherParam]?
Expand Down
36 changes: 36 additions & 0 deletions Tests/ZcashPaymentURITests/AmountTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}

0 comments on commit bd32863

Please sign in to comment.