Update to use Monty represenation for all 256 bit operations. #50
Conversation
coltfred
commented
Jan 24, 2019
coltfred
commented
- Update to use Monty once we get to the internal types.
| @@ -19,7 +19,7 @@ lazy_static = "~1.2" | |||
| arrayvec = "~0.4" | |||
| ed25519-dalek = "1.0.0-pre.1" | |||
| clear_on_drop = "~0.2" | |||
| gridiron = "0.5.0" | |||
| gridiron = "0.5.1" | |||
There was a problem hiding this comment.
Should we just depend on "0.5" or "~0.5"?
There was a problem hiding this comment.
If we did that it wouldn't actually work on 0.5.0, but maybe that's ok...
There was a problem hiding this comment.
I was just thinking we'd want to get new patch updates. Not sure what to say about 0.5.0 though...
| @@ -37,6 +37,23 @@ impl BytesDecoder for Fp256 { | |||
| } | |||
| } | |||
|
|
|||
| impl BytesDecoder for gridiron::fp_256::Monty { | |||
There was a problem hiding this comment.
I think we've been preferring the non ArrayVec implementation of this. See BytesDecoder for Fp480. Now that we have 3 of these, is it time to introduce a macro to reduce duplication?
There was a problem hiding this comment.
I propose we use arrayvec to shorten the code.
There was a problem hiding this comment.
That's a nice improvement. Will you fix the Fp480 instance below to maintain consistency? Or we can do it another PR.
There was a problem hiding this comment.
I'll fix it in my next pr.
src/api.rs
Outdated
| @@ -18,6 +18,7 @@ pub use crate::internal::ByteVector; | |||
| use crate::nonemptyvec::NonEmptyVec; | |||
| use crate::Revealed; | |||
| use clear_on_drop::clear::Clear; | |||
| use gridiron::fp_256; | |||
There was a problem hiding this comment.
Now that I'm seeing Monty in use, I wish the fp31 macro enabled us to set the name for the Montgomery Form type.
I think something like Fp256M or Fp256MF is nicer than fp_256::Monty, especially next to the "standard form" type of Fp256
| @@ -37,6 +37,23 @@ impl BytesDecoder for Fp256 { | |||
| } | |||
| } | |||
|
|
|||
| impl BytesDecoder for gridiron::fp_256::Monty { | |||
There was a problem hiding this comment.
That's a nice improvement. Will you fix the Fp480 instance below to maintain consistency? Or we can do it another PR.
| @@ -19,7 +19,7 @@ lazy_static = "~1.2" | |||
| arrayvec = "~0.4" | |||
| ed25519-dalek = "1.0.0-pre.1" | |||
| clear_on_drop = "~0.2" | |||
| gridiron = "0.5.0" | |||
| gridiron = "0.5.1" | |||
There was a problem hiding this comment.
I was just thinking we'd want to get new patch updates. Not sure what to say about 0.5.0 though...
