search

maidsafe / sn_dbc

Safe Network DBCs
BSD 3-Clause "New" or "Revised" License
15 stars 16 forks source link

Amount fixes and tests pr #99

Closed dan-da closed 3 years ago

dan-da commented 3 years ago

improves correctness and comments for Amount, Denomination and adds test cases for Amount.

dan-da commented 3 years ago

@davidrusu thx for the comments. just fyi, I will be updating this with a substantial simplification soon, and will incorporate your suggestions.

I have further refactored and was able to remove rug dependency entirely.... ie no more rug::Rational, rug::Integer. I like it because it reduces the code size/footprint in sn_dbc, all integer math and no need to compile gmp library.

The main things we lose are:

  1. the ability to parse fractions and repr/display fractions.
  2. Amount::to_rational(), which was quite useful as a double check in test cases, so I'm going to add test vectors for checked_add and checked_subtract instead.

But these are not things the Mint needs and perhaps not most clients.

Anyway, I'm creating a helper crate for client use (possibly) that has the removed logic in an AmountRational struct. Just to keep the code around in case it's useful at some point.... it took me a lot of work and head scratching to get impl From<Rational> for Amount working. ;)

davidrusu commented 3 years ago

Thanks for the heads up, let me know when the new work is ready for review

dan-da commented 3 years ago

@davidrusu ready for review when you have a chance.

dan-da commented 3 years ago

rebased on latest drusu/mint_blind_sigs_fork_squashed

davidrusu commented 3 years ago

Still have a few clippy failures

dan-da commented 3 years ago

Still have a few clippy failures

hmm, do you mean in the bench maybe? looks clean here.

$ cargo clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.36s
$ cargo clippy --tests
    Checking sn_dbc v2.7.1 (/home/danda/dev/maidsafe/sn_dbc)
    Finished dev [unoptimized + debuginfo] target(s) in 1.83s