Skip to content

Commit 1746cf4

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 5f4d964 commit 1746cf4

File tree

13 files changed

+178
-99
lines changed

13 files changed

+178
-99
lines changed

‎bitcoind-tests/tests/setup/test_util.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,10 @@ pub fn parse_test_desc(
279279
let desc = subs_hash_frag(desc, pubdata);
280280
let desc = Descriptor::<String>::from_str(&desc)?;
281281
let mut translator = StrDescPubKeyTranslator(0, pubdata);
282-
let desc: Result<_, ()> = desc.translate_pk(&mut translator);
283-
Ok(desc.expect("Translate must succeed"))
282+
let desc = desc
283+
.translate_pk(&mut translator)
284+
.expect("Translation failed");
285+
Ok(desc)
284286
}
285287

286288
// substitute hash fragments in the string as the per rules

‎src/descriptor/bare.rs

+9-6
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ use crate::policy::{semantic, Liftable};
2020
use crate::prelude::*;
2121
use crate::util::{varint_len, witness_to_scriptsig};
2222
use crate::{
23-
BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, TranslatePk,
24-
Translator,
23+
BareCtx, Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, ToPublicKey, TranslateErr,
24+
TranslatePk, Translator,
2525
};
2626

2727
/// Create a Bare Descriptor. That is descriptor that is
@@ -191,11 +191,11 @@ where
191191
{
192192
type Output = Bare<Q>;
193193

194-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
194+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Bare<Q>, TranslateErr<E>>
195195
where
196196
T: Translator<P, Q, E>,
197197
{
198-
Ok(Bare::new(self.ms.translate_pk(t)?).expect("Translation cannot fail inside Bare"))
198+
Ok(Bare::new(self.ms.translate_pk(t)?).map_err(TranslateErr::OuterError)?)
199199
}
200200
}
201201

@@ -379,11 +379,14 @@ where
379379
{
380380
type Output = Pkh<Q>;
381381

382-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
382+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
383383
where
384384
T: Translator<P, Q, E>,
385385
{
386386
let res = Pkh::new(t.pk(&self.pk)?);
387-
Ok(res.expect("Expect will be fixed in next commit"))
387+
match res {
388+
Ok(pk) => Ok(pk),
389+
Err(e) => Err(TranslateErr::OuterError(Error::from(e))),
390+
}
388391
}
389392
}

‎src/descriptor/mod.rs

+26-53
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::miniscript::{Legacy, Miniscript, Segwitv0};
2626
use crate::prelude::*;
2727
use crate::{
2828
expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, Satisfier,
29-
ToPublicKey, TranslatePk, Translator,
29+
ToPublicKey, TranslateErr, TranslatePk, Translator,
3030
};
3131

3232
mod bare;
@@ -519,7 +519,7 @@ where
519519
type Output = Descriptor<Q>;
520520

521521
/// Converts a descriptor using abstract keys to one using specific keys.
522-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
522+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
523523
where
524524
T: Translator<P, Q, E>,
525525
{
@@ -584,7 +584,10 @@ impl Descriptor<DescriptorPublicKey> {
584584

585585
translate_hash_clone!(DescriptorPublicKey, DescriptorPublicKey, ConversionError);
586586
}
587-
self.translate_pk(&mut Derivator(index))
587+
self.translate_pk(&mut Derivator(index)).map_err(|e| {
588+
e.try_into_translator_err()
589+
.expect("No Context errors while translating")
590+
})
588591
}
589592

590593
#[deprecated(note = "use at_derivation_index instead")]
@@ -697,9 +700,13 @@ impl Descriptor<DescriptorPublicKey> {
697700
}
698701

699702
let descriptor = Descriptor::<String>::from_str(s)?;
700-
let descriptor = descriptor
701-
.translate_pk(&mut keymap_pk)
702-
.map_err(|e| Error::Unexpected(e.to_string()))?;
703+
let descriptor = descriptor.translate_pk(&mut keymap_pk).map_err(|e| {
704+
Error::Unexpected(
705+
e.try_into_translator_err()
706+
.expect("No Outer context errors")
707+
.to_string(),
708+
)
709+
})?;
703710

704711
Ok((descriptor, keymap_pk.0))
705712
}
@@ -826,49 +833,16 @@ impl Descriptor<DescriptorPublicKey> {
826833

827834
for (i, desc) in descriptors.iter_mut().enumerate() {
828835
let mut index_choser = IndexChoser(i);
829-
*desc = desc.translate_pk(&mut index_choser)?;
836+
*desc = desc.translate_pk(&mut index_choser).map_err(|e| {
837+
e.try_into_translator_err()
838+
.expect("No Context errors possible")
839+
})?;
830840
}
831841

832842
Ok(descriptors)
833843
}
834844
}
835845

836-
impl<Pk: MiniscriptKey> Descriptor<Pk> {
837-
/// Whether this descriptor is a multipath descriptor that contains any 2 multipath keys
838-
/// with a different number of derivation paths.
839-
/// Such a descriptor is invalid according to BIP389.
840-
pub fn multipath_length_mismatch(&self) -> bool {
841-
// (Ab)use `for_each_key` to record the number of derivation paths a multipath key has.
842-
#[derive(PartialEq)]
843-
enum MultipathLenChecker {
844-
SinglePath,
845-
MultipathLen(usize),
846-
LenMismatch,
847-
}
848-
849-
let mut checker = MultipathLenChecker::SinglePath;
850-
self.for_each_key(|key| {
851-
match key.num_der_paths() {
852-
0 | 1 => {}
853-
n => match checker {
854-
MultipathLenChecker::SinglePath => {
855-
checker = MultipathLenChecker::MultipathLen(n);
856-
}
857-
MultipathLenChecker::MultipathLen(len) => {
858-
if len != n {
859-
checker = MultipathLenChecker::LenMismatch;
860-
}
861-
}
862-
MultipathLenChecker::LenMismatch => {}
863-
},
864-
}
865-
true
866-
});
867-
868-
checker == MultipathLenChecker::LenMismatch
869-
}
870-
}
871-
872846
impl Descriptor<DefiniteDescriptorKey> {
873847
/// Convert all the public keys in the descriptor to [`bitcoin::PublicKey`] by deriving them or
874848
/// otherwise converting them. All [`bitcoin::secp256k1::XOnlyPublicKey`]s are converted to by adding a
@@ -912,8 +886,14 @@ impl Descriptor<DefiniteDescriptorKey> {
912886
translate_hash_clone!(DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError);
913887
}
914888

915-
let derived = self.translate_pk(&mut Derivator(secp))?;
916-
Ok(derived)
889+
let derived = self.translate_pk(&mut Derivator(secp));
890+
match derived {
891+
Ok(derived) => Ok(derived),
892+
Err(e) => match e.try_into_translator_err() {
893+
Ok(e) => Err(e),
894+
Err(_) => unreachable!("No Context errors when deriving keys"),
895+
},
896+
}
917897
}
918898
}
919899

@@ -947,10 +927,6 @@ impl_from_str!(
947927
expression::FromTree::from_tree(&top)
948928
}?;
949929

950-
if desc.multipath_length_mismatch() {
951-
return Err(Error::MultipathDescLenMismatch);
952-
}
953-
954930
Ok(desc)
955931
}
956932
);
@@ -1995,7 +1971,6 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
19951971
// We can parse a multipath descriptors, and make it into separate single-path descriptors.
19961972
let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<7';8h;20>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/<0;1;987>/*)))").unwrap();
19971973
assert!(desc.is_multipath());
1998-
assert!(!desc.multipath_length_mismatch());
19991974
assert_eq!(desc.into_single_descriptors().unwrap(), vec![
20001975
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/7'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/0/*)))").unwrap(),
20011976
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/8h/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/1/*)))").unwrap(),
@@ -2005,7 +1980,6 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
20051980
// Even if only one of the keys is multipath.
20061981
let desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
20071982
assert!(desc.is_multipath());
2008-
assert!(!desc.multipath_length_mismatch());
20091983
assert_eq!(desc.into_single_descriptors().unwrap(), vec![
20101984
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/0/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
20111985
Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/1/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap(),
@@ -2014,7 +1988,6 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
20141988
// We can detect regular single-path descriptors.
20151989
let notmulti_desc = Descriptor::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/4567/*)))").unwrap();
20161990
assert!(!notmulti_desc.is_multipath());
2017-
assert!(!notmulti_desc.multipath_length_mismatch());
20181991
assert_eq!(
20191992
notmulti_desc.clone().into_single_descriptors().unwrap(),
20201993
vec![notmulti_desc]
@@ -2031,7 +2004,7 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
20312004
"02015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
20322005
)
20332006
.unwrap();
2034-
let x_only_key = bitcoin::XOnlyPublicKey::from_str(
2007+
let x_only_key = bitcoin::secp256k1::XOnlyPublicKey::from_str(
20352008
"015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
20362009
)
20372010
.unwrap();

‎src/descriptor/segwitv0.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use crate::policy::{semantic, Liftable};
1818
use crate::prelude::*;
1919
use crate::util::varint_len;
2020
use crate::{
21-
Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, TranslatePk,
22-
Translator,
21+
Error, ForEachKey, Miniscript, MiniscriptKey, Satisfier, Segwitv0, ToPublicKey, TranslateErr,
22+
TranslatePk, Translator,
2323
};
2424
/// A Segwitv0 wsh descriptor
2525
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
@@ -285,7 +285,7 @@ where
285285
{
286286
type Output = Wsh<Q>;
287287

288-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
288+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
289289
where
290290
T: Translator<P, Q, E>,
291291
{
@@ -486,10 +486,14 @@ where
486486
{
487487
type Output = Wpkh<Q>;
488488

489-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
489+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
490490
where
491491
T: Translator<P, Q, E>,
492492
{
493-
Ok(Wpkh::new(t.pk(&self.pk)?).expect("Uncompressed keys in Wpkh"))
493+
let res = Wpkh::new(t.pk(&self.pk)?);
494+
match res {
495+
Ok(pk) => Ok(pk),
496+
Err(e) => Err(TranslateErr::OuterError(Error::from(e))),
497+
}
494498
}
495499
}

‎src/descriptor/sh.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::prelude::*;
2323
use crate::util::{varint_len, witness_to_scriptsig};
2424
use crate::{
2525
push_opcode_size, Error, ForEachKey, Legacy, Miniscript, MiniscriptKey, Satisfier, Segwitv0,
26-
ToPublicKey, TranslatePk, Translator,
26+
ToPublicKey, TranslateErr, TranslatePk, Translator,
2727
};
2828

2929
/// A Legacy p2sh Descriptor
@@ -440,7 +440,7 @@ where
440440
{
441441
type Output = Sh<Q>;
442442

443-
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, E>
443+
fn translate_pk<T, E>(&self, t: &mut T) -> Result<Self::Output, TranslateErr<E>>
444444
where
445445
T: Translator<P, Q, E>,
446446
{

‎src/descriptor/sortedmulti.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
1818
use crate::prelude::*;
1919
use crate::{
2020
errstr, expression, miniscript, policy, script_num_size, Error, ForEachKey, Miniscript,
21-
MiniscriptKey, Satisfier, ToPublicKey, Translator,
21+
MiniscriptKey, Satisfier, ToPublicKey, TranslateErr, Translator,
2222
};
2323

2424
/// Contents of a "sortedmulti" descriptor
@@ -87,17 +87,14 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
8787
pub fn translate_pk<T, Q, FuncError>(
8888
&self,
8989
t: &mut T,
90-
) -> Result<SortedMultiVec<Q, Ctx>, FuncError>
90+
) -> Result<SortedMultiVec<Q, Ctx>, TranslateErr<FuncError>>
9191
where
9292
T: Translator<Pk, Q, FuncError>,
9393
Q: MiniscriptKey,
9494
{
9595
let pks: Result<Vec<Q>, _> = self.pks.iter().map(|pk| t.pk(pk)).collect();
96-
Ok(SortedMultiVec {
97-
k: self.k,
98-
pks: pks?,
99-
phantom: PhantomData,
100-
})
96+
let res = SortedMultiVec::new(self.k, pks?).map_err(TranslateErr::OuterError)?;
97+
Ok(res)
10198
}
10299
}
103100

‎src/descriptor/tr.rs

+5-5
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.
@@ -129,9 +129,9 @@ impl<Pk: MiniscriptKey> TapTree<Pk> {
129129
}
130130

131131
// Helper function to translate keys
132-
fn translate_helper<T, Q, Error>(&self, t: &mut T) -> Result<TapTree<Q>, Error>
132+
fn translate_helper<T, Q, E>(&self, t: &mut T) -> Result<TapTree<Q>, TranslateErr<E>>
133133
where
134-
T: Translator<Pk, Q, Error>,
134+
T: Translator<Pk, Q, E>,
135135
Q: MiniscriptKey,
136136
{
137137
let frag = match self {
@@ -632,7 +632,7 @@ where
632632
{
633633
type Output = Tr<Q>;
634634

635-
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, E>
635+
fn translate_pk<T, E>(&self, translate: &mut T) -> Result<Self::Output, TranslateErr<E>>
636636
where
637637
T: Translator<P, Q, E>,
638638
{
@@ -641,7 +641,7 @@ where
641641
None => None,
642642
};
643643
let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
644-
.expect("This will be removed in future");
644+
.map_err(|e| TranslateErr::OuterError(e))?;
645645
Ok(translate_desc)
646646
}
647647
}

0 commit comments

Comments
 (0)