Review feedback

Author: sanket1729

src/descriptor/covenants/mod.rs

@@ -739,7 +739,7 @@ mod tests {
     fn string_rtt(desc_str: &str) {
         let desc = Descriptor::<String>::from_str(desc_str).unwrap();
         assert_eq!(desc.to_string_no_chksum(), desc_str);
-        let cov_desc = desc.as_cov();
+        let cov_desc = desc.as_cov().unwrap();
         assert_eq!(cov_desc.to_string(), desc.to_string());
     }
     #[test]
@@ -836,7 +836,7 @@ mod tests {
         cov_sk: secp256k1::SecretKey,
     ) -> Result<(), Error> {
         assert_eq!(desc.desc_type(), DescriptorType::Cov);
-        let desc = desc.as_cov();
+        let desc = desc.as_cov().unwrap();
         // Now create a transaction spending this.
         let mut spend_tx = Transaction {
             version: 2,
@@ -1062,7 +1062,7 @@ mod tests {
         spend_tx.output[2].value = confidential::Value::Explicit(2_000);
 
         // Try to satisfy the covenant part
-        let desc = desc.as_cov();
+        let desc = desc.as_cov().unwrap();
         let script_code = desc.cov_script_code();
         let cov_sat = CovSatisfier::new_segwitv0(
             &spend_tx,

src/descriptor/mod.rs

@@ -432,11 +432,11 @@ impl<Pk: MiniscriptKey> Descriptor<Pk> {
 
     /// Unwrap a descriptor as a covenant descriptor
     /// Panics if the descriptor is not of [DescriptorType::Cov]
-    pub fn as_cov(&self) -> &CovenantDescriptor<Pk> {
+    pub fn as_cov(&self) -> Result<&CovenantDescriptor<Pk>, Error> {
         if let Descriptor::Cov(cov) = self {
-            cov
+            Ok(cov)
         } else {
-            panic!("Called as_cov on a non-covenant descriptor")
+            Err(Error::CovError(CovError::BadCovDescriptor))
         }
     }
 

src/interpreter/stack.rs

@@ -23,6 +23,7 @@ use elements::{self, opcodes, script};
 use {ElementsSig, ToPublicKey};
 
 use super::{verify_sersig, Error, HashLockType, SatisfiedConstraint};
+use miniscript::limits::{MAX_SCRIPT_ELEMENT_SIZE, MAX_STANDARD_P2WSH_STACK_ITEM_SIZE};
 use util;
 /// Definition of Stack Element of the Stack used for interpretation of Miniscript.
 /// All stack elements with vec![] go to Dissatisfied and vec![1] are marked to Satisfied.
@@ -431,21 +432,23 @@ impl<'txin> Stack<'txin> {
         if let Err(e) = hash_outputs.try_push() {
             return Some(Err(e));
         }
+        // Maximum number of suffix elements
+        let max_elems = MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE + 1;
         let hash_outputs = hash_outputs.as_push();
         if hash_outputs.len() == 32 {
             // We want to cat the last 6 elements(5 cats) in suffix
-            if self.len() < 6 {
+            if self.len() < max_elems {
                 return Some(Err(Error::UnexpectedStackEnd));
             }
             let mut outputs_builder = Vec::new();
             outputs_builder.extend(pref);
             let len = self.len();
-            // Add the 6 suffix elements
-            for i in 0..6 {
-                outputs_builder.extend(self[len - 6 + i].into_slice());
+            // Add the max_elems suffix elements
+            for i in 0..max_elems {
+                outputs_builder.extend(self[len - max_elems + i].into_slice());
             }
-            // Pop the 6 suffix elements
-            for _ in 0..6 {
+            // Pop the max_elems suffix elements
+            for _ in 0..max_elems {
                 self.pop().unwrap();
             }
             if sha256d::Hash::hash(&outputs_builder).as_inner() == hash_outputs {

src/miniscript/astelem.rs

@@ -632,7 +632,8 @@ impl StackCtxOperations for script::Builder {
     fn check_item_pref(self, idx: u32, pref: &[u8]) -> Self {
         let mut builder = self;
         // Initial Witness
-        let max_elems = MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE;
+        // The nuumber of maximum witness elements in the suffix
+        let max_elems = MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE + 1;
         for _ in 0..(max_elems - 1) {
             builder = builder.push_opcode(opcodes::all::OP_CAT);
         }
@@ -801,9 +802,9 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
             Terminal::False => 1,
             Terminal::Version(_n) => 4 + 1 + 1 + 4, // opcodes + push opcodes + target size
             Terminal::OutputsPref(ref pref) => {
-                // CAT CAT CAT CAT CAT <pref> SWAP CAT /*Now we hashoutputs on stack */
+                // CAT CAT CAT CAT CAT CAT <pref> SWAP CAT /*Now we hashoutputs on stack */
                 // HASH256 DEPTH <10> SUB PICK EQUAL
-                7 + pref.len() + 1 /* line1 opcodes + pref.push */
+                8 + pref.len() + 1 /* line1 opcodes + pref.push */
                 + 6 /* line 2 */
             }
             Terminal::Alt(ref sub) => sub.node.script_size() + 2,

src/miniscript/decode.rs

@@ -293,7 +293,7 @@ pub fn parse<Ctx: ScriptContext>(
                                 tokens,
                                 Tk::Num(10) => match_token!(
                                     tokens,
-                                    Tk::Hash256, Tk::Cat, Tk::Swap, Tk::Push(bytes), Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat =>
+                                    Tk::Hash256, Tk::Cat, Tk::Swap, Tk::Push(bytes), Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat =>
                                         {
                                             non_term.push(NonTerm::Verify);
                                             term.reduce0(Terminal::OutputsPref(bytes))?
@@ -368,7 +368,7 @@ pub fn parse<Ctx: ScriptContext>(
                             tokens,
                             Tk::Num(10) => match_token!(
                                 tokens,
-                                Tk::Hash256, Tk::Cat, Tk::Swap, Tk::Push(bytes), Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat =>
+                                Tk::Hash256, Tk::Cat, Tk::Swap, Tk::Push(bytes), Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat, Tk::Cat =>
                                     term.reduce0(Terminal::OutputsPref(bytes))?,
                             ),
                         ),

src/miniscript/satisfy.rs

@@ -750,7 +750,8 @@ impl Witness {
         match sat.lookup_outputs() {
             Some(outs) => {
                 let mut ser_out = Vec::new();
-                let num_wit_elems = MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE;
+                let num_wit_elems =
+                    MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE + 1;
                 let mut witness = Vec::with_capacity(num_wit_elems);
                 for out in outs {
                     ser_out.extend(serialize(out));

src/miniscript/types/extra_props.rs

@@ -347,13 +347,13 @@ impl Property for ExtData {
         // Assume txouts fill out all the 520 bytes
         let max_wit_sz = MAX_SCRIPT_ELEMENT_SIZE - pref.len();
         ExtData {
-            pk_cost: 7 + pref.len() + 1 + 6, // See script_size() in astelem.rs
+            pk_cost: 8 + pref.len() + 1 + 6, // See script_size() in astelem.rs
             has_free_verify: true,
-            ops_count_static: 12,
-            ops_count_sat: Some(12),
-            ops_count_nsat: Some(12),
-            stack_elem_count_sat: Some(6),
-            stack_elem_count_dissat: Some(6),
+            ops_count_static: 13,
+            ops_count_sat: Some(13),
+            ops_count_nsat: Some(13),
+            stack_elem_count_sat: Some(7),
+            stack_elem_count_dissat: Some(7),
             max_sat_size: Some((max_wit_sz, max_wit_sz)),
             max_dissat_size: Some((0, 0)), // all empty should dissatisfy
             timelock_info: TimeLockInfo::default(),