Merge pull request bitcoindevkit#196 from darosior/satisfy_error_max_…

…wit_size

Satisfy error: return a max witness size error for Segwitv0

doc/resource_limitations.md

@@ -3,12 +3,12 @@ TODO: Rust-miniscript behaviour for resource limitations:
 # Safe vs Valid vs Sanity/Analyzable/Liftable
 This document refers to bitcoin consensus and standardness rules as of bitcoin core release 0.20.
 
-One of Miniscript’s goals are to make advanced Script functionality accommodate both machine and human analysis. However such a analysis is not possible in all cases.
+One of Miniscript’s goals are to make advanced Script functionality accommodate both machine and human analysis. However such an analysis is not possible in all cases.
 
 - **Validity**: Validity refers to whether the Miniscript tree constructions follows the grammar rules. For eg: Top level must be `B`, or `thresh` must have all of it's arguments being dissatifyable.
 - **Safety**: Whether all satisfactions of Miniscript require a digital signature.
 - **Sanity/Analyzable/Liftable**: Even if the given is valid and safe, it does not imply that Miniscript is consensus and standardness complete. That is, there may exist some semantics implied by the lifted miniscript which cannot be realized in bitcoin network rules. This maybe because of three main reasons
-    - Miniscript may contain a invalid timelock and heightlock combination[article](https://medium.com/blockstream/dont-mix-your-timelocks-d9939b665094).
+    - Miniscript may contain an [invalid timelock and heightlock combination](https://medium.com/blockstream/dont-mix-your-timelocks-d9939b665094).
     - Resource limitations: Discussed in the next section
     - Repeated use of public keys or public key hashes
 
@@ -25,17 +25,17 @@ There are two types of limitations within the resource limitations: 1) Those tha
 Certain limitations like script size are independent of satisfactions and as such those can script creation time. If there is any script that does not satisfy these
 - Scripts over 520 bytes are invalid by consensus (P2SH).
 - Scripts over 10000 bytes are invalid by consensus (bare, P2SH, P2WSH, P2SH-P2WSH).
-- Anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) up to n=3 is invalid by standardness (bare).
+- For bare scripts (ie not P2PKH, P2SH, [p2sh-]P2WPKH, [p2sh-]P2WSH), anything but c:pk(key) (P2PK), c:pk_h(key) (P2PKH), and thresh_m(k,...) up to n=3 is invalid by standardness.
 - Scripts over 3600 bytes are invalid by standardness (P2WSH, P2SH-P2WSH).
 
 rust-miniscript errors on parsing descriptors with these limitations and the compiler would not create these scripts.
 
 ## Limitations dependent on satisfactions
 
-Some limitations are dependent on satisfaction path taken by script. It is possible that certain script satisfaction paths are not valid by consensus rules because they exceed the following limits:
+Some limitations are dependent on satisfaction path taken by script. It is possible that certain script satisfaction paths are not valid because they exceed the following limits:
 
 - Script satisfactions where the total number of non-push opcodes plus the number of keys participating in all executed thresh_ms, is above 201, are invalid by consensus (bare, P2SH, P2WSH, P2SH-P2WSH).
 - Script satisfactions with a serialized scriptSig over 1650 bytes are invalid by standardness (P2SH).
 - Script satisfactions with a witness consisting of over 100 stack elements (excluding the script itself) are invalid by standardness (P2WSH, P2SH-P2WSH).
 
-rust-miniscript correctly parses these miniscripts, but does not allow lifting/analyzing these scripts if any of the spend paths exceeds the above limits. The satisfier logic does **not** guarantee to find the satisfactions for these scripts.
+rust-miniscript correctly parses these miniscripts, but does not allow lifting/analyzing these scripts if any of the spend paths exceeds the above limits. The satisfier logic does **not** guarantee to find the satisfactions for these scripts. The policy compiler would not create such scripts.

src/miniscript/context.rs

@@ -12,13 +12,13 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use miniscript::types;
-use miniscript::types::extra_props::{
+use miniscript::limits::{
     MAX_OPS_PER_SCRIPT, MAX_SCRIPTSIG_SIZE, MAX_SCRIPT_ELEMENT_SIZE, MAX_SCRIPT_SIZE,
     MAX_STANDARD_P2WSH_SCRIPT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEMS,
 };
+use miniscript::types;
 use std::fmt;
-use util::{witness_size, witness_to_scriptsig};
+use util::witness_to_scriptsig;
 use Error;
 use {Miniscript, MiniscriptKey, Terminal};
 /// Error for Script Context
@@ -321,9 +321,7 @@ impl ScriptContext for Segwitv0 {
     fn check_witness<Pk: MiniscriptKey, Ctx: ScriptContext>(
         witness: &[Vec<u8>],
     ) -> Result<(), ScriptContextError> {
-        if witness_size(witness) > MAX_STANDARD_P2WSH_SCRIPT_SIZE {
-            return Err(ScriptContextError::MaxScriptSigSizeExceeded);
-        } else if witness.len() > MAX_STANDARD_P2WSH_STACK_ITEMS {
+        if witness.len() > MAX_STANDARD_P2WSH_STACK_ITEMS {
             return Err(ScriptContextError::MaxWitnessItemssExceeded);
         }
         Ok(())

src/miniscript/limits.rs

@@ -0,0 +1,42 @@
+//! Miscellaneous constraints imposed by Bitcoin.
+//! These constraints can be either Consensus or Policy (standardness) rules, for either Segwitv0
+//! or Legacy scripts.
+
+/// Maximum operations per script
+// https://github.com/bitcoin/bitcoin/blob/875e1ccc9fe01e026e564dfd39a64d9a4b332a89/src/script/script.h#L26
+pub const MAX_OPS_PER_SCRIPT: usize = 201;
+/// Maximum p2wsh initial stack items
+// https://github.com/bitcoin/bitcoin/blob/875e1ccc9fe01e026e564dfd39a64d9a4b332a89/src/policy/policy.h#L40
+pub const MAX_STANDARD_P2WSH_STACK_ITEMS: usize = 100;
+/// Maximum script size allowed by consensus rules
+// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/script/script.h#L32
+pub const MAX_SCRIPT_SIZE: usize = 10_000;
+/// Maximum script size allowed by standardness rules
+// https://github.com/bitcoin/bitcoin/blob/283a73d7eaea2907a6f7f800f529a0d6db53d7a6/src/policy/policy.h#L44
+pub const MAX_STANDARD_P2WSH_SCRIPT_SIZE: usize = 3600;
+/// The Threshold for deciding whether `nLockTime` is interpreted as
+/// time or height.
+// https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/script/script.h#L39
+pub const HEIGHT_TIME_THRESHOLD: u32 = 500_000_000;
+
+/// Bit flag for deciding whether sequence number is
+/// interpreted as height or time
+/* If nSequence encodes a relative lock-time and this flag
+ * is set, the relative lock-time has units of 512 seconds,
+ * otherwise it specifies blocks with a granularity of 1. */
+// https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki
+pub const SEQUENCE_LOCKTIME_TYPE_FLAG: u32 = 1 << 22;
+
+/// Disable flag for sequence locktime
+/* Below flags apply in the context of BIP 68*/
+/* If this flag set, nSequence is NOT interpreted as a
+ * relative lock-time. For future soft-fork compatibility*/
+// https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki
+pub const SEQUENCE_LOCKTIME_DISABLE_FLAG: u32 = 1 << 31;
+
+/// Maximum script element size allowed by consensus rules
+// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/script/script.h#L23
+pub const MAX_SCRIPT_ELEMENT_SIZE: usize = 520;
+/// Maximum script sig size allowed by standardness rules
+// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/policy/policy.cpp#L102
+pub const MAX_SCRIPTSIG_SIZE: usize = 1650;

src/miniscript/mod.rs

@@ -38,6 +38,7 @@ pub(crate) mod context;
 pub mod decode;
 pub mod iter;
 pub mod lex;
+pub mod limits;
 pub mod satisfy;
 pub mod types;
 

src/miniscript/satisfy.rs

@@ -26,7 +26,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d};
 use bitcoin::{self, secp256k1};
 use {MiniscriptKey, ToPublicKey};
 
-use miniscript::types::extra_props::{
+use miniscript::limits::{
     HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG,
 };
 use util::witness_size;

src/miniscript/types/extra_props.rs

@@ -1,51 +1,15 @@
 //! Other miscellaneous type properties which are not related to
 //! correctness or malleability.
 
+use miniscript::limits::{HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_TYPE_FLAG};
+
 use super::{Error, ErrorKind, Property, ScriptContext};
 use script_num_size;
 use std::cmp;
 use std::iter::once;
 use MiniscriptKey;
 use Terminal;
 
-/// Maximum operations per script
-// https://github.com/bitcoin/bitcoin/blob/875e1ccc9fe01e026e564dfd39a64d9a4b332a89/src/script/script.h#L26
-pub const MAX_OPS_PER_SCRIPT: usize = 201;
-/// Maximum p2wsh initial stack items
-// https://github.com/bitcoin/bitcoin/blob/875e1ccc9fe01e026e564dfd39a64d9a4b332a89/src/policy/policy.h#L40
-pub const MAX_STANDARD_P2WSH_STACK_ITEMS: usize = 100;
-/// Maximum script size allowed by consensus rules
-// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/script/script.h#L32
-pub const MAX_SCRIPT_SIZE: usize = 10_000;
-/// Maximum script size allowed by standardness rules
-// https://github.com/bitcoin/bitcoin/blob/283a73d7eaea2907a6f7f800f529a0d6db53d7a6/src/policy/policy.h#L44
-pub const MAX_STANDARD_P2WSH_SCRIPT_SIZE: usize = 3600;
-/// The Threshold for deciding whether `nLockTime` is interpreted as
-/// time or height.
-// https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/script/script.h#L39
-pub const HEIGHT_TIME_THRESHOLD: u32 = 500_000_000;
-
-/// Bit flag for deciding whether sequence number is
-/// interpreted as height or time
-/* If nSequence encodes a relative lock-time and this flag
- * is set, the relative lock-time has units of 512 seconds,
- * otherwise it specifies blocks with a granularity of 1. */
-// https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki
-pub const SEQUENCE_LOCKTIME_TYPE_FLAG: u32 = 1 << 22;
-
-/// Disable flag for sequence locktime
-/* Below flags apply in the context of BIP 68*/
-/* If this flag set, nSequence is NOT interpreted as a
- * relative lock-time. For future soft-fork compatibility*/
-// https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki
-pub const SEQUENCE_LOCKTIME_DISABLE_FLAG: u32 = 1 << 31;
-
-/// Maximum script element size allowed by consensus rules
-// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/script/script.h#L23
-pub const MAX_SCRIPT_ELEMENT_SIZE: usize = 520;
-/// Maximum script sig size allowed by standardness rules
-// https://github.com/bitcoin/bitcoin/blob/42b66a6b814bca130a9ccf0a3f747cf33d628232/src/policy/policy.cpp#L102
-pub const MAX_SCRIPTSIG_SIZE: usize = 1650;
 /// Helper struct Whether any satisfaction of this fragment contains any timelocks
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
 pub struct TimeLockInfo {

src/policy/concrete.rs

@@ -23,9 +23,8 @@ use std::{error, fmt, str};
 use super::ENTAILMENT_MAX_TERMINALS;
 use errstr;
 use expression::{self, FromTree};
-use miniscript::types::extra_props::{
-    TimeLockInfo, HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_TYPE_FLAG,
-};
+use miniscript::limits::{HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_TYPE_FLAG};
+use miniscript::types::extra_props::TimeLockInfo;
 #[cfg(feature = "compiler")]
 use miniscript::ScriptContext;
 #[cfg(feature = "compiler")]

src/psbt/mod.rs

@@ -27,8 +27,8 @@ use bitcoin::util::psbt::PartiallySignedTransaction as Psbt;
 use bitcoin::Script;
 
 use interpreter;
+use miniscript::limits::SEQUENCE_LOCKTIME_DISABLE_FLAG;
 use miniscript::satisfy::{bitcoinsig_from_rawsig, After, Older};
-use miniscript::types::extra_props::SEQUENCE_LOCKTIME_DISABLE_FLAG;
 use BitcoinSig;
 use Satisfier;
 use {MiniscriptKey, ToPublicKey};