feat: add op_checkmultisig (#198)

- [X] follows contribution [guide](https://github.com/keep-starknet-strange/shinigami/blob/main/CONTRIBUTING.md) - [X] code change includes tests  Add opcodes OP_CHECKMULTISIG and OP_CHECKMULTISIGVERIFY --------- Co-authored-by: j1mbo64 <[email protected]> Co-authored-by: Brandon Roberts <[email protected]>
keep-starknet-strange · Sep 5, 2024 · 6496bc1 · 6496bc1
1 parent e4c1717
commit 6496bc1
Show file tree

Hide file tree

Showing 16 changed files with 373 additions and 104 deletions.
diff --git a/README.md b/README.md
@@ -51,15 +51,15 @@ This will run the test-suite for all opcodes, integration, and testing Scripts.
 
 ## Supported Opcodes
 
-104/107 opcodes supported (97.20%).
+106/107 opcodes supported (99.07%).
 
 ```mermaid
 %%{init: {"pie": {"textPosition": 0.75}, "themeVariables": {"pieOuterStrokeWidth": "5px"}} }%%
 pie showData
     title Opcode Implementation Status
-    "Implemented" : 89
+    "Implemented" : 91
     "Disabled" : 15
-    "Not Implemented" : 3
+    "Not Implemented" : 1
 ```
 
 | Opcode                 | Hex       | Supported | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        |
@@ -164,8 +164,8 @@ pie showData
 | OP_CODESEPARATOR       | 0xab      |     ✅     | All of the signature checking words will only match signatures to the data after the most recently-executed OP_CODESEPARATOR.                                                                                                                                                                                                                                                                                                                                                                                                      |
 | OP_CHECKSIG            | 0xac      |     ✅     | The entire transaction's outputs, inputs, and script are hashed. The signature used by OP_CHECKSIG must be a valid signature for this hash and public key. If it is, 1 is returned, 0 otherwise.                                                                                                                                                                                                                                                                                                                                   |
 | OP_CHECKSIGVERIFY      | 0xad      |     ✅     | Same as OP_CHECKSIG, but OP_VERIFY is executed afterward.                                                                                                                                                                                                                                                                                                                                                                                                                                                                          |
-| OP_CHECKMULTISIG       | 0xae      |           | Compares the first signature against each public key until it finds an ECDSA match. Starting with the subsequent public key, it compares the second signature against each remaining public key until it finds an ECDSA match. The process is repeated until all signatures have been checked or not enough public keys remain to produce a successful result. All signatures need to match a public key. If all signatures are valid, 1 is returned, 0 otherwise. Due to a bug, one extra unused value is removed from the stack. |
-| OP_CHECKMULTISIGVERIFY | 0xaf      |           | Same as OP_CHECKMULTISIG, but OP_VERIFY is executed afterward.                                                                                                                                                                                                                                                                                                                                                                                                                                                                     |
+| OP_CHECKMULTISIG       | 0xae      |     ✅     | Compares the first signature against each public key until it finds an ECDSA match. Starting with the subsequent public key, it compares the second signature against each remaining public key until it finds an ECDSA match. The process is repeated until all signatures have been checked or not enough public keys remain to produce a successful result. All signatures need to match a public key. If all signatures are valid, 1 is returned, 0 otherwise. Due to a bug, one extra unused value is removed from the stack. |
+| OP_CHECKMULTISIGVERIFY | 0xaf      |     ✅     | Same as OP_CHECKMULTISIG, but OP_VERIFY is executed afterward.                                                                                                                                                                                                                                                                                                                                                                                                                                                                     |
 | OP_NOP1                | 0xb0      |     ✅     | The word is ignored. Does not mark transaction as invalid.                                                                                                                                                                                                                                                                                                                                                                                                                                                                         |
 | OP_CHECKLOCKTIMEVERIFY | 0xb1      |     ✅     | Marks transaction as invalid if the top stack item is greater than the transaction's nLockTime field, otherwise script evaluation continues as though an OP_NOP was executed.                                                                                                                                                                                                                                                                                                                                                      |
 | OP_CHECKSEQUENCEVERIFY | 0xb2      |     ✅     | Marks transaction as invalid if the relative lock time of the input is not equal to or longer than the value of the top stack item.                                                                                                                                                                                                                                                                                                                                                                                                |

diff --git a/Scarb.lock b/Scarb.lock
@@ -4,12 +4,12 @@ version = 1
 [[package]]
 name = "ripemd160"
 version = "0.1.0"
-source = "git+https://github.com/j1mbo64/ripemd160_cairo.git#bebe67c235b12e0a3668f49c78423d4eff6d7131"
+source = "git+https://github.com/j1mbo64/ripemd160_cairo.git#cdc5ab58b0acc64db87e0b03851fb18213977dc8"
 
 [[package]]
 name = "sha1"
 version = "0.1.0"
-source = "git+https://github.com/j1mbo64/sha1_cairo.git#a2fbea0d47adeb7c9c60d6fb97580e4084834cf5"
+source = "git+https://github.com/j1mbo64/sha1_cairo.git#280b4c64ae457fdc4bd7cd807efd17e8dced654e"
 
 [[package]]
 name = "shinigami"

diff --git a/src/compiler.cairo b/src/compiler.cairo
@@ -280,6 +280,8 @@ pub impl CompilerImpl of CompilerTrait {
         compiler.add_opcode('OP_HASH256', Opcode::OP_HASH256);
         compiler.add_opcode('OP_CHECKSIG', Opcode::OP_CHECKSIG);
         compiler.add_opcode('OP_CHECKSIGVERIFY', Opcode::OP_CHECKSIGVERIFY);
+        compiler.add_opcode('OP_CHECKMULTISIG', Opcode::OP_CHECKMULTISIG);
+        compiler.add_opcode('OP_CHECKMULTISIGVERIFY', Opcode::OP_CHECKMULTISIGVERIFY);
         compiler.add_opcode('OP_CODESEPARATOR', Opcode::OP_CODESEPARATOR);
         compiler.add_opcode('OP_CHECKLOCKTIMEVERIFY', Opcode::OP_CHECKLOCKTIMEVERIFY);
         compiler.add_opcode('OP_CLTV', Opcode::OP_CHECKLOCKTIMEVERIFY);

diff --git a/src/errors.cairo b/src/errors.cairo
@@ -12,6 +12,7 @@ pub mod Error {
     pub const OPCODE_DISABLED: felt252 = 'Opcode is disabled';
     pub const SCRIPT_DISCOURAGE_UPGRADABLE_NOPS: felt252 = 'Upgradable NOPs are discouraged';
     pub const UNSATISFIED_LOCKTIME: felt252 = 'Unsatisfied locktime';
+    pub const SCRIPT_STRICT_MULTISIG: felt252 = 'OP_CHECKMULTISIG invalid dummy';
     pub const FINALIZED_TX_CLTV: felt252 = 'Finalized tx in OP_CLTV';
     pub const INVALID_TX_VERSION: felt252 = 'Invalid transaction version';
     pub const SCRIPT_INVALID: felt252 = 'Invalid script data';

diff --git a/src/lib.cairo b/src/lib.cairo
@@ -39,15 +39,15 @@ pub mod scriptnum {
         #[cfg(test)]
         mod test_scriptnum;
     }
-    pub(crate) use scriptnum::ScriptNum;
+    pub use scriptnum::ScriptNum;
 }
 pub mod scriptflags;
 pub mod signature {
     pub mod signature;
     pub mod sighash;
     pub mod constants;
     pub mod utils;
-    pub(crate) use signature::{BaseSigVerifier, BaseSigVerifierTrait};
+    pub use signature::{BaseSigVerifier, BaseSigVerifierTrait};
 }
 pub mod transaction;
 #[cfg(test)]

diff --git a/src/opcodes/crypto.cairo b/src/opcodes/crypto.cairo
@@ -1,8 +1,16 @@
+use crate::engine::{Engine, EngineTrait};
+use crate::stack::ScriptStackTrait;
+use crate::scriptflags::ScriptFlags;
+use crate::signature::signature;
+use crate::signature::sighash;
+use crate::signature::signature::BaseSigVerifierTrait;
+use starknet::secp256_trait::{is_valid_signature};
 use core::sha256::compute_sha256_byte_array;
-use crate::engine::Engine;
 use crate::opcodes::utils;
-use crate::signature::BaseSigVerifierTrait;
-use crate::stack::ScriptStackTrait;
+use crate::scriptnum::ScriptNum;
+use crate::errors::Error;
+
+const MAX_KEYS_PER_MULTISIG: i64 = 20;
 
 pub fn opcode_sha256(ref engine: Engine) -> Result<(), felt252> {
     let arr = @engine.dstack.pop_byte_array()?;
@@ -91,6 +99,114 @@ pub fn opcode_checksig(ref engine: Engine) -> Result<(), felt252> {
     return Result::Ok(());
 }
 
+pub fn opcode_checkmultisig(ref engine: Engine) -> Result<(), felt252> {
+    // TODO Error on taproot exec
+    // TODO Numops
+
+    // Get number of public keys and construct array
+    let num_keys = engine.dstack.pop_int()?;
+    let mut num_pub_keys: i64 = ScriptNum::to_int32(num_keys).into();
+    if num_pub_keys < 0 {
+        return Result::Err('check multisig: num pk < 0');
+    }
+    if num_pub_keys > MAX_KEYS_PER_MULTISIG {
+        return Result::Err('check multisig: num pk > max');
+    }
+    let mut pub_keys = ArrayTrait::<ByteArray>::new();
+    let mut i: i64 = 0;
+    let mut err: felt252 = 0;
+    while i < num_pub_keys {
+        match engine.dstack.pop_byte_array() {
+            Result::Ok(pk) => pub_keys.append(pk),
+            Result::Err(e) => err = e
+        };
+        i += 1;
+    };
+    if err != 0 {
+        return Result::Err(err);
+    }
+
+    // Get number of required sigs and construct array
+    let num_sig_base = engine.dstack.pop_int()?;
+    let mut num_sigs: i64 = ScriptNum::to_int32(num_sig_base).into();
+    if num_sigs < 0 {
+        return Result::Err('check multisig: num sigs < 0');
+    }
+    if num_sigs > num_pub_keys {
+        return Result::Err('check multisig: num sigs > pk');
+    }
+    let mut sigs = ArrayTrait::<ByteArray>::new();
+    i = 0;
+    err = 0;
+    while i < num_sigs {
+        match engine.dstack.pop_byte_array() {
+            Result::Ok(s) => sigs.append(s),
+            Result::Err(e) => err = e
+        };
+        i += 1;
+    };
+    if err != 0 {
+        return Result::Err(err);
+    }
+
+    // Historical bug
+    let dummy = engine.dstack.pop_byte_array()?;
+
+    if engine.has_flag(ScriptFlags::ScriptStrictMultiSig) && dummy.len() != 0 {
+        return Result::Err(Error::SCRIPT_STRICT_MULTISIG);
+    }
+
+    let mut script = engine.sub_script();
+
+    // TODO: add witness context inside engine to check if witness is active
+    let mut s: u32 = 0;
+    while s < sigs.len() {
+        script = signature::remove_signature(script, sigs.at(s));
+        s += 1;
+    };
+
+    let mut success = true;
+    num_pub_keys += 1; // Offset due to decrementing it in the loop
+    let mut pub_key_idx: i64 = -1;
+    let mut sig_idx: i64 = 0;
+
+    while num_sigs > 0 {
+        pub_key_idx += 1;
+        num_pub_keys -= 1;
+        if num_sigs > num_pub_keys {
+            success = false;
+            break;
+        }
+
+        let sig = sigs.at(sig_idx.try_into().unwrap());
+        let pub_key = pub_keys.at(pub_key_idx.try_into().unwrap());
+        if sig.len() == 0 {
+            continue;
+        }
+
+        let res = signature::parse_base_sig_and_pk(ref engine, pub_key, sig);
+        if res.is_err() {
+            success = false;
+            err = res.unwrap_err();
+            break;
+        }
+        let (parsed_pub_key, parsed_sig, hash_type) = res.unwrap();
+        let sig_hash: u256 = sighash::calc_signature_hash(
+            @script, hash_type, ref engine.transaction, engine.tx_idx
+        );
+        if is_valid_signature(sig_hash, parsed_sig.r, parsed_sig.s, parsed_pub_key) {
+            sig_idx += 1;
+            num_sigs -= 1;
+        }
+    };
+    if err != 0 {
+        return Result::Err(err);
+    }
+
+    engine.dstack.push_bool(success);
+    Result::Ok(())
+}
+
 pub fn opcode_codeseparator(ref engine: Engine) -> Result<(), felt252> {
     engine.last_code_sep = engine.opcode_idx;
 
@@ -110,6 +226,12 @@ pub fn opcode_checksigverify(ref engine: Engine) -> Result<(), felt252> {
     return Result::Ok(());
 }
 
+pub fn opcode_checkmultisigverify(ref engine: Engine) -> Result<(), felt252> {
+    opcode_checkmultisig(ref engine)?;
+    utils::abstract_verify(ref engine)?;
+    return Result::Ok(());
+}
+
 pub fn opcode_sha1(ref engine: Engine) -> Result<(), felt252> {
     let m = engine.dstack.pop_byte_array()?;
     let h: ByteArray = sha1::sha1_hash(@m).into();

diff --git a/src/opcodes/opcodes.cairo b/src/opcodes/opcodes.cairo
@@ -174,6 +174,8 @@ pub mod Opcode {
     pub const OP_CODESEPARATOR: u8 = 171;
     pub const OP_CHECKSIG: u8 = 172;
     pub const OP_CHECKSIGVERIFY: u8 = 173;
+    pub const OP_CHECKMULTISIG: u8 = 174;
+    pub const OP_CHECKMULTISIGVERIFY: u8 = 175;
     pub const OP_NOP1: u8 = 176;
     pub const OP_CHECKLOCKTIMEVERIFY: u8 = 177;
     pub const OP_CHECKSEQUENCEVERIFY: u8 = 178;
@@ -366,8 +368,8 @@ pub mod Opcode {
             171 => crypto::opcode_codeseparator(ref engine),
             172 => crypto::opcode_checksig(ref engine),
             173 => crypto::opcode_checksigverify(ref engine),
-            174 => utils::not_implemented(ref engine),
-            175 => utils::not_implemented(ref engine),
+            174 => crypto::opcode_checkmultisig(ref engine),
+            175 => crypto::opcode_checkmultisigverify(ref engine),
             176 => flow::opcode_nop(),
             177 => locktime::opcode_checklocktimeverify(ref engine),
             178 => locktime::opcode_checksequenceverify(ref engine),