Skip to content

Commit

Permalink
checksum: use wrapper around fmt::Formatter for all descriptor types
Browse files Browse the repository at this point in the history
This allows us to remove the checksum with {:#}, and avoid extra
allocations in all cases. Deprecates the old to_string_no_checksum()
methods, which were inefficient and (in the case of Taproot) not
even exported.

Fixes rust-bitcoin#477
  • Loading branch information
apoelstra authored and joemphilips committed Oct 31, 2022
1 parent 7b193ca commit e664ad4
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 53 deletions.
16 changes: 9 additions & 7 deletions src/descriptor/bare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use core::fmt;
use bitcoin::blockdata::script;
use bitcoin::{Address, Network, Script};

use super::checksum::{desc_checksum, verify_checksum};
use super::checksum::{self, verify_checksum};
use crate::expression::{self, FromTree};
use crate::miniscript::context::ScriptContext;
use crate::policy::{semantic, Liftable};
Expand Down Expand Up @@ -132,9 +132,10 @@ impl<Pk: MiniscriptKey> fmt::Debug for Bare<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Bare<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = format!("{}", self.ms);
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
write!(wrapped_f, "{}", self.ms)?;
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down Expand Up @@ -285,9 +286,10 @@ impl<Pk: MiniscriptKey> fmt::Debug for Pkh<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Pkh<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = format!("pkh({})", self.pk);
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
write!(wrapped_f, "pkh({})", self.pk)?;
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/descriptor/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//! This module contains a re-implementation of the function used by Bitcoin Core to calculate the
//! checksum of a descriptor

#![allow(dead_code)] // will be removed in next commit
use core::fmt;
use core::iter::FromIterator;

Expand Down Expand Up @@ -145,6 +144,7 @@ impl<'f, 'a> Formatter<'f, 'a> {
}
}

/// Writes the checksum into the underlying `fmt::Formatter`
pub fn write_checksum(&mut self) -> fmt::Result {
use fmt::Write;
self.fmt.write_char('#')?;
Expand All @@ -153,6 +153,14 @@ impl<'f, 'a> Formatter<'f, 'a> {
}
Ok(())
}

/// Writes the checksum into the underlying `fmt::Formatter`, unless it has "alternate" display on
pub fn write_checksum_if_not_alt(&mut self) -> fmt::Result {
if !self.fmt.alternate() {
self.write_checksum()?;
}
Ok(())
}
}

impl<'f, 'a> fmt::Write for Formatter<'f, 'a> {
Expand Down
104 changes: 92 additions & 12 deletions src/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,25 +808,25 @@ impl_from_str!(
impl<Pk: MiniscriptKey> fmt::Debug for Descriptor<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Descriptor::Bare(ref sub) => write!(f, "{:?}", sub),
Descriptor::Pkh(ref pkh) => write!(f, "{:?}", pkh),
Descriptor::Wpkh(ref wpkh) => write!(f, "{:?}", wpkh),
Descriptor::Sh(ref sub) => write!(f, "{:?}", sub),
Descriptor::Wsh(ref sub) => write!(f, "{:?}", sub),
Descriptor::Tr(ref tr) => write!(f, "{:?}", tr),
Descriptor::Bare(ref sub) => fmt::Debug::fmt(sub, f),
Descriptor::Pkh(ref pkh) => fmt::Debug::fmt(pkh, f),
Descriptor::Wpkh(ref wpkh) => fmt::Debug::fmt(wpkh, f),
Descriptor::Sh(ref sub) => fmt::Debug::fmt(sub, f),
Descriptor::Wsh(ref sub) => fmt::Debug::fmt(sub, f),
Descriptor::Tr(ref tr) => fmt::Debug::fmt(tr, f),
}
}
}

impl<Pk: MiniscriptKey> fmt::Display for Descriptor<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Descriptor::Bare(ref sub) => write!(f, "{}", sub),
Descriptor::Pkh(ref pkh) => write!(f, "{}", pkh),
Descriptor::Wpkh(ref wpkh) => write!(f, "{}", wpkh),
Descriptor::Sh(ref sub) => write!(f, "{}", sub),
Descriptor::Wsh(ref sub) => write!(f, "{}", sub),
Descriptor::Tr(ref tr) => write!(f, "{}", tr),
Descriptor::Bare(ref sub) => fmt::Display::fmt(sub, f),
Descriptor::Pkh(ref pkh) => fmt::Display::fmt(pkh, f),
Descriptor::Wpkh(ref wpkh) => fmt::Display::fmt(wpkh, f),
Descriptor::Sh(ref sub) => fmt::Display::fmt(sub, f),
Descriptor::Wsh(ref sub) => fmt::Display::fmt(sub, f),
Descriptor::Tr(ref tr) => fmt::Display::fmt(tr, f),
}
}
}
Expand Down Expand Up @@ -1757,4 +1757,84 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
Ok(Some((1, expected_concrete)))
);
}

#[test]
fn display_alternate() {
let bare = StdDescriptor::from_str(
"pk(020000000000000000000000000000000000000000000000000000000000000002)",
)
.unwrap();
assert_eq!(
format!("{}", bare),
"pk(020000000000000000000000000000000000000000000000000000000000000002)#7yxkn84h",
);
assert_eq!(
format!("{:#}", bare),
"pk(020000000000000000000000000000000000000000000000000000000000000002)",
);

let pkh = StdDescriptor::from_str(
"pkh(020000000000000000000000000000000000000000000000000000000000000002)",
)
.unwrap();
assert_eq!(
format!("{}", pkh),
"pkh(020000000000000000000000000000000000000000000000000000000000000002)#ma7nspkf",
);
assert_eq!(
format!("{:#}", pkh),
"pkh(020000000000000000000000000000000000000000000000000000000000000002)",
);

let wpkh = StdDescriptor::from_str(
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)",
)
.unwrap();
assert_eq!(
format!("{}", wpkh),
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)#d3xz2xye",
);
assert_eq!(
format!("{:#}", wpkh),
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)",
);

let shwpkh = StdDescriptor::from_str(
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))",
)
.unwrap();
assert_eq!(
format!("{}", shwpkh),
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))#45zpjtet",
);
assert_eq!(
format!("{:#}", shwpkh),
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))",
);

let wsh = StdDescriptor::from_str("wsh(1)").unwrap();
assert_eq!(format!("{}", wsh), "wsh(1)#mrg7xj7p");
assert_eq!(format!("{:#}", wsh), "wsh(1)");

let sh = StdDescriptor::from_str("sh(1)").unwrap();
assert_eq!(format!("{}", sh), "sh(1)#l8r75ggs");
assert_eq!(format!("{:#}", sh), "sh(1)");

let shwsh = StdDescriptor::from_str("sh(wsh(1))").unwrap();
assert_eq!(format!("{}", shwsh), "sh(wsh(1))#hcyfl07f");
assert_eq!(format!("{:#}", shwsh), "sh(wsh(1))");

let tr = StdDescriptor::from_str(
"tr(020000000000000000000000000000000000000000000000000000000000000002)",
)
.unwrap();
assert_eq!(
format!("{}", tr),
"tr(020000000000000000000000000000000000000000000000000000000000000002)#8hc7wq5h",
);
assert_eq!(
format!("{:#}", tr),
"tr(020000000000000000000000000000000000000000000000000000000000000002)",
);
}
}
28 changes: 16 additions & 12 deletions src/descriptor/segwitv0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use core::fmt;

use bitcoin::{self, Address, Network, Script};

use super::checksum::{desc_checksum, verify_checksum};
use super::checksum::{self, verify_checksum};
use super::SortedMultiVec;
use crate::expression::{self, FromTree};
use crate::miniscript::context::{ScriptContext, ScriptContextError};
Expand Down Expand Up @@ -68,11 +68,9 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
}

/// Get the descriptor without the checksum
#[deprecated(since = "8.0.0", note = "use format!(\"{:#}\") instead")]
pub fn to_string_no_checksum(&self) -> String {
match self.inner {
WshInner::SortedMulti(ref smv) => format!("wsh({})", smv),
WshInner::Ms(ref ms) => format!("wsh({})", ms),
}
format!("{:#}", self)
}

/// Checks whether the descriptor is safe.
Expand Down Expand Up @@ -229,9 +227,13 @@ impl<Pk: MiniscriptKey> fmt::Debug for Wsh<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Wsh<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = self.to_string_no_checksum();
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
match self.inner {
WshInner::SortedMulti(ref smv) => write!(wrapped_f, "wsh({})", smv)?,
WshInner::Ms(ref ms) => write!(wrapped_f, "wsh({})", ms)?,
}
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down Expand Up @@ -307,8 +309,9 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
}

/// Get the descriptor without the checksum
#[deprecated(since = "8.0.0", note = "use format!(\"{:#}\") instead")]
pub fn to_string_no_checksum(&self) -> String {
format!("wpkh({})", self.pk)
format!("{:#}", self)
}

/// Checks whether the descriptor is safe.
Expand Down Expand Up @@ -398,9 +401,10 @@ impl<Pk: MiniscriptKey> fmt::Debug for Wpkh<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Wpkh<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = self.to_string_no_checksum();
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
write!(wrapped_f, "wpkh({})", self.pk)?;
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down
19 changes: 10 additions & 9 deletions src/descriptor/sh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use core::fmt;
use bitcoin::blockdata::script;
use bitcoin::{Address, Network, Script};

use super::checksum::{desc_checksum, verify_checksum};
use super::checksum::{self, verify_checksum};
use super::{SortedMultiVec, Wpkh, Wsh};
use crate::expression::{self, FromTree};
use crate::miniscript::context::ScriptContext;
Expand Down Expand Up @@ -79,14 +79,15 @@ impl<Pk: MiniscriptKey> fmt::Debug for Sh<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = match self.inner {
ShInner::Wsh(ref wsh) => format!("sh({})", wsh.to_string_no_checksum()),
ShInner::Wpkh(ref pk) => format!("sh({})", pk.to_string_no_checksum()),
ShInner::SortedMulti(ref smv) => format!("sh({})", smv),
ShInner::Ms(ref ms) => format!("sh({})", ms),
};
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
match self.inner {
ShInner::Wsh(ref wsh) => write!(wrapped_f, "sh({:#})", wsh)?,
ShInner::Wpkh(ref pk) => write!(wrapped_f, "sh({:#})", pk)?,
ShInner::SortedMulti(ref smv) => write!(wrapped_f, "sh({})", smv)?,
ShInner::Ms(ref ms) => write!(wrapped_f, "sh({})", ms)?,
}
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down
21 changes: 9 additions & 12 deletions src/descriptor/tr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use bitcoin::util::taproot::{
use bitcoin::{secp256k1, Address, Network, Script};
use sync::Arc;

use super::checksum::{desc_checksum, verify_checksum};
use super::checksum::{self, verify_checksum};
use crate::expression::{self, FromTree};
use crate::miniscript::Miniscript;
use crate::policy::semantic::Policy;
Expand Down Expand Up @@ -177,14 +177,6 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
}
}

fn to_string_no_checksum(&self) -> String {
let key = &self.internal_key;
match self.tree {
Some(ref s) => format!("tr({},{})", key, s),
None => format!("tr({})", key),
}
}

/// Obtain the internal key of [`Tr`] descriptor
pub fn internal_key(&self) -> &Pk {
&self.internal_key
Expand Down Expand Up @@ -463,9 +455,14 @@ impl<Pk: MiniscriptKey> fmt::Debug for Tr<Pk> {

impl<Pk: MiniscriptKey> fmt::Display for Tr<Pk> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let desc = self.to_string_no_checksum();
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
write!(f, "{}#{}", &desc, &checksum)
use fmt::Write;
let mut wrapped_f = checksum::Formatter::new(f);
let key = &self.internal_key;
match self.tree {
Some(ref s) => write!(wrapped_f, "tr({},{})", key, s)?,
None => write!(wrapped_f, "tr({})", key)?,
}
wrapped_f.write_checksum_if_not_alt()
}
}

Expand Down

0 comments on commit e664ad4

Please sign in to comment.