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

fix fpow overflow #161

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Conversation

neotheprogramist
Copy link
Contributor

Problem Description

In the implementation of bitshifts in lib.cairo, the existing fpow function exhibits an overflow error when computing 2^n for n ≥ 64 using the generic function for u128.

Details

When we want to compute 2^n for n ≥ 64 using the generic function for u128, we encounter an overflow error. The error occurs in the last steps where we need to compute x * x, and x is greater than or equal to 2^64. Thus, x * x is greater than or equal to 2^128, leading to an overflow error.

Proposed Solution

The proposed solution is to modify the fpow function with a specific condition for n == 1. Below are the old and new code snippets:

Old function code:

fn fpow(x: u128, n: u128) -> u128 {
    if n == 0 {
        1
    } else if (n & 1) == 1 {
        x * fpow(x * x, n / 2)
    } else {
        fpow(x * x, n / 2)
    }
}

New function code:

fn fpow(x: u128, n: u128) -> u128 {
    if n == 0 {
        1
    } else if n == 1 {
        x
    } else if (n & 1) == 1 {
        x * fpow(x * x, n / 2)
    } else {
        fpow(x * x, n / 2)
    }
}

This change fixes the issue by handling the specific case where n is 1, preventing the overflow error.

@0xLucqs 0xLucqs merged commit 0891274 into keep-starknet-strange:main Aug 11, 2023
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2023
@neotheprogramist neotheprogramist deleted the fpow-fix branch January 4, 2024 07:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants