Skip to content

Commit ee8de9a

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 fa2e8b4 commit ee8de9a

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, ScriptBuf};
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};
@@ -205,9 +205,12 @@ pub struct Pkh<Pk: MiniscriptKey> {
205205

206206
impl<Pk: MiniscriptKey> Pkh<Pk> {
207207
/// Create a new Pkh descriptor
208-
pub fn new(pk: Pk) -> Self {
208+
pub fn new(pk: Pk) -> Result<Self, ScriptContextError> {
209209
// do the top-level checks
210-
Self { pk }
210+
match BareCtx::check_pk(&pk) {
211+
Ok(()) => Ok(Pkh { pk }),
212+
Err(e) => Err(e),
213+
}
211214
}
212215

213216
/// Get a reference to the inner key
@@ -336,7 +339,7 @@ impl_from_tree!(
336339
if top.name == "pkh" && top.args.len() == 1 {
337340
Ok(Pkh::new(expression::terminal(&top.args[0], |pk| {
338341
Pk::from_str(pk)
339-
})?))
342+
})?)?)
340343
} else {
341344
Err(Error::Unexpected(format!(
342345
"{}({} args) while parsing pkh descriptor",
@@ -374,6 +377,7 @@ where
374377
where
375378
T: Translator<P, Q, E>,
376379
{
377-
Ok(Pkh::new(t.pk(&self.pk)?))
380+
let res = Pkh::new(t.pk(&self.pk)?);
381+
Ok(res.expect("Expect will be fixed in next commit"))
378382
}
379383
}

‎src/descriptor/mod.rs‎

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

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

184184
/// Create a new Wpkh descriptor
@@ -1297,7 +1297,7 @@ mod tests {
12971297
);
12981298
assert_eq!(bare.unsigned_script_sig(), bitcoin::ScriptBuf::new());
12991299

1300-
let pkh = Descriptor::new_pkh(pk);
1300+
let pkh = Descriptor::new_pkh(pk).unwrap();
13011301
pkh.satisfy(&mut txin, &satisfier).expect("satisfaction");
13021302
assert_eq!(
13031303
txin,
@@ -2021,4 +2021,54 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
20212021
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2;3;4>/*)))").unwrap_err();
20222022
Descriptor::<DescriptorPublicKey>::from_str("wsh(andor(pk(tpubDEN9WSToTyy9ZQfaYqSKfmVqmq1VVLNtYfj3Vkqh67et57eJ5sTKZQBkHqSwPUsoSskJeaYnPttHe2VrkCsKA27kUaN9SDc5zhqeLzKa1rr/0'/<0;1;2;3>/*),older(10000),pk(tpubD8LYfn6njiA2inCoxwM7EuN3cuLVcaHAwLYeups13dpevd3nHLRdK9NdQksWXrhLQVxcUZRpnp5CkJ1FhE61WRAsHxDNAkvGkoQkAeWDYjV/8/<0;1;2>/*)))").unwrap_err();
20232023
}
2024+
2025+
#[test]
2026+
fn test_context_pks() {
2027+
let comp_key = bitcoin::PublicKey::from_str(
2028+
"02015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
2029+
)
2030+
.unwrap();
2031+
let x_only_key = bitcoin::key::XOnlyPublicKey::from_str(
2032+
"015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab",
2033+
)
2034+
.unwrap();
2035+
let uncomp_key = bitcoin::PublicKey::from_str("04015e4cb53458bf813db8c79968e76e10d13ed6426a23fa71c2f41ba021c2a7ab0d46021e9e69ef061eb25eab41ae206187b2b05e829559df59d78319bd9267b4").unwrap();
2036+
2037+
type Desc = Descriptor<DescriptorPublicKey>;
2038+
2039+
// Legacy tests, x-only keys are not supported
2040+
Desc::from_str(&format!("sh(pk({}))", comp_key)).unwrap();
2041+
Desc::from_str(&format!("sh(pk({}))", uncomp_key)).unwrap();
2042+
Desc::from_str(&format!("sh(pk({}))", x_only_key)).unwrap_err();
2043+
2044+
// bare tests, x-only keys not supported
2045+
Desc::from_str(&format!("pk({})", comp_key)).unwrap();
2046+
Desc::from_str(&format!("pk({})", uncomp_key)).unwrap();
2047+
Desc::from_str(&format!("pk({})", x_only_key)).unwrap_err();
2048+
2049+
// pkh tests, x-only keys not supported
2050+
Desc::from_str(&format!("pkh({})", comp_key)).unwrap();
2051+
Desc::from_str(&format!("pkh({})", uncomp_key)).unwrap();
2052+
Desc::from_str(&format!("pkh({})", x_only_key)).unwrap_err();
2053+
2054+
// wpkh tests, uncompressed and x-only keys not supported
2055+
Desc::from_str(&format!("wpkh({})", comp_key)).unwrap();
2056+
Desc::from_str(&format!("wpkh({})", uncomp_key)).unwrap_err();
2057+
Desc::from_str(&format!("wpkh({})", x_only_key)).unwrap_err();
2058+
2059+
// Segwitv0 tests, uncompressed and x-only keys not supported
2060+
Desc::from_str(&format!("wsh(pk({}))", comp_key)).unwrap();
2061+
Desc::from_str(&format!("wsh(pk({}))", uncomp_key)).unwrap_err();
2062+
Desc::from_str(&format!("wsh(pk({}))", x_only_key)).unwrap_err();
2063+
2064+
// Tap tests, key path
2065+
Desc::from_str(&format!("tr({})", comp_key)).unwrap();
2066+
Desc::from_str(&format!("tr({})", uncomp_key)).unwrap_err();
2067+
Desc::from_str(&format!("tr({})", x_only_key)).unwrap();
2068+
2069+
// Tap tests, script path
2070+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, comp_key)).unwrap();
2071+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, uncomp_key)).unwrap_err();
2072+
Desc::from_str(&format!("tr({},pk({}))", x_only_key, x_only_key)).unwrap();
2073+
}
20242074
}

‎src/descriptor/segwitv0.rs‎

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

304304
impl<Pk: MiniscriptKey> Wpkh<Pk> {
305305
/// Create a new Wpkh descriptor
306-
pub fn new(pk: Pk) -> Result<Self, Error> {
306+
pub fn new(pk: Pk) -> Result<Self, ScriptContextError> {
307307
// do the top-level checks
308-
if pk.is_uncompressed() {
309-
Err(Error::ContextError(ScriptContextError::CompressedOnly(
310-
pk.to_string(),
311-
)))
312-
} else {
313-
Ok(Self { pk })
308+
match Segwitv0::check_pk(&pk) {
309+
Ok(_) => Ok(Wpkh { pk }),
310+
Err(e) => Err(e),
314311
}
315312
}
316313

‎src/descriptor/tr.rs‎

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

2526
/// 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 {
@@ -631,14 +633,12 @@ where
631633
where
632634
T: Translator<P, Q, E>,
633635
{
634-
let translate_desc = Tr {
635-
internal_key: translate.pk(&self.internal_key)?,
636-
tree: match &self.tree {
637-
Some(tree) => Some(tree.translate_helper(translate)?),
638-
None => None,
639-
},
640-
spend_info: Mutex::new(None),
636+
let tree = match &self.tree {
637+
Some(tree) => Some(tree.translate_helper(translate)?),
638+
None => None,
641639
};
640+
let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
641+
.expect("This will be removed in future");
642642
Ok(translate_desc)
643643
}
644644
}

‎src/miniscript/context.rs‎

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

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

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

374392
match ms.node {
375-
Terminal::PkK(ref key) if key.is_x_only_key() => {
376-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
377-
key.to_string(),
378-
Self::name_str(),
379-
))
380-
}
393+
Terminal::PkK(ref pk) => Self::check_pk(pk),
381394
Terminal::Multi(_k, ref pks) => {
382395
if pks.len() > MAX_PUBKEYS_PER_MULTISIG {
383396
return Err(ScriptContextError::CheckMultiSigLimitExceeded);
384397
}
385398
for pk in pks.iter() {
386-
if pk.is_x_only_key() {
387-
return Err(ScriptContextError::XOnlyKeysNotAllowed(
388-
pk.to_string(),
389-
Self::name_str(),
390-
));
391-
}
399+
Self::check_pk(pk)?;
392400
}
401+
Ok(())
393402
}
394403
Terminal::MultiA(..) => {
395404
return Err(ScriptContextError::MultiANotAllowed);
396405
}
397-
_ => {}
406+
_ => Ok(()),
398407
}
399-
Ok(())
400408
}
401409

402410
fn check_local_consensus_validity<Pk: MiniscriptKey>(
@@ -460,6 +468,20 @@ impl ScriptContext for Segwitv0 {
460468
Ok(())
461469
}
462470

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

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

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

608622
match ms.node {
609-
Terminal::PkK(ref pk) => {
610-
if pk.is_uncompressed() {
611-
return Err(ScriptContextError::UncompressedKeysNotAllowed);
623+
Terminal::PkK(ref pk) => Self::check_pk(pk),
624+
Terminal::MultiA(_, ref keys) => {
625+
for pk in keys.iter() {
626+
Self::check_pk(pk)?;
612627
}
613628
Ok(())
614629
}
@@ -693,30 +708,32 @@ impl ScriptContext for BareCtx {
693708
Ok(())
694709
}
695710

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

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

‎src/psbt/finalizer.rs‎

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

0 commit comments

Comments
 (0)