Merge rust-bitcoin/rust-bitcoin#1502: Fix bug in `ScriptBuf::extend` for short iterators

920599da94 Add test for previous commit (Martin Habovstiak)
a7f3458c27 Fix bug in `ScriptBuf::extend` for short iterators (Martin Habovstiak)

Pull request description:

  `ScriptBuf::extend` contained an optimization for short scripts that was
  supposed to preallocate the buffer and then fill it. By mistake it
  attempted to fill it from already-exhausted iterator instead of the
  temporary array it drained the items into. This obviously produced
  garbage (empty) values.

  This was not caught by tests because the optimization is only active for
  scripts with known maximum length and the test used `Instructions` which
  doesn't know the maximum length.

ACKs for top commit:
  sanket1729:
    ACK 920599da94 . Tested that the bug is correctly fixed and tested in the new test
  tcharding:
    ACK 920599da94

Tree-SHA512: a80f5f262a840d8e77efd42d63c511224380ee3efa6c31855233e81c90332ac15db228e8d552d039d729d7b642e03c3939c8b6a92d3279001377515acb83abea

bitcoin/src/blockdata/script.rs

@@ -1442,10 +1442,11 @@ impl<'a> core::iter::FromIterator<Instruction<'a>> for ScriptBuf {
 
 impl<'a> Extend<Instruction<'a>> for ScriptBuf {
     fn extend<T>(&mut self, iter: T) where T: IntoIterator<Item = Instruction<'a>> {
-        let mut iter = iter.into_iter();
+        let iter = iter.into_iter();
         // Most of Bitcoin scripts have only a few opcodes, so we can avoid reallocations in many
         // cases.
         if iter.size_hint().1.map(|max| max < 6).unwrap_or(false) {
+            let mut iter = iter.fuse();
             // `MaybeUninit` might be faster but we don't want to introduce more `unsafe` than
             // required.
             let mut head = [None; 5];
@@ -1454,8 +1455,10 @@ impl<'a> Extend<Instruction<'a>> for ScriptBuf {
                 total_size += instr.script_serialized_len();
                 *head = Some(instr);
             }
+            // Incorrect impl of `size_hint` breaks `Iterator` contract so we're free to panic.
+            assert!(iter.next().is_none(), "Buggy implementation of `Iterator` on {} returns invalid upper bound", core::any::type_name::<T::IntoIter>());
             self.reserve(total_size);
-            for instr in iter {
+            for instr in head.iter().cloned().flatten() {
                 self.push_instruction_no_opt(instr);
             }
         } else {
@@ -2600,6 +2603,50 @@ mod test {
         assert!(instructions.next().is_none());
     }
 
+    #[test]
+    fn script_extend() {
+        fn cmp_scripts(new_script: &Script, orig_script: &[Instruction<'_>]) {
+            let mut new_instr = new_script.instructions();
+            let mut orig_instr = orig_script.iter().cloned();
+            for (new, orig) in new_instr.by_ref().zip(orig_instr.by_ref()) {
+                assert_eq!(new.unwrap(), orig);
+            }
+            assert!(new_instr.next().is_none() && orig_instr.next().is_none())
+        }
+
+        let script_5_items = [
+            Instruction::Op(OP_DUP),
+            Instruction::Op(OP_HASH160),
+            Instruction::PushBytes(&[42; 20]),
+            Instruction::Op(OP_EQUALVERIFY),
+            Instruction::Op(OP_CHECKSIG),
+        ];
+        let new_script = script_5_items.iter().cloned().collect::<ScriptBuf>();
+        cmp_scripts(&new_script, &script_5_items);
+
+        let script_6_items = [
+            Instruction::Op(OP_DUP),
+            Instruction::Op(OP_HASH160),
+            Instruction::PushBytes(&[42; 20]),
+            Instruction::Op(OP_EQUALVERIFY),
+            Instruction::Op(OP_CHECKSIG),
+            Instruction::Op(OP_NOP),
+        ];
+        let new_script = script_6_items.iter().cloned().collect::<ScriptBuf>();
+        cmp_scripts(&new_script, &script_6_items);
+
+        let script_7_items = [
+            Instruction::Op(OP_DUP),
+            Instruction::Op(OP_HASH160),
+            Instruction::PushBytes(&[42; 20]),
+            Instruction::Op(OP_EQUALVERIFY),
+            Instruction::Op(OP_CHECKSIG),
+            Instruction::Op(OP_NOP),
+        ];
+        let new_script = script_7_items.iter().cloned().collect::<ScriptBuf>();
+        cmp_scripts(&new_script, &script_7_items);
+    }
+
     #[test]
     fn read_scriptbool_zero_is_false() {
         let v: Vec<u8> = vec![0x00, 0x00, 0x00, 0x00];