Skip to content

Commit c5463d2

Browse files
heap-coderheap-coder
committed
Merge rust-bitcoin/rust-miniscript#724: miniscript: non-recursive Clone
0d1a59a570cde5eb8e365b25a83e91179ee400a1 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 0d1a59a570cde5eb8e365b25a83e91179ee400a1. Tree-SHA512: eba53c2a61248444d0b43cc753535cdb6f1f7c9cccf720f66f378a041ab219533911500969bee7b025177524cefc48399fb88fee77239bf99bf4d1d28e949ad6
2 parents bf64fac + c8b1f92 commit c5463d2

File tree

2 files changed

+126
-2
lines changed

2 files changed

+126
-2
lines changed

‎src/miniscript/decode.rs‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ enum NonTerm {
115115
///
116116
/// The average user should always use the [`Descriptor`] APIs. Advanced users who want deal
117117
/// with Miniscript ASTs should use the [`Miniscript`] APIs.
118-
#[derive(Clone)]
119118
pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
120119
/// `1`
121120
True,
@@ -186,6 +185,66 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
186185
MultiA(Threshold<Pk, MAX_PUBKEYS_IN_CHECKSIGADD>),
187186
}
188187

188+
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Clone for Terminal<Pk, Ctx> {
189+
/// We implement clone as a "deep clone" which reconstructs the entire tree.
190+
///
191+
/// If users just want to clone Arcs they can use Arc::clone themselves.
192+
fn clone(&self) -> Self {
193+
match self {
194+
Terminal::PkK(ref p) => Terminal::PkK(p.clone()),
195+
Terminal::PkH(ref p) => Terminal::PkH(p.clone()),
196+
Terminal::RawPkH(ref p) => Terminal::RawPkH(*p),
197+
Terminal::After(ref n) => Terminal::After(*n),
198+
Terminal::Older(ref n) => Terminal::Older(*n),
199+
Terminal::Sha256(ref x) => Terminal::Sha256(x.clone()),
200+
Terminal::Hash256(ref x) => Terminal::Hash256(x.clone()),
201+
Terminal::Ripemd160(ref x) => Terminal::Ripemd160(x.clone()),
202+
Terminal::Hash160(ref x) => Terminal::Hash160(x.clone()),
203+
Terminal::True => Terminal::True,
204+
Terminal::False => Terminal::False,
205+
Terminal::Alt(ref sub) => Terminal::Alt(Arc::new(Miniscript::clone(sub))),
206+
Terminal::Swap(ref sub) => Terminal::Swap(Arc::new(Miniscript::clone(sub))),
207+
Terminal::Check(ref sub) => Terminal::Check(Arc::new(Miniscript::clone(sub))),
208+
Terminal::DupIf(ref sub) => Terminal::DupIf(Arc::new(Miniscript::clone(sub))),
209+
Terminal::Verify(ref sub) => Terminal::Verify(Arc::new(Miniscript::clone(sub))),
210+
Terminal::NonZero(ref sub) => Terminal::NonZero(Arc::new(Miniscript::clone(sub))),
211+
Terminal::ZeroNotEqual(ref sub) => {
212+
Terminal::ZeroNotEqual(Arc::new(Miniscript::clone(sub)))
213+
}
214+
Terminal::AndV(ref left, ref right) => Terminal::AndV(
215+
Arc::new(Miniscript::clone(left)),
216+
Arc::new(Miniscript::clone(right)),
217+
),
218+
Terminal::AndB(ref left, ref right) => Terminal::AndB(
219+
Arc::new(Miniscript::clone(left)),
220+
Arc::new(Miniscript::clone(right)),
221+
),
222+
Terminal::AndOr(ref a, ref b, ref c) => Terminal::AndOr(
223+
Arc::new(Miniscript::clone(a)),
224+
Arc::new(Miniscript::clone(b)),
225+
Arc::new(Miniscript::clone(c)),
226+
),
227+
Terminal::OrB(ref left, ref right) => {
228+
Terminal::OrB(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
229+
}
230+
Terminal::OrD(ref left, ref right) => {
231+
Terminal::OrD(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
232+
}
233+
Terminal::OrC(ref left, ref right) => {
234+
Terminal::OrC(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
235+
}
236+
Terminal::OrI(ref left, ref right) => {
237+
Terminal::OrI(Arc::new(Miniscript::clone(left)), Arc::new(Miniscript::clone(right)))
238+
}
239+
Terminal::Thresh(ref thresh) => {
240+
Terminal::Thresh(thresh.map_ref(|child| Arc::new(Miniscript::clone(child))))
241+
}
242+
Terminal::Multi(ref thresh) => Terminal::Multi(thresh.clone()),
243+
Terminal::MultiA(ref thresh) => Terminal::MultiA(thresh.clone()),
244+
}
245+
}
246+
}
247+
189248
impl<Pk: MiniscriptKey, Ctx: ScriptContext> PartialEq for Terminal<Pk, Ctx> {
190249
fn eq(&self, other: &Self) -> bool {
191250
for (me, you) in self.pre_order_iter().zip(other.pre_order_iter()) {

‎src/miniscript/mod.rs‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,13 @@ mod private {
5353
use core::marker::PhantomData;
5454

5555
use super::types::{ExtData, Type};
56+
use crate::iter::TreeLike as _;
5657
pub use crate::miniscript::context::ScriptContext;
5758
use crate::miniscript::types;
59+
use crate::prelude::sync::Arc;
5860
use crate::{Error, MiniscriptKey, Terminal, MAX_RECURSION_DEPTH};
5961

6062
/// The top-level miniscript abstract syntax tree (AST).
61-
#[derive(Clone)]
6263
pub struct Miniscript<Pk: MiniscriptKey, Ctx: ScriptContext> {
6364
/// A node in the AST.
6465
pub node: Terminal<Pk, Ctx>,
@@ -69,6 +70,70 @@ mod private {
6970
/// Context PhantomData. Only accessible inside this crate
7071
phantom: PhantomData<Ctx>,
7172
}
73+
74+
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Clone for Miniscript<Pk, Ctx> {
75+
/// We implement clone as a "deep clone" which reconstructs the entire tree.
76+
///
77+
/// If users just want to clone Arcs they can use Arc::clone themselves.
78+
/// Note that if a Miniscript was constructed using shared Arcs, the result
79+
/// of calling `clone` will no longer have shared Arcs. So there is no
80+
/// pleasing everyone. But for the two common cases:
81+
///
82+
/// * Users don't care about sharing at all, and they can call `Arc::clone`
83+
/// on an `Arc<Miniscript>`.
84+
/// * Users want a deep copy which does not share any nodes with the original
85+
/// (for example, because they have keys that have interior mutability),
86+
/// and they can call `Miniscript::clone`.
87+
fn clone(&self) -> Self {
88+
let mut stack = vec![];
89+
for item in self.post_order_iter() {
90+
let child_n = |n| Arc::clone(&stack[item.child_indices[n]]);
91+
92+
let new_term = match item.node.node {
93+
Terminal::PkK(ref p) => Terminal::PkK(p.clone()),
94+
Terminal::PkH(ref p) => Terminal::PkH(p.clone()),
95+
Terminal::RawPkH(ref p) => Terminal::RawPkH(*p),
96+
Terminal::After(ref n) => Terminal::After(*n),
97+
Terminal::Older(ref n) => Terminal::Older(*n),
98+
Terminal::Sha256(ref x) => Terminal::Sha256(x.clone()),
99+
Terminal::Hash256(ref x) => Terminal::Hash256(x.clone()),
100+
Terminal::Ripemd160(ref x) => Terminal::Ripemd160(x.clone()),
101+
Terminal::Hash160(ref x) => Terminal::Hash160(x.clone()),
102+
Terminal::True => Terminal::True,
103+
Terminal::False => Terminal::False,
104+
Terminal::Alt(..) => Terminal::Alt(child_n(0)),
105+
Terminal::Swap(..) => Terminal::Swap(child_n(0)),
106+
Terminal::Check(..) => Terminal::Check(child_n(0)),
107+
Terminal::DupIf(..) => Terminal::DupIf(child_n(0)),
108+
Terminal::Verify(..) => Terminal::Verify(child_n(0)),
109+
Terminal::NonZero(..) => Terminal::NonZero(child_n(0)),
110+
Terminal::ZeroNotEqual(..) => Terminal::ZeroNotEqual(child_n(0)),
111+
Terminal::AndV(..) => Terminal::AndV(child_n(0), child_n(1)),
112+
Terminal::AndB(..) => Terminal::AndB(child_n(0), child_n(1)),
113+
Terminal::AndOr(..) => Terminal::AndOr(child_n(0), child_n(1), child_n(2)),
114+
Terminal::OrB(..) => Terminal::OrB(child_n(0), child_n(1)),
115+
Terminal::OrD(..) => Terminal::OrD(child_n(0), child_n(1)),
116+
Terminal::OrC(..) => Terminal::OrC(child_n(0), child_n(1)),
117+
Terminal::OrI(..) => Terminal::OrI(child_n(0), child_n(1)),
118+
Terminal::Thresh(ref thresh) => Terminal::Thresh(
119+
thresh.map_from_post_order_iter(&item.child_indices, &stack),
120+
),
121+
Terminal::Multi(ref thresh) => Terminal::Multi(thresh.clone()),
122+
Terminal::MultiA(ref thresh) => Terminal::MultiA(thresh.clone()),
123+
};
124+
125+
stack.push(Arc::new(Miniscript {
126+
node: new_term,
127+
ty: item.node.ty,
128+
ext: item.node.ext,
129+
phantom: PhantomData,
130+
}));
131+
}
132+
133+
Arc::try_unwrap(stack.pop().unwrap()).unwrap()
134+
}
135+
}
136+
72137
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
73138
/// The `1` combinator.
74139
pub const TRUE: Self = Miniscript {

0 commit comments

Comments
 (0)