Skip to content

Remove a bit of validation-related cruft #871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 27, 2025
Merged

Remove a bit of validation-related cruft #871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3dd006a
miniscript: remove Ctx::check_witness
apoelstra May 12, 2025
bd8fca6
miniscript: replace decode_with_ext(., Ext::allow_all) with decode_co…
apoelstra May 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions src/interpreter/inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use bitcoin::Witness;
use super::{stack, BitcoinKey, Error, Stack};
use crate::miniscript::context::{NoChecks, ScriptContext, SigType};
use crate::prelude::*;
use crate::{BareCtx, ExtParams, Legacy, Miniscript, Segwitv0, Tap, ToPublicKey, Translator};
use crate::{BareCtx, Legacy, Miniscript, Segwitv0, Tap, ToPublicKey, Translator};

/// Attempts to parse a slice as a Bitcoin public key, checking compressedness
/// if asked to, but otherwise dropping it
Expand Down Expand Up @@ -43,8 +43,7 @@ fn script_from_stack_elem<Ctx: ScriptContext>(
) -> Result<Miniscript<Ctx::Key, Ctx>, Error> {
match *elem {
stack::Element::Push(sl) => {
Miniscript::decode_with_ext(bitcoin::Script::from_bytes(sl), &ExtParams::allow_all())
.map_err(Error::from)
Miniscript::decode_consensus(bitcoin::Script::from_bytes(sl)).map_err(Error::from)
}
stack::Element::Satisfied => Ok(Miniscript::TRUE),
stack::Element::Dissatisfied => Ok(Miniscript::FALSE),
Expand Down Expand Up @@ -327,10 +326,7 @@ pub(super) fn from_txdata<'txin>(
} else {
if wit_stack.is_empty() {
// Bare script parsed in BareCtx
let miniscript = Miniscript::<bitcoin::PublicKey, BareCtx>::decode_with_ext(
spk,
&ExtParams::allow_all(),
)?;
let miniscript = Miniscript::<bitcoin::PublicKey, BareCtx>::decode_consensus(spk)?;
let miniscript = miniscript.to_no_checks_ms();
Ok((Inner::Script(miniscript, ScriptType::Bare), ssig_stack, Some(spk.to_owned())))
} else {
Expand Down Expand Up @@ -678,8 +674,7 @@ mod tests {
}

fn ms_inner_script(ms: &str) -> (Miniscript<BitcoinKey, NoChecks>, bitcoin::ScriptBuf) {
let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::from_str_ext(ms, &ExtParams::insane())
.unwrap();
let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::from_str_insane(ms).unwrap();
let spk = ms.encode();
let miniscript = ms.to_no_checks_ms();
(miniscript, spk)
Expand Down
52 changes: 0 additions & 52 deletions src/miniscript/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::miniscript::limits::{
};
use crate::miniscript::types;
use crate::prelude::*;
use crate::util::witness_to_scriptsig;
use crate::{hash256, Error, ForEachKey, Miniscript, MiniscriptKey, Terminal};

/// Error for Script Context
Expand Down Expand Up @@ -189,16 +188,6 @@ where
_frag: &Terminal<Pk, Self>,
) -> Result<(), ScriptContextError>;

/// Check whether the given satisfaction is valid under the ScriptContext
/// For example, segwit satisfactions may fail if the witness len is more
/// 3600 or number of stack elements are more than 100.
fn check_witness(_witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
// Only really need to do this for segwitv0 and legacy
// Bare is already restricted by standardness rules
// and would reach these limits.
Ok(())
}

/// Each context has slightly different rules on what Pks are allowed in descriptors
/// Legacy/Bare does not allow x_only keys
/// Segwit does not allow uncompressed keys and x_only keys
Expand Down Expand Up @@ -389,19 +378,6 @@ impl ScriptContext for Legacy {
}
}

fn check_witness(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
// In future, we could avoid by having a function to count only
// len of script instead of converting it.
let script_sig = witness_to_scriptsig(witness);
if script_sig.len() > MAX_SCRIPTSIG_SIZE {
return Err(ScriptContextError::MaxScriptSigSizeExceeded {
actual: script_sig.len(),
limit: MAX_SCRIPTSIG_SIZE,
});
}
Ok(())
}

fn check_global_consensus_validity<Pk: MiniscriptKey>(
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
Expand Down Expand Up @@ -506,16 +482,6 @@ impl ScriptContext for Segwitv0 {
}
}

fn check_witness(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
if witness.len() > MAX_STANDARD_P2WSH_STACK_ITEMS {
return Err(ScriptContextError::MaxWitnessItemsExceeded {
actual: witness.len(),
limit: MAX_STANDARD_P2WSH_STACK_ITEMS,
});
}
Ok(())
}

fn check_global_consensus_validity<Pk: MiniscriptKey>(
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
Expand Down Expand Up @@ -627,17 +593,6 @@ impl ScriptContext for Tap {
}
}

fn check_witness(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
// Note that tapscript has a 1000 limit compared to 100 of segwitv0
if witness.len() > MAX_STACK_SIZE {
return Err(ScriptContextError::MaxWitnessItemsExceeded {
actual: witness.len(),
limit: MAX_STACK_SIZE,
});
}
Ok(())
}

fn check_global_consensus_validity<Pk: MiniscriptKey>(
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
Expand Down Expand Up @@ -882,13 +837,6 @@ impl ScriptContext for NoChecks {
"NochecksEcdsa"
}

fn check_witness(_witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
// Only really need to do this for segwitv0 and legacy
// Bare is already restricted by standardness rules
// and would reach these limits.
Ok(())
}

fn check_global_validity<Pk: MiniscriptKey>(
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
Expand Down
91 changes: 41 additions & 50 deletions src/miniscript/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,10 +471,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
Pk: ToPublicKey,
{
match satisfaction.stack {
satisfy::Witness::Stack(stack) => {
Ctx::check_witness(&stack)?;
Ok(stack)
}
satisfy::Witness::Stack(stack) => Ok(stack),
satisfy::Witness::Unavailable | satisfy::Witness::Impossible => {
Err(Error::CouldNotSatisfy)
}
Expand Down Expand Up @@ -522,24 +519,17 @@ impl Miniscript<<Tap as ScriptContext>::Key, Tap> {
}

impl<Ctx: ScriptContext> Miniscript<Ctx::Key, Ctx> {
/// Attempt to parse an insane(scripts don't clear sanity checks)
/// script into a Miniscript representation.
/// Use this to parse scripts with repeated pubkeys, timelock mixing, malleable
/// scripts without sig or scripts that can exceed resource limits.
/// Some of the analysis guarantees of miniscript are lost when dealing with
/// insane scripts. In general, in a multi-party setting users should only
/// accept sane scripts.
pub fn decode_insane(script: &script::Script) -> Result<Miniscript<Ctx::Key, Ctx>, Error> {
Miniscript::decode_with_ext(script, &ExtParams::insane())
/// Attempt to decode a Miniscript from Script, checking only for consensus compatibility,
/// and no other checks.
///
/// It may make sense to use this method when parsing Script that is already
/// embedded in the chain. While it is inadvisable to use insane Miniscripts,
/// once it's on the chain you don't have much choice anymore.
pub fn decode_consensus(script: &script::Script) -> Result<Miniscript<Ctx::Key, Ctx>, Error> {
Miniscript::decode_with_ext(script, &ExtParams::allow_all())
}

/// Attempt to parse an miniscript with extra features that not yet specified in the spec.
/// Users should not use this function unless they scripts can/will change in the future.
/// Currently, this function supports the following features:
/// - Parsing all insane scripts
/// - Parsing miniscripts with raw pubkey hashes
///
/// Allowed extra features can be specified by the ext [`ExtParams`] argument.
/// Attempt to decode a Miniscript from Script, specifying which validation parameters to apply.
pub fn decode_with_ext(
script: &script::Script,
ext: &ExtParams,
Expand All @@ -564,7 +554,7 @@ impl<Ctx: ScriptContext> Miniscript<Ctx::Key, Ctx> {
/// Attempt to parse a Script into Miniscript representation.
///
/// This function will fail parsing for scripts that do not clear the
/// [`Miniscript::sanity_check`] checks. Use [`Miniscript::decode_insane`] to
/// [`Miniscript::sanity_check`] checks. Use [`Miniscript::decode_consensus`] to
/// parse such scripts.
///
/// ## Decode/Parse a miniscript from script hex
Expand Down Expand Up @@ -1164,8 +1154,8 @@ mod tests {
assert_eq!(format!("{:x}", bitcoin_script), expected);
}
// Parse scripts with all extensions
let roundtrip = Segwitv0Script::decode_with_ext(&bitcoin_script, &ExtParams::allow_all())
.expect("parse string serialization");
let roundtrip =
Segwitv0Script::decode_consensus(&bitcoin_script).expect("parse string serialization");
assert_eq!(roundtrip, script);
}

Expand All @@ -1174,7 +1164,8 @@ mod tests {
let ser = tree.encode();
assert_eq!(ser.len(), tree.script_size());
assert_eq!(ser.to_string(), s);
let deser = Segwitv0Script::decode_insane(&ser).expect("deserialize result of serialize");
let deser =
Segwitv0Script::decode_consensus(&ser).expect("deserialize result of serialize");
assert_eq!(*tree, deser);
}

Expand Down Expand Up @@ -1315,19 +1306,19 @@ mod tests {
fn verify_parse() {
let ms = "and_v(v:hash160(20195b5a3d650c17f0f29f91c33f8f6335193d07),or_d(sha256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),older(16)))";
let ms: Segwitv0Script = Miniscript::from_str_insane(ms).unwrap();
assert_eq!(ms, Segwitv0Script::decode_insane(&ms.encode()).unwrap());
assert_eq!(ms, Segwitv0Script::decode_consensus(&ms.encode()).unwrap());

let ms = "and_v(v:sha256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),or_d(sha256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),older(16)))";
let ms: Segwitv0Script = Miniscript::from_str_insane(ms).unwrap();
assert_eq!(ms, Segwitv0Script::decode_insane(&ms.encode()).unwrap());
assert_eq!(ms, Segwitv0Script::decode_consensus(&ms.encode()).unwrap());

let ms = "and_v(v:ripemd160(20195b5a3d650c17f0f29f91c33f8f6335193d07),or_d(sha256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),older(16)))";
let ms: Segwitv0Script = Miniscript::from_str_insane(ms).unwrap();
assert_eq!(ms, Segwitv0Script::decode_insane(&ms.encode()).unwrap());
assert_eq!(ms, Segwitv0Script::decode_consensus(&ms.encode()).unwrap());

let ms = "and_v(v:hash256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),or_d(sha256(96de8fc8c256fa1e1556d41af431cace7dca68707c78dd88c3acab8b17164c47),older(16)))";
let ms: Segwitv0Script = Miniscript::from_str_insane(ms).unwrap();
assert_eq!(ms, Segwitv0Script::decode_insane(&ms.encode()).unwrap());
assert_eq!(ms, Segwitv0Script::decode_consensus(&ms.encode()).unwrap());
}

#[test]
Expand Down Expand Up @@ -1518,21 +1509,21 @@ mod tests {
#[test]
fn deserialize() {
// Most of these came from fuzzing, hence the increasing lengths
assert!(Segwitv0Script::decode_insane(&hex_script("")).is_err()); // empty
assert!(Segwitv0Script::decode_insane(&hex_script("00")).is_ok()); // FALSE
assert!(Segwitv0Script::decode_insane(&hex_script("51")).is_ok()); // TRUE
assert!(Segwitv0Script::decode_insane(&hex_script("69")).is_err()); // VERIFY
assert!(Segwitv0Script::decode_insane(&hex_script("0000")).is_err()); //and_v(FALSE,FALSE)
assert!(Segwitv0Script::decode_insane(&hex_script("1001")).is_err()); // incomplete push
assert!(Segwitv0Script::decode_insane(&hex_script("03990300b2")).is_err()); // non-minimal #
assert!(Segwitv0Script::decode_insane(&hex_script("8559b2")).is_err()); // leading bytes
assert!(Segwitv0Script::decode_insane(&hex_script("4c0169b2")).is_err()); // non-minimal push
assert!(Segwitv0Script::decode_insane(&hex_script("0000af0000ae85")).is_err()); // OR not BOOLOR
assert!(Segwitv0Script::decode_consensus(&hex_script("")).is_err()); // empty
assert!(Segwitv0Script::decode_consensus(&hex_script("00")).is_ok()); // FALSE
assert!(Segwitv0Script::decode_consensus(&hex_script("51")).is_ok()); // TRUE
assert!(Segwitv0Script::decode_consensus(&hex_script("69")).is_err()); // VERIFY
assert!(Segwitv0Script::decode_consensus(&hex_script("0000")).is_err()); //and_v(FALSE,FALSE)
assert!(Segwitv0Script::decode_consensus(&hex_script("1001")).is_err()); // incomplete push
assert!(Segwitv0Script::decode_consensus(&hex_script("03990300b2")).is_err()); // non-minimal #
assert!(Segwitv0Script::decode_consensus(&hex_script("8559b2")).is_err()); // leading bytes
assert!(Segwitv0Script::decode_consensus(&hex_script("4c0169b2")).is_err()); // non-minimal push
assert!(Segwitv0Script::decode_consensus(&hex_script("0000af0000ae85")).is_err()); // OR not BOOLOR

// misc fuzzer problems
assert!(Segwitv0Script::decode_insane(&hex_script("0000000000af")).is_err());
assert!(Segwitv0Script::decode_insane(&hex_script("04009a2970af00")).is_err()); // giant CMS key num
assert!(Segwitv0Script::decode_insane(&hex_script(
assert!(Segwitv0Script::decode_consensus(&hex_script("0000000000af")).is_err());
assert!(Segwitv0Script::decode_consensus(&hex_script("04009a2970af00")).is_err()); // giant CMS key num
assert!(Segwitv0Script::decode_consensus(&hex_script(
"2102ffffffffffffffefefefefefefefefefefef394c0fe5b711179e124008584753ac6900"
))
.is_err());
Expand Down Expand Up @@ -1572,22 +1563,22 @@ mod tests {

//---------------- test script <-> miniscript ---------------
// Test parsing from scripts: x-only fails decoding in segwitv0 ctx
Segwitv0Script::decode_insane(&hex_script(
Segwitv0Script::decode_consensus(&hex_script(
"202788ee41e76f4f3af603da5bc8fa22997bc0344bb0f95666ba6aaff0242baa99ac",
))
.unwrap_err();
// x-only succeeds in tap ctx
Tapscript::decode_insane(&hex_script(
Tapscript::decode_consensus(&hex_script(
"202788ee41e76f4f3af603da5bc8fa22997bc0344bb0f95666ba6aaff0242baa99ac",
))
.unwrap();
// tapscript fails decoding with compressed
Tapscript::decode_insane(&hex_script(
Tapscript::decode_consensus(&hex_script(
"21022788ee41e76f4f3af603da5bc8fa22997bc0344bb0f95666ba6aaff0242baa99ac",
))
.unwrap_err();
// Segwitv0 succeeds decoding with tapscript.
Segwitv0Script::decode_insane(&hex_script(
Segwitv0Script::decode_consensus(&hex_script(
"21022788ee41e76f4f3af603da5bc8fa22997bc0344bb0f95666ba6aaff0242baa99ac",
))
.unwrap();
Expand Down Expand Up @@ -1623,7 +1614,7 @@ mod tests {
.unwrap();
// script rtt test
assert_eq!(
Miniscript::<XOnlyPublicKey, Tap>::decode_insane(&tap_ms.encode()).unwrap(),
Miniscript::<XOnlyPublicKey, Tap>::decode_consensus(&tap_ms.encode()).unwrap(),
tap_ms
);
assert_eq!(tap_ms.script_size(), 104);
Expand Down Expand Up @@ -1664,7 +1655,7 @@ mod tests {
.unwrap();
let ms_trans = ms.translate_pk(&mut StrKeyTranslator::new()).unwrap();
let enc = ms_trans.encode();
let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::decode_insane(&enc).unwrap();
let ms = Miniscript::<bitcoin::PublicKey, Segwitv0>::decode_consensus(&enc).unwrap();
assert_eq!(ms_trans.encode(), ms.encode());
}

Expand All @@ -1687,8 +1678,8 @@ mod tests {
let script = ms.encode();
// The same test, but parsing from script
SegwitMs::decode(&script).unwrap_err();
SegwitMs::decode_insane(&script).unwrap_err();
SegwitMs::decode_with_ext(&script, &ExtParams::allow_all()).unwrap();
SegwitMs::decode_with_ext(&script, &ExtParams::insane()).unwrap_err();
SegwitMs::decode_consensus(&script).unwrap();

// Try replacing the raw_pkh with a pkh
let mut map = BTreeMap::new();
Expand Down Expand Up @@ -1880,7 +1871,7 @@ mod tests {
for _ in 0..10000 {
script = script.push_opcode(bitcoin::opcodes::all::OP_0NOTEQUAL);
}
Tapscript::decode_insane(&script.into_script()).unwrap_err();
Tapscript::decode_consensus(&script.into_script()).unwrap_err();
}

#[test]
Expand Down
Loading
Loading