Skip to content

Commit 3ebdf4c

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 6c4bc9f commit 3ebdf4c

File tree

6 files changed

+145
-72
lines changed

6 files changed

+145
-72
lines changed

‎src/descriptor/bare.rs

+9-5
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};
@@ -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
@@ -339,7 +342,7 @@ impl_from_tree!(
339342
if top.name == "pkh" && top.args.len() == 1 {
340343
Ok(Pkh::new(expression::terminal(&top.args[0], |pk| {
341344
Pk::from_str(pk)
342-
})?))
345+
})?)?)
343346
} else {
344347
Err(Error::Unexpected(format!(
345348
"{}({} args) while parsing pkh descriptor",
@@ -380,6 +383,7 @@ where
380383
where
381384
T: Translator<P, Q, E>,
382385
{
383-
Ok(Pkh::new(t.pk(&self.pk)?))
386+
let res = Pkh::new(t.pk(&self.pk)?);
387+
Ok(res.expect("Expect will be fixed in next commit"))
384388
}
385389
}

‎src/descriptor/mod.rs

+53-3
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
@@ -1300,7 +1300,7 @@ mod tests {
13001300
);
13011301
assert_eq!(bare.unsigned_script_sig(), bitcoin::ScriptBuf::new());
13021302

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

‎src/descriptor/segwitv0.rs

+4-7
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

+8-8
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 {
@@ -634,14 +636,12 @@ where
634636
where
635637
T: Translator<P, Q, E>,
636638
{
637-
let translate_desc = Tr {
638-
internal_key: translate.pk(&self.internal_key)?,
639-
tree: match &self.tree {
640-
Some(tree) => Some(tree.translate_helper(translate)?),
641-
None => None,
642-
},
643-
spend_info: Mutex::new(None),
639+
let tree = match &self.tree {
640+
Some(tree) => Some(tree.translate_helper(translate)?),
641+
None => None,
644642
};
643+
let translate_desc = Tr::new(translate.pk(&self.internal_key)?, tree)
644+
.expect("This will be removed in future");
645645
Ok(translate_desc)
646646
}
647647
}

‎src/miniscript/context.rs

+70-48
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

+1-1
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)