Revert "Handle invalid scripts correctly (ordinals#3390)"

This reverts commit 40b3d99.
brc721-core · Apr 2, 2024 · a85ba4a · a85ba4a
1 parent 60fb3da
commit a85ba4a
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 136 deletions.
diff --git a/src/index/updater/rune_updater.rs b/src/index/updater/rune_updater.rs
@@ -362,10 +362,7 @@ impl<'a, 'tx, 'client> RuneUpdater<'a, 'tx, 'client> {
       };
 
       for instruction in tapscript.instructions() {
-        // ignore errors, since the extracted script may not be valid
-        let Ok(instruction) = instruction else {
-          break;
-        };
+        let instruction = instruction?;
 
         let Some(pushbytes) = instruction.push_bytes() else {
           continue;

diff --git a/src/runes.rs b/src/runes.rs
@@ -5392,39 +5392,4 @@ mod tests {
 
     context.assert_runes([], []);
   }
-
-  #[test]
-  fn tx_commits_to_rune_ignores_invalid_script() {
-    let context = Context::builder().arg("--index-runes").build();
-
-    context.mine_blocks(1);
-
-    let runestone = Runestone {
-      etching: Some(Etching {
-        rune: Some(Rune(RUNE)),
-        terms: Some(Terms {
-          amount: Some(1000),
-          ..default()
-        }),
-        ..default()
-      }),
-      ..default()
-    };
-
-    let mut witness = Witness::new();
-
-    witness.push([opcodes::all::OP_PUSHDATA4.to_u8()]);
-    witness.push([]);
-
-    context.rpc_server.broadcast_tx(TransactionTemplate {
-      inputs: &[(1, 0, 0, witness)],
-      op_return: Some(runestone.encipher()),
-      outputs: 1,
-      ..default()
-    });
-
-    context.mine_blocks(1);
-
-    context.assert_runes([], []);
-  }
 }
diff --git a/src/runes/runestone.rs b/src/runes/runestone.rs
@@ -17,7 +17,6 @@ struct Message {
   fields: HashMap<u128, VecDeque<u128>>,
 }
 
-#[derive(Debug, PartialEq)]
 enum Payload {
   Valid(Vec<u8>),
   Invalid,
@@ -265,17 +264,16 @@ impl Runestone {
         continue;
       }
 
-      // followed by the protocol identifier, ignoring errors, since OP_RETURN
-      // scripts may be invalid
-      if instructions.next().transpose().ok().flatten() != Some(Instruction::Op(MAGIC_NUMBER)) {
+      // followed by the protocol identifier
+      if instructions.next().transpose()? != Some(Instruction::Op(MAGIC_NUMBER)) {
         continue;
       }
 
       // construct the payload by concatinating remaining data pushes
       let mut payload = Vec::new();
 
       for result in instructions {
-        if let Ok(Instruction::PushBytes(push)) = result {
+        if let Instruction::PushBytes(push) = result? {
           payload.extend_from_slice(push.as_bytes());
         } else {
           return Ok(Some(Payload::Invalid));
@@ -435,7 +433,7 @@ mod tests {
   }
 
   #[test]
-  fn deciphering_valid_runestone_with_invalid_script_postfix_returns_invalid_payload() {
+  fn deciphering_valid_runestone_with_invalid_script_postfix_returns_script_error() {
     let mut script_pubkey = script::Builder::new()
       .push_opcode(opcodes::all::OP_RETURN)
       .push_opcode(MAGIC_NUMBER)
@@ -444,18 +442,16 @@ mod tests {
 
     script_pubkey.push(opcodes::all::OP_PUSHBYTES_4.to_u8());
 
-    assert_eq!(
-      Runestone::payload(&Transaction {
-        input: Vec::new(),
-        output: vec![TxOut {
-          script_pubkey: ScriptBuf::from_bytes(script_pubkey),
-          value: 0,
-        }],
-        lock_time: LockTime::ZERO,
-        version: 2,
-      }),
-      Ok(Some(Payload::Invalid))
-    );
+    Runestone::decipher(&Transaction {
+      input: Vec::new(),
+      output: vec![TxOut {
+        script_pubkey: ScriptBuf::from_bytes(script_pubkey),
+        value: 0,
+      }],
+      lock_time: LockTime::ZERO,
+      version: 2,
+    })
+    .unwrap_err();
   }
 
   #[test]
@@ -538,8 +534,8 @@ mod tests {
   }
 
   #[test]
-  fn invalid_input_scripts_are_skipped_when_searching_for_runestone() {
-    let payload = payload(&[Tag::Mint.into(), 1, Tag::Mint.into(), 1]);
+  fn error_in_input_aborts_search_for_runestone() {
+    let payload = payload(&[0, 1, 2, 3]);
 
     let payload: &PushBytes = payload.as_slice().try_into().unwrap();
 
@@ -550,33 +546,26 @@ mod tests {
       opcodes::all::OP_PUSHBYTES_4.to_u8(),
     ];
 
-    assert_eq!(
-      Runestone::decipher(&Transaction {
-        input: Vec::new(),
-        output: vec![
-          TxOut {
-            script_pubkey: ScriptBuf::from_bytes(script_pubkey),
-            value: 0,
-          },
-          TxOut {
-            script_pubkey: script::Builder::new()
-              .push_opcode(opcodes::all::OP_RETURN)
-              .push_opcode(MAGIC_NUMBER)
-              .push_slice(payload)
-              .into_script(),
-            value: 0,
-          },
-        ],
-        lock_time: LockTime::ZERO,
-        version: 2,
-      })
-      .unwrap()
-      .unwrap(),
-      Runestone {
-        mint: Some(RuneId::new(1, 1).unwrap()),
-        ..default()
-      },
-    );
+    Runestone::decipher(&Transaction {
+      input: Vec::new(),
+      output: vec![
+        TxOut {
+          script_pubkey: ScriptBuf::from_bytes(script_pubkey),
+          value: 0,
+        },
+        TxOut {
+          script_pubkey: script::Builder::new()
+            .push_opcode(opcodes::all::OP_RETURN)
+            .push_opcode(MAGIC_NUMBER)
+            .push_slice(payload)
+            .into_script(),
+          value: 0,
+        },
+      ],
+      lock_time: LockTime::ZERO,
+      version: 2,
+    })
+    .unwrap_err();
   }
 
   #[test]
@@ -1966,54 +1955,4 @@ mod tests {
       .cenotaph
     );
   }
-
-  #[test]
-  fn invalid_scripts_in_op_returns_are_ignored() {
-    let transaction = Transaction {
-      version: 2,
-      lock_time: LockTime::ZERO,
-      input: vec![TxIn {
-        previous_output: OutPoint::null(),
-        script_sig: ScriptBuf::new(),
-        sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-        witness: Witness::new(),
-      }],
-      output: vec![TxOut {
-        script_pubkey: ScriptBuf::from(vec![
-          opcodes::all::OP_RETURN.to_u8(),
-          opcodes::all::OP_PUSHBYTES_4.to_u8(),
-        ]),
-        value: 0,
-      }],
-    };
-
-    assert_eq!(Runestone::decipher(&transaction).unwrap(), None);
-
-    let transaction = Transaction {
-      version: 2,
-      lock_time: LockTime::ZERO,
-      input: vec![TxIn {
-        previous_output: OutPoint::null(),
-        script_sig: ScriptBuf::new(),
-        sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-        witness: Witness::new(),
-      }],
-      output: vec![TxOut {
-        script_pubkey: ScriptBuf::from(vec![
-          opcodes::all::OP_RETURN.to_u8(),
-          MAGIC_NUMBER.to_u8(),
-          opcodes::all::OP_PUSHBYTES_4.to_u8(),
-        ]),
-        value: 0,
-      }],
-    };
-
-    assert_eq!(
-      Runestone::decipher(&transaction).unwrap(),
-      Some(Runestone {
-        cenotaph: true,
-        ..default()
-      })
-    );
-  }
 }