From 84f02921b7b5a617959137c7c9a136e3c1d88f61 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Oct 2022 17:25:17 +0000 Subject: [PATCH 1/3] checksum: pull computation apart into an engine This will let us avoid some allocations when formatting descriptors, and make it simpler to selectively append a checksum. --- src/descriptor/checksum.rs | 137 +++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 35 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 8ecced007..93d0418dc 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -3,13 +3,15 @@ //! 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; use crate::prelude::*; use crate::Error; const INPUT_CHARSET: &str = "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ "; -const CHECKSUM_CHARSET: &str = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; +const CHECKSUM_CHARSET: &[u8] = b"qpzry9x8gf2tvdw0s3jn54khce6mua7l"; fn poly_mod(mut c: u64, val: u64) -> u64 { let c0 = c >> 35; @@ -39,40 +41,9 @@ fn poly_mod(mut c: u64, val: u64) -> u64 { /// descriptor string is syntactically correct or not. /// This only computes the checksum pub fn desc_checksum(desc: &str) -> Result { - let mut c = 1; - let mut cls = 0; - let mut clscount = 0; - - for ch in desc.chars() { - let pos = INPUT_CHARSET.find(ch).ok_or_else(|| { - Error::BadDescriptor(format!("Invalid character in checksum: '{}'", ch)) - })? as u64; - c = poly_mod(c, pos & 31); - cls = cls * 3 + (pos >> 5); - clscount += 1; - if clscount == 3 { - c = poly_mod(c, cls); - cls = 0; - clscount = 0; - } - } - if clscount > 0 { - c = poly_mod(c, cls); - } - (0..8).for_each(|_| c = poly_mod(c, 0)); - c ^= 1; - - let mut chars = Vec::with_capacity(8); - for j in 0..8 { - chars.push( - CHECKSUM_CHARSET - .chars() - .nth(((c >> (5 * (7 - j))) & 31) as usize) - .unwrap(), - ); - } - - Ok(String::from_iter(chars)) + let mut eng = Engine::new(); + eng.input(desc)?; + Ok(eng.checksum()) } /// Helper function for FromStr for various @@ -99,6 +70,102 @@ pub(super) fn verify_checksum(s: &str) -> Result<&str, Error> { } Ok(desc_str) } + +/// An engine to compute a checksum from a string +pub struct Engine { + c: u64, + cls: u64, + clscount: u64, +} + +impl Engine { + /// Construct an engine with no input + pub fn new() -> Self { + Engine { + c: 1, + cls: 0, + clscount: 0, + } + } + + /// Checksum some data + /// + /// If this function returns an error, the `Engine` will be left in an indeterminate + /// state! It is safe to continue feeding it data but the result will not be meaningful. + pub fn input(&mut self, s: &str) -> Result<(), Error> { + for ch in s.chars() { + let pos = INPUT_CHARSET.find(ch).ok_or_else(|| { + Error::BadDescriptor(format!("Invalid character in checksum: '{}'", ch)) + })? as u64; + self.c = poly_mod(self.c, pos & 31); + self.cls = self.cls * 3 + (pos >> 5); + self.clscount += 1; + if self.clscount == 3 { + self.c = poly_mod(self.c, self.cls); + self.cls = 0; + self.clscount = 0; + } + } + Ok(()) + } + + /// Obtain the checksum of all the data thus-far fed to the engine + pub fn checksum_chars(&mut self) -> [char; 8] { + if self.clscount > 0 { + self.c = poly_mod(self.c, self.cls); + } + (0..8).for_each(|_| self.c = poly_mod(self.c, 0)); + self.c ^= 1; + + let mut chars = [0 as char; 8]; + for j in 0..8 { + chars[j] = CHECKSUM_CHARSET[((self.c >> (5 * (7 - j))) & 31) as usize] as char; + } + chars + } + + /// Obtain the checksum of all the data thus-far fed to the engine + pub fn checksum(&mut self) -> String { + String::from_iter(self.checksum_chars()) + } +} + +/// A wrapper around a `fmt::Formatter` which provides checksumming ability +pub struct Formatter<'f, 'a> { + fmt: &'f mut fmt::Formatter<'a>, + eng: Engine, +} + +impl<'f, 'a> Formatter<'f, 'a> { + /// Contruct a new `Formatter`, wrapping a given `fmt::Formatter` + pub fn new(f: &'f mut fmt::Formatter<'a>) -> Self { + Formatter { + fmt: f, + eng: Engine::new(), + } + } + + pub fn write_checksum(&mut self) -> fmt::Result { + use fmt::Write; + self.fmt.write_char('#')?; + for ch in self.eng.checksum_chars() { + self.fmt.write_char(ch)?; + } + Ok(()) + } +} + +impl<'f, 'a> fmt::Write for Formatter<'f, 'a> { + fn write_str(&mut self, s: &str) -> fmt::Result { + self.fmt.write_str(s)?; + if self.eng.input(s).is_ok() { + Ok(()) + } else { + Err(fmt::Error) + } + } +} + #[cfg(test)] mod test { use core::str; From 7577e8ceda6d7c83be4f4422ec9be39c11f9050c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Oct 2022 17:50:11 +0000 Subject: [PATCH 2/3] 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 --- src/descriptor/bare.rs | 16 +++--- src/descriptor/checksum.rs | 10 +++- src/descriptor/mod.rs | 104 ++++++++++++++++++++++++++++++++----- src/descriptor/segwitv0.rs | 28 +++++----- src/descriptor/sh.rs | 19 +++---- src/descriptor/tr.rs | 21 ++++---- 6 files changed, 145 insertions(+), 53 deletions(-) diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 2495f16b1..65fa01a66 100644 --- a/src/descriptor/bare.rs +++ b/src/descriptor/bare.rs @@ -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}; @@ -132,9 +132,10 @@ impl fmt::Debug for Bare { impl fmt::Display for Bare { 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() } } @@ -285,9 +286,10 @@ impl fmt::Debug for Pkh { impl fmt::Display for Pkh { 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() } } diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 93d0418dc..c26e4e09b 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -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; @@ -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('#')?; @@ -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> { diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 6b8b2c87d..31a30b2e3 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -808,12 +808,12 @@ impl_from_str!( impl fmt::Debug for Descriptor { 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), } } } @@ -821,12 +821,12 @@ impl fmt::Debug for Descriptor { impl fmt::Display for Descriptor { 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), } } } @@ -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)", + ); + } } diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 118a859f9..28d65b6f8 100644 --- a/src/descriptor/segwitv0.rs +++ b/src/descriptor/segwitv0.rs @@ -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}; @@ -68,11 +68,9 @@ impl Wsh { } /// 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. @@ -229,9 +227,13 @@ impl fmt::Debug for Wsh { impl fmt::Display for Wsh { 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() } } @@ -307,8 +309,9 @@ impl Wpkh { } /// 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. @@ -398,9 +401,10 @@ impl fmt::Debug for Wpkh { impl fmt::Display for Wpkh { 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() } } diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 0c60c7b4b..fd1f29d6f 100644 --- a/src/descriptor/sh.rs +++ b/src/descriptor/sh.rs @@ -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; @@ -79,14 +79,15 @@ impl fmt::Debug for Sh { impl fmt::Display for Sh { 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() } } diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 1daaea51e..3607461b9 100644 --- a/src/descriptor/tr.rs +++ b/src/descriptor/tr.rs @@ -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; @@ -177,14 +177,6 @@ impl Tr { } } - 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 @@ -463,9 +455,14 @@ impl fmt::Debug for Tr { impl fmt::Display for Tr { 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() } } From 7f5bbdf379db2357dd6449e991805dcceb93106a Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Oct 2022 18:04:02 +0000 Subject: [PATCH 3/3] fixes for 1.41.0 --- src/descriptor/checksum.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index c26e4e09b..16fbec7c0 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -125,7 +125,7 @@ impl Engine { /// Obtain the checksum of all the data thus-far fed to the engine pub fn checksum(&mut self) -> String { - String::from_iter(self.checksum_chars()) + String::from_iter(self.checksum_chars().iter().copied()) } } @@ -148,7 +148,7 @@ impl<'f, 'a> Formatter<'f, 'a> { pub fn write_checksum(&mut self) -> fmt::Result { use fmt::Write; self.fmt.write_char('#')?; - for ch in self.eng.checksum_chars() { + for ch in self.eng.checksum_chars().iter().copied() { self.fmt.write_char(ch)?; } Ok(())