Skip to content

Commit 7d4a621

Browse files
sanket1729sanket1729
committed
Cleanly check validity rules for DescriptorKeys
Each context has slightly different rules on what Pks are allowed in descriptors Legacy/Bare does not allow x_only keys SegwitV0 does not allow uncompressed keys and x_only keys Tapscript does not allow uncompressed keys Note that String types are allowed everywhere
1 parent c907ee0 commit 7d4a621

File tree

6 files changed

+145
-72
lines changed

6 files changed

+145
-72
lines changed

‎src/descriptor/bare.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bitcoin::{Address, Network, Script};
1515

1616
use super::checksum::{self, verify_checksum};
1717
use crate::expression::{self, FromTree};
18-
use crate::miniscript::context::ScriptContext;
18+
use crate::miniscript::context::{ScriptContext, ScriptContextError};
1919
use crate::policy::{semantic, Liftable};
2020
use crate::prelude::*;
2121
use crate::util::{varint_len, witness_to_scriptsig};
@@ -208,9 +208,12 @@ pub struct Pkh<Pk: MiniscriptKey> {
208208

209209
impl<Pk: MiniscriptKey> Pkh<Pk> {
210210
/// Create a new Pkh descriptor
211-
pub fn new(pk: Pk) -> Self {
211+
pub fn new(pk: Pk) -> Result<Self, ScriptContextError> {
212212
// do the top-level checks
213-
Self { pk }
213+
match BareCtx::check_pk(&pk) {
214+
Ok(()) => Ok(Pkh { pk }),
215+
Err(e) => Err(e),
216+
}
214217
}
215218

216219
/// Get a reference to the inner key
@@ -337,7 +340,7 @@ impl_from_tree!(
337340
if top.name == "pkh" && top.args.len() == 1 {
338341
Ok(Pkh::new(expression::terminal(&top.args[0], |pk| {
339342
Pk::from_str(pk)
340-
})?))
343+
})?)?)
341344
} else {
342345
Err(Error::Unexpected(format!(
343346
"{}({} args) while parsing pkh descriptor",
@@ -378,6 +381,7 @@ where
378381
where
379382
T: Translator<P, Q, E>,
380383
{
381-
Ok(Pkh::new(t.pk(&self.pk)?))
384+
let res = Pkh::new(t.pk(&self.pk)?);
385+
Ok(res.expect("Expect will be fixed in next commit"))
382386
}
383387
}

‎src/descriptor/mod.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
178178
}
179179

180180
/// Create a new PkH descriptor
181-
pub fn new_pkh(pk: Pk) -> Self {
182-
Descriptor::Pkh(Pkh::new(pk))
181+
pub fn new_pkh(pk: Pk) -> Result<Self, Error> {
182+
Ok(Descriptor::Pkh(Pkh::new(pk)?))
183183
}
184184

185185
/// Create a new Wpkh descriptor
@@ -1288,7 +1288,7 @@ mod tests {
12881288
);
12891289
assert_eq!(bare.unsigned_script_sig(), bitcoin::Script::new());
12901290

1291-
let pkh = Descriptor::new_pkh(pk);
1291+
let pkh = Descriptor::new_pkh(pk).unwrap();
12921292
pkh.satisfy(&mut txin, &satisfier).expect("satisfaction");
12931293
assert_eq!(
12941294
txin,
@@ -2006,4 +2006,54 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
20062006
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2;3;4>/*)))").unwrap_err();
20072007
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1;2;3>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2>/*)))").unwrap_err();
20082008
}
2009+
2010+
#[test]
2011+
fn test_context_pks() {
2012+
let comp_key = bitcoin::PublicKey::from_str(
2013+
"02015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
2014+
)
2015+
.unwrap();
2016+
let x_only_key = bitcoin::XOnlyPublicKey::from_str(
2017+
"015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
2018+
)
2019+
.unwrap();
2020+
let uncomp_key = bitcoin::PublicKey::from_str("04015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab0d46021e9e69ef061eb25eab41ae206187b2b05e829559df59d78319bd9267b4").unwrap();
2021+
2022+
type Desc = Descriptor<DescriptorPublicKey>;
2023+
2024+
// Legacy tests, x-only keys are not supported
2025+
Desc::from_str(&format!("sh(pk({}))", comp_key)).unwrap();
2026+
Desc::from_str(&format!("sh(pk({}))", uncomp_key)).unwrap();
2027+
Desc::from_str(&format!("sh(pk({}))", x_only_key)).unwrap_err();
2028+
2029+
// bare tests, x-only keys not supported
2030+
Desc::from_str(&format!("pk({})", comp_key)).unwrap();
2031+
Desc::from_str(&format!("pk({})", uncomp_key)).unwrap();
2032+
Desc::from_str(&format!("pk({})", x_only_key)).unwrap_err();
2033+
2034+
// pkh tests, x-only keys not supported
2035+
Desc::from_str(&format!("pkh({})", comp_key)).unwrap();
2036+
Desc::from_str(&format!("pkh({})", uncomp_key)).unwrap();
2037+
Desc::from_str(&format!("pkh({})", x_only_key)).unwrap_err();
2038+
2039+
// wpkh tests, uncompressed and x-only keys not supported
2040+
Desc::from_str(&format!("wpkh({})", comp_key)).unwrap();
2041+
Desc::from_str(&format!("wpkh({})", uncomp_key)).unwrap_err();
2042+
Desc::from_str(&format!("wpkh({})", x_only_key)).unwrap_err();
2043+
2044+
// Segwitv0 tests, uncompressed and x-only keys not supported
2045+
Desc::from_str(&format!("wsh(pk({}))", comp_key)).unwrap();
2046+
Desc::from_str(&format!("wsh(pk({}))", uncomp_key)).unwrap_err();
2047+
Desc::from_str(&format!("wsh(pk({}))", x_only_key)).unwrap_err();
2048+
2049+
// Tap tests, key path
2050+
Desc::from_str(&format!("tr({})", comp_key)).unwrap();
2051+
Desc::from_str(&format!("tr({})", uncomp_key)).unwrap_err();
2052+
Desc::from_str(&format!("tr({})", x_only_key)).unwrap();
2053+
2054+
// Tap tests, script path
2055+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, comp_key)).unwrap();
2056+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, uncomp_key)).unwrap_err();
2057+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, x_only_key)).unwrap();
2058+
}
20092059
}

‎src/descriptor/segwitv0.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -306,14 +306,11 @@ pub struct Wpkh<Pk: MiniscriptKey> {
306306

307307
impl<Pk: MiniscriptKey> Wpkh<Pk> {
308308
/// Create a new Wpkh descriptor
309-
pub fn new(pk: Pk) -> Result<Self, Error> {
309+
pub fn new(pk: Pk) -> Result<Self, ScriptContextError> {
310310
// do the top-level checks
311-
if pk.is_uncompressed() {
312-
Err(Error::ContextError(ScriptContextError::CompressedOnly(
313-
pk.to_string(),
314-
)))
315-
} else {
316-
Ok(Self { pk })
311+
match Segwitv0::check_pk(&pk) {
312+
Ok(_) => Ok(Wpkh { pk }),
313+
Err(e) => Err(e),
317314
}
318315
}
319316

‎src/descriptor/tr.rs

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

2627
/// A Taproot Tree representation.
@@ -165,6 +166,7 @@ impl<Pk: MiniscriptKey> fmt::Debug for TapTree<Pk> {
165166
impl<Pk: MiniscriptKey> Tr<Pk> {
166167
/// Create a new [`Tr`] descriptor from internal key and [`TapTree`]
167168
pub fn new(internal_key: Pk, tree: Option<TapTree<Pk>>) -> Result<Self, Error> {
169+
Tap::check_pk(&internal_key)?;
168170
let nodes = tree.as_ref().map(|t| t.taptree_height()).unwrap_or(0);
169171

170172
if nodes <= TAPROOT_CONTROL_MAX_NODE_COUNT {
@@ -636,14 +638,12 @@ where
636638
where
637639
T: Translator<P, Q, E>,
638640
{
639-
let translate_desc = Tr {
640-
internal_key: translate.pk(&self.internal_key)?,
641-
tree: match &self.tree {
642-
Some(tree) => Some(tree.translate_helper(translate)?),
643-
None => None,
644-
},
645-
spend_info: Mutex::new(None),
641+
let tree = match &self.tree {
642+
Some(tree) => Some(tree.translate_helper(translate)?),
643+
None => None,
646644
};
645+
let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
646+
.expect("This will be removed in future");
647647
Ok(translate_desc)
648648
}
649649
}

‎src/miniscript/context.rs

Lines changed: 70 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ where
209209
Ok(())
210210
}
211211

212+
/// Each context has slightly different rules on what Pks are allowed in descriptors
213+
/// Legacy/Bare does not allow x_only keys
214+
/// Segwit does not allow uncompressed keys and x_only keys
215+
/// Tapscript does not allow uncompressed keys
216+
fn check_pk<Pk: MiniscriptKey>(pk: &Pk) -> Result<(), ScriptContextError>;
217+
212218
/// Depending on script context, the size of a satifaction witness may slightly differ.
213219
fn max_satisfaction_size<Pk: MiniscriptKey>(ms: &Miniscript<Pk, Self>) -> Option<usize>;
214220
/// Depending on script Context, some of the Terminals might not
@@ -356,6 +362,18 @@ impl ScriptContext for Legacy {
356362
}
357363
}
358364

365+
// Only compressed and uncompressed public keys are allowed in Legacy context
366+
fn check_pk<Pk: MiniscriptKey>(pk: &Pk) -> Result<(), ScriptContextError> {
367+
if pk.is_x_only_key() {
368+
Err(ScriptContextError::XOnlyKeysNotAllowed(
369+
pk.to_string(),
370+
Self::name_str(),
371+
))
372+
} else {
373+
Ok(())
374+
}
375+
}
376+
359377
fn check_witness<Pk: MiniscriptKey>(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
360378
// In future, we could avoid by having a function to count only
361379
// len of script instead of converting it.
@@ -373,31 +391,21 @@ impl ScriptContext for Legacy {
373391
}
374392

375393
match ms.node {
376-
Terminal::PkK(ref key) if key.is_x_only_key() => {
377-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
378-
key.to_string(),
379-
Self::name_str(),
380-
))
381-
}
394+
Terminal::PkK(ref pk) => Self::check_pk(pk),
382395
Terminal::Multi(_k, ref pks) => {
383396
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
384397
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
385398
}
386399
for pk in pks.iter() {
387-
if pk.is_x_only_key() {
388-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
389-
pk.to_string(),
390-
Self::name_str(),
391-
));
392-
}
400+
Self::check_pk(pk)?;
393401
}
402+
Ok(())
394403
}
395404
Terminal::MultiA(..) => {
396405
return Err(ScriptContextError::MultiANotAllowed);
397406
}
398-
_ => {}
407+
_ => Ok(()),
399408
}
400-
Ok(())
401409
}
402410

403411
fn check_local_consensus_validity<Pk: MiniscriptKey>(
@@ -461,6 +469,20 @@ impl ScriptContext for Segwitv0 {
461469
Ok(())
462470
}
463471

472+
// No x-only keys or uncompressed keys in Segwitv0 context
473+
fn check_pk<Pk: MiniscriptKey>(pk: &Pk) -> Result<(), ScriptContextError> {
474+
if pk.is_uncompressed() {
475+
Err(ScriptContextError::UncompressedKeysNotAllowed)
476+
} else if pk.is_x_only_key() {
477+
Err(ScriptContextError::XOnlyKeysNotAllowed(
478+
pk.to_string(),
479+
Self::name_str(),
480+
))
481+
} else {
482+
Ok(())
483+
}
484+
}
485+
464486
fn check_witness<Pk: MiniscriptKey>(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
465487
if witness.len() > MAX_STANDARD_P2WSH_STACK_ITEMS {
466488
return Err(ScriptContextError::MaxWitnessItemssExceeded {
@@ -479,30 +501,13 @@ impl ScriptContext for Segwitv0 {
479501
}
480502

481503
match ms.node {
482-
Terminal::PkK(ref pk) => {
483-
if pk.is_uncompressed() {
484-
return Err(ScriptContextError::CompressedOnly(pk.to_string()));
485-
} else if pk.is_x_only_key() {
486-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
487-
pk.to_string(),
488-
Self::name_str(),
489-
));
490-
}
491-
Ok(())
492-
}
504+
Terminal::PkK(ref pk) => Self::check_pk(pk),
493505
Terminal::Multi(_k, ref pks) => {
494506
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
495507
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
496508
}
497509
for pk in pks.iter() {
498-
if pk.is_uncompressed() {
499-
return Err(ScriptContextError::CompressedOnly(pk.to_string()));
500-
} else if pk.is_x_only_key() {
501-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
502-
pk.to_string(),
503-
Self::name_str(),
504-
));
505-
}
510+
Self::check_pk(pk)?;
506511
}
507512
Ok(())
508513
}
@@ -583,6 +588,15 @@ impl ScriptContext for Tap {
583588
Ok(())
584589
}
585590

591+
// No uncompressed keys in Tap context
592+
fn check_pk<Pk: MiniscriptKey>(pk: &Pk) -> Result<(), ScriptContextError> {
593+
if pk.is_uncompressed() {
594+
Err(ScriptContextError::UncompressedKeysNotAllowed)
595+
} else {
596+
Ok(())
597+
}
598+
}
599+
586600
fn check_witness<Pk: MiniscriptKey>(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
587601
// Note that tapscript has a 1000 limit compared to 100 of segwitv0
588602
if witness.len() > MAX_STACK_SIZE {
@@ -607,9 +621,10 @@ impl ScriptContext for Tap {
607621
}
608622

609623
match ms.node {
610-
Terminal::PkK(ref pk) => {
611-
if pk.is_uncompressed() {
612-
return Err(ScriptContextError::UncompressedKeysNotAllowed);
624+
Terminal::PkK(ref pk) => Self::check_pk(pk),
625+
Terminal::MultiA(_, ref keys) => {
626+
for pk in keys.iter() {
627+
Self::check_pk(pk)?;
613628
}
614629
Ok(())
615630
}
@@ -694,30 +709,32 @@ impl ScriptContext for BareCtx {
694709
Ok(())
695710
}
696711

712+
// No x-only keys in Bare context
713+
fn check_pk<Pk: MiniscriptKey>(pk: &Pk) -> Result<(), ScriptContextError> {
714+
if pk.is_x_only_key() {
715+
Err(ScriptContextError::XOnlyKeysNotAllowed(
716+
pk.to_string(),
717+
Self::name_str(),
718+
))
719+
} else {
720+
Ok(())
721+
}
722+
}
723+
697724
fn check_global_consensus_validity<Pk: MiniscriptKey>(
698725
ms: &Miniscript<Pk, Self>,
699726
) -> Result<(), ScriptContextError> {
700727
if ms.ext.pk_cost > MAX_SCRIPT_SIZE {
701728
return Err(ScriptContextError::MaxWitnessScriptSizeExceeded);
702729
}
703730
match ms.node {
704-
Terminal::PkK(ref key) if key.is_x_only_key() => {
705-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
706-
key.to_string(),
707-
Self::name_str(),
708-
))
709-
}
731+
Terminal::PkK(ref key) => Self::check_pk(key),
710732
Terminal::Multi(_k, ref pks) => {
711733
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
712734
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
713735
}
714736
for pk in pks.iter() {
715-
if pk.is_x_only_key() {
716-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
717-
pk.to_string(),
718-
Self::name_str(),
719-
));
720-
}
737+
Self::check_pk(pk)?;
721738
}
722739
Ok(())
723740
}
@@ -788,6 +805,11 @@ impl ScriptContext for NoChecks {
788805
Ok(())
789806
}
790807

808+
// No checks in NoChecks
809+
fn check_pk<Pk: MiniscriptKey>(_pk: &Pk) -> Result<(), ScriptContextError> {
810+
Ok(())
811+
}
812+
791813
fn check_global_policy_validity<Pk: MiniscriptKey>(
792814
_ms: &Miniscript<Pk, Self>,
793815
) -> Result<(), ScriptContextError> {

‎src/psbt/finalizer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ fn get_descriptor(psbt: &Psbt, index: usize) -> Result<Descriptor<PublicKey>, In
147147
*script_pubkey == addr.script_pubkey()
148148
});
149149
match partial_sig_contains_pk {
150-
Some((pk, _sig)) => Ok(Descriptor::new_pkh(*pk)),
150+
Some((pk, _sig)) => Descriptor::new_pkh(*pk).map_err(|e| InputError::from(e)),
151151
None => Err(InputError::MissingPubkey),
152152
}
153153
} else if script_pubkey.is_v0_p2wpkh() {

0 commit comments

Comments
 (0)