[feat] Implement OP_SUB opcode

[TAdev0]

• [x] issue #11
• [x] follows contribution guide
• [x] code change includes tests

Hi @b-j-roberts

I also added OP_2 opcode for testing.

I have 2 questions:

• bitcoin wiki says arithmetic operations operates on i32 not i64, is it normal that we use i64? https://en.bitcoin.it/wiki/Script

wiki says : "Note: Arithmetic inputs are limited to signed 32-bit integers, but may overflow their output."

this is why I tested the OP_SUB for

• 1 - 1
• 2 - 1
• 1 - 2

in the last case, i guess an overflow is supposed to happen and return -1 , currently represented as a i64 But as you can see in test, i get an unwrap error, related to the assert_eq! usage on the last line. is it an issue with the macro? but it should work and print the result right?

when i print the ByteArray that will be added to the ScriptStack , i get this :

    fn push_int(ref self: ScriptStack, value: i64) {
        let mut bytes = "";
        bytes.append_word(value.into(), 8);
        println!("{}", bytes);
        self.push_byte_array(bytes);
    }

[DEBUG] 0x46a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3
[DEBUG] 0x0 ('')
[DEBUG] 0x800000000000010ffffffffffffffffffffffffffffffffffffffffffffff0b
[DEBUG] 0x9 ('  ')

[b-j-roberts]

Thanks for looking into this!

So the stack values for arithmetic operations are i32, but they will be represented as i64 to be safe in cases of overflow. You can see more info here from btcd : https://github.com/btcsuite/btcd/blob/b161cd6a199b4e35acec66afc5aad221f05fe1e3/txscript/scriptnum.go#L32

I can look more at this error it is giving you soon, but if you want to try and dig deeper, I think we might need to implement this function from btcd to handle cases of negative numbers : https://github.com/btcsuite/btcd/blob/b161cd6a199b4e35acec66afc5aad221f05fe1e3/txscript/scriptnum.go#L105

[TAdev0]

@b-j-roberts thanks, now I understand why we use i64

It seems indeed this function will be required

[b-j-roberts]

It seems indeed this function will be required

Would you be interested in implementing that function and making any other required changes to get this working? If not, I'll try and get someone on it asap

[TAdev0]

Yeah I can work on it

[TAdev0]

@b-j-roberts just implemented the Bytes function. Thus, we can remove the append_word method from ByteArray corelib.

Needed changes in tests because the formatting of the stack is now different, doesnt appear with 8 elements all the time, just like the Go comment here:

// Example encodings:
//
//     127 -> [0x7f]
//    -127 -> [0xff]
//     128 -> [0x80 0x00]
//    -128 -> [0x80 0x80]
//     129 -> [0x81 0x00]