Skip to content

Commit

Permalink
Merge #148: Use pass-by-copy for Hrp when possible
Browse files Browse the repository at this point in the history
faf8651 Use pass-by-copy for Hrp when possible (Tobin C. Harding)

Pull request description:

  We currently have some code that uses pass-by-reference and some that uses pass-by-copy.

  Further investigation shows that when messing with iterators we have to have a reference to the `Hrp` so that the iterator has somewhere in memory to point to while iterating.

  The iterator API is intentionally low level, we can make the higher level APIs more ergonomic by using pass-by-copy.

ACKs for top commit:
  apoelstra:
    ACK faf8651

Tree-SHA512: c7e547033cdec865d887b82ddc0a7c38762693dbf77851355c584afde1082bac1c046f4b4c2ac22a2f016b0a599974701a0ac0914c91629787259747df47d8c2
  • Loading branch information
apoelstra committed Oct 21, 2023
2 parents 413e8b5 + faf8651 commit 7207926
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 37 deletions.
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
//! assert_eq!(address, ADDR);
//!
//! // Encode arbitrary data as a Bitcoin taproot address.
//! let taproot_address = segwit::encode(&hrp::BC, segwit::VERSION_1, &DATA).expect("valid witness version and program");
//! let taproot_address = segwit::encode(hrp::BC, segwit::VERSION_1, &DATA).expect("valid witness version and program");
//! assert_eq!(taproot_address, TAP_ADDR);
//!
//! // No-alloc: Encode without allocating (ignoring that String::new() allocates :).
Expand Down
4 changes: 2 additions & 2 deletions src/primitives/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ impl<Ck: Checksum> Engine<Ck> {

/// Feeds `hrp` into the checksum engine.
#[inline]
pub fn input_hrp(&mut self, hrp: &Hrp) {
for fe in HrpFe32Iter::new(hrp) {
pub fn input_hrp(&mut self, hrp: Hrp) {
for fe in HrpFe32Iter::new(&hrp) {
self.input_fe(fe)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/primitives/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<'s> UncheckedHrpstring<'s> {
}

let mut checksum_eng = checksum::Engine::<Ck>::new();
checksum_eng.input_hrp(&self.hrp());
checksum_eng.input_hrp(self.hrp());

// Unwrap ok since we checked all characters in our constructor.
for fe in self.data.iter().map(|&b| Fe32::from_char_unchecked(b)) {
Expand Down
4 changes: 2 additions & 2 deletions src/primitives/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ where
/// Adapts the `Fe32Iter` iterator to yield characters representing the bech32 encoding.
#[inline]
pub fn new(hrp: &'hrp Hrp, data: WitnessVersionIter<I>) -> Self {
let checksummed = Checksummed::new_hrp(hrp, data);
let checksummed = Checksummed::new_hrp(*hrp, data);
Self { hrp_iter: Some(hrp.lowercase_char_iter()), checksummed }
}
}
Expand Down Expand Up @@ -258,7 +258,7 @@ where
#[inline]
pub fn new(hrp: &'hrp Hrp, data: WitnessVersionIter<I>) -> Self {
let hrp_iter = HrpFe32Iter::new(hrp);
let checksummed = Checksummed::new_hrp(hrp, data);
let checksummed = Checksummed::new_hrp(*hrp, data);
Self { hrp_iter: Some(hrp_iter), checksummed }
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/primitives/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ where
/// Creates a new checksummed iterator which adapts a data iterator of field elements by
/// first inputting the [`Hrp`] and then appending a checksum.
#[inline]
pub fn new_hrp(hrp: &Hrp, data: I) -> Checksummed<I, Ck> {
pub fn new_hrp(hrp: Hrp, data: I) -> Checksummed<I, Ck> {
let mut ret = Self::new(data);
ret.checksum_engine.input_hrp(hrp);
ret
Expand Down
56 changes: 28 additions & 28 deletions src/segwit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@
//! ];
//!
//! // Encode a taproot address suitable for use on mainnet.
//! let _ = segwit::encode_v1(&hrp::BC, &witness_prog);
//! let _ = segwit::encode_v1(hrp::BC, &witness_prog);
//!
//! // Encode a segwit v0 address suitable for use on testnet.
//! let _ = segwit::encode_v0(&hrp::TB, &witness_prog);
//! let _ = segwit::encode_v0(hrp::TB, &witness_prog);
//!
//! // If you have the witness version already you can use:
//! # let witness_version = segwit::VERSION_0;
//! let _ = segwit::encode(&hrp::BC, witness_version, &witness_prog);
//! let _ = segwit::encode(hrp::BC, witness_version, &witness_prog);
//!
//! // Decode a Bitcoin bech32 segwit address.
//! let address = "bc1q2s3rjwvam9dt2ftt4sqxqjf3twav0gdx0k0q2etxflx38c3x8tnssdmnjq";
Expand Down Expand Up @@ -94,7 +94,7 @@ pub fn decode(s: &str) -> Result<(Hrp, Fe32, Vec<u8>), DecodeError> {
#[cfg(feature = "alloc")]
#[inline]
pub fn encode(
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> Result<String, EncodeError> {
Expand All @@ -109,14 +109,14 @@ pub fn encode(
/// Encodes a segwit version 0 address.
#[cfg(feature = "alloc")]
#[inline]
pub fn encode_v0(hrp: &Hrp, witness_program: &[u8]) -> Result<String, EncodeError> {
pub fn encode_v0(hrp: Hrp, witness_program: &[u8]) -> Result<String, EncodeError> {
encode(hrp, VERSION_0, witness_program)
}

/// Encodes a segwit version 1 address.
#[cfg(feature = "alloc")]
#[inline]
pub fn encode_v1(hrp: &Hrp, witness_program: &[u8]) -> Result<String, EncodeError> {
pub fn encode_v1(hrp: Hrp, witness_program: &[u8]) -> Result<String, EncodeError> {
encode(hrp, VERSION_1, witness_program)
}

Expand All @@ -127,7 +127,7 @@ pub fn encode_v1(hrp: &Hrp, witness_program: &[u8]) -> Result<String, EncodeErro
#[inline]
pub fn encode_to_fmt_unchecked<W: fmt::Write>(
fmt: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> fmt::Result {
Expand All @@ -140,19 +140,19 @@ pub fn encode_to_fmt_unchecked<W: fmt::Write>(
/// the [`crate::primitives::segwit`] module for validation functions).
pub fn encode_lower_to_fmt_unchecked<W: fmt::Write>(
fmt: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> fmt::Result {
let iter = witness_program.iter().copied().bytes_to_fes();
match witness_version {
VERSION_0 => {
for c in iter.with_checksum::<Bech32>(hrp).with_witness_version(VERSION_0).chars() {
for c in iter.with_checksum::<Bech32>(&hrp).with_witness_version(VERSION_0).chars() {
fmt.write_char(c)?;
}
}
version => {
for c in iter.with_checksum::<Bech32m>(hrp).with_witness_version(version).chars() {
for c in iter.with_checksum::<Bech32m>(&hrp).with_witness_version(version).chars() {
fmt.write_char(c)?;
}
}
Expand All @@ -169,19 +169,19 @@ pub fn encode_lower_to_fmt_unchecked<W: fmt::Write>(
#[inline]
pub fn encode_upper_to_fmt_unchecked<W: fmt::Write>(
fmt: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> fmt::Result {
let iter = witness_program.iter().copied().bytes_to_fes();
match witness_version {
VERSION_0 => {
for c in iter.with_checksum::<Bech32>(hrp).with_witness_version(VERSION_0).chars() {
for c in iter.with_checksum::<Bech32>(&hrp).with_witness_version(VERSION_0).chars() {
fmt.write_char(c.to_ascii_uppercase())?;
}
}
version => {
for c in iter.with_checksum::<Bech32m>(hrp).with_witness_version(version).chars() {
for c in iter.with_checksum::<Bech32m>(&hrp).with_witness_version(version).chars() {
fmt.write_char(c.to_ascii_uppercase())?;
}
}
Expand All @@ -198,7 +198,7 @@ pub fn encode_upper_to_fmt_unchecked<W: fmt::Write>(
#[inline]
pub fn encode_to_writer_unchecked<W: std::io::Write>(
w: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> std::io::Result<()> {
Expand All @@ -213,20 +213,20 @@ pub fn encode_to_writer_unchecked<W: std::io::Write>(
#[inline]
pub fn encode_lower_to_writer_unchecked<W: std::io::Write>(
w: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> std::io::Result<()> {
let iter = witness_program.iter().copied().bytes_to_fes();
match witness_version {
VERSION_0 => {
for c in iter.with_checksum::<Bech32>(hrp).with_witness_version(VERSION_0).chars() {
w.write_all(&[c as u8])?;
for c in iter.with_checksum::<Bech32>(&hrp).with_witness_version(VERSION_0).chars() {
w.write_all(&[c.to_ascii_lowercase() as u8])?;
}
}
version => {
for c in iter.with_checksum::<Bech32m>(hrp).with_witness_version(version).chars() {
w.write_all(&[c as u8])?;
for c in iter.with_checksum::<Bech32m>(&hrp).with_witness_version(version).chars() {
w.write_all(&[c.to_ascii_lowercase() as u8])?;
}
}
}
Expand All @@ -243,19 +243,19 @@ pub fn encode_lower_to_writer_unchecked<W: std::io::Write>(
#[inline]
pub fn encode_upper_to_writer_unchecked<W: std::io::Write>(
w: &mut W,
hrp: &Hrp,
hrp: Hrp,
witness_version: Fe32,
witness_program: &[u8],
) -> std::io::Result<()> {
let iter = witness_program.iter().copied().bytes_to_fes();
match witness_version {
VERSION_0 => {
for c in iter.with_checksum::<Bech32>(hrp).with_witness_version(VERSION_0).chars() {
for c in iter.with_checksum::<Bech32>(&hrp).with_witness_version(VERSION_0).chars() {
w.write_all(&[c.to_ascii_uppercase() as u8])?;
}
}
version => {
for c in iter.with_checksum::<Bech32m>(hrp).with_witness_version(version).chars() {
for c in iter.with_checksum::<Bech32m>(&hrp).with_witness_version(version).chars() {
w.write_all(&[c.to_ascii_uppercase() as u8])?;
}
}
Expand Down Expand Up @@ -357,7 +357,7 @@ mod tests {

for address in addresses {
let (hrp, version, program) = decode(address).expect("failed to decode valid address");
let encoded = encode(&hrp, version, &program).expect("failed to encode address");
let encoded = encode(hrp, version, &program).expect("failed to encode address");
assert_eq!(encoded, address);
}
}
Expand All @@ -373,7 +373,7 @@ mod tests {
fn encode_lower_to_fmt() {
let program = witness_program();
let mut address = String::new();
encode_to_fmt_unchecked(&mut address, &hrp::BC, VERSION_0, &program)
encode_to_fmt_unchecked(&mut address, hrp::BC, VERSION_0, &program)
.expect("failed to encode address to QR code");

let want = "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4";
Expand All @@ -384,7 +384,7 @@ mod tests {
fn encode_upper_to_fmt() {
let program = witness_program();
let mut address = String::new();
encode_upper_to_fmt_unchecked(&mut address, &hrp::BC, VERSION_0, &program)
encode_upper_to_fmt_unchecked(&mut address, hrp::BC, VERSION_0, &program)
.expect("failed to encode address to QR code");

let want = "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3T4";
Expand All @@ -396,7 +396,7 @@ mod tests {
fn encode_lower_to_writer() {
let program = witness_program();
let mut buf = Vec::new();
encode_lower_to_writer_unchecked(&mut buf, &hrp::BC, VERSION_0, &program)
encode_lower_to_writer_unchecked(&mut buf, hrp::BC, VERSION_0, &program)
.expect("failed to encode");

let address = std::str::from_utf8(&buf).expect("ascii is valid utf8");
Expand All @@ -409,7 +409,7 @@ mod tests {
fn encode_upper_to_writer() {
let program = witness_program();
let mut buf = Vec::new();
encode_upper_to_writer_unchecked(&mut buf, &hrp::BC, VERSION_0, &program)
encode_upper_to_writer_unchecked(&mut buf, hrp::BC, VERSION_0, &program)
.expect("failed to encode");

let address = std::str::from_utf8(&buf).expect("ascii is valid utf8");
Expand All @@ -423,7 +423,7 @@ mod tests {
let program = witness_program();
let mut buf = Vec::new();
let hrp = Hrp::parse_unchecked("BC");
encode_lower_to_writer_unchecked(&mut buf, &hrp, VERSION_0, &program)
encode_lower_to_writer_unchecked(&mut buf, hrp, VERSION_0, &program)
.expect("failed to encode");

let address = std::str::from_utf8(&buf).expect("ascii is valid utf8");
Expand Down
2 changes: 1 addition & 1 deletion tests/bip_173_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ macro_rules! check_valid_address_roundtrip {
// tested by the test vectors. However when BIP-350 came into effect only witness
// version 0 uses bech32 (and this is enforced by encode/decode).
if let Ok((hrp, bech32::Fe32::Q, program)) = bech32::segwit::decode($addr) {
let encoded = bech32::segwit::encode_v0(&hrp, &program).expect("failed to encode address");
let encoded = bech32::segwit::encode_v0(hrp, &program).expect("failed to encode address");
// The bips specifically say that encoder should output lowercase characters so we uppercase manually.
if encoded != $addr {
let got = encoded.to_uppercase();
Expand Down
2 changes: 1 addition & 1 deletion tests/bip_350_test_vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ macro_rules! check_valid_address_roundtrip {
#[cfg(feature = "alloc")]
fn $test_name() {
let (hrp, version, program) = bech32::segwit::decode($addr).expect("failed to decode valid address");
let encoded = bech32::segwit::encode(&hrp, version, &program).expect("failed to encode address");
let encoded = bech32::segwit::encode(hrp, version, &program).expect("failed to encode address");

// The bips specifically say that encoder should output lowercase characters so we uppercase manually.
if encoded != $addr {
Expand Down

0 comments on commit 7207926

Please sign in to comment.