Skip to content

Commit

Permalink
WIP: Remove restriction on segwit HRP
Browse files Browse the repository at this point in the history
  • Loading branch information
tcharding committed Jul 21, 2023
1 parent 35ebe2e commit e3d62d4
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 26 deletions.
34 changes: 16 additions & 18 deletions src/primitives/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,6 @@ impl<'s> CheckedHrpstring<'s> {

self.validate_padding()?;
self.validate_witness_length(witness_version)?;
if !self.hrp().is_valid_segwit() {
return Err(SegwitHrpstringError::InvalidHrp(self.hrp()));
}

Ok(SegwitHrpstring { hrp: self.hrp(), witness_version, data: self.data })
}
Expand Down Expand Up @@ -360,15 +357,14 @@ pub struct SegwitHrpstring<'s> {
impl<'s> SegwitHrpstring<'s> {
/// Parses an HRP string, treating the first data character as a witness version.
///
/// The version byte does not appear in the extracted binary data, but is covered
/// by the checksum. It can be accessed with [`Self::witness_version`].
/// The version byte does not appear in the extracted binary data, but is covered by the
/// checksum. It can be accessed with [`Self::witness_version`].
///
/// NOTE: We do not enforce any restrictions on the HRP, use [`has_valid_hrp`] to get strict BIP
/// conformance (see [`has_valid_hrp`]).
pub fn new(s: &'s str) -> Result<Self, SegwitHrpstringError> {
let unchecked = UncheckedHrpstring::new(s)?;

if !unchecked.hrp().is_valid_segwit() {
return Err(SegwitHrpstringError::InvalidHrp(unchecked.hrp()));
}

// Unwrap ok since check_characters (in `Self::new`) checked the bech32-ness of this char.
let witness_version = Fe32::from_char(unchecked.data[0].into()).unwrap();
if witness_version.to_u8() > 16 {
Expand Down Expand Up @@ -398,10 +394,6 @@ impl<'s> SegwitHrpstring<'s> {
pub fn new_bech32(s: &'s str) -> Result<Self, SegwitHrpstringError> {
let unchecked = UncheckedHrpstring::new(s)?;

if !unchecked.hrp().is_valid_segwit() {
return Err(SegwitHrpstringError::InvalidHrp(unchecked.hrp()));
}

// Unwrap ok since check_characters (in `Self::new`) checked the bech32-ness of this char.
let witness_version = Fe32::from_char(unchecked.data[0].into()).unwrap();
if witness_version.to_u8() > 16 {
Expand All @@ -412,6 +404,16 @@ impl<'s> SegwitHrpstring<'s> {
checked.validate_segwit()
}

/// Returns `true` if the HRP is "bc" or "tb".
///
/// BIP-173 requires that the HRP is "bc" or "tb" but software in the Bitcoin ecosystem uses
/// other HRPs, specifically "bcrt" for regtest addresses. We provide this function in order to
/// be BIP-173 compliant but their are no restrictions on the HRP of [`SegwitHrpstring`].
pub fn has_valid_hrp(&self) -> bool {
let hrp = self.hrp();
hrp == Hrp::parse_unchecked("bc") || hrp == Hrp::parse_unchecked("tb")
}

/// Returns the human-readable part.
pub fn hrp(&self) -> Hrp { self.hrp }

Expand Down Expand Up @@ -527,8 +529,6 @@ where
pub enum SegwitHrpstringError {
/// Error while parsing the encoded address string.
Unchecked(UncheckedHrpstringError),
/// BIP-173 requires the HRP to be either "bc" or "tb".
InvalidHrp(Hrp),
/// The witness version byte is missing.
MissingWitnessVersion,
/// Invalid witness version (must be 0-16 inclusive).
Expand All @@ -547,8 +547,6 @@ impl fmt::Display for SegwitHrpstringError {

match *self {
Unchecked(ref e) => write_err!(f, "parsing unchecked hrpstring failed"; e),
InvalidHrp(hrp) =>
write!(f, "BIP-173 requires the HRP to be either \"bc\" or \"tb\": {}", hrp),
MissingWitnessVersion => write!(f, "the witness version byte is missing"),
InvalidWitnessVersion(fe) => write!(f, "invalid segwit witness version: {}", fe),
Padding(ref e) => write_err!(f, "invalid padding on the witness data"; e),
Expand All @@ -568,7 +566,7 @@ impl std::error::Error for SegwitHrpstringError {
Padding(ref e) => Some(e),
WitnessLength(ref e) => Some(e),
Checksum(ref e) => Some(e),
InvalidHrp(_) | MissingWitnessVersion | InvalidWitnessVersion(_) => None,
MissingWitnessVersion | InvalidWitnessVersion(_) => None,
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions tests/bip_173_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use bech32::primitives::decode::{
CheckedHrpstring, ChecksumError, SegwitHrpstring, UncheckedHrpstring,
};
use bech32::{Bech32, Bech32m, ByteIterExt, Fe32IterExt, Hrp};
use bech32::{Bech32, Bech32m, ByteIterExt, Fe32IterExt};

// This is a separate test because we correctly identify this string as invalid but not for the
// reason given in the bip.
Expand All @@ -15,15 +15,15 @@ fn bip_173_checksum_calculated_with_uppercase_form() {

// BIP-173 states reason for error should be: "checksum calculated with uppercase form of HRP".
let s = "A1G7SGD8";

assert_eq!(
CheckedHrpstring::new::<Bech32>(s).unwrap_err(),
CheckedHrpstringError::Checksum(ChecksumError::InvalidChecksum)
);

// After BIP-350 is introduced this test vector now fails because of invalid HRP.
assert_eq!(
SegwitHrpstring::new(s).unwrap_err(),
SegwitHrpstringError::InvalidHrp(Hrp::parse_unchecked("A"))
SegwitHrpstringError::Checksum(ChecksumError::InvalidChecksum)
);
}

Expand Down Expand Up @@ -86,7 +86,10 @@ macro_rules! check_invalid_address {
#[test]
#[cfg(feature = "alloc")]
fn $test_name() {
assert!(SegwitHrpstring::new($addr).is_err());
match SegwitHrpstring::new($addr) {
Err(_) => {},
Ok(segwit) => assert!(!segwit.has_valid_hrp()),
}
}
)*
}
Expand Down
11 changes: 7 additions & 4 deletions tests/bip_350_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@ use bech32::primitives::decode::{
CheckedHrpstring, CheckedHrpstringError, ChecksumError, SegwitHrpstring, SegwitHrpstringError,
UncheckedHrpstring,
};
use bech32::{Bech32, Bech32m, ByteIterExt, Fe32, Fe32IterExt, Hrp};
use bech32::{Bech32, Bech32m, ByteIterExt, Fe32, Fe32IterExt};

// This is a separate test because we correctly identify this string as invalid but not for the
// reason given in the bip.
#[test]
fn bip_350_checksum_calculated_with_uppercase_form() {
// BIP-350 states reason for error should be: "checksum calculated with uppercase form of HRP".
let s = "M1VUXWEZ";

assert_eq!(
CheckedHrpstring::new::<Bech32m>(s).unwrap_err(),
CheckedHrpstringError::Checksum(ChecksumError::InvalidChecksum)
);

// But if parsed as a BIP-350 address we fail this because of the invalid HRP.
assert_eq!(
SegwitHrpstring::new(s).unwrap_err(),
SegwitHrpstringError::InvalidHrp(Hrp::parse_unchecked("M"))
SegwitHrpstringError::Checksum(ChecksumError::InvalidChecksum)
);
}

Expand Down Expand Up @@ -91,7 +91,10 @@ macro_rules! check_invalid_address {
#[test]
#[cfg(feature = "alloc")]
fn $test_name() {
assert!(SegwitHrpstring::new($addr).is_err());
match SegwitHrpstring::new($addr) {
Err(_) => {},
Ok(segwit) => assert!(!segwit.has_valid_hrp()),
}
}
)*
}
Expand Down

0 comments on commit e3d62d4

Please sign in to comment.