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

Is the function from_hardened_idx correctly implemented? #2643

Closed
ywiyogo opened this issue Mar 31, 2024 · 3 comments
Closed

Is the function from_hardened_idx correctly implemented? #2643

ywiyogo opened this issue Mar 31, 2024 · 3 comments

Comments

@ywiyogo
Copy link

ywiyogo commented Mar 31, 2024

In https://github.com/rust-bitcoin/rust-bitcoin/blob/master/bitcoin/src/bip32.rs#L149, I see that the if check is as same as the from_normal_idx. Shouldn't the implementation like this:

        if index & (1 << 31) == 1 {
            Ok(ChildNumber::Hardened { index })
        } else {
            Err(Error::InvalidChildNumber(index))
        }       
@tcharding
Copy link
Member

tcharding commented Apr 1, 2024

The ChildNumber enum maintains the invariant that each value does not have the top bit set. In other words the inner values of ChildNumber::Hardened and ChildNumber::Normal do not use the top bit. The from_hardened_idx and from_normal_idx expect the input value to not use the top bit (to be within the range [0, 2^31 - 1]). There is also the From<u32> impl that accepts a u32 with the top bit specifying hardened/normal

impl From<u32> for ChildNumber {
    fn from(number: u32) -> Self {
        if number & (1 << 31) != 0 {
            ChildNumber::Hardened { index: number ^ (1 << 31) }
        } else {
            ChildNumber::Normal { index: number }
        }
    }
}

@apoelstra
Copy link
Member

Yep, this is correct. Though it could perhaps be commented better.

@ywiyogo
Copy link
Author

ywiyogo commented Apr 28, 2024

Thanks for the clarification!

@ywiyogo ywiyogo closed this as completed Apr 28, 2024
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

No branches or pull requests

3 participants