Skip to content

wnaf: support windows of size 64#72

Open
tarcieri wants to merge 3 commits intozkcrypto:mainfrom
tarcieri:wnaf/support-window-size-64
Open

wnaf: support windows of size 64#72
tarcieri wants to merge 3 commits intozkcrypto:mainfrom
tarcieri:wnaf/support-window-size-64

Conversation

@tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Feb 6, 2026

Followup to #46 which attempted to change an assertion for the window size from <= 64 to < 64, which was in response to the width type being u64 and its value computed as 1 << window, which would overflow a u64.

This means width needs an extra bit, so this promotes it to a u128 while keeping any variables that can remain u64 as they were.

Followup to zkcrypto#46 which attempted to change an assertion for the window
size from `<= 64` to `< 64`, which was in response to the `width` type
being `u64` and its value computed as `1 << window`, which would
overflow a `u64`.

This means `width` needs an extra bit, so this promotes it to a `u128`
while keeping any variables that can remain `u64` as they were.
@tarcieri
Copy link
Contributor Author

tarcieri commented Feb 7, 2026

Is there a good way to test this?

@tarcieri tarcieri mentioned this pull request Feb 7, 2026
@tarcieri
Copy link
Contributor Author

tarcieri commented Feb 7, 2026

As I was looking at this I thought there was already a lot of casting, and adding in wider types only makes things worse.

Since we need to compute width / 2 anyway, it seems like if you calculated width_div_2 instead of width, it would always fit in u64, which gets rid of the u128, e.g.:

let width_div_2 = 1u64 << (window - 1); // window is asserted >= 2
let window_mask = width_div_2 | (width_div_2 - 1); // fill in 1s for all the lower 0 bits

I guess you still need i128 here though? (window_val can overflow i64 if window == 64, right?)

let width = (width_div_2 as i128) * 2;
((window_val as i128) - width) as i64

Edit: went ahead and made this change in 6a7d96a

- uses: actions/checkout@v4
- run: cargo fetch
# Requires #![deny(rustdoc::broken_intra_doc_links)] in crates.
- run: sudo apt-get -y install libfontconfig1-dev
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same change I made in #71: CI fails with this line present

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.

1 participant