Accept Apple Pay amounts in minor units - #53
Conversation
ana-maksimovskikh
left a comment
There was a problem hiding this comment.
Thanks Max!
Approved with the nitties nit possible
| formatter.numberStyle = .currency | ||
| formatter.currencyCode = currency | ||
| return NSDecimalNumber( | ||
| mantissa: UInt64(abs(minorUnits)), |
There was a problem hiding this comment.
Maybe
mantissa: UInt64(minorUnits.magnitude),?
Almost nonexistent opportunity for a bug here 🤡
minorUnits is a plain Int (signed, 64-bit). Signed integers are asymmetric: the range is -9223372036854775808 (Int.min) to 9223372036854775807 (Int.max) — one more negative value than positive.
abs(x) is implemented as roughly "if negative, negate it." For every value except Int.min, that's fine. For Int.min specifically, the negation itself overflows.
So -Int.min would need to equal 9223372036854775808, which is one larger than Int.max and literally cannot be represented as an Int.
Int.magnitude returns the absolute value as UInt, whose range fully covers Int.min's magnitude, so it never overflows:
| case let .decimal(decimal): | ||
| return !decimal.decimalValue.isNaN | ||
| case .minorUnits: | ||
| return true |
There was a problem hiding this comment.
Maybe
return minorUnits > Int.min
Same reason as below
Why
Amountonly accepted a major-unit decimal string, so converting from minor units was left to merchant code. The web SDKs take minor-unit integers, so a merchant integrating both handles the same charge two different ways, and the exponent logic (JPY has no minor unit, KWD has three) sits in their code rather than ours. Nothing validated the string either:NSDecimalNumber(string:)returns NaN for an unparseable value, and PassKit raises on a NaN summary item, so a typo surfaced as a crash when the sheet was presented.Part of the cross-SDK amount work. Android is #52, web is evervault/evervault-js#977 and #978.
How
Amountnow holds either a decimal or a minor-unit integer, resolved against the transaction's currency when the request is built. That way there is one currency and it is always the transaction's, rather than anAmountcarrying a currency that could disagree with the transaction it belongs to.Amount(minorUnits: 5499)is the new entry point. The exponent comes fromNumberFormatterin.currencystyle, andNSDecimalNumber(mantissa:exponent:isNegative:)keeps the conversion exact.Amount("54.99")is unchanged. An unparseable string is now rejected when the transaction is constructed, throwing the newEvervaultError.InvalidAmountError. That boundary already throws and already validates currency and country, so no merchant call site needs a newtry.totalPriceformat does not permit them.Source-breaking in one spot:
Amount.amountis replaced byresolve(currency:), since the value now depends on the currency. The Demo app is updated to match. Construction is unchanged, which is what merchant code mostly does.I could not build this locally (no Xcode on this machine, and PassKit is iOS-only) — relying on CI.