Skip to content

Commit 2b40679

Browse files
sanket1729sanket1729
committed
Update TranslatePk again
This fixes of the long last standing bug in rust-miniscript that allowed creating unsound miniscript using the translate APIs
1 parent 960d394 commit 2b40679

File tree

12 files changed

+139
-53
lines changed

12 files changed

+139
-53
lines changed

‎src/descriptor/bare.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use crate::policy::{semantic, Liftable};
3030
use crate::prelude::*;
3131
use crate::util::{varint_len, witness_to_scriptsig};
3232
use crate::{
33-
BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, TranslatePk,
34-
Translator,
33+
BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, TranslateErr,
34+
TranslatePk, Translator,
3535
};
3636

3737
/// Create a Bare Descriptor. That is descriptor that is
@@ -180,11 +180,11 @@ where
180180
{
181181
type Output = Bare<Q>;
182182

183-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
183+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Bare<Q>, TranslateErr<E>>
184184
where
185185
T: Translator<P, Q, E>,
186186
{
187-
Ok(Bare::new(self.ms.translate_pk(t)?).expect("Translation cannot fail inside Bare"))
187+
Ok(Bare::new(self.ms.translate_pk(t)?).map_err(TranslateErr::OuterError)?)
188188
}
189189
}
190190

@@ -345,11 +345,14 @@ where
345345
{
346346
type Output = Pkh<Q>;
347347

348-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
348+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
349349
where
350350
T: Translator<P, Q, E>,
351351
{
352352
let res = Pkh::new(t.pk(&self.pk)?);
353-
Ok(res.expect("Expect will be fixed in next commit"))
353+
match res {
354+
Ok(pk) => Ok(pk),
355+
Err(e) => Err(TranslateErr::OuterError(Error::from(e))),
356+
}
354357
}
355358
}

‎src/descriptor/mod.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use crate::miniscript::{Legacy, Miniscript, Segwitv0};
3838
use crate::prelude::*;
3939
use crate::{
4040
expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, Satisfier,
41-
ToPublicKey, TranslatePk, Translator,
41+
ToPublicKey, TranslateErr, TranslatePk, Translator,
4242
};
4343

4444
mod bare;
@@ -478,7 +478,7 @@ where
478478
type Output = Descriptor<Q>;
479479

480480
/// Converts a descriptor using abstract keys to one using specific keys.
481-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
481+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
482482
where
483483
T: Translator<P, Q, E>,
484484
{
@@ -539,7 +539,7 @@ impl Descriptor<DescriptorPublicKey> {
539539
translate_hash_clone!(DescriptorPublicKey, DescriptorPublicKey, ());
540540
}
541541
self.translate_pk(&mut Derivator(index))
542-
.expect("BIP 32 key index substitution cannot fail")
542+
.expect("No Errors in BIP32 substitution")
543543
}
544544

545545
#[deprecated(note = "use at_derivation_index instead")]
@@ -652,9 +652,13 @@ impl Descriptor<DescriptorPublicKey> {
652652
}
653653

654654
let descriptor = Descriptor::<String>::from_str(s)?;
655-
let descriptor = descriptor
656-
.translate_pk(&mut keymap_pk)
657-
.map_err(|e| Error::Unexpected(e.to_string()))?;
655+
let descriptor = descriptor.translate_pk(&mut keymap_pk).map_err(|e| {
656+
Error::Unexpected(
657+
e.try_into_translator_err()
658+
.expect("No Outer context errors")
659+
.to_string(),
660+
)
661+
})?;
658662

659663
Ok((descriptor, keymap_pk.0))
660664
}
@@ -768,8 +772,14 @@ impl Descriptor<DefiniteDescriptorKey> {
768772
translate_hash_clone!(DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError);
769773
}
770774

771-
let derived = self.translate_pk(&mut Derivator(secp))?;
772-
Ok(derived)
775+
let derived = self.translate_pk(&mut Derivator(secp));
776+
match derived {
777+
Ok(derived) => Ok(derived),
778+
Err(e) => match e.try_into_translator_err() {
779+
Ok(e) => Err(e),
780+
Err(_) => unreachable!("No Context errors when deriving keys"),
781+
},
782+
}
773783
}
774784
}
775785

‎src/descriptor/segwitv0.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use crate::policy::{semantic, Liftable};
2828
use crate::prelude::*;
2929
use crate::util::varint_len;
3030
use crate::{
31-
Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, TranslatePk,
32-
Translator,
31+
Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, TranslateErr,
32+
TranslatePk, Translator,
3333
};
3434
/// A Segwitv0 wsh descriptor
3535
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
@@ -266,7 +266,7 @@ where
266266
{
267267
type Output = Wsh<Q>;
268268

269-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
269+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
270270
where
271271
T: Translator<P, Q, E>,
272272
{
@@ -454,10 +454,14 @@ where
454454
{
455455
type Output = Wpkh<Q>;
456456

457-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
457+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
458458
where
459459
T: Translator<P, Q, E>,
460460
{
461-
Ok(Wpkh::new(t.pk(&self.pk)?).expect("Uncompressed keys in Wpkh"))
461+
let res = Wpkh::new(t.pk(&self.pk)?);
462+
match res {
463+
Ok(pk) => Ok(pk),
464+
Err(e) => Err(TranslateErr::OuterError(Error::from(e))),
465+
}
462466
}
463467
}

‎src/descriptor/sh.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::prelude::*;
3232
use crate::util::{varint_len, witness_to_scriptsig};
3333
use crate::{
3434
push_opcode_size, Error, ForEachKey, Legacy, Miniscript, MiniscriptKey, Satisfier, Segwitv0,
35-
ToPublicKey, TranslatePk, Translator,
35+
ToPublicKey, TranslateErr, TranslatePk, Translator,
3636
};
3737

3838
/// A Legacy p2sh Descriptor
@@ -398,7 +398,7 @@ where
398398
{
399399
type Output = Sh<Q>;
400400

401-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
401+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
402402
where
403403
T: Translator<P, Q, E>,
404404
{

‎src/descriptor/sortedmulti.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
2828
use crate::prelude::*;
2929
use crate::{
3030
errstr, expression, miniscript, policy, script_num_size, Error, ForEachKey, Miniscript,
31-
MiniscriptKey, Satisfier, ToPublicKey, Translator,
31+
MiniscriptKey, Satisfier, ToPublicKey, TranslateErr, Translator,
3232
};
3333

3434
/// Contents of a "sortedmulti" descriptor
@@ -97,17 +97,14 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
9797
pub fn translate_pk<T, Q, FuncError>(
9898
&self,
9999
t: &mut T,
100-
) -> Result<SortedMultiVec<Q, Ctx>, FuncError>
100+
) -> Result<SortedMultiVec<Q, Ctx>, TranslateErr<FuncError>>
101101
where
102102
T: Translator<Pk, Q, FuncError>,
103103
Q: MiniscriptKey,
104104
{
105105
let pks: Result<Vec<Q>, _> = self.pks.iter().map(|pk| t.pk(pk)).collect();
106-
Ok(SortedMultiVec {
107-
k: self.k,
108-
pks: pks?,
109-
phantom: PhantomData,
110-
})
106+
let res = SortedMultiVec::new(self.k, pks?).map_err(TranslateErr::OuterError)?;
107+
Ok(res)
111108
}
112109
}
113110

‎src/descriptor/tr.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::prelude::*;
2020
use crate::util::{varint_len, witness_size};
2121
use crate::{
2222
errstr, Error, ForEachKey, MiniscriptKey, Satisfier, ScriptContext, Tap, ToPublicKey,
23-
TranslatePk, Translator,
23+
TranslateErr, TranslatePk, Translator,
2424
};
2525

2626
/// A Taproot Tree representation.
@@ -128,9 +128,9 @@ impl<Pk: MiniscriptKey> TapTree<Pk> {
128128
}
129129

130130
// Helper function to translate keys
131-
fn translate_helper<T, Q, Error>(&self, t: &mut T) -> Result<TapTree<Q>, Error>
131+
fn translate_helper<T, Q, E>(&self, t: &mut T) -> Result<TapTree<Q>, TranslateErr<E>>
132132
where
133-
T: Translator<Pk, Q, Error>,
133+
T: Translator<Pk, Q, E>,
134134
Q: MiniscriptKey,
135135
{
136136
let frag = match self {
@@ -583,7 +583,7 @@ where
583583
{
584584
type Output = Tr<Q>;
585585

586-
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, E>
586+
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
587587
where
588588
T: Translator<P, Q, E>,
589589
{
@@ -592,7 +592,7 @@ where
592592
None => None,
593593
};
594594
let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
595-
.expect("This will be removed in future");
595+
.map_err(|e| TranslateErr::OuterError(e))?;
596596
Ok(translate_desc)
597597
}
598598
}

‎src/lib.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,57 @@ where
542542
fn hash160(&mut self, hash160: &P::Hash160) -> Result<Q::Hash160, E>;
543543
}
544544

545+
/// An enum for representing translation errors
546+
pub enum TranslateErr<E> {
547+
/// Error inside in the underlying key translation
548+
TranslatorErr(E),
549+
/// Error in the final translated structure. In some cases, the translated
550+
/// structure might not be valid under the given context. For example, translating
551+
/// from string keys to x-only keys in wsh descriptors.
552+
OuterError(Error),
553+
}
554+
555+
impl<E> TranslateErr<E> {
556+
/// Enum used to capture errors from the [`Translator`] trait as well as
557+
/// context errors from the translated structure.
558+
/// The errors occurred in translation are captured in the [`TranslateErr::TranslatorErr`]
559+
/// while the errors in the translated structure are captured in the [`TranslateErr::OuterError`]
560+
///
561+
/// As of taproot upgrade: The following rules apply to the translation of descriptors:
562+
/// - Legacy/Bare does not allow x_only keys
563+
/// - SegwitV0 does not allow uncompressed keys and x_only keys
564+
/// - Tapscript does not allow uncompressed keys
565+
/// - Translating into multi-path descriptors should have same number of path
566+
/// for all the keys in the descriptor
567+
///
568+
/// # Errors
569+
///
570+
/// This function will return an error if the Error is OutError.
571+
pub fn try_into_translator_err(self) -> Result<E, Self> {
572+
if let Self::TranslatorErr(v) = self {
573+
Ok(v)
574+
} else {
575+
Err(self)
576+
}
577+
}
578+
}
579+
580+
impl<E> From<E> for TranslateErr<E> {
581+
fn from(v: E) -> Self {
582+
Self::TranslatorErr(v)
583+
}
584+
}
585+
586+
// Required for unwrap
587+
impl<E: fmt::Debug> fmt::Debug for TranslateErr<E> {
588+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
589+
match self {
590+
Self::TranslatorErr(e) => write!(f, "TranslatorErr({:?})", e),
591+
Self::OuterError(e) => write!(f, "OuterError({:?})", e),
592+
}
593+
}
594+
}
595+
545596
/// Converts a descriptor using abstract keys to one using specific keys. Uses translator `t` to do
546597
/// the actual translation function calls.
547598
pub trait TranslatePk<P, Q>
@@ -554,7 +605,7 @@ where
554605

555606
/// Translates a struct from one generic to another where the translations
556607
/// for Pk are provided by the given [`Translator`].
557-
fn translate_pk<T, E>(&self, translator: &mut T) -> Result<Self::Output, E>
608+
fn translate_pk<T, E>(&self, translator: &mut T) -> Result<Self::Output, TranslateErr<E>>
558609
where
559610
T: Translator<P, Q, E>;
560611
}

‎src/miniscript/astelem.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use crate::prelude::*;
3434
use crate::util::MsKeyBuilder;
3535
use crate::{
3636
errstr, expression, script_num_size, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal,
37-
ToPublicKey, TranslatePk, Translator,
37+
ToPublicKey, TranslateErr, TranslatePk, Translator,
3838
};
3939

4040
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
@@ -67,7 +67,7 @@ where
6767
type Output = Terminal<Q, Ctx>;
6868

6969
/// Converts an AST element with one public key type to one of another public key type.
70-
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, E>
70+
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
7171
where
7272
T: Translator<Pk, Q, E>,
7373
{
@@ -119,7 +119,10 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
119119
}
120120
}
121121

122-
pub(super) fn real_translate_pk<Q, CtxQ, T, E>(&self, t: &mut T) -> Result<Terminal<Q, CtxQ>, E>
122+
pub(super) fn real_translate_pk<Q, CtxQ, T, E>(
123+
&self,
124+
t: &mut T,
125+
) -> Result<Terminal<Q, CtxQ>, TranslateErr<E>>
123126
where
124127
Q: MiniscriptKey,
125128
CtxQ: ScriptContext,

‎src/miniscript/mod.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use bitcoin::util::taproot::{LeafVersion, TapLeafHash};
3333
use self::analyzable::ExtParams;
3434
pub use self::context::{BareCtx, Legacy, Segwitv0, Tap};
3535
use crate::prelude::*;
36+
use crate::TranslateErr;
3637

3738
pub mod analyzable;
3839
pub mod astelem;
@@ -121,12 +122,14 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
121122
/// `AstElem` fragment. Dependent on display and clone because of Error
122123
/// Display code of type_check.
123124
pub fn from_ast(t: Terminal<Pk, Ctx>) -> Result<Miniscript<Pk, Ctx>, Error> {
124-
Ok(Miniscript {
125+
let res = Miniscript {
125126
ty: Type::type_check(&t, |_| None)?,
126127
ext: ExtData::type_check(&t, |_| None)?,
127128
node: t,
128129
phantom: PhantomData,
129-
})
130+
};
131+
Ctx::check_global_consensus_validity(&res)?;
132+
Ok(res)
130133
}
131134

132135
/// Create a new `Miniscript` from a `Terminal` node and a `Type` annotation
@@ -321,7 +324,7 @@ where
321324

322325
/// Translates a struct from one generic to another where the translation
323326
/// for Pk is provided by [`Translator`]
324-
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, E>
327+
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
325328
where
326329
T: Translator<Pk, Q, E>,
327330
{
@@ -340,14 +343,14 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
340343
pub(super) fn real_translate_pk<Q, CtxQ, T, FuncError>(
341344
&self,
342345
t: &mut T,
343-
) -> Result<Miniscript<Q, CtxQ>, FuncError>
346+
) -> Result<Miniscript<Q, CtxQ>, TranslateErr<FuncError>>
344347
where
345348
Q: MiniscriptKey,
346349
CtxQ: ScriptContext,
347350
T: Translator<Pk, Q, FuncError>,
348351
{
349352
let inner = self.node.real_translate_pk(t)?;
350-
Ok(Miniscript::from_ast(inner).expect("This will be removed in the next commit"))
353+
Miniscript::from_ast(inner).map_err(TranslateErr::OuterError)
351354
}
352355
}
353356

@@ -1133,4 +1136,13 @@ mod tests {
11331136
SegwitMs::parse_insane(&script).unwrap_err();
11341137
SegwitMs::parse_with_ext(&script, &ExtParams::allow_all()).unwrap();
11351138
}
1139+
1140+
#[test]
1141+
fn translate_tests() {
1142+
let ms = Miniscript::<String, Segwitv0>::from_str("pk(A)").unwrap();
1143+
let mut t = StrKeyTranslator::new();
1144+
let uncompressed = bitcoin::PublicKey::from_str("0400232a2acfc9b43fa89f1b4f608fde335d330d7114f70ea42bfb4a41db368a3e3be6934a4097dd25728438ef73debb1f2ffdb07fec0f18049df13bdc5285dc5b").unwrap();
1145+
t.pk_map.insert(String::from("A"), uncompressed);
1146+
ms.translate_pk(&mut t).unwrap_err();
1147+
}
11361148
}

0 commit comments

Comments
 (0)