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

Introduce get_checksum_bytes method and improvements #671

Merged
merged 1 commit into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Add `descriptor::checksum::get_checksum_bytes` method.

## [v0.20.0] - [v0.19.0]

Expand Down
44 changes: 20 additions & 24 deletions src/descriptor/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@
//! This module contains a re-implementation of the function used by Bitcoin Core to calculate the
//! checksum of a descriptor

use std::iter::FromIterator;

use crate::descriptor::DescriptorError;

const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
const CHECKSUM_CHARSET: &str = "qpzry9x8gf2tvdw0s3jn54khce6mua7l";
const INPUT_CHARSET: &[u8] = b"0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
const CHECKSUM_CHARSET: &[u8] = b"qpzry9x8gf2tvdw0s3jn54khce6mua7l";

fn poly_mod(mut c: u64, val: u64) -> u64 {
let c0 = c >> 35;
Expand All @@ -43,15 +41,17 @@ fn poly_mod(mut c: u64, val: u64) -> u64 {
c
}

/// Compute the checksum of a descriptor
pub fn get_checksum(desc: &str) -> Result<String, DescriptorError> {
Copy link
Member

@notmandatory notmandatory Jul 16, 2022

Choose a reason for hiding this comment

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

I don't know if anyone is using get_checksum() but since it's public our policy is to deprecate it for at least one release before completely removing it. We haven't been super strict about it, but we do need to be more consistent. I think you could make a simple function to call your new get_checksum_bytes and convert to a String.

Copy link
Member Author

@evanlinjin evanlinjin Jul 16, 2022

Choose a reason for hiding this comment

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

@notmandatory I haven't removed it at all! And yes, get_checksum calls get_checksum_bytes 😅 (line 79)

/// Computes the checksum bytes of a descriptor
pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> {
let mut c = 1;
let mut cls = 0;
let mut clscount = 0;
for ch in desc.chars() {

for ch in desc.as_bytes() {
let pos = INPUT_CHARSET
.find(ch)
.ok_or(DescriptorError::InvalidDescriptorCharacter(ch))? as u64;
.iter()
.position(|b| b == ch)
.ok_or(DescriptorError::InvalidDescriptorCharacter(*ch))? as u64;
c = poly_mod(c, pos & 31);
cls = cls * 3 + (pos >> 5);
clscount += 1;
Expand All @@ -67,17 +67,18 @@ pub fn get_checksum(desc: &str) -> Result<String, DescriptorError> {
(0..8).for_each(|_| c = poly_mod(c, 0));
c ^= 1;

let mut chars = Vec::with_capacity(8);
let mut checksum = [0_u8; 8];
for j in 0..8 {
chars.push(
CHECKSUM_CHARSET
.chars()
.nth(((c >> (5 * (7 - j))) & 31) as usize)
.unwrap(),
);
checksum[j] = CHECKSUM_CHARSET[((c >> (5 * (7 - j))) & 31) as usize];
}

Ok(String::from_iter(chars))
Ok(checksum)
}

/// Compute the checksum of a descriptor
pub fn get_checksum(desc: &str) -> Result<String, DescriptorError> {
// unsafe is okay here as the checksum only uses bytes in `CHECKSUM_CHARSET`
get_checksum_bytes(desc).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) })
}

#[cfg(test)]
Expand All @@ -97,17 +98,12 @@ mod test {

#[test]
fn test_get_checksum_invalid_character() {
let sparkle_heart = vec![240, 159, 146, 150];
let sparkle_heart = std::str::from_utf8(&sparkle_heart)
.unwrap()
.chars()
.next()
.unwrap();
let sparkle_heart = unsafe { std::str::from_utf8_unchecked(&[240, 159, 146, 150]) };
let invalid_desc = format!("wpkh(tprv8ZgxMBicQKsPdpkqS7Eair4YxjcuuvDPNYmKX3sCniCf16tHEVrjjiSXEkFRnUH77yXc6ZcwHHcL{}fjdi5qUvw3VDfgYiH5mNsj5izuiu2N/1/2/*)", sparkle_heart);

assert!(matches!(
get_checksum(&invalid_desc).err(),
Some(DescriptorError::InvalidDescriptorCharacter(invalid_char)) if invalid_char == sparkle_heart
Some(DescriptorError::InvalidDescriptorCharacter(invalid_char)) if invalid_char == sparkle_heart.as_bytes()[0]
));
}
}
4 changes: 2 additions & 2 deletions src/descriptor/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pub enum Error {
/// Error while extracting and manipulating policies
Policy(crate::descriptor::policy::PolicyError),

/// Invalid character found in the descriptor checksum
InvalidDescriptorCharacter(char),
/// Invalid byte found in the descriptor checksum
InvalidDescriptorCharacter(u8),

/// BIP32 error
Bip32(bitcoin::util::bip32::Error),
Expand Down