From 20450b177bfe17fe5ef0cbf49d3d6a942a91c637 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 14 Jul 2023 22:41:40 +0000 Subject: [PATCH] miniscript: eliminate recursion from TranslatePk This is again a breaking change because we remove the trait impl from `Terminal`, but again I don't think the trait impl should've existed. This also exposed an "off-label" use of `real_translate_pk` to convert context types as well as pk types, which I apparently explicitly reviewed and ACKed when reviewing #426, but which in retrospect looks a little funny. Rename this function to translate_pk_ctx so it's clear that it's doing two jobs. --- src/interpreter/inner.rs | 4 +- src/miniscript/astelem.rs | 97 +-------------------------------------- src/miniscript/mod.rs | 57 +++++++++++++++++++++-- 3 files changed, 55 insertions(+), 103 deletions(-) diff --git a/src/interpreter/inner.rs b/src/interpreter/inner.rs index 538cacb7e..59de53324 100644 --- a/src/interpreter/inner.rs +++ b/src/interpreter/inner.rs @@ -380,7 +380,7 @@ impl ToNoChecks for Miniscript { translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, ()); } - self.real_translate_pk(&mut TranslateFullPk) + self.translate_pk_ctx(&mut TranslateFullPk) .expect("Translation should succeed") } } @@ -397,7 +397,7 @@ impl ToNoChecks for Miniscript Terminal { @@ -46,102 +46,7 @@ impl Terminal { } } -impl TranslatePk for Terminal -where - Pk: MiniscriptKey, - Q: MiniscriptKey, - Ctx: ScriptContext, -{ - type Output = Terminal; - - /// Converts an AST element with one public key type to one of another public key type. - fn translate_pk(&self, translate: &mut T) -> Result> - where - T: Translator, - { - self.real_translate_pk(translate) - } -} - impl Terminal { - pub(super) fn real_translate_pk( - &self, - t: &mut T, - ) -> Result, TranslateErr> - where - Q: MiniscriptKey, - CtxQ: ScriptContext, - T: Translator, - { - let frag: Terminal = match *self { - Terminal::PkK(ref p) => Terminal::PkK(t.pk(p)?), - Terminal::PkH(ref p) => Terminal::PkH(t.pk(p)?), - Terminal::RawPkH(ref p) => Terminal::RawPkH(*p), - Terminal::After(n) => Terminal::After(n), - Terminal::Older(n) => Terminal::Older(n), - Terminal::Sha256(ref x) => Terminal::Sha256(t.sha256(x)?), - Terminal::Hash256(ref x) => Terminal::Hash256(t.hash256(x)?), - Terminal::Ripemd160(ref x) => Terminal::Ripemd160(t.ripemd160(x)?), - Terminal::Hash160(ref x) => Terminal::Hash160(t.hash160(x)?), - Terminal::True => Terminal::True, - Terminal::False => Terminal::False, - Terminal::Alt(ref sub) => Terminal::Alt(Arc::new(sub.real_translate_pk(t)?)), - Terminal::Swap(ref sub) => Terminal::Swap(Arc::new(sub.real_translate_pk(t)?)), - Terminal::Check(ref sub) => Terminal::Check(Arc::new(sub.real_translate_pk(t)?)), - Terminal::DupIf(ref sub) => Terminal::DupIf(Arc::new(sub.real_translate_pk(t)?)), - Terminal::Verify(ref sub) => Terminal::Verify(Arc::new(sub.real_translate_pk(t)?)), - Terminal::NonZero(ref sub) => Terminal::NonZero(Arc::new(sub.real_translate_pk(t)?)), - Terminal::ZeroNotEqual(ref sub) => { - Terminal::ZeroNotEqual(Arc::new(sub.real_translate_pk(t)?)) - } - Terminal::AndV(ref left, ref right) => Terminal::AndV( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::AndB(ref left, ref right) => Terminal::AndB( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::AndOr(ref a, ref b, ref c) => Terminal::AndOr( - Arc::new(a.real_translate_pk(t)?), - Arc::new(b.real_translate_pk(t)?), - Arc::new(c.real_translate_pk(t)?), - ), - Terminal::OrB(ref left, ref right) => Terminal::OrB( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::OrD(ref left, ref right) => Terminal::OrD( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::OrC(ref left, ref right) => Terminal::OrC( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::OrI(ref left, ref right) => Terminal::OrI( - Arc::new(left.real_translate_pk(t)?), - Arc::new(right.real_translate_pk(t)?), - ), - Terminal::Thresh(k, ref subs) => { - let subs: Result>>, _> = subs - .iter() - .map(|s| s.real_translate_pk(t).map(Arc::new)) - .collect(); - Terminal::Thresh(k, subs?) - } - Terminal::Multi(k, ref keys) => { - let keys: Result, _> = keys.iter().map(|k| t.pk(k)).collect(); - Terminal::Multi(k, keys?) - } - Terminal::MultiA(k, ref keys) => { - let keys: Result, _> = keys.iter().map(|k| t.pk(k)).collect(); - Terminal::MultiA(k, keys?) - } - }; - Ok(frag) - } - /// Substitutes raw public keys hashes with the public keys as provided by map. pub fn substitute_raw_pkh(&self, pk_map: &BTreeMap) -> Terminal { match self { diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 0dd62e72c..f6e71d53e 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -319,16 +319,16 @@ where /// Translates a struct from one generic to another where the translation /// for Pk is provided by [`Translator`] - fn translate_pk(&self, translate: &mut T) -> Result> + fn translate_pk(&self, t: &mut T) -> Result> where T: Translator, { - self.real_translate_pk(translate) + self.translate_pk_ctx(t) } } impl Miniscript { - pub(super) fn real_translate_pk( + pub(super) fn translate_pk_ctx( &self, t: &mut T, ) -> Result, TranslateErr> @@ -337,8 +337,55 @@ impl Miniscript { CtxQ: ScriptContext, T: Translator, { - let inner = self.node.real_translate_pk(t)?; - Miniscript::from_ast(inner).map_err(TranslateErr::OuterError) + let mut translated = vec![]; + for data in Arc::new(self.clone()).post_order_iter() { + // convenience method to reduce typing + let child_n = |n| Arc::clone(&translated[data.child_indices[n]]); + + let new_term = match data.node.node { + Terminal::PkK(ref p) => Terminal::PkK(t.pk(p)?), + Terminal::PkH(ref p) => Terminal::PkH(t.pk(p)?), + Terminal::RawPkH(ref p) => Terminal::RawPkH(*p), + Terminal::After(n) => Terminal::After(n), + Terminal::Older(n) => Terminal::Older(n), + Terminal::Sha256(ref x) => Terminal::Sha256(t.sha256(x)?), + Terminal::Hash256(ref x) => Terminal::Hash256(t.hash256(x)?), + Terminal::Ripemd160(ref x) => Terminal::Ripemd160(t.ripemd160(x)?), + Terminal::Hash160(ref x) => Terminal::Hash160(t.hash160(x)?), + 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(k, ref subs) => Terminal::Thresh( + k, + (0..subs.len()).map(child_n).collect(), + ), + Terminal::Multi(k, ref keys) => { + let keys: Result, _> = keys.iter().map(|k| t.pk(k)).collect(); + Terminal::Multi(k, keys?) + } + Terminal::MultiA(k, ref keys) => { + let keys: Result, _> = keys.iter().map(|k| t.pk(k)).collect(); + Terminal::MultiA(k, keys?) + } + }; + let new_ms = Miniscript::from_ast(new_term).map_err(TranslateErr::OuterError)?; + translated.push(Arc::new(new_ms)); + } + + Ok(Arc::try_unwrap(translated.pop().unwrap()).unwrap()) } /// Substitutes raw public keys hashes with the public keys as provided by map.