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

more type safe currency unit calculation #423

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

hussein-aitlahcen
Copy link
Contributor

Use 12 decimals for all currencies and move unit/milli etc to the trait definition

@dzmitry-lahoda
Copy link
Contributor

what i know about decimals,

  • right, in ORML these called same, so they have special trait to know it. so CurrencyId does not knows its decimals.
  • decimals are only for currencies with decimals, cannot represent fiat with non decimals increases - ok
  • because of benchmarks does not allow using other pallets than tested (cannot use others config), i cannot use CurrencyFactory or NativeCurrencyId traits from other pallets to get currency. so people hacked that by stating that currency id is from 128 on pallet levels, which is false. it is try from. decode is little bit better.
  • really we have MayBeCurrency in all extrinsic, they are converted to currency by calling operations (transfer accepts MayBeCurrency) and returns not error if currency exists. like checking by doing.
  • from 2 above type system kind of broken as of now.
  • decimals, could we hedge against possible currencies with 15-16 decimals? this number is good binary also (12 has zeroes)
  • really, it does not matter how much decimal something have. there are only UI and oracle who cares. we do not do Domain Design here. All calculation should be done in smalles units. Decimals are only for external bounded contexts.

So having currency type safe in substrate is broken in Substrate more or less (at least by benchmarks).

dzmitry-lahoda
dzmitry-lahoda previously approved these changes Dec 29, 2021
KaiserKarel
KaiserKarel previously approved these changes Dec 30, 2021
@hussein-aitlahcen hussein-aitlahcen merged commit 8e570a1 into main Jan 5, 2022
@hussein-aitlahcen hussein-aitlahcen deleted the safe-currency-unit branch January 5, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants