Make currency conversions more intuitive.

[Sventimir]

This is a small improvement to Currency API. In Currency submodules (Currency.Balance, Currency.Amount, Currency.Fee) functions of_int and to_int are repleced by more intuitive nanomina, centimina, mina, int_of_nanomina, int_of_centimina, int_of_mina. This way there is no cofusion about the unit of currency we're converting to (or from). This also provides a way to easily round the amounts upon conversion. Finally thanks to these conversion functions we can use much smaller numbers in tests, which makes it easier to read and understand those tests.

Explain how you tested your changes:

• Just made sure the project still does compile and passes the CI. Removing old-style functions from the signature forced the propagation of the change to other parts of the system.

Checklist:

• [ ] Modified the current draft of release notes with details on what is completed or incomplete within this project
• [ ] Document code purpose, how to use it
  • Mention expected invariants, implicit constraints
• [ ] All tests pass (CI will check this if you didn't)
• [x] Serialized types are in stable-versioned modules

[mitschabaude]

I'll mention one aspect to this @Sventimir

When OCaml is compiled to JavaScript, which is currently the case for the entire mina_base module, then using large int is unsafe. js_of_ocaml will simple truncate ints to the 32 bits that ints have in JS. Therefore, to_int / of_int and all the functions added instead of it in this PR should probably NOT be the default way of converting to/from a Currency, at least not in libraries that will be compiled to JS. Instead, its safe to use int64 or uint64, both of which are correctly represented in JS.

I checked this PR for introducing unsafe usage of int in parts that are compiled to JS, and didn't find any. Therefore, I don't think it has to be addressed in this PR, but it's something that the people involved should know. If we design "the one cool way of creating a Currency.t", I'd argue for making that expect an int64 rather than an int

[Sventimir]

@mitschabaude, thank you, that's an important thing to keep in mind indeed. I already had an idea to add more conversions like these, from/to int64 and unint64, but decided it would be too much for a single PR. Indeed, there's more work to do in this direction: replace _exn (unsafe) calls with safe ones wherever it makes sense and probably switch to using int64 rather than int where possible. I will make a note of it and try to further improve the situation in follow-up PRs.

[Sventimir]

I would dispense with the centimina functions. In the economics whitepaper, there is indeed mention of centimina (and micromina). But so far, in the codebase, only Mina and nanoMina are used. I think it's better to keep that simplicity.

centimina is actually used a lot in tests, so I wouldn't get rid of it. I'd rather consider replacing it with milimina, as there are a couple of occurrences that would get simplified with it. But as humans we;re used to splitting currency into hundredths rather than into thousandths parts, so I'm not really sure.

Can mina_exn be mina_of_int_exn, similarly for nanomina_exn? That symmetry in naming would be nice.

One more request: can of_formatted_string be renamed to of_mina_string, similarly to_mina_string, analogous to of_mina_int_exn and mina_to_int?

Done in 1f25adc, 4ddd42a and 920da6a .

[psteckler]

centimina is actually used a lot in tests

Sorry, which tests? git grep centimina doesn't show anything in this repo, or in snarkyjs.

[Sventimir]

Sorry, which tests? git grep centimina doesn't show anything in this repo, or in snarkyjs.

Are you sure you're on the right branch?

$ grep -rni centimina src
src/app/test_executive/zkapps.ml:149:      let fee = Currency.Fee.centimina_of_int_exn 2 in
src/app/test_executive/zkapps.ml:175:      let fee = Currency.Fee.centimina_of_int_exn 1 in
src/app/test_executive/zkapps.ml:229:      let fee = Currency.Fee.centimina_of_int_exn 1 in
src/app/test_executive/zkapps.ml:355:      let fee = Currency.Fee.centimina_of_int_exn 1 in
src/app/test_executive/block_production_priority.ml:36:  let fee = Currency.Fee.centimina_of_int_exn 1
src/app/test_executive/block_production_priority.ml:38:  let amount = Currency.Amount.centimina_of_int_exn 1
src/app/test_executive/gossip_consistency.ml:89:    let fee = Currency.Fee.centimina_of_int_exn 1 in
src/app/test_executive/gossip_consistency.ml:90:    let amount = Currency.Amount.centimina_of_int_exn 1 in
src/app/test_executive/delegation_test.ml:35:    let fee = Currency.Fee.centimina_of_int_exn 1 in
src/app/test_executive/peers_reliability_test.ml:71:             ~fee:(Currency.Fee.centimina_of_int_exn 1)
src/app/test_executive/peers_reliability_test.ml:95:           let fee = Currency.Fee.centimina_of_int_exn 2 in
src/lib/currency/intf.ml:54:     It is advisable to use nanomina, centimina and mina wherever
src/lib/currency/intf.ml:60:  val centimina_of_int_exn : int -> t
src/lib/currency/intf.ml:66:  val centimina_of_int : int -> t option
src/lib/currency/intf.ml:72:  val int_of_centimina : t -> int
src/lib/currency/currency.ml:369:    let int_of_centimina m = to_int m / centi_to_nano
src/lib/currency/currency.ml:412:       It is advisable to use nanomina, centimina and mina wherever
src/lib/currency/currency.ml:418:    let centimina_of_int i =
src/lib/currency/currency.ml:431:    let centimina_of_int_exn i =
src/lib/currency/currency.ml:432:      match centimina_of_int i with
src/lib/transaction_logic/mina_transaction_logic.ml:2205:          gen_incl (nanomina_of_int_exn 1_000_000) (centimina_of_int_exn 10))
src/lib/transaction_logic/mina_transaction_logic.ml:2209:          gen_incl (nanomina_of_int_exn 1_000_000) (centimina_of_int_exn 10))
src/lib/transaction_snark/test/account_timing/account_timing.ml:87:      let cliff_amount = Amount.centimina_of_int_exn 50 in
src/lib/transaction_snark/test/account_timing/account_timing.ml:147:      let cliff_amount = Amount.centimina_of_int_exn 90 in
src/lib/transaction_snark/test/account_timing/account_timing.ml:1137:                   ; cliff_amount = Currency.Amount.centimina_of_int_exn 10
src/lib/transaction_snark/test/account_timing/account_timing.ml:1139:                   ; vesting_increment = Currency.Amount.centimina_of_int_exn 10

[psteckler]

Are you sure you're on the right branch?

In develop, where the changes in the PR are not present, there are no uses of centimina.

The PR is introducing that new naming, and I'm not sure it's desirable. It adds complexity that hasn't existed.

[Sventimir]

Okay, done.

[psteckler]

!ci-build-me

[ylecornec]

!ci-build-me

[Sventimir]

!ci-build-me

[kantp]

!ci-build-me

[kantp]

(integration tests failed due to preemptible nodes, so re-running the CI)

[Sventimir]

!ci-build-me

[Sventimir]

!ci-build-me

[Sventimir]

!ci-build-me

[Sventimir]

!ci-build-me