Skip to content

Constant-time square root and division #376

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

Merged
merged 8 commits into from
Dec 3, 2023
Merged

Constant-time square root and division #376

merged 8 commits into from
Dec 3, 2023

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Dec 2, 2023

This is the implementation from #277, rebased and with most of the review comments addressed.

This was already getting a bit hard to rebase, so if we can get it over the finish line before it becomes too stale that seems good.

cc @HastD

HastD and others added 6 commits December 2, 2023 12:59
The initial guess is 2^(ceil(bits/2)), which is always greater than sqrt(self), so initially `xn < guess`, which means the removed while loop never runs.
This makes `Uint::sqrt` constant-time as well.
@tarcieri tarcieri requested a review from fjarri December 2, 2023 20:56
Casts to `Word` are causing issues on 32-bit platforms
@fjarri
Copy link
Contributor

fjarri commented Dec 2, 2023

I am still uncertain about whether there's an off-by-1 error.

First, I think I made a mistake in my previous comment saying that we can use LOG2_BITS. In the paper, the bound depends on floor(log2(BITS)), but LOG2_BITS == ceil(log2(BITS)). The difference will come up for sizes like U192.

Second, I wrote a Python implementation to test it, and it seems that while the paper claims that one needs to take min(x_n, x_{n+1}) for n = floor(log2(BITS)) + 1, in reality n = floor(log2(BITS)) is enough. In the Rust implementation it means one can have the loop while i < Self::LOG2_BITS - 1 (because the first iteration is already done outside of it). The test that uses up all the iterations is, for example

        assert_eq!(
            U256::from_be_hex("4bb750738e25a8f82940737d94a48a91f8cd918a3679ff90c1a631f2bd6c3597").sqrt(),
            U256::from_be_hex("000000000000000000000000000000008b3956339e8315cff66eb6107b610075"));

Here the number is

x = 34247351911670871285154124444449115000627881786205705581610587444645680919959

and the square root is r = 185060400711959097619664627767588225141 such that x = (r + 1)^2 - 205.

For reference, the Python implementation:

import math
import random

def sqrt_test(x, x_bits):
    log2_bits = int(math.log2(x_bits))
    max_bits = (x.bit_length() + 1) // 2

    xn = 1 << max_bits # x_0
    i = 0

    print(f"x = {x}")
    print(f"log2(b)+1 =", log2_bits + 1)
    print(f"x_{i} = {xn}")
    while i < log2_bits - 1:
        guess = xn
        xn = (guess + x // guess) // 2
        i += 1
        print(f"x_{i} = {xn}")

    return min(guess, xn)


if __name__ == '__main__':

    for x in range(1, 2**16):
        t = sqrt_test(x, 16)
        assert t**2 <= x < (t+1)**2

@fjarri
Copy link
Contributor

fjarri commented Dec 2, 2023

If you want to finalize it, we can:

  • keep the current number of iterations (the first iteration, let mut xn = ..., can be brought inside it to avoid repetition)
  • add the test above
  • fix the LOG2_BITS problem
  • and make a TODO & an issue to reduce the number of iterations, which will hopefully be looked at by @HastD.

@fjarri
Copy link
Contributor

fjarri commented Dec 2, 2023

A test for U192 (to check a non-power-of-2 bit size) can be, for example:

x = 131760037584061584202147902974795492358573409165975616061
r = 11478677518950586156071689761
where x = (r + 1)^2 - 583

This also uses up the maximum number of iterations

@tarcieri
Copy link
Member Author

tarcieri commented Dec 3, 2023

@fjarri pushed up the U192 and U256 edge cases tests you suggested in 927ebaf

Wasn't sure if you were suggesting translating the Python example into Rust as well.

@fjarri
Copy link
Contributor

fjarri commented Dec 3, 2023

No, the Python code is just to document what I was using for cross-checking. I'm approving this, you can merge, and then I'll make a separate PR for the items from #376 (comment)

@tarcieri tarcieri merged commit 64add42 into master Dec 3, 2023
@tarcieri tarcieri deleted the tarcieri/ct-sqrt branch December 3, 2023 14:58
@fjarri fjarri mentioned this pull request Dec 3, 2023
@tarcieri tarcieri mentioned this pull request Jan 22, 2025
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.

3 participants