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/miniscript/astelem.rs

@@ -633,7 +633,7 @@ impl StackCtxOperations for script::Builder {
         let mut builder = self;
         // Initial Witness
         let max_elems = MAX_SCRIPT_ELEMENT_SIZE / MAX_STANDARD_P2WSH_STACK_ITEM_SIZE;
-        for _ in 0..(max_elems - 1) {
+        for _ in 0..max_elems {
             builder = builder.push_opcode(opcodes::all::OP_CAT);
         }
         builder = builder
@@ -801,9 +801,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/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(),