miniscript: remove Ctx::check_witness

This is a weird function. Its role is to double-check a witness after it
has been produced to ensure that it falls within limits. If you are
working with a sane miniscript this is impossible to fail, and if you're
working with an insane miniscript then the checks are too strong -- it
verifies standardness rules rather than consensus.

This was introduced in #189 without much discussion. It had no tests
then and has no tests now -- everything continues to pass.

I am removing it for now. I will later replace it with an API where you
call Plan::validate if you want to do checks like this. If the user wants
to do something oddball like "parse a miniscript without checking satisfaction
limits, then produce a satisfaction and see if the result is nonetheless
within limits" the workflow will be:

1. Parse an insane Miniscript
2. Produce a Plan
3. Run Plan::validate with tighter validation params than were used in
   step 1.

This gives the user a fair bit more flexibility. It is an awkward and
hard to discover workflow, but IMO it's better than we have now.

Author: apoelstra

src/miniscript/context.rs

@@ -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
@@ -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
@@ -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> {
@@ -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> {
@@ -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> {
@@ -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> {

src/miniscript/mod.rs

@@ -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)
             }