Extra utils for working with Bitcoin Script

[stevenroose]

Got these piled up from some BitVM work I did last year. They are used in bitcoin-scriptexec.

I am no expert on fuzzing, so please someone that does, check out that commit especially :)

I'm open to the idea of putting all the parsing code in script/parse.rs or so..

[coveralls]

Pull Request Test Coverage Report for Build 8523679171

Details

• 150 of 187 (80.21%) changed or added relevant lines in 4 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.04%) to 83.099%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
bitcoin/src/blockdata/script/owned.rs	5	7	71.43%
bitcoin/src/blockdata/script/mod.rs	92	127	72.44%

Files with Coverage Reduction	New Missed Lines	%
bitcoin/src/blockdata/opcodes.rs	1	94.4%

Totals
Change from base Build 8523508788:	-0.04%
Covered Lines:	19220
Relevant Lines:	23129

---

💛 - Coveralls

[apoelstra]

utACK a5b7a5f though this definitely needs to wait for the next release :) and maybe we should make an executive decision about whether we want to drop the OP_ prefix from all our opcodes. (Maybe in the parser we could just support both variants.)

Also I'm seeing some clippy issues with a5b7a5f ... not sure why CI is not complaining.

error: the item `FromStr` is imported redundantly
 --> bitcoin/src/blockdata/script/tests.rs:3:5
  |
3 | use core::str::FromStr;
  |     ^^^^^^^^^^^^^^^^^^
...
8 | use super::*;
  |     -------- the item `FromStr` is already imported here
  |
  = note: `-D unused-imports` implied by `-D warnings`
  = help: to override `-D warnings` add `#[allow(unused_imports)]`

error: the item `DisplayHex` is imported redundantly
  --> bitcoin/src/blockdata/script/tests.rs:12:5
   |
8  | use super::*;
   |     -------- the item `DisplayHex` is already imported here
...
12 | use crate::hex::DisplayHex;
   |     ^^^^^^^^^^^^^^^^^^^^^^

[tcharding]

We can get rid of try_parse_raw_hex and the mutable buffer and just use:

let v = Vec::from_hex(push).map_err(|_| err(next, ParseAsmErrorKind::InvalidHex))?;

[apoelstra]

Does using Vec::from_hex instead of a mutable buffer introduce an allocation where previously there wasn't one?

[tcharding]

oooo, yes it does. Now I see why the mutable vector and buf.clear is used.

Note that this method has no effect on the allocated capacity of the vector.

ref: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.clear

[tcharding]

I don't think we want to unwrap here, any random data is not going to be parsable asm, right?

I think we want:

    let asm = bitcoin::Script::from_bytes(data).to_asm_string();

    match bitcoin::ScriptBuf::parse_asm(&asm) {
        Ok(parsed) => assert_eq!(parsed.as_bytes(), data),
        Err(_) => return,
    }

[apoelstra]

@tcharding no, the data being parsed is coming out of to_asm_string, so we definitely do want to unwrap it.

[tcharding]

Just to clarify, any random bytes [0] can be converted to asm using to_asm_string and should then be parsable?

[0] ScriptBuf::from_bytes does not do any checks, just wraps the vector.

[apoelstra]

@tcharding with the current API, yes. We allow the user to embed any sequence of bytes as a Script, which does mean that they can do invalid things like having incomplete pushes or the 0xff "not an opcode" opcode or whatever. However this behavior matches the Bitcoin network, where the scriptPubkey field of the transaction outputs is allowed to be any random crap.

When we do script tagging it might be interesting to try to make scriptSig and witnessScripts tighter, since unlike scriptPubKeys they have to be legal for their containing transaction to be legal.

[tcharding]

Cool, I got ya, thanks. I'm keen to see script tagging.