Skip to content

Commit

Permalink
miniscript: eliminate recursion from TranslatePk
Browse files Browse the repository at this point in the history 
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 rust-bitcoin#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.
  • Loading branch information
@apoelstra
apoelstra committed Jul 14, 2023
1 parent ea70d8a commit 20450b1
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 103 deletions.
4 changes: 2 additions & 2 deletions src/interpreter/inner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::PublicKey, Ctx> {
translate_hash_clone!(bitcoin::PublicKey, BitcoinKey, ());
}

self.real_translate_pk(&mut TranslateFullPk)
self.translate_pk_ctx(&mut TranslateFullPk)
.expect("Translation should succeed")
}
}
Expand All @@ -397,7 +397,7 @@ impl<Ctx: ScriptContext> ToNoChecks for Miniscript<bitcoin::key::XOnlyPublicKey,

translate_hash_clone!(bitcoin::key::XOnlyPublicKey, BitcoinKey, ());
}
self.real_translate_pk(&mut TranslateXOnlyPk)
self.translate_pk_ctx(&mut TranslateXOnlyPk)
.expect("Translation should succeed")
}
}
Expand Down
97 changes: 1 addition & 96 deletions src/miniscript/astelem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::prelude::*;
use crate::util::MsKeyBuilder;
use crate::{
errstr, expression, script_num_size, AbsLockTime, Error, Miniscript, MiniscriptKey,
Terminal, ToPublicKey, TranslateErr, TranslatePk, Translator,
Terminal, ToPublicKey,
};

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
Expand All @@ -46,102 +46,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
}
}

impl<Pk, Q, Ctx> TranslatePk<Pk, Q> for Terminal<Pk, Ctx>
where
Pk: MiniscriptKey,
Q: MiniscriptKey,
Ctx: ScriptContext,
{
type Output = Terminal<Q, Ctx>;

/// Converts an AST element with one public key type to one of another public key type.
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
where
T: Translator<Pk, Q, E>,
{
self.real_translate_pk(translate)
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
pub(super) fn real_translate_pk<Q, CtxQ, T, E>(
&self,
t: &mut T,
) -> Result<Terminal<Q, CtxQ>, TranslateErr<E>>
where
Q: MiniscriptKey,
CtxQ: ScriptContext,
T: Translator<Pk, Q, E>,
{
let frag: Terminal<Q, CtxQ> = 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<Vec<Arc<Miniscript<Q, _>>>, _> = 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<Vec<Q>, _> = keys.iter().map(|k| t.pk(k)).collect();
Terminal::Multi(k, keys?)
}
Terminal::MultiA(k, ref keys) => {
let keys: Result<Vec<Q>, _> = 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<hash160::Hash, Pk>) -> Terminal<Pk, Ctx> {
match self {
Expand Down
57 changes: 52 additions & 5 deletions src/miniscript/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
where
T: Translator<Pk, Q, E>,
{
self.real_translate_pk(translate)
self.translate_pk_ctx(t)
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
pub(super) fn real_translate_pk<Q, CtxQ, T, FuncError>(
pub(super) fn translate_pk_ctx<Q, CtxQ, T, FuncError>(
&self,
t: &mut T,
) -> Result<Miniscript<Q, CtxQ>, TranslateErr<FuncError>>
Expand All @@ -337,8 +337,55 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
CtxQ: ScriptContext,
T: Translator<Pk, Q, FuncError>,
{
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<Vec<Q>, _> = keys.iter().map(|k| t.pk(k)).collect();
Terminal::Multi(k, keys?)
}
Terminal::MultiA(k, ref keys) => {
let keys: Result<Vec<Q>, _> = 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.
Expand Down

0 comments on commit 20450b1

Please sign in to comment.