RustCrypto / crypto-bigint

Cryptography-oriented big integer library with constant-time, stack-allocated (no_std-friendly) implementations of modern formulas
Apache License 2.0
182 stars 51 forks source link

`Monty::new` takes values, but impl takes ref #599

Closed dvdplm closed 3 months ago

dvdplm commented 4 months ago

The Monty trait's new method takes its arguments by value, but the implementation for MontyForm uses references when calling into MontyForm::new.

What are your thoughts on changing the trait in v0.6 to take a reference as well? The new method only ever need read access to the data so why impose a clone/copy on users if they need to keep using their Integer?

Sort of like so:

src/traits.rs --- Rust
799     /// Convert the value into the representation using precomputed data.
800     fn new(value: &Self::Integer, params: Self::Params) -> Self;
801 

src/modular/monty_form.rs --- Rust
243     fn new(value: &Self::Integer, params: Self::Params) -> Self {
244         MontyForm::new(value, params)
245     } 
tarcieri commented 4 months ago

Yeah, I think it would make sense for it to borrow

dvdplm commented 4 months ago

Cool, I'll open a PR later tonight. :)