Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Cryptocurrencies #3

Open
Mordil opened this issue Jan 14, 2020 · 2 comments
Open

Support Cryptocurrencies #3

Mordil opened this issue Jan 14, 2020 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Mordil
Copy link
Member

Mordil commented Jan 14, 2020

No description provided.

@Mordil Mordil added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jan 20, 2020
@rnapier
Copy link

rnapier commented Jan 11, 2021

Note that the current approach probably can't work for ETH. There are 1e18 Wei in an ETH, and if you try to implement that in an obvious way:

public struct ETH: CurrencyProtocol, CurrencyMetadata {
  public static var name: String { return "Eth" }
  public static var alphabeticCode: String { return "ETH" }
  public static var numericCode: UInt16 { return 999 }
  public static var minorUnits: UInt8 { return 18 }

  public var minorUnits: Int64 { return self._minorUnits }

  public init<T: BinaryInteger>(minorUnits: T) { self._minorUnits = .init(minorUnits) }

  private let _minorUnits: Int64
}

then you'll wind up with 5.01 ETH == 5.009999999999998976 ETH. The rounding hack to support ExpressibleByFloatLiteral is convenient, but it fails silently when minorUnits is large. ETH is a pathological case and fails easily. But BTC fails in edge cases that are still legal. For example, BTC(20_000_000.10000001) is rounded to 20000000.1 BTC.

Using String rather than double literals would fix this. Decimal(string: "20000000.10000001") does not fail this way. This is clearly less convenient. The question is whether that convenience is worth introducing rounding errors in money, or limiting the range of minorUnits to fairly small values.

EDIT: After thinking about this a bit more, I realized that rounding isn't even the biggest problem facing ETH. Int64 overflows for anything larger than about 10 ETH (on the order of US $13k in 2021). This could be fixed by using Decimal instead of Int64 for minorUnits, which would support a quintillion ETH, without causing much headache for callers.

@Mordil
Copy link
Member Author

Mordil commented Apr 9, 2021

@rnapier Thank you for your feedback, we are addressing those points in our next major release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants