Skip to content

Conversation

@chaoticlonghair
Copy link
Contributor

@chaoticlonghair chaoticlonghair commented Mar 26, 2019

  • Fix bug: in division, when borrow a higher unit, do not forget check if the higher unit was reduced to zero after subtraction.

    The reason was written in this comment.

  • Fix bug: in division, when doing estimate is too cautious, so the estimated quotient is too small each time, led to an overflow.

    The reason was written in this comment.

@mohanson
Copy link

New bugs happened.

use numext_fixed_uint::U4096;

fn main() {
    let a = U4096::from_dec_str("77207453359070457604620928442586248569527320043595348948921733518358535471105").unwrap();
    let b = U4096::from_dec_str("115792089237316195423570985008687907853269984665640564039457584007913129639936").unwrap();

    let r1 = &a * &a;
    println!("{}", r1);

    let r2 = r1 % b;
    println!("{}", r2);
}
thread 'test_state_work' panicked at 'attempt to add with overflow'

@mohanson
Copy link

Build failed with message

error[E0425]: cannot find value `of` in this scope
  --> C:\Users\mohan\.cargo\git\checkouts\rust-numext-836c791cc312aa11\cfab40d\fixed-uint\src\lib.rs:11:1
   |
11 | / numext_constructor::construct_fixed_uints!(
12 | |     U128 {
13 | |         size = 128,
14 | |     },
...  |
41 | |     },
42 | | );
   | |__^ not found in this scope

@chaoticlonghair
Copy link
Contributor Author

That is not my fault, no matter I push which commit, travis ci always run a wrong commit.

I have passed all test in my laptop.

…if the higher unit was reduced to zero after subtraction.
@chaoticlonghair
Copy link
Contributor Author

Copy link

@mohanson mohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine in my test cases now.

@chaoticlonghair chaoticlonghair merged commit d1662f4 into cryptape:develop Mar 27, 2019
@chaoticlonghair chaoticlonghair deleted the fix-div-slow-bug branch April 3, 2019 07:44
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