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

pow_vartime([n, 0, 0, 0] => pow_vartime([n] #792

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

Conversation

zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented Aug 22, 2023

pow_vartime([n, 0, 0, 0] == pow_vartime([n] always holds, so it seems there's no point in wasting resource computing pow_vartime([n, 0, 0, 0] instead of pow_vartime([n].

@zhiqiangxu zhiqiangxu changed the title pow_vartime([n, 0, 0, 0] => pow_vartime([n] pow_vartime([n, 0, 0, 0] => pow_vartime([n] Aug 22, 2023
@str4d
Copy link
Contributor

str4d commented Aug 22, 2023

This is probably left over from when pow_vartime was on FieldExt and took a fixed-length array (IIRC). Generally +1 on the PR, but I'm on PTO and am not reviewing Zcash PRs currently.

@zhiqiangxu
Copy link
Contributor Author

zhiqiangxu commented Aug 22, 2023

This is probably left over from when pow_vartime was on FieldExt and took a fixed-length array (IIRC). Generally +1 on the PR, but I'm on PTO and am not reviewing Zcash PRs currently.

No hurry:)

BTW, it seems pow_vartime and pow are almost the same, is the difference really that big or another left over to be unified?

UPDATE

I see what constant time means here now: it doesn't mean it's faster, just that the time is constant no matter which branch is chosen, to anti timing attack.

E.g:

            fn conditional_select(a: &Self, b: &Self, choice: Choice) -> Self {
                // if choice = 0, mask = (-0) = 0000...0000
                // if choice = 1, mask = (-1) = 1111...1111
                let mask = -(choice.unwrap_u8() as to_signed_int!($t)) as $t;
                a ^ (mask & (a ^ b))
            }

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.

2 participants