Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dead code. #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove dead code. #5

wants to merge 1 commit into from

Conversation

shekohex
Copy link
Owner

After making some code refactoring, i found that

curve25519-rs/src/lib.rs

Lines 1152 to 1156 in 540914f

fn is_nonzero(&self) -> bool {
let bs = self.to_bytes();
let zero = [0; 32];
!fixed_time_eq(bs.as_ref(), zero.as_ref())
}

is only used here GeP3::from_bytes_negate_vartime

pub fn from_bytes_negate_vartime(s: &[u8]) -> Option<GeP3> {

...

curve25519-rs/src/lib.rs

Lines 1402 to 1412 in 540914f

let mut x = uv7.pow25523() * v_raise_3 * u;
let vxx = x.square() * v;
let check = vxx - u;
if check.is_nonzero() {
let check2 = vxx + u;
if check2.is_nonzero() {
return None;
}
x = x * FE_SQRTM1;
}

which the GeP3::from_bytes_negate_vartime is never used in the code.
so after removing it, we now can get rid of some other stuff like fixed_time_eq which is unsafe linked from other c code and written in asm.

@shekohex shekohex added enhancement New feature or request needs-review labels Dec 18, 2018
@shekohex
Copy link
Owner Author

cc @jamesmunns could you review this ? thanks.

@jamesmunns
Copy link
Contributor

Hey @shekohex, in general I am definitely not a crypto expert, and removing anything marked "fixed time" is something that would make me very nervous.

@shekohex
Copy link
Owner Author

shekohex commented Dec 18, 2018

@jamesmunns
oh, i know that maybe a bad thing, since i'm not a crypto expert too, but actually that code never have been referenced, so why we need it?

again, yesterday i was trying to port it to WASM using wasm-pack of course, but i got a linkage error from rust-lld, and guess what? the object causes the problem was that one from the cc compiler util_helpers.o.
which made me to search the full code, and i found that we never uses it.

and after removing that dead_code, it works perfectly in browser !

@jamesmunns
Copy link
Contributor

Yeah, I would expect that this code would not work for targets other than x86[_64] or ARM, as the C code is only implemented for these targets. Additionally, I don't think it's possible to link in non-Rust code using the wasm32-unknown-unknown target (I think you need to use the emscripten one? I might be wrong about this)

@shekohex
Copy link
Owner Author

Ok, I'll create a new branch which i will remove c code from the base, and start to use rusty_wasm crate which i think would be nice to inline asm.
(but I'm not sure, if that could be a solution for compiling it to wasm target).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants