From 67d6ff78ad044ab0048e796e376529b6d2be9191 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 18 Aug 2024 13:40:38 +0000 Subject: [PATCH 1/5] iter: introduce Ternary variant It's lazy and takes some extra allocations to use Nary for the andor combinator; introduce a dedicated Ternary variant for this. This also means that Nary will be exclusively used for thresholds, which should give us some more freedom to mess with it. --- src/iter/mod.rs | 4 ++-- src/iter/tree.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/iter/mod.rs b/src/iter/mod.rs index e3fb12a9d..52377b8ad 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -36,7 +36,7 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript Tree::Binary(left, right), - AndOr(ref a, ref b, ref c) => Tree::Nary(Arc::from([a.as_ref(), b, c])), + AndOr(ref a, ref b, ref c) => Tree::Ternary(a, b, c), Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()), } } @@ -62,7 +62,7 @@ impl TreeLike for Arc | OrC(ref left, ref right) | OrI(ref left, ref right) => Tree::Binary(Arc::clone(left), Arc::clone(right)), AndOr(ref a, ref b, ref c) => { - Tree::Nary(Arc::from([Arc::clone(a), Arc::clone(b), Arc::clone(c)])) + Tree::Ternary(Arc::clone(a), Arc::clone(b), Arc::clone(c)) } Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()), } diff --git a/src/iter/tree.rs b/src/iter/tree.rs index cc58a6cf7..784f40a7a 100644 --- a/src/iter/tree.rs +++ b/src/iter/tree.rs @@ -20,6 +20,8 @@ pub enum Tree { Unary(T), /// Combinator with two children. Binary(T, T), + /// Combinator with two children. + Ternary(T, T, T), /// Combinator with more than two children. Nary(Arc<[T]>), } @@ -45,6 +47,7 @@ pub trait TreeLike: Clone + Sized { Tree::Nullary => 0, Tree::Unary(..) => 1, Tree::Binary(..) => 2, + Tree::Ternary(..) => 3, Tree::Nary(children) => children.len(), } } @@ -58,6 +61,10 @@ pub trait TreeLike: Clone + Sized { (0, Tree::Binary(sub, _)) => Some(sub), (1, Tree::Binary(_, sub)) => Some(sub), (_, Tree::Binary(..)) => None, + (0, Tree::Ternary(sub, _, _)) => Some(sub), + (1, Tree::Ternary(_, sub, _)) => Some(sub), + (2, Tree::Ternary(_, _, sub)) => Some(sub), + (_, Tree::Ternary(..)) => None, (n, Tree::Nary(children)) => children.get(n).cloned(), } } @@ -210,6 +217,11 @@ impl Iterator for PreOrderIter { self.stack.push(right); self.stack.push(left); } + Tree::Ternary(a, b, c) => { + self.stack.push(c); + self.stack.push(b); + self.stack.push(a); + } Tree::Nary(children) => { self.stack.extend(children.iter().rev().cloned()); } From 47bed0c281aaf67cba19eb4b7feda86e464b29cb Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 18 Aug 2024 16:18:35 +0000 Subject: [PATCH 2/5] iter: get rid of Arc::clones and allocations for n-ary nodes This somewhat complicates implementation of the trait, but not by much, and eliminates a ton of refcount accesses and increments, and a ton of Arc::clones. As we will see, having an Arc<[T]> is somewhat restrictive in addition to requiring allocations. Later we will want to replace this with an enum that covers multiple kinds of arrays. I was unsure whether to name the new associated type NAry or Nary. It seems more natural to use NAry but the existing Tree::Nary variant uses the lowercase spelling so I stuck with that. (And similarly used nary_ for the method names rather than n_ary.) --- src/iter/mod.rs | 28 +++++++++++------- src/iter/tree.rs | 41 ++++++++++++++++++-------- src/policy/concrete.rs | 65 +++++++++++++++++++++++++++++++++++------- src/policy/semantic.rs | 28 ++++++++++++------ 4 files changed, 120 insertions(+), 42 deletions(-) diff --git a/src/iter/mod.rs b/src/iter/mod.rs index 52377b8ad..052675376 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -18,7 +18,12 @@ use crate::sync::Arc; use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal}; impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript { - fn as_node(&self) -> Tree { + type NaryChildren = &'a [Arc>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { Arc::as_ref(&tc[idx]) } + + fn as_node(&self) -> Tree { use Terminal::*; match self.node { PkK(..) | PkH(..) | RawPkH(..) | After(..) | Older(..) | Sha256(..) | Hash256(..) @@ -37,13 +42,18 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Miniscript Tree::Binary(left, right), AndOr(ref a, ref b, ref c) => Tree::Ternary(a, b, c), - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()), + Thresh(ref thresh) => Tree::Nary(thresh.data()), } } } -impl TreeLike for Arc> { - fn as_node(&self) -> Tree { +impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Arc> { + type NaryChildren = &'a [Arc>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] } + + fn as_node(&self) -> Tree { use Terminal::*; match self.node { PkK(..) | PkH(..) | RawPkH(..) | After(..) | Older(..) | Sha256(..) | Hash256(..) @@ -54,17 +64,15 @@ impl TreeLike for Arc | DupIf(ref sub) | Verify(ref sub) | NonZero(ref sub) - | ZeroNotEqual(ref sub) => Tree::Unary(Arc::clone(sub)), + | ZeroNotEqual(ref sub) => Tree::Unary(sub), AndV(ref left, ref right) | AndB(ref left, ref right) | OrB(ref left, ref right) | OrD(ref left, ref right) | OrC(ref left, ref right) - | OrI(ref left, ref right) => Tree::Binary(Arc::clone(left), Arc::clone(right)), - AndOr(ref a, ref b, ref c) => { - Tree::Ternary(Arc::clone(a), Arc::clone(b), Arc::clone(c)) - } - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()), + | OrI(ref left, ref right) => Tree::Binary(left, right), + AndOr(ref a, ref b, ref c) => Tree::Ternary(a, b, c), + Thresh(ref thresh) => Tree::Nary(thresh.data()), } } } diff --git a/src/iter/tree.rs b/src/iter/tree.rs index 784f40a7a..53208894d 100644 --- a/src/iter/tree.rs +++ b/src/iter/tree.rs @@ -7,13 +7,12 @@ //! use crate::prelude::*; -use crate::sync::Arc; /// Abstract node of a tree. /// /// Tracks the arity (out-degree) of a node, which is the only thing that /// is needed for iteration purposes. -pub enum Tree { +pub enum Tree { /// Combinator with no children. Nullary, /// Combinator with one child. @@ -23,7 +22,7 @@ pub enum Tree { /// Combinator with two children. Ternary(T, T, T), /// Combinator with more than two children. - Nary(Arc<[T]>), + Nary(NT), } /// A trait for any structure which has the shape of a Miniscript tree. @@ -32,14 +31,27 @@ pub enum Tree { /// rather than nodes themselves, because it provides algorithms that /// assume copying is cheap. /// -/// To implement this trait, you only need to implement the [`TreeLike::as_node`] -/// method, which will usually be very mechanical. Everything else is provided. -/// However, to avoid allocations, it may make sense to also implement -/// [`TreeLike::n_children`] and [`TreeLike::nth_child`] because the default -/// implementations will allocate vectors for n-ary nodes. +/// To implement this trait, you only need to implement the [`TreeLike::as_node`], +/// [`TreeLike::nary_len`] and `[TreeLike::nary_index'] methods, which should +/// be very mechanical. Everything else is provided. pub trait TreeLike: Clone + Sized { + /// An abstraction over the children of n-ary nodes. Typically when + /// implementing the trait for `&a T` this will be `&'a [T]`. + type NaryChildren: Clone; + + /// Accessor for the length of a [`Self::NaryChildren`]. + fn nary_len(tc: &Self::NaryChildren) -> usize; + + /// Accessor for a specific child of a [`Self::NaryChildren`]. + /// + /// # Panics + /// + /// May panic if asked for an element outside of the range + /// `0..Self::nary_len(&tc)`. + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self; + /// Interpret the node as an abstract node. - fn as_node(&self) -> Tree; + fn as_node(&self) -> Tree; /// Accessor for the number of children this node has. fn n_children(&self) -> usize { @@ -48,7 +60,7 @@ pub trait TreeLike: Clone + Sized { Tree::Unary(..) => 1, Tree::Binary(..) => 2, Tree::Ternary(..) => 3, - Tree::Nary(children) => children.len(), + Tree::Nary(ref children) => Self::nary_len(children), } } @@ -65,7 +77,10 @@ pub trait TreeLike: Clone + Sized { (1, Tree::Ternary(_, sub, _)) => Some(sub), (2, Tree::Ternary(_, _, sub)) => Some(sub), (_, Tree::Ternary(..)) => None, - (n, Tree::Nary(children)) => children.get(n).cloned(), + (n, Tree::Nary(children)) if n < Self::nary_len(&children) => { + Some(Self::nary_index(children, n)) + } + (_, Tree::Nary(..)) => None, } } @@ -223,7 +238,9 @@ impl Iterator for PreOrderIter { self.stack.push(a); } Tree::Nary(children) => { - self.stack.extend(children.iter().rev().cloned()); + for i in (0..T::nary_len(&children)).rev() { + self.stack.push(T::nary_index(children.clone(), i)); + } } } Some(top) diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index a07bf262a..d1b3b6353 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -594,7 +594,7 @@ impl Policy { }; match new_policy { Some(new_policy) => translated.push(Arc::new(new_policy)), - None => translated.push(Arc::clone(&data.node)), + None => translated.push(Arc::clone(data.node)), } } // Ok to unwrap because we know we processed at least one node. @@ -1044,30 +1044,73 @@ fn generate_combination( ret } +/// A type used within the iterator API to abstract over the two kinds +/// of n-ary nodes in a [`Policy`]. +/// +/// Generally speaking, users should never need to see or be aware of this +/// type. But when directly implementing iterators it may come in handy. +#[derive(Clone)] +pub enum TreeChildren<'a, Pk: MiniscriptKey> { + /// A conjunction or threshold node's children. + And(&'a [Arc>]), + /// A disjunction node's children. + Or(&'a [(usize, Arc>)]), +} + impl<'a, Pk: MiniscriptKey> TreeLike for &'a Policy { - fn as_node(&self) -> Tree { + type NaryChildren = TreeChildren<'a, Pk>; + + fn nary_len(tc: &Self::NaryChildren) -> usize { + match tc { + TreeChildren::And(sl) => sl.len(), + TreeChildren::Or(sl) => sl.len(), + } + } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { + match tc { + TreeChildren::And(sl) => &sl[idx], + TreeChildren::Or(sl) => &sl[idx].1, + } + } + + fn as_node(&self) -> Tree { use Policy::*; match *self { Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_) | Ripemd160(_) | Hash160(_) => Tree::Nullary, - And(ref subs) => Tree::Nary(subs.iter().map(Arc::as_ref).collect()), - Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| p.as_ref()).collect()), - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()), + And(ref subs) => Tree::Nary(TreeChildren::And(subs)), + Or(ref v) => Tree::Nary(TreeChildren::Or(v)), + Thresh(ref thresh) => Tree::Nary(TreeChildren::And(thresh.data())), } } } -impl TreeLike for Arc> { - fn as_node(&self) -> Tree { +impl<'a, Pk: MiniscriptKey> TreeLike for &'a Arc> { + type NaryChildren = TreeChildren<'a, Pk>; + + fn nary_len(tc: &Self::NaryChildren) -> usize { + match tc { + TreeChildren::And(sl) => sl.len(), + TreeChildren::Or(sl) => sl.len(), + } + } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { + match tc { + TreeChildren::And(sl) => &sl[idx], + TreeChildren::Or(sl) => &sl[idx].1, + } + } + + fn as_node(&self) -> Tree { use Policy::*; - match self.as_ref() { + match ***self { Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_) | Ripemd160(_) | Hash160(_) => Tree::Nullary, - And(ref subs) => Tree::Nary(subs.iter().map(Arc::clone).collect()), - Or(ref v) => Tree::Nary(v.iter().map(|(_, p)| Arc::clone(p)).collect()), - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()), + And(ref subs) => Tree::Nary(TreeChildren::And(subs)), + Or(ref v) => Tree::Nary(TreeChildren::Or(v)), + Thresh(ref thresh) => Tree::Nary(TreeChildren::And(thresh.data())), } } } diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index cd7d61002..d7258e6d1 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -499,7 +499,7 @@ impl Policy { }; match new_policy { Some(new_policy) => at_age.push(Arc::new(new_policy)), - None => at_age.push(Arc::clone(&data.node)), + None => at_age.push(Arc::clone(data.node)), } } // Unwrap is ok because we know we processed at least one node. @@ -531,7 +531,7 @@ impl Policy { }; match new_policy { Some(new_policy) => at_age.push(Arc::new(new_policy)), - None => at_age.push(Arc::clone(&data.node)), + None => at_age.push(Arc::clone(data.node)), } } // Unwrap is ok because we know we processed at least one node. @@ -609,7 +609,7 @@ impl Policy { }; match new_policy { Some(new_policy) => sorted.push(Arc::new(new_policy)), - None => sorted.push(Arc::clone(&data.node)), + None => sorted.push(Arc::clone(data.node)), } } // Unwrap is ok because we know we processed at least one node. @@ -620,25 +620,35 @@ impl Policy { } impl<'a, Pk: MiniscriptKey> TreeLike for &'a Policy { - fn as_node(&self) -> Tree { + type NaryChildren = &'a [Arc>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] } + + fn as_node(&self) -> Tree { use Policy::*; match *self { Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_) | Ripemd160(_) | Hash160(_) => Tree::Nullary, - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::as_ref).collect()), + Thresh(ref thresh) => Tree::Nary(thresh.data()), } } } -impl TreeLike for Arc> { - fn as_node(&self) -> Tree { +impl<'a, Pk: MiniscriptKey> TreeLike for &'a Arc> { + type NaryChildren = &'a [Arc>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { &tc[idx] } + + fn as_node(&self) -> Tree { use Policy::*; - match self.as_ref() { + match ***self { Unsatisfiable | Trivial | Key(_) | After(_) | Older(_) | Sha256(_) | Hash256(_) | Ripemd160(_) | Hash160(_) => Tree::Nullary, - Thresh(ref thresh) => Tree::Nary(thresh.iter().map(Arc::clone).collect()), + Thresh(ref thresh) => Tree::Nary(thresh.data()), } } } From cccaaec8bc189f5dcf8212c40d0f752ce90d9063 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 18 Aug 2024 20:10:29 +0000 Subject: [PATCH 3/5] miniscript: non-recursive Display implementation This uses the new iterator extensions to implement a `DisplayNode` wrapper around a `Terminal` which understands that pubkeys, threshold k values, etc., count as "children" for display purposes. We can then embed all the alias/wrapper logic in the `as_node` method and the new `fragment_name` method. The latter is generally useful and ought to be public; given a fragment, it outputs its name, without recursing but with consideration of the aliasing rules. Previously this logic was embedded in the Terminal::condition_fmt method. The resulting display algorithm is much easier to follow, although it uses more lines of code (primarily in the form of large repetitive match statements). cargo bloat shows there is a slight reduction in code size attributed to the miniscript crate, though a slight increase in the total code size for an example binary which basically implements the `string_rtt` test. I think with some effort we should be able to reduce the code size of this algorithm. The important thing though is that it's not recursive. The exact results are as follows (though they have limited use without my committing the actual example program, which is a little ugly and not generally useful). Before: File .text Size Crate Name 0.0% 0.9% 7.3KiB miniscript miniscript::miniscript::astelem::>::from_tree 0.0% 0.7% 5.8KiB miniscript miniscript::miniscript::types::Type::type_check 0.0% 0.6% 4.4KiB miniscript miniscript::miniscript::types::extra_props::ExtData::threshold 0.0% 0.5% 4.0KiB miniscript ::fmt 0.0% 0.5% 4.0KiB miniscript miniscript::miniscript::astelem::>::conditional_fmt 0.0% 0.5% 3.7KiB miniscript miniscript::miniscript::wrap_into_miniscript 0.0% 0.4% 3.0KiB miniscript miniscript::miniscript::types::extra_props::ExtData::or_i 0.0% 0.4% 3.0KiB miniscript as core::cmp::PartialEq>::eq 0.0% 0.3% 2.8KiB miniscript ::fmt 0.0% 0.3% 2.5KiB miniscript miniscript::expression::Tree::from_slice_delim 0.0% 0.3% 2.4KiB miniscript miniscript::miniscript::types::extra_props::ExtData::type_check 0.0% 0.3% 2.4KiB miniscript alloc::collections::btree::node::BalancingContext::bulk_steal_left 0.0% 0.3% 2.2KiB miniscript miniscript::miniscript::split_expression_name 0.0% 0.3% 2.2KiB miniscript ::fmt 0.0% 0.3% 2.1KiB miniscript miniscript::miniscript::types::extra_props::ExtData::and_or 0.0% 0.2% 1.9KiB miniscript miniscript::miniscript::types::extra_props::ExtData::or_b 0.0% 0.2% 1.9KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.7% 33.0% 262.1KiB And 1454 smaller methods. Use -n N to show more. 0.9% 40.7% 323.2KiB filtered data size, the file size is 34.8MiB After: File .text Size Crate Name 0.0% 0.9% 7.3KiB miniscript miniscript::miniscript::astelem::>::from_tree 0.0% 0.7% 5.8KiB miniscript miniscript::miniscript::types::Type::type_check 0.0% 0.7% 5.7KiB miniscript miniscript::miniscript::display::>::conditional_fmt 0.0% 0.6% 4.4KiB miniscript miniscript::miniscript::types::extra_props::ExtData::threshold 0.0% 0.5% 4.3KiB miniscript as miniscript::iter::tree::TreeLike>::as_node 0.0% 0.5% 4.0KiB miniscript ::fmt 0.0% 0.5% 3.7KiB miniscript miniscript::miniscript::wrap_into_miniscript 0.0% 0.4% 3.0KiB miniscript miniscript::miniscript::types::extra_props::ExtData::or_i 0.0% 0.3% 2.8KiB miniscript ::fmt 0.0% 0.3% 2.5KiB miniscript miniscript::expression::Tree::from_slice_delim 0.0% 0.3% 2.4KiB miniscript miniscript::miniscript::types::extra_props::ExtData::type_check 0.0% 0.3% 2.4KiB miniscript alloc::collections::btree::node::BalancingContext::bulk_steal_left 0.0% 0.3% 2.2KiB miniscript miniscript::miniscript::split_expression_name 0.0% 0.3% 2.2KiB miniscript ::fmt 0.0% 0.3% 2.1KiB miniscript miniscript::miniscript::types::extra_props::ExtData::and_or 0.0% 0.2% 1.9KiB miniscript miniscript::miniscript::types::extra_props::ExtData::or_b 0.0% 0.2% 1.9KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.0% 0.2% 1.8KiB miniscript core::slice::sort::merge 0.7% 32.2% 256.3KiB And 1423 smaller methods. Use -n N to show more. 0.9% 40.2% 320.4KiB filtered data size, the file size is 35.2MiB --- src/miniscript/astelem.rs | 219 +------------------------ src/miniscript/display.rs | 314 ++++++++++++++++++++++++++++++++++++ src/miniscript/mod.rs | 11 +- src/miniscript/types/mod.rs | 37 +++++ 4 files changed, 354 insertions(+), 227 deletions(-) create mode 100644 src/miniscript/display.rs diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index 58c91befb..08ddd71d3 100644 --- a/src/miniscript/astelem.rs +++ b/src/miniscript/astelem.rs @@ -7,7 +7,6 @@ //! encoding in Bitcoin script, as well as a datatype. Full details //! are given on the Miniscript website. -use core::fmt; use core::str::FromStr; use bitcoin::hashes::{hash160, Hash}; @@ -15,7 +14,7 @@ use bitcoin::{absolute, opcodes, script}; use sync::Arc; use crate::miniscript::context::SigType; -use crate::miniscript::{types, ScriptContext}; +use crate::miniscript::ScriptContext; use crate::prelude::*; use crate::util::MsKeyBuilder; use crate::{ @@ -23,222 +22,6 @@ use crate::{ ToPublicKey, }; -impl Terminal { - /// Internal helper function for displaying wrapper types; returns - /// a character to display before the `:` as well as a reference - /// to the wrapped type to allow easy recursion - fn wrap_char(&self) -> Option<(char, &Arc>)> { - match *self { - Terminal::Alt(ref sub) => Some(('a', sub)), - Terminal::Swap(ref sub) => Some(('s', sub)), - Terminal::Check(ref sub) => Some(('c', sub)), - Terminal::DupIf(ref sub) => Some(('d', sub)), - Terminal::Verify(ref sub) => Some(('v', sub)), - Terminal::NonZero(ref sub) => Some(('j', sub)), - Terminal::ZeroNotEqual(ref sub) => Some(('n', sub)), - Terminal::AndV(ref sub, ref r) if r.node == Terminal::True => Some(('t', sub)), - Terminal::OrI(ref sub, ref r) if r.node == Terminal::False => Some(('u', sub)), - Terminal::OrI(ref l, ref sub) if l.node == Terminal::False => Some(('l', sub)), - _ => None, - } - } - - fn conditional_fmt(&self, f: &mut fmt::Formatter, is_debug: bool) -> fmt::Result { - match *self { - Terminal::PkK(ref pk) => fmt_1(f, "pk_k(", pk, is_debug), - Terminal::PkH(ref pk) => fmt_1(f, "pk_h(", pk, is_debug), - Terminal::RawPkH(ref pkh) => fmt_1(f, "expr_raw_pk_h(", pkh, is_debug), - Terminal::After(ref t) => fmt_1(f, "after(", t, is_debug), - Terminal::Older(ref t) => fmt_1(f, "older(", t, is_debug), - Terminal::Sha256(ref h) => fmt_1(f, "sha256(", h, is_debug), - Terminal::Hash256(ref h) => fmt_1(f, "hash256(", h, is_debug), - Terminal::Ripemd160(ref h) => fmt_1(f, "ripemd160(", h, is_debug), - Terminal::Hash160(ref h) => fmt_1(f, "hash160(", h, is_debug), - Terminal::True => f.write_str("1"), - Terminal::False => f.write_str("0"), - Terminal::AndV(ref l, ref r) if r.node != Terminal::True => { - fmt_2(f, "and_v(", l, r, is_debug) - } - Terminal::AndB(ref l, ref r) => fmt_2(f, "and_b(", l, r, is_debug), - Terminal::AndOr(ref a, ref b, ref c) => { - if c.node == Terminal::False { - fmt_2(f, "and_b(", a, b, is_debug) - } else { - f.write_str("andor(")?; - conditional_fmt(f, a, is_debug)?; - f.write_str(",")?; - conditional_fmt(f, b, is_debug)?; - f.write_str(",")?; - conditional_fmt(f, c, is_debug)?; - f.write_str(")") - } - } - Terminal::OrB(ref l, ref r) => fmt_2(f, "or_b(", l, r, is_debug), - Terminal::OrD(ref l, ref r) => fmt_2(f, "or_d(", l, r, is_debug), - Terminal::OrC(ref l, ref r) => fmt_2(f, "or_c(", l, r, is_debug), - Terminal::OrI(ref l, ref r) - if l.node != Terminal::False && r.node != Terminal::False => - { - fmt_2(f, "or_i(", l, r, is_debug) - } - Terminal::Thresh(ref thresh) => { - if is_debug { - fmt::Debug::fmt(&thresh.debug("thresh", true), f) - } else { - fmt::Display::fmt(&thresh.display("thresh", true), f) - } - } - Terminal::Multi(ref thresh) => { - if is_debug { - fmt::Debug::fmt(&thresh.debug("multi", true), f) - } else { - fmt::Display::fmt(&thresh.display("multi", true), f) - } - } - Terminal::MultiA(ref thresh) => { - if is_debug { - fmt::Debug::fmt(&thresh.debug("multi_a", true), f) - } else { - fmt::Display::fmt(&thresh.display("multi_a", true), f) - } - } - // wrappers - _ => { - if let Some((ch, sub)) = self.wrap_char() { - if ch == 'c' { - if let Terminal::PkK(ref pk) = sub.node { - // alias: pk(K) = c:pk_k(K) - return fmt_1(f, "pk(", pk, is_debug); - } else if let Terminal::RawPkH(ref pkh) = sub.node { - // `RawPkH` is currently unsupported in the descriptor spec - // alias: pkh(K) = c:pk_h(K) - // We temporarily display there using raw_pkh, but these descriptors - // are not defined in the spec yet. These are prefixed with `expr` - // in the descriptor string. - // We do not support parsing these descriptors yet. - return fmt_1(f, "expr_raw_pkh(", pkh, is_debug); - } else if let Terminal::PkH(ref pk) = sub.node { - // alias: pkh(K) = c:pk_h(K) - return fmt_1(f, "pkh(", pk, is_debug); - } - } - - fmt::Write::write_char(f, ch)?; - match sub.node.wrap_char() { - None => { - f.write_str(":")?; - } - // Add a ':' wrapper if there are other wrappers apart from c:pk_k() - // tvc:pk_k() -> tv:pk() - Some(('c', ms)) => match ms.node { - Terminal::PkK(_) | Terminal::PkH(_) | Terminal::RawPkH(_) => { - f.write_str(":")?; - } - _ => {} - }, - _ => {} - }; - if is_debug { - write!(f, "{:?}", sub) - } else { - write!(f, "{}", sub) - } - } else { - unreachable!(); - } - } - } - } -} - -fn fmt_1( - f: &mut fmt::Formatter, - name: &str, - a: &D, - is_debug: bool, -) -> fmt::Result { - f.write_str(name)?; - conditional_fmt(f, a, is_debug)?; - f.write_str(")") -} -fn fmt_2( - f: &mut fmt::Formatter, - name: &str, - a: &D, - b: &D, - is_debug: bool, -) -> fmt::Result { - f.write_str(name)?; - conditional_fmt(f, a, is_debug)?; - f.write_str(",")?; - conditional_fmt(f, b, is_debug)?; - f.write_str(")") -} -fn conditional_fmt( - f: &mut fmt::Formatter, - data: &D, - is_debug: bool, -) -> fmt::Result { - if is_debug { - fmt::Debug::fmt(data, f) - } else { - fmt::Display::fmt(data, f) - } -} - -impl fmt::Debug for Terminal { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - fn fmt_type_map(f: &mut fmt::Formatter<'_>, type_map: types::Type) -> fmt::Result { - f.write_str(match type_map.corr.base { - types::Base::B => "B", - types::Base::K => "K", - types::Base::V => "V", - types::Base::W => "W", - })?; - f.write_str("/")?; - f.write_str(match type_map.corr.input { - types::Input::Zero => "z", - types::Input::One => "o", - types::Input::OneNonZero => "on", - types::Input::Any => "", - types::Input::AnyNonZero => "n", - })?; - if type_map.corr.dissatisfiable { - f.write_str("d")?; - } - if type_map.corr.unit { - f.write_str("u")?; - } - f.write_str(match type_map.mall.dissat { - types::Dissat::None => "f", - types::Dissat::Unique => "e", - types::Dissat::Unknown => "", - })?; - if type_map.mall.safe { - f.write_str("s")?; - } - if type_map.mall.non_malleable { - f.write_str("m")?; - } - Ok(()) - } - - f.write_str("[")?; - if let Ok(type_map) = types::Type::type_check(self) { - fmt_type_map(f, type_map)?; - } else { - f.write_str("TYPECHECK FAILED")?; - } - f.write_str("]")?; - - self.conditional_fmt(f, true) - } -} - -impl fmt::Display for Terminal { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.conditional_fmt(f, false) } -} - impl crate::expression::FromTree for Arc> { fn from_tree(top: &expression::Tree) -> Result>, Error> { Ok(Arc::new(expression::FromTree::from_tree(top)?)) diff --git a/src/miniscript/display.rs b/src/miniscript/display.rs new file mode 100644 index 000000000..29a6cf596 --- /dev/null +++ b/src/miniscript/display.rs @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Miniscript Node Display + +use core::fmt; + +use bitcoin::hashes::hash160; + +use crate::iter::{Tree, TreeLike}; +use crate::miniscript::types::Type; +use crate::miniscript::Terminal; +use crate::prelude::sync::Arc; +use crate::{Miniscript, MiniscriptKey, ScriptContext}; + +#[derive(Clone)] +enum DisplayNode<'a, Pk: MiniscriptKey, Ctx: ScriptContext> { + Node(Type, &'a Terminal), + ThresholdK(usize), + Key(&'a Pk), + RawKeyHash(&'a hash160::Hash), + After(&'a crate::AbsLockTime), + Older(&'a crate::RelLockTime), + Sha256(&'a Pk::Sha256), + Hash256(&'a Pk::Hash256), + Ripemd160(&'a Pk::Ripemd160), + Hash160(&'a Pk::Hash160), +} + +#[derive(Clone)] +enum NaryChildren<'a, Pk: MiniscriptKey, Ctx: ScriptContext> { + Nodes(usize, &'a [Arc>]), + Keys(usize, &'a [Pk]), +} + +impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for DisplayNode<'a, Pk, Ctx> { + type NaryChildren = NaryChildren<'a, Pk, Ctx>; + + fn nary_len(tc: &Self::NaryChildren) -> usize { + match tc { + NaryChildren::Nodes(_, n) => 1 + n.len(), + NaryChildren::Keys(_, k) => 1 + k.len(), + } + } + + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { + if idx == 0 { + match tc { + NaryChildren::Nodes(k, _) => DisplayNode::ThresholdK(k), + NaryChildren::Keys(k, _) => DisplayNode::ThresholdK(k), + } + } else { + match tc { + NaryChildren::Nodes(_, n) => { + DisplayNode::Node(n[idx - 1].ty, n[idx - 1].as_inner()) + } + NaryChildren::Keys(_, k) => DisplayNode::Key(&k[idx - 1]), + } + } + } + + fn as_node(&self) -> Tree { + match self { + DisplayNode::Node(_, ref node) => match node { + Terminal::True | Terminal::False => Tree::Nullary, + Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => Tree::Unary(DisplayNode::Key(pk)), + Terminal::RawPkH(ref pkh) => Tree::Unary(DisplayNode::RawKeyHash(pkh)), + Terminal::After(ref t) => Tree::Unary(DisplayNode::After(t)), + Terminal::Older(ref t) => Tree::Unary(DisplayNode::Older(t)), + Terminal::Sha256(ref h) => Tree::Unary(DisplayNode::Sha256(h)), + Terminal::Hash256(ref h) => Tree::Unary(DisplayNode::Hash256(h)), + Terminal::Ripemd160(ref h) => Tree::Unary(DisplayNode::Ripemd160(h)), + Terminal::Hash160(ref h) => Tree::Unary(DisplayNode::Hash160(h)), + // Check hash to be treated specially as always.. + Terminal::Check(ref sub) => match sub.as_inner() { + Terminal::PkK(ref pk) | Terminal::PkH(ref pk) => { + Tree::Unary(DisplayNode::Key(pk)) + } + Terminal::RawPkH(ref pkh) => Tree::Unary(DisplayNode::RawKeyHash(pkh)), + _ => Tree::Unary(DisplayNode::Node(sub.ty, sub.as_inner())), + }, + Terminal::Alt(ref sub) + | Terminal::Swap(ref sub) + | Terminal::DupIf(ref sub) + | Terminal::Verify(ref sub) + | Terminal::NonZero(ref sub) + | Terminal::ZeroNotEqual(ref sub) => { + Tree::Unary(DisplayNode::Node(sub.ty, sub.as_inner())) + } + Terminal::AndV(ref left, ref right) + if matches!(right.as_inner(), Terminal::True) => + { + Tree::Unary(DisplayNode::Node(left.ty, left.as_inner())) + } + Terminal::OrI(ref left, ref right) + if matches!(left.as_inner(), Terminal::False) => + { + Tree::Unary(DisplayNode::Node(right.ty, right.as_inner())) + } + Terminal::OrI(ref left, ref right) + if matches!(right.as_inner(), Terminal::False) => + { + Tree::Unary(DisplayNode::Node(left.ty, left.as_inner())) + } + Terminal::AndV(ref left, ref right) + | Terminal::AndB(ref left, ref right) + | Terminal::OrB(ref left, ref right) + | Terminal::OrD(ref left, ref right) + | Terminal::OrC(ref left, ref right) + | Terminal::OrI(ref left, ref right) => Tree::Binary( + DisplayNode::Node(left.ty, left.as_inner()), + DisplayNode::Node(right.ty, right.as_inner()), + ), + Terminal::AndOr(ref a, ref b, ref c) if matches!(c.as_inner(), Terminal::False) => { + Tree::Binary( + DisplayNode::Node(a.ty, a.as_inner()), + DisplayNode::Node(b.ty, b.as_inner()), + ) + } + Terminal::AndOr(ref a, ref b, ref c) => Tree::Ternary( + DisplayNode::Node(a.ty, a.as_inner()), + DisplayNode::Node(b.ty, b.as_inner()), + DisplayNode::Node(c.ty, c.as_inner()), + ), + Terminal::Thresh(ref thresh) => { + Tree::Nary(NaryChildren::Nodes(thresh.k(), thresh.data())) + } + Terminal::Multi(ref thresh) => { + Tree::Nary(NaryChildren::Keys(thresh.k(), thresh.data())) + } + Terminal::MultiA(ref thresh) => { + Tree::Nary(NaryChildren::Keys(thresh.k(), thresh.data())) + } + }, + // Only nodes have children; the rest are terminals. + _ => Tree::Nullary, + } + } +} + +#[derive(Copy, Clone)] +enum DisplayTypes { + /// Display no types. + None, + /// Display all types, including the initial type. + All(Type), + /// Display all types, except that the initial type should be written as [TYPECHECK FAILED]. + AllBadFirst, +} + +impl Terminal { + fn conditional_fmt(&self, f: &mut fmt::Formatter, display_types: DisplayTypes) -> fmt::Result { + let initial_type = match display_types { + DisplayTypes::None => Type::FALSE, + DisplayTypes::All(ty) => ty, + DisplayTypes::AllBadFirst => Type::FALSE, + }; + + for item in DisplayNode::Node(initial_type, self).verbose_pre_order_iter() { + let show_type = match display_types { + DisplayTypes::None => false, + DisplayTypes::All(_) => true, + DisplayTypes::AllBadFirst => item.index > 0, + }; + + match (display_types, item.node) { + (_, DisplayNode::Node(ty, node)) => { + if node.is_wrapper() { + // Wrappers are very easy: just write the one-character name and maybe the + // type and we are done. No parens, no :s, no commas, etc. + if item.n_children_yielded == 0 { + if show_type { + f.write_str("[")?; + fmt::Display::fmt(&ty, f)?; + f.write_str("]")?; + } + f.write_str(node.fragment_name())?; + } + } else { + // Non-wrappers are a little more involved. + if item.n_children_yielded == 0 { + if let Some(DisplayNode::Node(_, parent)) = item.parent { + if parent.is_wrapper() { + f.write_str(":")?; + } + } + + if show_type { + f.write_str("[")?; + fmt::Display::fmt(&ty, f)?; + f.write_str("]")?; + } + f.write_str(node.fragment_name())?; + + if !item.is_complete { + f.write_str("(")?; + } + } else if item.is_complete { + f.write_str(")")?; + } else { + f.write_str(",")?; + } + } + } + // Only nodes have a complicated algorithm. The other objects we just print. + (DisplayTypes::None, DisplayNode::ThresholdK(ref k)) => fmt::Display::fmt(k, f)?, + (DisplayTypes::None, DisplayNode::Key(ref pk)) => fmt::Display::fmt(pk, f)?, + (DisplayTypes::None, DisplayNode::RawKeyHash(ref h)) => fmt::Display::fmt(h, f)?, + (DisplayTypes::None, DisplayNode::After(ref t)) => fmt::Display::fmt(t, f)?, + (DisplayTypes::None, DisplayNode::Older(ref t)) => fmt::Display::fmt(t, f)?, + (DisplayTypes::None, DisplayNode::Sha256(ref h)) => fmt::Display::fmt(h, f)?, + (DisplayTypes::None, DisplayNode::Hash256(ref h)) => fmt::Display::fmt(h, f)?, + (DisplayTypes::None, DisplayNode::Ripemd160(ref h)) => fmt::Display::fmt(h, f)?, + (DisplayTypes::None, DisplayNode::Hash160(ref h)) => fmt::Display::fmt(h, f)?, + (_, DisplayNode::ThresholdK(ref k)) => fmt::Debug::fmt(k, f)?, + (_, DisplayNode::Key(ref pk)) => fmt::Debug::fmt(pk, f)?, + (_, DisplayNode::RawKeyHash(ref h)) => fmt::Debug::fmt(h, f)?, + (_, DisplayNode::After(ref t)) => fmt::Debug::fmt(t, f)?, + (_, DisplayNode::Older(ref t)) => fmt::Debug::fmt(t, f)?, + (_, DisplayNode::Sha256(ref h)) => fmt::Debug::fmt(h, f)?, + (_, DisplayNode::Hash256(ref h)) => fmt::Debug::fmt(h, f)?, + (_, DisplayNode::Ripemd160(ref h)) => fmt::Debug::fmt(h, f)?, + (_, DisplayNode::Hash160(ref h)) => fmt::Debug::fmt(h, f)?, + } + } + Ok(()) + } + + /// A string representation of the fragment's name. + /// + /// This is **not** a recursive representation of the whole fragment; + /// it does not contain or indicate any children. + /// + /// Not public since we intend to move it to the Inner type once that exists. + fn fragment_name(&self) -> &'static str { + match *self { + Terminal::True => "1", + Terminal::False => "0", + Terminal::PkK(..) => "pk_k", + Terminal::PkH(..) => "pk_h", + // `RawPkH` is currently unsupported in the descriptor spec. We temporarily + // display and parse these by prefixing them with 'expr'. + Terminal::RawPkH(..) => "expr_raw_pk_h", + Terminal::After(..) => "after", + Terminal::Older(..) => "older", + Terminal::Sha256(..) => "sha256", + Terminal::Hash256(..) => "hash256", + Terminal::Ripemd160(..) => "ripemd160", + Terminal::Hash160(..) => "hash160", + Terminal::Alt(..) => "a", + Terminal::Swap(..) => "s", + Terminal::Check(ref sub) if matches!(sub.as_inner(), Terminal::PkK(..)) => "pk", + Terminal::Check(ref sub) if matches!(sub.as_inner(), Terminal::PkH(..)) => "pkh", + Terminal::Check(ref sub) if matches!(sub.as_inner(), Terminal::RawPkH(..)) => { + "expr_raw_pkh" + } + Terminal::Check(..) => "c", + Terminal::DupIf(..) => "d", + Terminal::Verify(..) => "v", + Terminal::NonZero(..) => "j", + Terminal::ZeroNotEqual(..) => "n", + Terminal::AndV(_, ref r) if matches!(r.as_inner(), Terminal::True) => "t", + Terminal::AndV(..) => "and_v", + Terminal::AndB(..) => "and_b", + Terminal::AndOr(_, _, ref c) if matches!(c.as_inner(), Terminal::False) => "and_n", + Terminal::AndOr(..) => "andor", + Terminal::OrB(..) => "or_b", + Terminal::OrD(..) => "or_d", + Terminal::OrC(..) => "or_c", + Terminal::OrI(ref l, _) if matches!(l.as_inner(), Terminal::False) => "l", + Terminal::OrI(_, ref r) if matches!(r.as_inner(), Terminal::False) => "u", + Terminal::OrI(..) => "or_i", + Terminal::Thresh(..) => "thresh", + Terminal::Multi(..) => "multi", + Terminal::MultiA(..) => "multi_a", + } + } + + /// Whether the fragment in question is a "wrapper" such as `s:` or `a:`. + /// + /// Not public since we intend to move it to the Inner type once that exists. + fn is_wrapper(&self) -> bool { + !matches!(self, Terminal::True | Terminal::False) && self.fragment_name().len() == 1 + } +} + +impl fmt::Debug for Miniscript { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.as_inner() + .conditional_fmt(f, DisplayTypes::All(self.ty)) + } +} + +impl fmt::Display for Miniscript { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.as_inner().conditional_fmt(f, DisplayTypes::None) + } +} + +impl fmt::Debug for Terminal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let display_types = if let Ok(ty) = Type::type_check(self) { + DisplayTypes::All(ty) + } else { + DisplayTypes::AllBadFirst + }; + self.conditional_fmt(f, display_types) + } +} + +impl fmt::Display for Terminal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.conditional_fmt(f, DisplayTypes::None) + } +} diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 6cc419823..c8d3a953c 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -12,7 +12,7 @@ //! components of the AST. //! -use core::{fmt, hash, str}; +use core::{hash, str}; use bitcoin::hashes::hash160; use bitcoin::script; @@ -28,6 +28,7 @@ pub mod analyzable; pub mod astelem; pub(crate) mod context; pub mod decode; +mod display; pub mod iter; pub mod lex; pub mod limits; @@ -408,14 +409,6 @@ impl hash::Hash for Miniscript { fn hash(&self, state: &mut H) { self.node.hash(state); } } -impl fmt::Debug for Miniscript { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{:?}", self.node) } -} - -impl fmt::Display for Miniscript { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.node) } -} - impl ForEachKey for Miniscript { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool { for ms in self.pre_order_iter() { diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 84fb85e4f..174a5e5dd 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -157,6 +157,43 @@ pub struct Type { pub mall: Malleability, } +impl fmt::Display for Type { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(match self.corr.base { + Base::B => "B", + Base::K => "K", + Base::V => "V", + Base::W => "W", + })?; + f.write_str("/")?; + f.write_str(match self.corr.input { + Input::Zero => "z", + Input::One => "o", + Input::OneNonZero => "on", + Input::Any => "", + Input::AnyNonZero => "n", + })?; + if self.corr.dissatisfiable { + f.write_str("d")?; + } + if self.corr.unit { + f.write_str("u")?; + } + f.write_str(match self.mall.dissat { + Dissat::None => "f", + Dissat::Unique => "e", + Dissat::Unknown => "", + })?; + if self.mall.safe { + f.write_str("s")?; + } + if self.mall.non_malleable { + f.write_str("m")?; + } + Ok(()) + } +} + impl Type { /// Type of the `0` combinator pub const TRUE: Self = Type { corr: Correctness::TRUE, mall: Malleability::TRUE }; From 117003f3d1c89129734288153a870abb7e940e45 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 18 Aug 2024 23:04:29 +0000 Subject: [PATCH 4/5] miniscript: implement PartialOrd/Ord nonrecursively This implements an ordering based on the display ordering, though a more efficiently than just serializing to strings and comparing those. Empirically this adds a couple kb to the total code size, but not as much as the nonrecursive fmt shrunk it. We expect somewhat larger code to implement such a nontrivial algorithm nonrecursively (and in fact the old algorithm was "wrong" in that it did not implement any coherent ordering). This is a breaking change because it changes the order of Miniscripts. --- src/miniscript/decode.rs | 2 +- src/miniscript/display.rs | 50 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index 3af3afe2a..c06063586 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -114,7 +114,7 @@ enum NonTerm { /// /// The average user should always use the [`Descriptor`] APIs. Advanced users who want deal /// with Miniscript ASTs should use the [`Miniscript`] APIs. -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, PartialEq, Eq, Hash)] pub enum Terminal { /// `1` True, diff --git a/src/miniscript/display.rs b/src/miniscript/display.rs index 29a6cf596..2a4e9f757 100644 --- a/src/miniscript/display.rs +++ b/src/miniscript/display.rs @@ -2,7 +2,7 @@ //! Miniscript Node Display -use core::fmt; +use core::{cmp, fmt}; use bitcoin::hashes::hash160; @@ -312,3 +312,51 @@ impl fmt::Display for Terminal { self.conditional_fmt(f, DisplayTypes::None) } } + +impl PartialOrd for Terminal { + fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } +} + +impl Ord for Terminal { + fn cmp(&self, other: &Self) -> cmp::Ordering { + // First try matching directly on the fragment name to avoid the + // complexity of building an iterator. + match self.fragment_name().cmp(other.fragment_name()) { + cmp::Ordering::Less => cmp::Ordering::Less, + cmp::Ordering::Greater => cmp::Ordering::Greater, + cmp::Ordering::Equal => { + // But if they are equal then we need to iterate + for (me, you) in DisplayNode::Node(Type::FALSE, self) + .pre_order_iter() + .zip(DisplayNode::Node(Type::FALSE, other).pre_order_iter()) + { + let me_you_cmp = match (me, you) { + (DisplayNode::Node(_, me), DisplayNode::Node(_, you)) => { + me.fragment_name().cmp(you.fragment_name()) + } + (DisplayNode::ThresholdK(me), DisplayNode::ThresholdK(you)) => me.cmp(&you), + (DisplayNode::Key(me), DisplayNode::Key(you)) => me.cmp(you), + (DisplayNode::RawKeyHash(me), DisplayNode::RawKeyHash(you)) => me.cmp(you), + (DisplayNode::After(me), DisplayNode::After(you)) => me.cmp(you), + (DisplayNode::Older(me), DisplayNode::Older(you)) => me.cmp(you), + (DisplayNode::Sha256(me), DisplayNode::Sha256(you)) => me.cmp(you), + (DisplayNode::Hash256(me), DisplayNode::Hash256(you)) => me.cmp(you), + (DisplayNode::Ripemd160(me), DisplayNode::Ripemd160(you)) => me.cmp(you), + (DisplayNode::Hash160(me), DisplayNode::Hash160(you)) => me.cmp(you), + _ => unreachable!( + "if the type of a node differs, its parent must have differed" + ), + }; + + match me_you_cmp { + cmp::Ordering::Less => return cmp::Ordering::Less, + cmp::Ordering::Greater => return cmp::Ordering::Greater, + cmp::Ordering::Equal => {} + } + } + + cmp::Ordering::Equal + } + } + } +} From d0c0c14ae26e353c950319182fd853ee90b8eb54 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 18 Aug 2024 21:34:13 +0000 Subject: [PATCH 5/5] miniscript: nonrecursive implementation of PartialEq/Eq/Hash --- src/iter/mod.rs | 30 ++++++++++++++++++++ src/miniscript/decode.rs | 59 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/iter/mod.rs b/src/iter/mod.rs index 052675376..abb8eda7b 100644 --- a/src/iter/mod.rs +++ b/src/iter/mod.rs @@ -76,3 +76,33 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> TreeLike for &'a Arc TreeLike for &'a Terminal { + type NaryChildren = &'a [Arc>]; + + fn nary_len(tc: &Self::NaryChildren) -> usize { tc.len() } + fn nary_index(tc: Self::NaryChildren, idx: usize) -> Self { tc[idx].as_inner() } + + fn as_node(&self) -> Tree { + use Terminal::*; + match self { + PkK(..) | PkH(..) | RawPkH(..) | After(..) | Older(..) | Sha256(..) | Hash256(..) + | Ripemd160(..) | Hash160(..) | True | False | Multi(..) | MultiA(..) => Tree::Nullary, + Alt(ref sub) + | Swap(ref sub) + | Check(ref sub) + | DupIf(ref sub) + | Verify(ref sub) + | NonZero(ref sub) + | ZeroNotEqual(ref sub) => Tree::Unary(sub.as_inner()), + AndV(ref left, ref right) + | AndB(ref left, ref right) + | OrB(ref left, ref right) + | OrD(ref left, ref right) + | OrC(ref left, ref right) + | OrI(ref left, ref right) => Tree::Binary(left.as_inner(), right.as_inner()), + AndOr(ref a, ref b, ref c) => Tree::Ternary(a.as_inner(), b.as_inner(), c.as_inner()), + Thresh(ref thresh) => Tree::Nary(thresh.data()), + } + } +} diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index c06063586..2db7cf443 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -5,13 +5,14 @@ //! Functionality to parse a Bitcoin Script into a `Miniscript` //! -use core::fmt; +use core::{fmt, mem}; #[cfg(feature = "std")] use std::error; use bitcoin::hashes::{hash160, ripemd160, sha256, Hash}; use sync::Arc; +use crate::iter::TreeLike; use crate::miniscript::lex::{Token as Tk, TokenIter}; use crate::miniscript::limits::{MAX_PUBKEYS_IN_CHECKSIGADD, MAX_PUBKEYS_PER_MULTISIG}; use crate::miniscript::ScriptContext; @@ -114,7 +115,7 @@ enum NonTerm { /// /// The average user should always use the [`Descriptor`] APIs. Advanced users who want deal /// with Miniscript ASTs should use the [`Miniscript`] APIs. -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Clone)] pub enum Terminal { /// `1` True, @@ -185,6 +186,60 @@ pub enum Terminal { MultiA(Threshold), } +impl PartialEq for Terminal { + fn eq(&self, other: &Self) -> bool { + for (me, you) in self.pre_order_iter().zip(other.pre_order_iter()) { + match (me, you) { + (Terminal::PkK(key1), Terminal::PkK(key2)) if key1 != key2 => return false, + (Terminal::PkH(key1), Terminal::PkH(key2)) if key1 != key2 => return false, + (Terminal::RawPkH(h1), Terminal::RawPkH(h2)) if h1 != h2 => return false, + (Terminal::After(t1), Terminal::After(t2)) if t1 != t2 => return false, + (Terminal::Older(t1), Terminal::Older(t2)) if t1 != t2 => return false, + (Terminal::Sha256(h1), Terminal::Sha256(h2)) if h1 != h2 => return false, + (Terminal::Hash256(h1), Terminal::Hash256(h2)) if h1 != h2 => return false, + (Terminal::Ripemd160(h1), Terminal::Ripemd160(h2)) if h1 != h2 => return false, + (Terminal::Hash160(h1), Terminal::Hash160(h2)) if h1 != h2 => return false, + (Terminal::Multi(th1), Terminal::Multi(th2)) if th1 != th2 => return false, + (Terminal::MultiA(th1), Terminal::MultiA(th2)) if th1 != th2 => return false, + _ => { + if mem::discriminant(me) != mem::discriminant(you) { + return false; + } + } + } + } + true + } +} +impl Eq for Terminal {} + +impl core::hash::Hash for Terminal { + fn hash(&self, hasher: &mut H) { + for term in self.pre_order_iter() { + mem::discriminant(term).hash(hasher); + match term { + Terminal::PkK(key) => key.hash(hasher), + Terminal::PkH(key) => key.hash(hasher), + Terminal::RawPkH(h) => h.hash(hasher), + Terminal::After(t) => t.hash(hasher), + Terminal::Older(t) => t.hash(hasher), + Terminal::Sha256(h) => h.hash(hasher), + Terminal::Hash256(h) => h.hash(hasher), + Terminal::Ripemd160(h) => h.hash(hasher), + Terminal::Hash160(h) => h.hash(hasher), + Terminal::Thresh(th) => { + th.k().hash(hasher); + th.n().hash(hasher); + // The actual children will be hashed when we iterate + } + Terminal::Multi(th) => th.hash(hasher), + Terminal::MultiA(th) => th.hash(hasher), + _ => {} + } + } + } +} + macro_rules! match_token { // Base case ($tokens:expr => $sub:expr,) => { $sub };