Skip to content

Commit 7577e8c

Browse files
apoelstraapoelstra
committed
checksum: use wrapper around fmt::Formatter for all descriptor types
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 #477
1 parent 84f0292 commit 7577e8c

File tree

6 files changed

+145
-53
lines changed

6 files changed

+145
-53
lines changed

‎src/descriptor/bare.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use core::fmt;
2323
use bitcoin::blockdata::script;
2424
use bitcoin::{Address, Network, Script};
2525

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

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

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

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

‎src/descriptor/checksum.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
//! This module contains a re-implementation of the function used by Bitcoin Core to calculate the
44
//! checksum of a descriptor
55
6-
#![allow(dead_code)] // will be removed in next commit
76
use core::fmt;
87
use core::iter::FromIterator;
98

@@ -145,6 +144,7 @@ impl<'f, 'a> Formatter<'f, 'a> {
145144
}
146145
}
147146

147+
/// Writes the checksum into the underlying `fmt::Formatter`
148148
pub fn write_checksum(&mut self) -> fmt::Result {
149149
use fmt::Write;
150150
self.fmt.write_char('#')?;
@@ -153,6 +153,14 @@ impl<'f, 'a> Formatter<'f, 'a> {
153153
}
154154
Ok(())
155155
}
156+
157+
/// Writes the checksum into the underlying `fmt::Formatter`, unless it has "alternate" display on
158+
pub fn write_checksum_if_not_alt(&mut self) -> fmt::Result {
159+
if !self.fmt.alternate() {
160+
self.write_checksum()?;
161+
}
162+
Ok(())
163+
}
156164
}
157165

158166
impl<'f, 'a> fmt::Write for Formatter<'f, 'a> {

‎src/descriptor/mod.rs‎

Lines changed: 92 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -808,25 +808,25 @@ impl_from_str!(
808808
impl<Pk: MiniscriptKey> fmt::Debug for Descriptor<Pk> {
809809
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
810810
match *self {
811-
Descriptor::Bare(ref sub) => write!(f, "{:?}", sub),
812-
Descriptor::Pkh(ref pkh) => write!(f, "{:?}", pkh),
813-
Descriptor::Wpkh(ref wpkh) => write!(f, "{:?}", wpkh),
814-
Descriptor::Sh(ref sub) => write!(f, "{:?}", sub),
815-
Descriptor::Wsh(ref sub) => write!(f, "{:?}", sub),
816-
Descriptor::Tr(ref tr) => write!(f, "{:?}", tr),
811+
Descriptor::Bare(ref sub) => fmt::Debug::fmt(sub, f),
812+
Descriptor::Pkh(ref pkh) => fmt::Debug::fmt(pkh, f),
813+
Descriptor::Wpkh(ref wpkh) => fmt::Debug::fmt(wpkh, f),
814+
Descriptor::Sh(ref sub) => fmt::Debug::fmt(sub, f),
815+
Descriptor::Wsh(ref sub) => fmt::Debug::fmt(sub, f),
816+
Descriptor::Tr(ref tr) => fmt::Debug::fmt(tr, f),
817817
}
818818
}
819819
}
820820

821821
impl<Pk: MiniscriptKey> fmt::Display for Descriptor<Pk> {
822822
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
823823
match *self {
824-
Descriptor::Bare(ref sub) => write!(f, "{}", sub),
825-
Descriptor::Pkh(ref pkh) => write!(f, "{}", pkh),
826-
Descriptor::Wpkh(ref wpkh) => write!(f, "{}", wpkh),
827-
Descriptor::Sh(ref sub) => write!(f, "{}", sub),
828-
Descriptor::Wsh(ref sub) => write!(f, "{}", sub),
829-
Descriptor::Tr(ref tr) => write!(f, "{}", tr),
824+
Descriptor::Bare(ref sub) => fmt::Display::fmt(sub, f),
825+
Descriptor::Pkh(ref pkh) => fmt::Display::fmt(pkh, f),
826+
Descriptor::Wpkh(ref wpkh) => fmt::Display::fmt(wpkh, f),
827+
Descriptor::Sh(ref sub) => fmt::Display::fmt(sub, f),
828+
Descriptor::Wsh(ref sub) => fmt::Display::fmt(sub, f),
829+
Descriptor::Tr(ref tr) => fmt::Display::fmt(tr, f),
830830
}
831831
}
832832
}
@@ -1757,4 +1757,84 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
17571757
Ok(Some((1, expected_concrete)))
17581758
);
17591759
}
1760+
1761+
#[test]
1762+
fn display_alternate() {
1763+
let bare = StdDescriptor::from_str(
1764+
"pk(020000000000000000000000000000000000000000000000000000000000000002)",
1765+
)
1766+
.unwrap();
1767+
assert_eq!(
1768+
format!("{}", bare),
1769+
"pk(020000000000000000000000000000000000000000000000000000000000000002)#7yxkn84h",
1770+
);
1771+
assert_eq!(
1772+
format!("{:#}", bare),
1773+
"pk(020000000000000000000000000000000000000000000000000000000000000002)",
1774+
);
1775+
1776+
let pkh = StdDescriptor::from_str(
1777+
"pkh(020000000000000000000000000000000000000000000000000000000000000002)",
1778+
)
1779+
.unwrap();
1780+
assert_eq!(
1781+
format!("{}", pkh),
1782+
"pkh(020000000000000000000000000000000000000000000000000000000000000002)#ma7nspkf",
1783+
);
1784+
assert_eq!(
1785+
format!("{:#}", pkh),
1786+
"pkh(020000000000000000000000000000000000000000000000000000000000000002)",
1787+
);
1788+
1789+
let wpkh = StdDescriptor::from_str(
1790+
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)",
1791+
)
1792+
.unwrap();
1793+
assert_eq!(
1794+
format!("{}", wpkh),
1795+
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)#d3xz2xye",
1796+
);
1797+
assert_eq!(
1798+
format!("{:#}", wpkh),
1799+
"wpkh(020000000000000000000000000000000000000000000000000000000000000002)",
1800+
);
1801+
1802+
let shwpkh = StdDescriptor::from_str(
1803+
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))",
1804+
)
1805+
.unwrap();
1806+
assert_eq!(
1807+
format!("{}", shwpkh),
1808+
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))#45zpjtet",
1809+
);
1810+
assert_eq!(
1811+
format!("{:#}", shwpkh),
1812+
"sh(wpkh(020000000000000000000000000000000000000000000000000000000000000002))",
1813+
);
1814+
1815+
let wsh = StdDescriptor::from_str("wsh(1)").unwrap();
1816+
assert_eq!(format!("{}", wsh), "wsh(1)#mrg7xj7p");
1817+
assert_eq!(format!("{:#}", wsh), "wsh(1)");
1818+
1819+
let sh = StdDescriptor::from_str("sh(1)").unwrap();
1820+
assert_eq!(format!("{}", sh), "sh(1)#l8r75ggs");
1821+
assert_eq!(format!("{:#}", sh), "sh(1)");
1822+
1823+
let shwsh = StdDescriptor::from_str("sh(wsh(1))").unwrap();
1824+
assert_eq!(format!("{}", shwsh), "sh(wsh(1))#hcyfl07f");
1825+
assert_eq!(format!("{:#}", shwsh), "sh(wsh(1))");
1826+
1827+
let tr = StdDescriptor::from_str(
1828+
"tr(020000000000000000000000000000000000000000000000000000000000000002)",
1829+
)
1830+
.unwrap();
1831+
assert_eq!(
1832+
format!("{}", tr),
1833+
"tr(020000000000000000000000000000000000000000000000000000000000000002)#8hc7wq5h",
1834+
);
1835+
assert_eq!(
1836+
format!("{:#}", tr),
1837+
"tr(020000000000000000000000000000000000000000000000000000000000000002)",
1838+
);
1839+
}
17601840
}

‎src/descriptor/segwitv0.rs‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use core::fmt;
2020

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

23-
use super::checksum::{desc_checksum, verify_checksum};
23+
use super::checksum::{self, verify_checksum};
2424
use super::SortedMultiVec;
2525
use crate::expression::{self, FromTree};
2626
use crate::miniscript::context::{ScriptContext, ScriptContextError};
@@ -68,11 +68,9 @@ impl<Pk: MiniscriptKey> Wsh<Pk> {
6868
}
6969

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

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

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

@@ -307,8 +309,9 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> {
307309
}
308310

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

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

399402
impl<Pk: MiniscriptKey> fmt::Display for Wpkh<Pk> {
400403
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
401-
let desc = self.to_string_no_checksum();
402-
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
403-
write!(f, "{}#{}", &desc, &checksum)
404+
use fmt::Write;
405+
let mut wrapped_f = checksum::Formatter::new(f);
406+
write!(wrapped_f, "wpkh({})", self.pk)?;
407+
wrapped_f.write_checksum_if_not_alt()
404408
}
405409
}
406410

‎src/descriptor/sh.rs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use core::fmt;
2323
use bitcoin::blockdata::script;
2424
use bitcoin::{Address, Network, Script};
2525

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

8080
impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
8181
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
82-
let desc = match self.inner {
83-
ShInner::Wsh(ref wsh) => format!("sh({})", wsh.to_string_no_checksum()),
84-
ShInner::Wpkh(ref pk) => format!("sh({})", pk.to_string_no_checksum()),
85-
ShInner::SortedMulti(ref smv) => format!("sh({})", smv),
86-
ShInner::Ms(ref ms) => format!("sh({})", ms),
87-
};
88-
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
89-
write!(f, "{}#{}", &desc, &checksum)
82+
use fmt::Write;
83+
let mut wrapped_f = checksum::Formatter::new(f);
84+
match self.inner {
85+
ShInner::Wsh(ref wsh) => write!(wrapped_f, "sh({:#})", wsh)?,
86+
ShInner::Wpkh(ref pk) => write!(wrapped_f, "sh({:#})", pk)?,
87+
ShInner::SortedMulti(ref smv) => write!(wrapped_f, "sh({})", smv)?,
88+
ShInner::Ms(ref ms) => write!(wrapped_f, "sh({})", ms)?,
89+
}
90+
wrapped_f.write_checksum_if_not_alt()
9091
}
9192
}
9293

‎src/descriptor/tr.rs‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use bitcoin::util::taproot::{
1111
use bitcoin::{secp256k1, Address, Network, Script};
1212
use sync::Arc;
1313

14-
use super::checksum::{desc_checksum, verify_checksum};
14+
use super::checksum::{self, verify_checksum};
1515
use crate::expression::{self, FromTree};
1616
use crate::miniscript::Miniscript;
1717
use crate::policy::semantic::Policy;
@@ -177,14 +177,6 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
177177
}
178178
}
179179

180-
fn to_string_no_checksum(&self) -> String {
181-
let key = &self.internal_key;
182-
match self.tree {
183-
Some(ref s) => format!("tr({},{})", key, s),
184-
None => format!("tr({})", key),
185-
}
186-
}
187-
188180
/// Obtain the internal key of [`Tr`] descriptor
189181
pub fn internal_key(&self) -> &Pk {
190182
&self.internal_key
@@ -463,9 +455,14 @@ impl<Pk: MiniscriptKey> fmt::Debug for Tr<Pk> {
463455

464456
impl<Pk: MiniscriptKey> fmt::Display for Tr<Pk> {
465457
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
466-
let desc = self.to_string_no_checksum();
467-
let checksum = desc_checksum(&desc).map_err(|_| fmt::Error)?;
468-
write!(f, "{}#{}", &desc, &checksum)
458+
use fmt::Write;
459+
let mut wrapped_f = checksum::Formatter::new(f);
460+
let key = &self.internal_key;
461+
match self.tree {
462+
Some(ref s) => write!(wrapped_f, "tr({},{})", key, s)?,
463+
None => write!(wrapped_f, "tr({})", key)?,
464+
}
465+
wrapped_f.write_checksum_if_not_alt()
469466
}
470467
}
471468

0 commit comments

Comments
 (0)