Skip to content

Commit

Permalink
Merge #724: miniscript: non-recursive Clone
Browse files Browse the repository at this point in the history 
0d1a59a miniscript: non-recursive Clone (Andrew Poelstra)

Pull request description:

  This is mostly just annoying/mechanical code. But it needs to be written because the derive(Clone) logic, I think, was only doing a "shallow clone" by cloning the Arcs in the Miniscript.

  In my view this is undesirable behavior. If users want to do a shallow clone, they can hold an Arc<Miniscript> and call Arc::clone on that. But there are some cases where they might want to do a deep clone, and currently there isn't really any way to do so (you can do it with the translator API and an "identity" translator, but this is awkward to do and will be slow because it redoes typechecking and rechecks for duplicate keys).

ACKs for top commit:
  sanket1729:
    ACK 0d1a59a.

Tree-SHA512: eba53c2a61248444d0b43cc753535cdb6f1f7c9cccf720f66f378a041ab219533911500969bee7b025177524cefc48399fb88fee77239bf99bf4d1d28e949ad6
  • Loading branch information
@apoelstra
apoelstra committed Aug 19, 2024
2 parents d67fb00 + 0d1a59a commit cab7762
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 2 deletions.
61 changes: 60 additions & 1 deletion src/miniscript/decode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ 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)]
pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
/// `1`
True,
Expand Down Expand Up @@ -186,6 +185,66 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
MultiA(Threshold<Pk, MAX_PUBKEYS_IN_CHECKSIGADD>),
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Clone for Terminal<Pk, Ctx> {
/// We implement clone as a "deep clone" which reconstructs the entire tree.
///
/// If users just want to clone Arcs they can use Arc::clone themselves.
fn clone(&self) -> Self {
match self {
Terminal::PkK(ref p) => Terminal::PkK(p.clone()),
Terminal::PkH(ref p) => Terminal::PkH(p.clone()),
Terminal::RawPkH(ref p) => Terminal::RawPkH(*p),
Terminal::After(ref n) => Terminal::After(*n),
Terminal::Older(ref n) => Terminal::Older(*n),
Terminal::Sha256(ref x) => Terminal::Sha256(x.clone()),
Terminal::Hash256(ref x) => Terminal::Hash256(x.clone()),
Terminal::Ripemd160(ref x) => Terminal::Ripemd160(x.clone()),
Terminal::Hash160(ref x) => Terminal::Hash160(x.clone()),
Terminal::True => Terminal::True,
Terminal::False => Terminal::False,
Terminal::Alt(ref sub) => Terminal::Alt(Arc::new(Miniscript::clone(sub))),
Terminal::Swap(ref sub) => Terminal::Swap(Arc::new(Miniscript::clone(sub))),
Terminal::Check(ref sub) => Terminal::Check(Arc::new(Miniscript::clone(sub))),
Terminal::DupIf(ref sub) => Terminal::DupIf(Arc::new(Miniscript::clone(sub))),
Terminal::Verify(ref sub) => Terminal::Verify(Arc::new(Miniscript::clone(sub))),
Terminal::NonZero(ref sub) => Terminal::NonZero(Arc::new(Miniscript::clone(sub))),
Terminal::ZeroNotEqual(ref sub) => {
Terminal::ZeroNotEqual(Arc::new(Miniscript::clone(sub)))
}
Terminal::AndV(ref left, ref right) => Terminal::AndV(
Arc::new(Miniscript::clone(left)),
Arc::new(Miniscript::clone(right)),
),
Terminal::AndB(ref left, ref right) => Terminal::AndB(
Arc::new(Miniscript::clone(left)),
Arc::new(Miniscript::clone(right)),
),
Terminal::AndOr(ref a, ref b, ref c) => Terminal::AndOr(
Arc::new(Miniscript::clone(a)),
Arc::new(Miniscript::clone(b)),
Arc::new(Miniscript::clone(c)),
),
Terminal::OrB(ref left, ref right) => {
Terminal::OrB(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
}
Terminal::OrD(ref left, ref right) => {
Terminal::OrD(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
}
Terminal::OrC(ref left, ref right) => {
Terminal::OrC(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
}
Terminal::OrI(ref left, ref right) => {
Terminal::OrI(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
}
Terminal::Thresh(ref thresh) => {
Terminal::Thresh(thresh.map_ref(|child| Arc::new(Miniscript::clone(child))))
}
Terminal::Multi(ref thresh) => Terminal::Multi(thresh.clone()),
Terminal::MultiA(ref thresh) => Terminal::MultiA(thresh.clone()),
}
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> PartialEq for Terminal<Pk, Ctx> {
fn eq(&self, other: &Self) -> bool {
for (me, you) in self.pre_order_iter().zip(other.pre_order_iter()) {
Expand Down
67 changes: 66 additions & 1 deletion src/miniscript/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ mod private {
use core::marker::PhantomData;

use super::types::{ExtData, Type};
use crate::iter::TreeLike as _;
pub use crate::miniscript::context::ScriptContext;
use crate::miniscript::types;
use crate::prelude::sync::Arc;
use crate::{Error, MiniscriptKey, Terminal, MAX_RECURSION_DEPTH};

/// The top-level miniscript abstract syntax tree (AST).
#[derive(Clone)]
pub struct Miniscript<Pk: MiniscriptKey, Ctx: ScriptContext> {
/// A node in the AST.
pub node: Terminal<Pk, Ctx>,
Expand All @@ -69,6 +70,70 @@ mod private {
/// Context PhantomData. Only accessible inside this crate
phantom: PhantomData<Ctx>,
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Clone for Miniscript<Pk, Ctx> {
/// We implement clone as a "deep clone" which reconstructs the entire tree.
///
/// If users just want to clone Arcs they can use Arc::clone themselves.
/// Note that if a Miniscript was constructed using shared Arcs, the result
/// of calling `clone` will no longer have shared Arcs. So there is no
/// pleasing everyone. But for the two common cases:
///
/// * Users don't care about sharing at all, and they can call `Arc::clone`
/// on an `Arc<Miniscript>`.
/// * Users want a deep copy which does not share any nodes with the original
/// (for example, because they have keys that have interior mutability),
/// and they can call `Miniscript::clone`.
fn clone(&self) -> Self {
let mut stack = vec![];
for item in self.post_order_iter() {
let child_n = |n| Arc::clone(&stack[item.child_indices[n]]);

let new_term = match item.node.node {
Terminal::PkK(ref p) => Terminal::PkK(p.clone()),
Terminal::PkH(ref p) => Terminal::PkH(p.clone()),
Terminal::RawPkH(ref p) => Terminal::RawPkH(*p),
Terminal::After(ref n) => Terminal::After(*n),
Terminal::Older(ref n) => Terminal::Older(*n),
Terminal::Sha256(ref x) => Terminal::Sha256(x.clone()),
Terminal::Hash256(ref x) => Terminal::Hash256(x.clone()),
Terminal::Ripemd160(ref x) => Terminal::Ripemd160(x.clone()),
Terminal::Hash160(ref x) => Terminal::Hash160(x.clone()),
Terminal::True => Terminal::True,
Terminal::False => Terminal::False,
Terminal::Alt(..) => Terminal::Alt(child_n(0)),
Terminal::Swap(..) => Terminal::Swap(child_n(0)),
Terminal::Check(..) => Terminal::Check(child_n(0)),
Terminal::DupIf(..) => Terminal::DupIf(child_n(0)),
Terminal::Verify(..) => Terminal::Verify(child_n(0)),
Terminal::NonZero(..) => Terminal::NonZero(child_n(0)),
Terminal::ZeroNotEqual(..) => Terminal::ZeroNotEqual(child_n(0)),
Terminal::AndV(..) => Terminal::AndV(child_n(0), child_n(1)),
Terminal::AndB(..) => Terminal::AndB(child_n(0), child_n(1)),
Terminal::AndOr(..) => Terminal::AndOr(child_n(0), child_n(1), child_n(2)),
Terminal::OrB(..) => Terminal::OrB(child_n(0), child_n(1)),
Terminal::OrD(..) => Terminal::OrD(child_n(0), child_n(1)),
Terminal::OrC(..) => Terminal::OrC(child_n(0), child_n(1)),
Terminal::OrI(..) => Terminal::OrI(child_n(0), child_n(1)),
Terminal::Thresh(ref thresh) => Terminal::Thresh(
thresh.map_from_post_order_iter(&item.child_indices, &stack),
),
Terminal::Multi(ref thresh) => Terminal::Multi(thresh.clone()),
Terminal::MultiA(ref thresh) => Terminal::MultiA(thresh.clone()),
};

stack.push(Arc::new(Miniscript {
node: new_term,
ty: item.node.ty,
ext: item.node.ext,
phantom: PhantomData,
}));
}

Arc::try_unwrap(stack.pop().unwrap()).unwrap()
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
/// The `1` combinator.
pub const TRUE: Self = Miniscript {
Expand Down

0 comments on commit cab7762

Please sign in to comment.