feat: biginteger implementation #495
Conversation
✅ Deploy Preview for contracts-stylus canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
There was a problem hiding this comment.
It's great you have done these benchmarks but I am not a fan of maintaining akr-ff or poseidon-renegades. We may want. to have just a separated branch with these benchmarks.
There was a problem hiding this comment.
I was about to drop it. It is just for comparison, when I was optimizing it
There was a problem hiding this comment.
Left some comments + many warnings from CI.
Overall great progress @qalisander!
| erc1155::bench().boxed(), | ||
| erc1155_metadata_uri::bench().boxed(), | ||
| poseidon::bench().boxed(), | ||
| // access_control::bench().boxed(), |
There was a problem hiding this comment.
To be reverted or maintained on a dedicated branch.
| @@ -84,12 +84,12 @@ pub trait Field: | |||
| + for<'a> Div<&'a mut Self, Output = Self> | |||
| + for<'a> DivAssign<&'a mut Self> | |||
| + for<'a> Product<&'a Self> | |||
| + From<u128> | |||
| // + From<u128> // TODO#q: add u128 conversion | |||
| educe.workspace = true | ||
| hex-literal.workspace = true | ||
| # TODO#q: have num-bigint dependency optional |
| const_for!((i in 0..N) { | ||
| let a = self.limbs[N - i - 1]; | ||
| let b = other.limbs[N - i - 1]; | ||
| if a < b { |
There was a problem hiding this comment.
- Can't you have jus
return a < b? - What if
a == b?
| while self.const_is_even() { | ||
| self = self.const_shr(); | ||
| } | ||
| assert!(self.const_is_odd()); |
There was a problem hiding this comment.
I do not like these asserts but I believe it is the only way to assure proper execution :(
| } | ||
| } | ||
|
|
||
| // TODO#q: rename to checked_add? |
There was a problem hiding this comment.
If you are about checked_add you should implement the proper trait then.
| } | ||
| */ | ||
|
|
||
| impl<const N: usize> From<u64> for Uint<N> { |
There was a problem hiding this comment.
You should use macros to implement these traits.
There was a problem hiding this comment.
Fuzzing here is super needed 😅 @0xNeshi
There was a problem hiding this comment.
Proptests would be a bare minimum I'd say 😃
Resolves #481
PR Checklist