From ff58ba0ff7bfbea0105d875f3491a89225022205 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Tue, 6 Aug 2024 15:53:21 -0400 Subject: [PATCH 01/13] mcms initial --- contracts/src/lib.cairo | 1 + contracts/src/mcms.cairo | 101 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 contracts/src/mcms.cairo diff --git a/contracts/src/lib.cairo b/contracts/src/lib.cairo index eef049d8c..71ffd2336 100644 --- a/contracts/src/lib.cairo +++ b/contracts/src/lib.cairo @@ -8,6 +8,7 @@ mod emergency; mod multisig; mod token; mod access_control; +mod mcms; #[cfg(test)] mod tests; diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo new file mode 100644 index 000000000..bed175737 --- /dev/null +++ b/contracts/src/mcms.cairo @@ -0,0 +1,101 @@ +use starknet::ContractAddress; +use starknet::{ + EthAddress, + secp256_trait::{ + Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature + }, + secp256k1::Secp256k1Point, SyscallResult, SyscallResultTrait +}; + +#[starknet::interface] +trait IManyChainMultiSig { + // todo: check length of byte array is 32 + fn set_root( + ref self: TContractState, + root: u256, + valid_until: u32, + metadata: RootMetadata, + metadata_proof: Span, + // note: v is a boolean and not uint8 + signaures: Span + ); + fn execute(ref self: TContractState, op: Op, proof: Span); + // todo: check length of group_quorums and group_parents + fn set_config( + ref self: TContractState, + signer_addresses: Span, + signer_groups: Span, + group_quorums: Span, + group_parents: Span, + clear_root: bool + ); + fn get_config(self: @TContractState) -> Config; + fn get_op_count(self: @TContractState) -> u64; + fn get_root(self: @TContractState) -> (u256, u32); + fn get_root_metadata(self: @TContractState) -> RootMetadata; +} + +#[derive(Copy, Drop, Serde, starknet::Store)] +struct Signer { + address: EthAddress, + index: u8, + group: u8 +} + +#[derive(Copy, Drop, Serde, starknet::Store)] +struct RootMetadata { + chain_id: u256, + multisig: ContractAddress, + pre_opcount: u64, + post_opcount: u64, + override_previous_root: bool +} + +#[derive(Copy, Drop, Serde, starknet::Store)] +struct Op { + chain_id: u256, + multisig: ContractAddress, + nonce: u64, + to: ContractAddress, + data: ByteArray +} + +// does not implement Storage trait because structs cannot support arrays or maps +#[derive(Copy, Drop, Serde)] +struct Config { + signers: Span, + group_quorums: Span, + group_parents: Span +} + +#[derive(Copy, Drop, Serde, starknet::Store)] +struct ExpiringRootAndOpCount { + root: u256, + valid_until: u32, + op_count: u64 +} + + +#[starknet::contract] +mod ManyChainMultiSig { + use super::{ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op}; + use starknet::{EthAddress}; + const NUM_GROUPS: u8 = 32; + const MAX_NUM_SIGNERS: u8 = 200; + + #[storage] + struct Storage { + s_signers: LegacyMap, + // begin s_config (defined in storage bc Config struct cannot support maps) + _s_config_signers_len: u8, + _s_config_signers: LegacyMap, + // no _s_config_group_len because there are always 32 groups + _s_config_group_quorums: LegacyMap, + _s_config_group_parents: LegacyMap, + // end s_config + s_seen_signed_hashes: LegacyMap, + s_expiring_root_and_op_count: ExpiringRootAndOpCount, + s_root_metadata: RootMetadata + } +} + From cda1cf6c39e01abaca465324cc1a378b2e3c9ada Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Thu, 8 Aug 2024 16:25:51 -0400 Subject: [PATCH 02/13] finish set_root --- .vscode/settings.json | 2 +- contracts/Scarb.lock | 54 +++++++++ contracts/Scarb.toml | 3 + contracts/src/mcms.cairo | 243 +++++++++++++++++++++++++++++++++++---- 4 files changed, 281 insertions(+), 21 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index bb4c99574..3f77e7328 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,7 +2,7 @@ "editor.formatOnSave": true, "editor.formatOnSaveTimeout": 1500, "editor.codeActionsOnSave": { - "source.fixAll.eslint": true + "source.fixAll.eslint": "explicit" }, "files.insertFinalNewline": true } diff --git a/contracts/Scarb.lock b/contracts/Scarb.lock index 88a112553..cfdbfc0f4 100644 --- a/contracts/Scarb.lock +++ b/contracts/Scarb.lock @@ -1,10 +1,64 @@ # Code generated by scarb DO NOT EDIT. version = 1 +[[package]] +name = "alexandria_bytes" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", + "alexandria_math", +] + +[[package]] +name = "alexandria_data_structures" +version = "0.2.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_encoding", +] + +[[package]] +name = "alexandria_encoding" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_bytes", + "alexandria_math", + "alexandria_numeric", +] + +[[package]] +name = "alexandria_math" +version = "0.2.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", +] + +[[package]] +name = "alexandria_numeric" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_math", + "alexandria_searching", +] + +[[package]] +name = "alexandria_searching" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", +] + [[package]] name = "chainlink" version = "0.1.0" dependencies = [ + "alexandria_bytes", + "alexandria_encoding", "openzeppelin", ] diff --git a/contracts/Scarb.toml b/contracts/Scarb.toml index 69c519dfd..4a89900c9 100644 --- a/contracts/Scarb.toml +++ b/contracts/Scarb.toml @@ -14,6 +14,9 @@ sierra = "cairo-compile . -r" [dependencies] starknet = ">=2.6.3" openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.10.0" } +alexandria_bytes = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "bcdca70afdf59c9976148e95cebad5cf63d75a7f" } +alexandria_encoding = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "bcdca70afdf59c9976148e95cebad5cf63d75a7f" } + [lib] diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index bed175737..56b723399 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -1,11 +1,14 @@ use starknet::ContractAddress; use starknet::{ - EthAddress, + eth_signature::public_key_point_to_eth_address, EthAddress, secp256_trait::{ Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature }, secp256k1::Secp256k1Point, SyscallResult, SyscallResultTrait }; +use alexandria_bytes::{Bytes, BytesTrait}; +use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; +use alexandria_encoding::sol_abi::encode::SolAbiEncodeTrait; #[starknet::interface] trait IManyChainMultiSig { @@ -17,22 +20,22 @@ trait IManyChainMultiSig { metadata: RootMetadata, metadata_proof: Span, // note: v is a boolean and not uint8 - signaures: Span + signatures: Array ); fn execute(ref self: TContractState, op: Op, proof: Span); - // todo: check length of group_quorums and group_parents - fn set_config( - ref self: TContractState, - signer_addresses: Span, - signer_groups: Span, - group_quorums: Span, - group_parents: Span, - clear_root: bool - ); - fn get_config(self: @TContractState) -> Config; - fn get_op_count(self: @TContractState) -> u64; - fn get_root(self: @TContractState) -> (u256, u32); - fn get_root_metadata(self: @TContractState) -> RootMetadata; +// // todo: check length of group_quorums and group_parents +// fn set_config( +// ref self: TContractState, +// signer_addresses: Span, +// signer_groups: Span, +// group_quorums: Span, +// group_parents: Span, +// clear_root: bool +// ); +// fn get_config(self: @TContractState) -> Config; +// fn get_op_count(self: @TContractState) -> u64; +// fn get_root(self: @TContractState) -> (u256, u32); +// fn get_root_metadata(self: @TContractState) -> RootMetadata; } #[derive(Copy, Drop, Serde, starknet::Store)] @@ -46,12 +49,13 @@ struct Signer { struct RootMetadata { chain_id: u256, multisig: ContractAddress, - pre_opcount: u64, - post_opcount: u64, + pre_op_count: u64, + post_op_count: u64, override_previous_root: bool } -#[derive(Copy, Drop, Serde, starknet::Store)] +// todo: maybe use copy +#[derive(Drop, Serde, starknet::Store)] struct Op { chain_id: u256, multisig: ContractAddress, @@ -75,13 +79,69 @@ struct ExpiringRootAndOpCount { op_count: u64 } +// based of https://github.com/starkware-libs/cairo/blob/1b747da1ec7e43a6fd0c0a4cbce302616408bc72/corelib/src/starknet/eth_signature.cairo#L25 +pub fn recover_eth_ecdsa(msg_hash: u256, signature: Signature) -> Result { + if !is_signature_entry_valid::(signature.r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(signature.s) { + return Result::Err('Signature out of range'); + } + + let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); + // calculated eth address + return Result::Ok(public_key_point_to_eth_address(:public_key_point)); +} + +pub fn to_u256(address: EthAddress) -> u256 { + let temp: felt252 = address.into(); + temp.into() +} + +pub fn verify_merkle_proof(proof: Span, root: u256, leaf: u256) -> bool { + let mut computed_hash = leaf; + + let mut i = 0; + + while i < proof.len() { + computed_hash = hash_pair(computed_hash, *proof.at(i)); + i += 1; + }; + + computed_hash == root +} + +fn hash_pair(a: u256, b: u256) -> u256 { + let (lower, higher) = if a < b { + (a, b) + } else { + (b, a) + }; + BytesTrait::new_empty().encode(lower).encode(higher).keccak() +} #[starknet::contract] mod ManyChainMultiSig { - use super::{ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op}; - use starknet::{EthAddress}; + use core::dict::Felt252Dict; + use core::traits::PanicDestruct; + use super::{ + ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op, Signature, recover_eth_ecdsa, + to_u256, verify_merkle_proof + }; + use starknet::{EthAddress, EthAddressZeroable, EthAddressIntoFelt252}; + use starknet::eth_signature::is_eth_signature_valid; + use alexandria_bytes::{Bytes, BytesTrait}; + use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; + use alexandria_encoding::sol_abi::encode::SolAbiEncodeTrait; + const NUM_GROUPS: u8 = 32; const MAX_NUM_SIGNERS: u8 = 200; + // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP") + const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPERATOR_OP: u256 = + 0x08d275622006c4ca82d03f498e90163cafd53c663a48470c3b52ac8bfbd9f52c; + // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA") + const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA: u256 = + 0xe6b82be989101b4eb519770114b997b97b3c8707515286748a871717f0e4ea1c; #[storage] struct Storage { @@ -97,5 +157,148 @@ mod ManyChainMultiSig { s_expiring_root_and_op_count: ExpiringRootAndOpCount, s_root_metadata: RootMetadata } + + #[derive(Drop, starknet::Event)] + struct NewRoot { + #[key] + root: u256, + valid_until: u32, + metadata: RootMetadata, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + NewRoot: NewRoot + } + + + #[abi(embed_v0)] + impl ManyChainMultiSigImpl of super::IManyChainMultiSig { + fn set_root( + ref self: ContractState, + root: u256, + valid_until: u32, + metadata: RootMetadata, + metadata_proof: Span, + // note: v is a boolean and not uint8 + mut signatures: Array + ) { + let encoded_root: Bytes = BytesTrait::new_empty().encode(root).encode(valid_until); + + let mut eip_191_msg: Bytes = BytesTrait::new_empty(); + eip_191_msg.append_felt252('\x19Ethereum Signed Message:\n32'); + eip_191_msg.append_u256(encoded_root.keccak()); + let msg_hash = eip_191_msg.keccak(); + + assert(!self.s_seen_signed_hashes.read(msg_hash), 'signed hash already seen'); + + let prev_address = EthAddressZeroable::zero(); + let mut group_vote_counts: Felt252Dict = Default::default(); + while let Option::Some(signature) = signatures + .pop_front() { + let signer_address = match recover_eth_ecdsa(msg_hash, signature) { + Result::Ok(signer_address) => signer_address, + Result::Err(e) => panic_with_felt252(e), + }; + + assert( + to_u256(prev_address) < to_u256(signer_address), + 'signer address must increase' + ); + + let signer = self.s_signers.read(signer_address); + assert(signer.address == signer_address, 'invalid signer'); + + let mut group = signer.group; + loop { + // todo: may be unnecessary assert + assert(group < NUM_GROUPS, 'invalid group number'); + let counts = group_vote_counts.get(group.into()); + group_vote_counts.insert(group.into(), counts + 1); + if counts + 1 != self._s_config_group_quorums.read(group) { + break; + } + if group == 0 { + // reached root + break; + } + group = self._s_config_group_parents.read(group) + }; + }; + + let root_group_quorum = self._s_config_group_quorums.read(0); + assert(root_group_quorum > 0, 'missing config'); + assert(group_vote_counts.get(0) >= root_group_quorum, 'insufficient signers'); + assert( + valid_until.into() >= starknet::info::get_block_timestamp(), + 'valid until has passed' + ); + // verify metadataProof + // todo: make sure this is the right way to encode the struct + let encoded_metadata: Bytes = BytesTrait::new_empty() + .encode(MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA) + .encode(valid_until) + .encode(metadata.chain_id) + .encode(metadata.multisig) + .encode(metadata.pre_op_count) + .encode(metadata.post_op_count) + .encode(metadata.override_previous_root); + + let hashed_leaf = encoded_metadata.keccak(); + assert( + verify_merkle_proof(metadata_proof, root, hashed_leaf), 'proof verification failed' + ); + + // maybe move to beginning of function + assert( + starknet::info::get_tx_info().unbox().chain_id.into() == metadata.chain_id, + 'wrong chain id' + ); + assert( + starknet::info::get_contract_address() == metadata.multisig, + 'wrong multisig address' + ); + + let op_count = self.s_expiring_root_and_op_count.read().op_count; + let current_root_metadata = self.s_root_metadata.read(); + assert( + op_count == current_root_metadata.post_op_count + || current_root_metadata.override_previous_root, + 'expect pending operations' + ); + assert(op_count == metadata.pre_op_count, 'wrong pre-operation count'); + assert(metadata.pre_op_count <= metadata.post_op_count, 'wrong post-operation count'); + + self.s_seen_signed_hashes.write(msg_hash, true); + self + .s_expiring_root_and_op_count + .write( + ExpiringRootAndOpCount { + root: root, valid_until: valid_until, op_count: metadata.pre_op_count + } + ); + + self + .emit( + Event::NewRoot( + NewRoot { root: root, valid_until: valid_until, metadata: metadata, } + ) + ); + } + + fn execute(ref self: ContractState, op: Op, proof: Span) { + let current_expiring_root_and_op_count = self.s_expiring_root_and_op_count.read(); + + assert( + self + .s_root_metadata + .read() + .post_op_count > current_expiring_root_and_op_count + .op_count, + 'post-operation count reached' + ); + } + } } From c36a52d69785bcd07a80f72539564e7415526525 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 9 Aug 2024 10:58:13 -0400 Subject: [PATCH 03/13] update mcms --- contracts/src/mcms.cairo | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index 56b723399..1216a35a3 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -298,6 +298,13 @@ mod ManyChainMultiSig { .op_count, 'post-operation count reached' ); + + assert(starknet::info::get_tx_info().unbox().chain_id.into() == op.chain_id, 'wrong chain id'); + + assert( + starknet::info::get_contract_address() == op.multisig, + 'wrong multisig address' + ); } } } From f6208568bc75e13ceda13f25c52bcbbf2bd9584c Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 9 Aug 2024 17:14:25 -0400 Subject: [PATCH 04/13] wip --- contracts/src/mcms.cairo | 70 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 7 deletions(-) diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index 1216a35a3..a283153c5 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -55,12 +55,14 @@ struct RootMetadata { } // todo: maybe use copy +// todo: figure out how this works off-chain with MCMS since we have a new selector field here #[derive(Drop, Serde, starknet::Store)] struct Op { chain_id: u256, multisig: ContractAddress, nonce: u64, to: ContractAddress, + selector: felt252, data: ByteArray } @@ -128,7 +130,7 @@ mod ManyChainMultiSig { ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op, Signature, recover_eth_ecdsa, to_u256, verify_merkle_proof }; - use starknet::{EthAddress, EthAddressZeroable, EthAddressIntoFelt252}; + use starknet::{EthAddress, EthAddressZeroable, EthAddressIntoFelt252, ContractAddress}; use starknet::eth_signature::is_eth_signature_valid; use alexandria_bytes::{Bytes, BytesTrait}; use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; @@ -137,7 +139,7 @@ mod ManyChainMultiSig { const NUM_GROUPS: u8 = 32; const MAX_NUM_SIGNERS: u8 = 200; // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP") - const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPERATOR_OP: u256 = + const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP: u256 = 0x08d275622006c4ca82d03f498e90163cafd53c663a48470c3b52ac8bfbd9f52c; // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA") const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA: u256 = @@ -166,10 +168,20 @@ mod ManyChainMultiSig { metadata: RootMetadata, } + #[derive(Drop, starknet::Event)] + struct OpExecuted { + #[key] + nonce: u64, + to: ContractAddress, + data: ByteArray + // no value because value is sent through ERC20 tokens, even the native STRK token + } + #[event] #[derive(Drop, starknet::Event)] enum Event { - NewRoot: NewRoot + NewRoot: NewRoot, + OpExecuted: OpExecuted, } @@ -299,13 +311,57 @@ mod ManyChainMultiSig { 'post-operation count reached' ); - assert(starknet::info::get_tx_info().unbox().chain_id.into() == op.chain_id, 'wrong chain id'); - assert( - starknet::info::get_contract_address() == op.multisig, - 'wrong multisig address' + starknet::info::get_tx_info().unbox().chain_id.into() == op.chain_id, + 'wrong chain id' ); + + assert(starknet::info::get_contract_address() == op.multisig, 'wrong multisig address'); + + assert( + current_expiring_root_and_op_count + .valid_until + .into() >= starknet::info::get_block_timestamp(), + 'root has expired' + ); + + assert(op.nonce == current_expiring_root_and_op_count.op_count, 'wrong nonce'); + + // verify op exists in merkle tree + let encoded_leaf: Bytes = BytesTrait::new_empty() + .encode(MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP) + .encode(op.chain_id) + .encode(op.multisig) + .encode(op.nonce) + .encode(op.to) + .encode(op.selector) // todo: selector doesn't currently exist + .encode(op.data.clone()); + + let hashed_leaf = encoded_leaf.keccak(); + + assert( + verify_merkle_proof(proof, current_expiring_root_and_op_count.root, hashed_leaf), + 'proof verification failed' + ); + + let mut new_expiring_root_and_op_count = current_expiring_root_and_op_count; + new_expiring_root_and_op_count.op_count += 1; + + self.s_expiring_root_and_op_count.write(new_expiring_root_and_op_count); + // todo: execute + + self + .emit( + Event::OpExecuted( + OpExecuted { nonce: op.nonce, to: op.to, data: op.data.clone() } + ) + ); } } + + #[generate_trait] + impl InternalFunctions of InternalFunctionsTrait { + fn _execute(target: ContractAddress, ) + } } From 43c5b9e92a5cd8071876560c7bc8e285816df653 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Mon, 26 Aug 2024 11:59:32 -0400 Subject: [PATCH 05/13] midway --- contracts/src/mcms.cairo | 79 ++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index a283153c5..95df9625b 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -23,15 +23,15 @@ trait IManyChainMultiSig { signatures: Array ); fn execute(ref self: TContractState, op: Op, proof: Span); -// // todo: check length of group_quorums and group_parents -// fn set_config( -// ref self: TContractState, -// signer_addresses: Span, -// signer_groups: Span, -// group_quorums: Span, -// group_parents: Span, -// clear_root: bool -// ); + // // todo: check length of group_quorums and group_parents + fn set_config( + ref self: TContractState, + signer_addresses: Span, + signer_groups: Span, + group_quorums: Span, + group_parents: Span, + clear_root: bool + ); // fn get_config(self: @TContractState) -> Config; // fn get_op_count(self: @TContractState) -> u64; // fn get_root(self: @TContractState) -> (u256, u32); @@ -56,14 +56,14 @@ struct RootMetadata { // todo: maybe use copy // todo: figure out how this works off-chain with MCMS since we have a new selector field here -#[derive(Drop, Serde, starknet::Store)] +#[derive(Copy, Drop, Serde)] struct Op { chain_id: u256, multisig: ContractAddress, nonce: u64, to: ContractAddress, selector: felt252, - data: ByteArray + data: Span } // does not implement Storage trait because structs cannot support arrays or maps @@ -124,13 +124,18 @@ fn hash_pair(a: u256, b: u256) -> u256 { #[starknet::contract] mod ManyChainMultiSig { + use core::starknet::SyscallResultTrait; + use core::array::SpanTrait; use core::dict::Felt252Dict; use core::traits::PanicDestruct; use super::{ ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op, Signature, recover_eth_ecdsa, to_u256, verify_merkle_proof }; - use starknet::{EthAddress, EthAddressZeroable, EthAddressIntoFelt252, ContractAddress}; + use starknet::{ + EthAddress, EthAddressZeroable, EthAddressIntoFelt252, ContractAddress, + call_contract_syscall + }; use starknet::eth_signature::is_eth_signature_valid; use alexandria_bytes::{Bytes, BytesTrait}; use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; @@ -173,7 +178,8 @@ mod ManyChainMultiSig { #[key] nonce: u64, to: ContractAddress, - data: ByteArray + selector: felt252, + data: Span // no value because value is sent through ERC20 tokens, even the native STRK token } @@ -328,15 +334,19 @@ mod ManyChainMultiSig { assert(op.nonce == current_expiring_root_and_op_count.op_count, 'wrong nonce'); // verify op exists in merkle tree - let encoded_leaf: Bytes = BytesTrait::new_empty() + let mut encoded_leaf: Bytes = BytesTrait::new_empty() .encode(MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP) .encode(op.chain_id) .encode(op.multisig) .encode(op.nonce) .encode(op.to) - .encode(op.selector) // todo: selector doesn't currently exist - .encode(op.data.clone()); - + .encode(op.selector); + // encode the data field by looping through + let mut i = 0; + while i < op.data.len() { + encoded_leaf = encoded_leaf.encode(*op.data.at(i)); + i += 1; + }; let hashed_leaf = encoded_leaf.keccak(); assert( @@ -349,19 +359,50 @@ mod ManyChainMultiSig { self.s_expiring_root_and_op_count.write(new_expiring_root_and_op_count); // todo: execute + self._execute(op.to, op.selector, op.data); self .emit( Event::OpExecuted( - OpExecuted { nonce: op.nonce, to: op.to, data: op.data.clone() } + OpExecuted { + nonce: op.nonce, to: op.to, selector: op.selector, data: op.data + } ) ); } + + // todo: make onlyOwner + fn set_config( + ref self: ContractState, + signer_addresses: Span, + signer_groups: Span, + group_quorums: Span, + group_parents: Span, + clear_root: bool + ) { + assert( + signer_addresses.len() != 0 && signer_addresses.len() <= MAX_NUM_SIGNERS.into(), + 'out of bound signers len' + ); + + assert(signer_addresses.len() == signer_groups.len(), 'signer groups len mismatch'); + + assert( + group_quorums.len() == NUM_GROUPS.into() + && group_quorums.len() == group_parents.len(), + 'group quorums/parents mismatch' + ); + // let mut group_children_counts = + } } #[generate_trait] impl InternalFunctions of InternalFunctionsTrait { - fn _execute(target: ContractAddress, ) + fn _execute( + ref self: ContractState, target: ContractAddress, selector: felt252, data: Span + ) { + let _response = call_contract_syscall(target, selector, data).unwrap_syscall(); + } } } From f7249c6aabcf82184ac2281068c54137f66654e4 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Thu, 29 Aug 2024 10:28:48 -0400 Subject: [PATCH 06/13] in progress - set root --- contracts/src/mcms.cairo | 129 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 127 insertions(+), 2 deletions(-) diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index 95df9625b..d09dcd4a2 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -149,9 +149,11 @@ mod ManyChainMultiSig { // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA") const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA: u256 = 0xe6b82be989101b4eb519770114b997b97b3c8707515286748a871717f0e4ea1c; + const S_CONFIG_GROUP_LEN: u8 = NUM_GROUPS; #[storage] struct Storage { + // s_signers is used to easily validate the existence of the signer by its address. s_signers: LegacyMap, // begin s_config (defined in storage bc Config struct cannot support maps) _s_config_signers_len: u8, @@ -179,15 +181,22 @@ mod ManyChainMultiSig { nonce: u64, to: ContractAddress, selector: felt252, - data: Span + data: Span, // no value because value is sent through ERC20 tokens, even the native STRK token } + #[derive(Drop, starknet::Event)] + struct ConfigSet { + config: Config, + is_root_cleared: bool, + } + #[event] #[derive(Drop, starknet::Event)] enum Event { NewRoot: NewRoot, OpExecuted: OpExecuted, + ConfigSet: ConfigSet, } @@ -392,7 +401,123 @@ mod ManyChainMultiSig { && group_quorums.len() == group_parents.len(), 'group quorums/parents mismatch' ); - // let mut group_children_counts = + + let mut group_children_counts: Felt252Dict = Default::default(); + let mut i = 0; + while i < signer_groups + .len() { + let group = *signer_groups.at(i); + assert(group < NUM_GROUPS, 'out of bounds group'); + // increment count for each group + group_children_counts + .insert(group.into(), group_children_counts.get(group.into()) + 1); + i += 1; + }; + + let mut j = 0; + while j < NUM_GROUPS { + // iterate backwards: i is the group # + let i = NUM_GROUPS - 1 - j; + let group_parent = (*group_parents.at(i.into())); + + assert( + (i == 0 || group_parent < i) && (i != 0 || group_parent == 0), + 'group tree malformed' + ); + + let disabled = *group_quorums.at(i.into()) == 0; + + if disabled { + assert(group_children_counts.get(i.into()) == 0, 'signer in disabled group'); + } else { + assert( + group_children_counts.get(i.into()) >= *group_quorums.at(i.into()), + 'quorum impossible' + ); + + group_children_counts + .insert( + group_parent.into(), group_children_counts.get(group_parent.into()) + 1 + ); + // the above line clobbers group_children_counts[0] in last iteration, don't use it after the loop ends + } + j += 1; + }; + // SDFSDF:Kj + // remove any old signer addresses + let mut i: u8 = 0; + let signers_len = self._s_config_signers_len.read(); + + // create signers array (for event e) + while i < signers_len { + let mut old_signer = self._s_config_signers.read(i); + let empty_signer = Signer { + address: EthAddressZeroable::zero(), index: 0, group: 0 + }; + // reset s_signers + self.s_signers.write(old_signer.address, empty_signer); + // reset _s_config_signers + self._s_config_signers.write(i.into(), empty_signer); + }; + // reset _s_config_signers_len + self._s_config_signers_len.write(0); + + let mut i: u8 = 0; + while i < NUM_GROUPS { + self._s_config_group_quorums.write(i, *group_quorums.at(i.into())); + self._s_config_group_parents.write(i, *group_parents.at(i.into())); + i += 1; + }; + + let mut prev_signer_address = EthAddressZeroable::zero(); + let mut i: u8 = 0; + while i + .into() < signer_addresses + .len() { + let signer_address = *signer_addresses.at(i.into()); + assert( + to_u256(prev_signer_address) < to_u256(signer_address), + 'addresses not sorted' + ); + + let signer = Signer { + address: signer_address, index: i, group: *signer_groups.at(i.into()) + }; + + self.s_signers.write(signer_address, signer); + self._s_config_signers.write(i.into(), signer); + + prev_signer_address = signer_address; + i += 1; + }; + + if clear_root { + let op_count = self.s_expiring_root_and_op_count.read().op_count; + self + .s_expiring_root_and_op_count + .write(ExpiringRootAndOpCount { root: 0, valid_until: 0, op_count: op_count }); + self + .s_root_metadata + .write( + RootMetadata { + chain_id: starknet::info::get_tx_info().unbox().chain_id.into(), + multisig: starknet::info::get_contract_address(), + pre_op_count: op_count, + post_op_count: op_count, + override_previous_root: true + } + ); + } + + // self + // .emit( + // Event::ConfigSet( + // ConfigSet { config: Config { + // signers: + // }, is_root_cleared: clear_root } + // ) + // ); + } } From eae6c6a3475319c9b6683078f915da61e68268b1 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Thu, 29 Aug 2024 11:57:48 -0400 Subject: [PATCH 07/13] mcm done --- contracts/src/mcms.cairo | 78 ++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo index d09dcd4a2..3f3e8d613 100644 --- a/contracts/src/mcms.cairo +++ b/contracts/src/mcms.cairo @@ -32,10 +32,10 @@ trait IManyChainMultiSig { group_parents: Span, clear_root: bool ); -// fn get_config(self: @TContractState) -> Config; -// fn get_op_count(self: @TContractState) -> u64; -// fn get_root(self: @TContractState) -> (u256, u32); -// fn get_root_metadata(self: @TContractState) -> RootMetadata; + fn get_config(self: @TContractState) -> Config; + fn get_op_count(self: @TContractState) -> u64; + fn get_root(self: @TContractState) -> (u256, u32); + fn get_root_metadata(self: @TContractState) -> RootMetadata; } #[derive(Copy, Drop, Serde, starknet::Store)] @@ -124,6 +124,7 @@ fn hash_pair(a: u256, b: u256) -> u256 { #[starknet::contract] mod ManyChainMultiSig { + use core::array::ArrayTrait; use core::starknet::SyscallResultTrait; use core::array::SpanTrait; use core::dict::Felt252Dict; @@ -142,6 +143,7 @@ mod ManyChainMultiSig { use alexandria_encoding::sol_abi::encode::SolAbiEncodeTrait; const NUM_GROUPS: u8 = 32; + const MAX_NUM_SIGNERS: u8 = 200; // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP") const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP: u256 = @@ -149,7 +151,6 @@ mod ManyChainMultiSig { // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA") const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA: u256 = 0xe6b82be989101b4eb519770114b997b97b3c8707515286748a871717f0e4ea1c; - const S_CONFIG_GROUP_LEN: u8 = NUM_GROUPS; #[storage] struct Storage { @@ -443,12 +444,11 @@ mod ManyChainMultiSig { } j += 1; }; - // SDFSDF:Kj + // remove any old signer addresses let mut i: u8 = 0; let signers_len = self._s_config_signers_len.read(); - // create signers array (for event e) while i < signers_len { let mut old_signer = self._s_config_signers.read(i); let empty_signer = Signer { @@ -469,6 +469,8 @@ mod ManyChainMultiSig { i += 1; }; + // create signers array (for event logs) + let mut signers = ArrayTrait::::new(); let mut prev_signer_address = EthAddressZeroable::zero(); let mut i: u8 = 0; while i @@ -487,6 +489,8 @@ mod ManyChainMultiSig { self.s_signers.write(signer_address, signer); self._s_config_signers.write(i.into(), signer); + signers.append(signer); + prev_signer_address = signer_address; i += 1; }; @@ -509,15 +513,59 @@ mod ManyChainMultiSig { ); } - // self - // .emit( - // Event::ConfigSet( - // ConfigSet { config: Config { - // signers: - // }, is_root_cleared: clear_root } - // ) - // ); + self + .emit( + Event::ConfigSet( + ConfigSet { + config: Config { + signers: signers.span(), + group_quorums: group_quorums, + group_parents: group_parents, + }, + is_root_cleared: clear_root + } + ) + ); + } + + fn get_config(self: @ContractState) -> Config { + let mut signers = ArrayTrait::::new(); + + let mut i = 0; + let signers_len = self._s_config_signers_len.read(); + while i < signers_len { + let signer = self._s_config_signers.read(i); + signers.append(signer); + i += 1 + }; + + let mut group_quorums = ArrayTrait::::new(); + let mut group_parents = ArrayTrait::::new(); + + let mut i = 0; + while i < NUM_GROUPS { + group_quorums.append(self._s_config_group_quorums.read(i)); + group_parents.append(self._s_config_group_parents.read(i)); + }; + + Config { + signers: signers.span(), + group_quorums: group_quorums.span(), + group_parents: group_parents.span() + } + } + + fn get_op_count(self: @ContractState) -> u64 { + self.s_expiring_root_and_op_count.read().op_count + } + + fn get_root(self: @ContractState) -> (u256, u32) { + let current = self.s_expiring_root_and_op_count.read(); + (current.root, current.valid_until) + } + fn get_root_metadata(self: @ContractState) -> RootMetadata { + self.s_root_metadata.read() } } From 44bee96e31d0e74840aab998055291c2ef1b5586 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Thu, 29 Aug 2024 18:31:39 -0400 Subject: [PATCH 08/13] fix examples with updated foundry --- .tool-versions | 1 + contracts/Scarb.lock | 6 ++ contracts/Scarb.toml | 3 +- contracts/src/tests/test_aggregator.cairo | 2 + contracts/src/tests/test_mcms.cairo | 15 +++++ .../contracts/aggregator_consumer/Scarb.lock | 59 ++++++++++++++++++- .../contracts/aggregator_consumer/Scarb.toml | 5 +- .../tests/test_consumer.cairo | 18 +++++- .../test_price_consumer_with_sequencer.cairo | 26 +++++--- 9 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 contracts/src/tests/test_mcms.cairo diff --git a/.tool-versions b/.tool-versions index b00c44d37..e3f251c90 100644 --- a/.tool-versions +++ b/.tool-versions @@ -11,6 +11,7 @@ actionlint 1.6.12 shellcheck 0.8.0 scarb 2.6.5 postgres 15.1 +starknet-foundry 0.27.0 # Kubernetes k3d 5.4.4 diff --git a/contracts/Scarb.lock b/contracts/Scarb.lock index cfdbfc0f4..0c0ba3523 100644 --- a/contracts/Scarb.lock +++ b/contracts/Scarb.lock @@ -60,9 +60,15 @@ dependencies = [ "alexandria_bytes", "alexandria_encoding", "openzeppelin", + "snforge_std", ] [[package]] name = "openzeppelin" version = "0.10.0" source = "git+https://github.com/OpenZeppelin/cairo-contracts.git?tag=v0.10.0#d77082732daab2690ba50742ea41080eb23299d3" + +[[package]] +name = "snforge_std" +version = "0.27.0" +source = "git+https://github.com/foundry-rs/starknet-foundry.git?tag=v0.27.0#2d99b7c00678ef0363881ee0273550c44a9263de" diff --git a/contracts/Scarb.toml b/contracts/Scarb.toml index 4a89900c9..261bc54b7 100644 --- a/contracts/Scarb.toml +++ b/contracts/Scarb.toml @@ -7,6 +7,7 @@ homepage = "https://github.com/smartcontractkit/chainlink-starknet" [scripts] sierra = "cairo-compile . -r" +# test = "snforge test" # Add your own custom commands and run them with scarb run # Uncomment if you want to use dependencies @@ -16,7 +17,7 @@ starknet = ">=2.6.3" openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.10.0" } alexandria_bytes = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "bcdca70afdf59c9976148e95cebad5cf63d75a7f" } alexandria_encoding = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "bcdca70afdf59c9976148e95cebad5cf63d75a7f" } - +snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry.git", tag = "v0.27.0" } [lib] diff --git a/contracts/src/tests/test_aggregator.cairo b/contracts/src/tests/test_aggregator.cairo index e2c3d81c3..8204bfaf2 100644 --- a/contracts/src/tests/test_aggregator.cairo +++ b/contracts/src/tests/test_aggregator.cairo @@ -25,6 +25,8 @@ use chainlink::token::link_token::LinkToken; use chainlink::tests::test_ownable::should_implement_ownable; use chainlink::tests::test_access_controller::should_implement_access_control; +use snforge_std::{declare, ContractClassTrait}; + #[test] fn test_pow_2_0() { assert(pow(2, 0) == 0x1, 'expected 0x1'); diff --git a/contracts/src/tests/test_mcms.cairo b/contracts/src/tests/test_mcms.cairo new file mode 100644 index 000000000..7233889b2 --- /dev/null +++ b/contracts/src/tests/test_mcms.cairo @@ -0,0 +1,15 @@ +// set_config tests + +// 1. test if lena(signer_address) = 0 => revert +// 2. test if lena(signer_address) > MAX_NUM_SIGNERS => revert +// 3. test if signer addresses and signer groups not same size +// 4. test if group_quorum and group_parents not same size +// 5. test if one of signer_group #'s is out of bounds NUM_GROUPS +// 6. test if group_parents[i] is greater than or equal to i (when not 0) there is revert +// 7. test if i is 0 and group_parents[i] != 0 and revert +// 8. test if there is a signer in a group where group_quorum[i] == 0 => revert +// 9. test if there are not enough signers to meet a quorum => revert +// 10. test if signer addresses are not in ascending order +// 11. successful => test without clearing root. test the state of storage variables and that event was emitted + + diff --git a/examples/contracts/aggregator_consumer/Scarb.lock b/examples/contracts/aggregator_consumer/Scarb.lock index 586d04a23..5bdff8546 100644 --- a/examples/contracts/aggregator_consumer/Scarb.lock +++ b/examples/contracts/aggregator_consumer/Scarb.lock @@ -9,11 +9,66 @@ dependencies = [ "snforge_std", ] +[[package]] +name = "alexandria_bytes" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", + "alexandria_math", +] + +[[package]] +name = "alexandria_data_structures" +version = "0.2.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_encoding", +] + +[[package]] +name = "alexandria_encoding" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_bytes", + "alexandria_math", + "alexandria_numeric", +] + +[[package]] +name = "alexandria_math" +version = "0.2.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", +] + +[[package]] +name = "alexandria_numeric" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_math", + "alexandria_searching", +] + +[[package]] +name = "alexandria_searching" +version = "0.1.0" +source = "git+https://github.com/keep-starknet-strange/alexandria.git?rev=bcdca70afdf59c9976148e95cebad5cf63d75a7f#bcdca70afdf59c9976148e95cebad5cf63d75a7f" +dependencies = [ + "alexandria_data_structures", +] + [[package]] name = "chainlink" version = "0.1.0" dependencies = [ + "alexandria_bytes", + "alexandria_encoding", "openzeppelin", + "snforge_std", ] [[package]] @@ -23,5 +78,5 @@ source = "git+https://github.com/OpenZeppelin/cairo-contracts.git?tag=v0.10.0#d7 [[package]] name = "snforge_std" -version = "0.21.0" -source = "git+https://github.com/foundry-rs/starknet-foundry.git?tag=v0.21.0#2996b8c1dd66b2715fc67e69578089f278a46790" +version = "0.27.0" +source = "git+https://github.com/foundry-rs/starknet-foundry.git?tag=v0.27.0#2d99b7c00678ef0363881ee0273550c44a9263de" diff --git a/examples/contracts/aggregator_consumer/Scarb.toml b/examples/contracts/aggregator_consumer/Scarb.toml index e7868b138..8da76d784 100644 --- a/examples/contracts/aggregator_consumer/Scarb.toml +++ b/examples/contracts/aggregator_consumer/Scarb.toml @@ -8,10 +8,13 @@ name = "aggregator_consumer" version = "0.1.0" cairo-version = "2.6.3" +# [scripts] +# test = "snforge test" + # See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html [dependencies] -snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry.git", tag = "v0.21.0" } +snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry.git", tag = "v0.27.0" } chainlink = { path = "../../../contracts" } starknet = ">=2.6.3" diff --git a/examples/contracts/aggregator_consumer/tests/test_consumer.cairo b/examples/contracts/aggregator_consumer/tests/test_consumer.cairo index 31d86386c..b7afc0b70 100644 --- a/examples/contracts/aggregator_consumer/tests/test_consumer.cairo +++ b/examples/contracts/aggregator_consumer/tests/test_consumer.cairo @@ -13,13 +13,23 @@ use starknet::ContractAddress; fn deploy_mock_aggregator(decimals: u8) -> ContractAddress { let mut calldata = ArrayTrait::new(); calldata.append(decimals.into()); - return declare("MockAggregator").deploy(@calldata).unwrap(); + + let contract = declare("MockAggregator").unwrap(); + + let (contract_address, _) = contract.deploy(@calldata).unwrap(); + + contract_address } fn deploy_consumer(aggregator_address: ContractAddress) -> ContractAddress { let mut calldata = ArrayTrait::new(); calldata.append(aggregator_address.into()); - return declare("AggregatorConsumer").deploy(@calldata).unwrap(); + + let contract = declare("AggregatorConsumer").unwrap(); + + let (contract_address, _) = contract.deploy(@calldata).unwrap(); + + contract_address } #[test] @@ -79,7 +89,9 @@ fn test_set_and_read_answer() { let consumer_dispatcher = IAggregatorConsumerDispatcher { contract_address: consumer_address }; // Let's make sure the AggregatorConsumer was initialized correctly - assert(consumer_dispatcher.read_ocr_address() == mock_aggregator_address, 'Invalid OCR address'); + assert( + consumer_dispatcher.read_ocr_address() == mock_aggregator_address, 'Invalid OCR address' + ); assert(consumer_dispatcher.read_answer() == 0, 'Invalid initial answer'); // No round data has been initialized, so reading the latest round should return no data diff --git a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo index 9582c8030..9826331de 100644 --- a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo +++ b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo @@ -1,4 +1,6 @@ -use snforge_std::{declare, ContractClassTrait, start_prank, stop_prank, CheatTarget}; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; use chainlink::emergency::sequencer_uptime_feed::ISequencerUptimeFeedDispatcherTrait; use chainlink::emergency::sequencer_uptime_feed::ISequencerUptimeFeedDispatcher; @@ -17,14 +19,16 @@ use starknet::ContractAddress; fn deploy_mock_aggregator(decimals: u8) -> ContractAddress { let mut calldata = ArrayTrait::new(); calldata.append(decimals.into()); - return declare("MockAggregator").deploy(@calldata).unwrap(); + let (contract_address, _) = declare("MockAggregator").unwrap().deploy(@calldata).unwrap(); + contract_address } fn deploy_uptime_feed(initial_status: u128, owner_address: ContractAddress) -> ContractAddress { let mut calldata = ArrayTrait::new(); calldata.append(initial_status.into()); calldata.append(owner_address.into()); - return declare("SequencerUptimeFeed").deploy(@calldata).unwrap(); + let (contract_address, _) = declare("SequencerUptimeFeed").unwrap().deploy(@calldata).unwrap(); + contract_address } fn deploy_price_consumer( @@ -33,7 +37,11 @@ fn deploy_price_consumer( let mut calldata = ArrayTrait::new(); calldata.append(uptime_feed_address.into()); calldata.append(aggregator_address.into()); - return declare("AggregatorPriceConsumer").deploy(@calldata).unwrap(); + let (contract_address, _) = declare("AggregatorPriceConsumer") + .unwrap() + .deploy(@calldata) + .unwrap(); + contract_address } #[test] @@ -52,7 +60,8 @@ fn test_get_latest_price() { // Adds the price consumer contract to the sequencer uptime feed access control list // which allows the price consumer to call the get_latest_price function - start_prank(CheatTarget::All, owner); + start_cheat_caller_address_global(owner); + // start_prank(CheatTarget::All, owner); IAccessControllerDispatcher { contract_address: uptime_feed_address } .add_access(price_consumer_address); @@ -62,7 +71,8 @@ fn test_get_latest_price() { // a new round is initialized using its initial status as the round's answer, so the // latest price should be the initial status that was passed into the sequencer uptime // feed's constructor. - start_prank(CheatTarget::All, price_consumer_address); + start_cheat_caller_address_global(price_consumer_address); + // start_prank(CheatTarget::All, price_consumer_address); let latest_price = IAggregatorPriceConsumerDispatcher { contract_address: price_consumer_address } @@ -70,7 +80,7 @@ fn test_get_latest_price() { assert(latest_price == init_status, 'latest price is incorrect'); // Now let's update the round - stop_prank(CheatTarget::All); + stop_cheat_caller_address_global(); let answer = 1; let block_num = 12345; let observation_timestamp = 100000; @@ -79,7 +89,7 @@ fn test_get_latest_price() { .set_latest_round_data(answer, block_num, observation_timestamp, transmission_timestamp); // This should now return the updated answer - start_prank(CheatTarget::All, price_consumer_address); + start_cheat_caller_address_global(price_consumer_address); let updated_latest_price = IAggregatorPriceConsumerDispatcher { contract_address: price_consumer_address } From f009b2a89682207e07f2a29a4c5e8d8a8c21e836 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 30 Aug 2024 12:27:44 -0400 Subject: [PATCH 09/13] tests all pass with snforge --- .gitignore | 1 + contracts/src/libraries/mocks.cairo | 1 + .../mocks/mock_multisig_target.cairo | 16 + .../src/tests/test_access_controller.cairo | 22 +- contracts/src/tests/test_aggregator.cairo | 104 ++++-- .../src/tests/test_aggregator_proxy.cairo | 60 +++- contracts/src/tests/test_erc677.cairo | 33 +- contracts/src/tests/test_link_token.cairo | 19 +- .../src/tests/test_mock_aggregator.cairo | 7 +- contracts/src/tests/test_multisig.cairo | 336 +++++++++++------- contracts/src/tests/test_ownable.cairo | 10 +- .../tests/test_sequencer_uptime_feed.cairo | 28 +- contracts/src/tests/test_upgradeable.cairo | 23 +- .../test_price_consumer_with_sequencer.cairo | 8 +- 14 files changed, 436 insertions(+), 232 deletions(-) create mode 100644 contracts/src/libraries/mocks/mock_multisig_target.cairo diff --git a/.gitignore b/.gitignore index 1ec5b7415..bf22810e3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +.snfoundry_cache .direnv artifacts/ vendor/ diff --git a/contracts/src/libraries/mocks.cairo b/contracts/src/libraries/mocks.cairo index 9a58bab7d..a6e8a7395 100644 --- a/contracts/src/libraries/mocks.cairo +++ b/contracts/src/libraries/mocks.cairo @@ -1,2 +1,3 @@ mod mock_upgradeable; mod mock_non_upgradeable; +mod mock_multisig_target; diff --git a/contracts/src/libraries/mocks/mock_multisig_target.cairo b/contracts/src/libraries/mocks/mock_multisig_target.cairo new file mode 100644 index 000000000..686fa976d --- /dev/null +++ b/contracts/src/libraries/mocks/mock_multisig_target.cairo @@ -0,0 +1,16 @@ +#[starknet::contract] +mod MockMultisigTarget { + use array::ArrayTrait; + + #[storage] + struct Storage {} + + #[abi(per_item)] + #[generate_trait] + impl HelperImpl of HelperTrait { + #[external(v0)] + fn increment(ref self: ContractState, val1: felt252, val2: felt252) -> Array { + array![val1 + 1, val2 + 1] + } + } +} diff --git a/contracts/src/tests/test_access_controller.cairo b/contracts/src/tests/test_access_controller.cairo index 17d570978..5f20b2da2 100644 --- a/contracts/src/tests/test_access_controller.cairo +++ b/contracts/src/tests/test_access_controller.cairo @@ -19,13 +19,19 @@ use chainlink::libraries::access_control::{ IAccessController, IAccessControllerDispatcher, IAccessControllerDispatcherTrait }; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + + fn STATE() -> AccessController::ContractState { AccessController::contract_state_for_testing() } fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); account } @@ -44,10 +50,13 @@ fn test_access_control() { // Deploy access controller let calldata = array![owner.into(), // owner ]; - let (accessControllerAddr, _) = deploy_syscall( - AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (accessControllerAddr, _) = declare("AccessController").unwrap().deploy(@calldata).unwrap(); + + // let (accessControllerAddr, _) = deploy_syscall( + // AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_access_control(accessControllerAddr, owner); } @@ -62,7 +71,8 @@ fn should_implement_access_control(contract_addr: ContractAddress, owner: Contra let contract = IAccessControllerDispatcher { contract_address: contract_addr }; let acc2: ContractAddress = contract_address_const::<2222987765>(); - set_contract_address(owner); // required to call contract as owner + start_cheat_caller_address_global(owner); + // set_contract_address(owner); // required to call contract as owner // access check is enabled by default assert(!contract.has_access(acc2, array![]), 'should not have access'); diff --git a/contracts/src/tests/test_aggregator.cairo b/contracts/src/tests/test_aggregator.cairo index 8204bfaf2..cdf12e938 100644 --- a/contracts/src/tests/test_aggregator.cairo +++ b/contracts/src/tests/test_aggregator.cairo @@ -25,7 +25,9 @@ use chainlink::token::link_token::LinkToken; use chainlink::tests::test_ownable::should_implement_ownable; use chainlink::tests::test_access_controller::should_implement_access_control; -use snforge_std::{declare, ContractClassTrait}; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; #[test] fn test_pow_2_0() { @@ -80,15 +82,23 @@ fn setup() -> ( let acc1: ContractAddress = contract_address_const::<777>(); let acc2: ContractAddress = contract_address_const::<888>(); // set acc1 as default caller - set_caller_address(acc1); + start_cheat_caller_address_global(acc1); + + // set_caller_address(acc1); // deploy billing access controller let calldata = array![acc1.into(), // owner = acc1; ]; - let (billingAccessControllerAddr, _) = deploy_syscall( - AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) + + let (billingAccessControllerAddr, _) = declare("AccessController") + .unwrap() + .deploy(@calldata) .unwrap(); + + // let (billingAccessControllerAddr, _) = deploy_syscall( + // AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let billingAccessController = IAccessControllerDispatcher { contract_address: billingAccessControllerAddr }; @@ -97,10 +107,13 @@ fn setup() -> ( let calldata = array![acc1.into(), // minter = acc1; acc1.into(), // owner = acc1; ]; - let (linkTokenAddr, _) = deploy_syscall( - LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (linkTokenAddr, _) = declare("LinkToken").unwrap().deploy(@calldata).unwrap(); + + // let (linkTokenAddr, _) = deploy_syscall( + // LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let linkToken = ILinkTokenDispatcher { contract_address: linkTokenAddr }; // return accounts, billing access controller, link token @@ -120,10 +133,13 @@ fn test_ownable() { 8, // decimals 123, // description ]; - let (aggregatorAddr, _) = deploy_syscall( - Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (aggregatorAddr, _) = declare("Aggregator").unwrap().deploy(@calldata).unwrap(); + + // let (aggregatorAddr, _) = deploy_syscall( + // Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_ownable(aggregatorAddr, account); } @@ -141,10 +157,13 @@ fn test_access_control() { 8, // decimals 123, // description ]; - let (aggregatorAddr, _) = deploy_syscall( - Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (aggregatorAddr, _) = declare("Aggregator").unwrap().deploy(@calldata).unwrap(); + + // let (aggregatorAddr, _) = deploy_syscall( + // Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_access_control(aggregatorAddr, account); } @@ -171,7 +190,8 @@ fn test_set_billing_access_controller_not_owner() { ); // set billing access controller should revert if caller is not owner - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); BillingImpl::set_billing_access_controller(ref state, billingAccessController.contract_address); } @@ -198,7 +218,8 @@ fn test_set_billing_config_no_access() { gas_base: 1, gas_per_signature: 1, }; - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); BillingImpl::set_billing(ref state, config); } @@ -239,7 +260,8 @@ fn test_set_billing_config_as_acc_with_access() { let (owner, acc2, billingAccessController, _) = setup(); let mut state = STATE(); // grant acc2 access on access controller - set_contract_address(owner); + start_cheat_caller_address_global(owner); + // set_contract_address(owner); billingAccessController.add_access(acc2); Aggregator::constructor( @@ -260,7 +282,8 @@ fn test_set_billing_config_as_acc_with_access() { gas_base: 1, gas_per_signature: 1, }; - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); BillingImpl::set_billing(ref state, config); // check billing config @@ -283,9 +306,9 @@ fn test_set_payees_caller_not_owner() { ); let payees = array![PayeeConfig { transmitter: acc2, payee: acc2, },]; - // set payee should revert if caller is not owner - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); PayeeManagementImpl::set_payees(ref state, payees); } @@ -298,8 +321,8 @@ fn test_set_single_payee() { ); let payees = array![PayeeConfig { transmitter: acc2, payee: acc2, },]; - - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); } @@ -315,8 +338,8 @@ fn test_set_multiple_payees() { PayeeConfig { transmitter: acc2, payee: acc2, }, PayeeConfig { transmitter: owner, payee: owner, }, ]; - - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); } @@ -332,7 +355,8 @@ fn test_transfer_payeeship_caller_not_payee() { let transmitter = contract_address_const::<123>(); let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); } @@ -349,9 +373,11 @@ fn test_transfer_payeeship_to_self() { let transmitter = contract_address_const::<123>(); let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, acc2); } @@ -367,9 +393,11 @@ fn test_accept_payeeship_caller_not_proposed_payee() { let transmitter = contract_address_const::<123>(); let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); PayeeManagementImpl::accept_payeeship(ref state, transmitter); } @@ -385,11 +413,14 @@ fn test_transfer_and_accept_payeeship() { let transmitter = contract_address_const::<123>(); let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); - set_caller_address(acc2); + start_cheat_caller_address_global(acc2); + // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::accept_payeeship(ref state, transmitter); } // --- Payments and Withdrawals Tests --- @@ -410,7 +441,8 @@ fn test_owed_payment_no_rounds() { let transmitter = contract_address_const::<123>(); let mut payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; - set_caller_address(owner); + start_cheat_caller_address_global(owner); + // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); let owed = BillingImpl::owed_payment(@state, transmitter); diff --git a/contracts/src/tests/test_aggregator_proxy.cairo b/contracts/src/tests/test_aggregator_proxy.cairo index ef33d0f66..2a2c091d4 100644 --- a/contracts/src/tests/test_aggregator_proxy.cairo +++ b/contracts/src/tests/test_aggregator_proxy.cairo @@ -24,6 +24,11 @@ use chainlink::utils::split_felt; use chainlink::tests::test_ownable::should_implement_ownable; use chainlink::tests::test_access_controller::should_implement_access_control; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + + fn STATE() -> AggregatorProxy::ContractState { AggregatorProxy::contract_state_for_testing() } @@ -37,15 +42,22 @@ fn setup() -> ( ) { // Set account as default caller let account: ContractAddress = contract_address_const::<1>(); - set_caller_address(account); + + start_cheat_caller_address_global(account); + // set_caller_address(account); // Deploy mock aggregator 1 let mut calldata = ArrayTrait::new(); calldata.append(8); // decimals = 8 - let (mockAggregatorAddr1, _) = deploy_syscall( - MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let contract_class = declare("MockAggregator").unwrap(); + + let (mockAggregatorAddr1, _) = contract_class.deploy(@calldata).unwrap(); + + // let (mockAggregatorAddr1, _) = deploy_syscall( + // MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let mockAggregator1 = IMockAggregatorDispatcher { contract_address: mockAggregatorAddr1 }; // Deploy mock aggregator 2 @@ -53,10 +65,13 @@ fn setup() -> ( // so we need to change the decimals parameter to avoid an address conflict with mock aggregator 1 let mut calldata2 = ArrayTrait::new(); calldata2.append(10); // decimals = 10 - let (mockAggregatorAddr2, _) = deploy_syscall( - MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata2.span(), false - ) - .unwrap(); + + let (mockAggregatorAddr2, _) = contract_class.deploy(@calldata).unwrap(); + + // let (mockAggregatorAddr2, _) = deploy_syscall( + // MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata2.span(), false + // ) + // .unwrap(); let mockAggregator2 = IMockAggregatorDispatcher { contract_address: mockAggregatorAddr2 }; // Return account, mock aggregator address and mock aggregator contract @@ -69,10 +84,12 @@ fn test_ownable() { // Deploy aggregator proxy let calldata = array![account.into(), // owner = account mockAggregatorAddr.into(),]; - let (aggregatorProxyAddr, _) = deploy_syscall( - AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + let (aggregatorProxyAddr, _) = declare("AggregatorProxy").unwrap().deploy(@calldata).unwrap(); + + // let (aggregatorProxyAddr, _) = deploy_syscall( + // AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_ownable(aggregatorProxyAddr, account); } @@ -83,10 +100,13 @@ fn test_access_control() { // Deploy aggregator proxy let calldata = array![account.into(), // owner = account mockAggregatorAddr.into(),]; - let (aggregatorProxyAddr, _) = deploy_syscall( - AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (aggregatorProxyAddr, _) = declare("AggregatorProxy").unwrap().deploy(@calldata).unwrap(); + + // let (aggregatorProxyAddr, _) = deploy_syscall( + // AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_access_control(aggregatorProxyAddr, account); } @@ -133,7 +153,8 @@ fn test_query_latest_round_data_without_access() { // insert round into mock aggregator mockAggregator.set_latest_round_data(10, 1, 9, 8); // set caller to non-owner address with no read access - set_caller_address(contract_address_const::<2>()); + start_cheat_caller_address_global(contract_address_const::<2>()); + // set_caller_address(contract_address_const::<2>()); // query latest round AggregatorProxyImpl::latest_round_data(@state); } @@ -149,7 +170,8 @@ fn test_query_latest_answer_without_access() { // insert round into mock aggregator mockAggregator.set_latest_round_data(10, 1, 9, 8); // set caller to non-owner address with no read access - set_caller_address(contract_address_const::<2>()); + start_cheat_caller_address_global(contract_address_const::<2>()); + // set_caller_address(contract_address_const::<2>()); // query latest round AggregatorProxyImpl::latest_answer(@state); } diff --git a/contracts/src/tests/test_erc677.cairo b/contracts/src/tests/test_erc677.cairo index 99b9c8fbf..796bec043 100644 --- a/contracts/src/tests/test_erc677.cairo +++ b/contracts/src/tests/test_erc677.cairo @@ -16,6 +16,10 @@ use chainlink::token::mock::invalid_erc667_receiver::InvalidReceiver; use chainlink::libraries::token::erc677::ERC677Component; use chainlink::libraries::token::erc677::ERC677Component::ERC677Impl; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + #[starknet::interface] trait MockInvalidReceiver { fn set_supports(ref self: TContractState, value: bool); @@ -30,16 +34,20 @@ use chainlink::token::mock::valid_erc667_receiver::{ fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<1>(); // Set account as default caller - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); account } fn setup_valid_receiver() -> (ContractAddress, MockValidReceiverDispatcher) { let calldata = ArrayTrait::new(); - let (address, _) = deploy_syscall( - ValidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (address, _) = declare("ValidReceiver").unwrap().deploy(@calldata).unwrap(); + + // let (address, _) = deploy_syscall( + // ValidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let contract = MockValidReceiverDispatcher { contract_address: address }; (address, contract) } @@ -47,10 +55,13 @@ fn setup_valid_receiver() -> (ContractAddress, MockValidReceiverDispatcher) { fn setup_invalid_receiver() -> (ContractAddress, MockInvalidReceiverDispatcher) { let calldata = ArrayTrait::new(); - let (address, _) = deploy_syscall( - InvalidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (address, _) = declare("InvalidReceiver").unwrap().deploy(@calldata).unwrap(); + + // let (address, _) = deploy_syscall( + // InvalidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let contract = MockInvalidReceiverDispatcher { contract_address: address }; (address, contract) } @@ -83,7 +94,7 @@ fn test_valid_transfer_and_call() { } #[test] -#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] +#[should_panic] fn test_invalid_receiver_supports_interface_true() { setup(); let (receiver_address, receiver) = setup_invalid_receiver(); @@ -103,7 +114,7 @@ fn test_invalid_receiver_supports_interface_false() { #[test] -#[should_panic(expected: ('CONTRACT_NOT_DEPLOYED',))] +#[should_panic] fn test_nonexistent_receiver() { setup(); diff --git a/contracts/src/tests/test_link_token.cairo b/contracts/src/tests/test_link_token.cairo index 0c0fde3ed..4892ea93b 100644 --- a/contracts/src/tests/test_link_token.cairo +++ b/contracts/src/tests/test_link_token.cairo @@ -17,6 +17,11 @@ use chainlink::token::link_token::LinkToken::{MintableToken, UpgradeableImpl}; use openzeppelin::token::erc20::ERC20Component::{ERC20Impl, ERC20MetadataImpl}; use chainlink::tests::test_ownable::should_implement_ownable; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + + // only tests link token specific functionality // erc20 and erc677 functionality is already tested elsewhere @@ -27,7 +32,8 @@ fn STATE() -> LinkToken::ContractState { fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<1>(); // Set account as default caller - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); account } @@ -38,10 +44,13 @@ fn test_ownable() { let mut calldata = ArrayTrait::new(); calldata.append(class_hash_const::<123>().into()); // minter calldata.append(account.into()); // owner - let (linkAddr, _) = deploy_syscall( - LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (linkAddr, _) = declare("LinkToken").unwrap().deploy(@calldata).unwrap(); + + // let (linkAddr, _) = deploy_syscall( + // LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); should_implement_ownable(linkAddr, account); } diff --git a/contracts/src/tests/test_mock_aggregator.cairo b/contracts/src/tests/test_mock_aggregator.cairo index e95b7b87e..81cc8871e 100644 --- a/contracts/src/tests/test_mock_aggregator.cairo +++ b/contracts/src/tests/test_mock_aggregator.cairo @@ -4,6 +4,10 @@ use chainlink::ocr2::mocks::mock_aggregator::MockAggregator; use starknet::contract_address_const; use chainlink::ocr2::aggregator::Round; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + fn STATE() -> MockAggregator::ContractState { MockAggregator::contract_state_for_testing() } @@ -11,7 +15,8 @@ fn STATE() -> MockAggregator::ContractState { fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); // Set account as default caller - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); account } diff --git a/contracts/src/tests/test_multisig.cairo b/contracts/src/tests/test_multisig.cairo index becfe4d7d..cd32be7ae 100644 --- a/contracts/src/tests/test_multisig.cairo +++ b/contracts/src/tests/test_multisig.cairo @@ -17,23 +17,10 @@ use chainlink::multisig::Multisig; use chainlink::multisig::Multisig::{MultisigImpl, UpgradeableImpl}; use chainlink::multisig::{IMultisigDispatcher}; -#[starknet::contract] -mod MultisigTest { - use array::ArrayTrait; - - #[storage] - struct Storage {} - - #[abi(per_item)] - #[generate_trait] - impl HelperImpl of HelperTrait { - #[external(v0)] - fn increment(ref self: ContractState, val1: felt252, val2: felt252) -> Array { - array![val1 + 1, val2 + 1] - } - } -} - +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, + stop_cheat_caller_address_global, cheat_caller_address, CheatSpan +}; fn STATE() -> Multisig::ContractState { Multisig::contract_state_for_testing() @@ -123,7 +110,8 @@ fn test_submit_transaction() { let signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 1); - set_caller_address(signer); + start_cheat_caller_address_global(signer); + // set_caller_address(signer); let to = contract_address_const::<42>(); let function_selector = 10; MultisigImpl::submit_transaction( @@ -147,7 +135,8 @@ fn test_submit_transaction_not_signer() { let signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 1); - set_caller_address(contract_address_const::<3>()); + start_cheat_caller_address_global(contract_address_const::<3>()); + // set_caller_address(contract_address_const::<3>()); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -164,7 +153,8 @@ fn test_confirm_transaction() { let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -189,15 +179,16 @@ fn test_confirm_transaction_not_signer() { let not_signer = contract_address_const::<2>(); let signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 1); - set_caller_address(signer); + start_cheat_caller_address_global(signer); + // set_caller_address(signer); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), ); - - set_caller_address(not_signer); + start_cheat_caller_address_global(not_signer); + // set_caller_address(not_signer); MultisigImpl::confirm_transaction(ref state, nonce: 0); } @@ -208,7 +199,8 @@ fn test_revoke_confirmation() { let signer2 = contract_address_const::<2>(); let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -237,7 +229,8 @@ fn test_revoke_confirmation_not_signer() { let not_signer = contract_address_const::<2>(); let mut signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer); + start_cheat_caller_address_global(signer); + // set_caller_address(signer); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -245,8 +238,8 @@ fn test_revoke_confirmation_not_signer() { calldata: sample_calldata(), ); MultisigImpl::confirm_transaction(ref state, nonce: 0); - - set_caller_address(not_signer); + start_cheat_caller_address_global(not_signer); + // set_caller_address(not_signer); MultisigImpl::revoke_confirmation(ref state, nonce: 0); } @@ -257,8 +250,10 @@ fn test_execute_confirmation_below_threshold() { let signer1 = contract_address_const::<1>(); let signer2 = contract_address_const::<2>(); let signers = array![signer1, signer2]; + Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -274,7 +269,8 @@ fn test_execute_confirmation_below_threshold() { fn test_upgrade_not_multisig() { let mut state = STATE(); let account = contract_address_const::<777>(); - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); UpgradeableImpl::upgrade(ref state, class_hash_const::<1>()) } @@ -286,11 +282,18 @@ fn test_execute() { let signer2 = contract_address_const::<2>(); let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); - let (test_address, _) = deploy_syscall( - MultisigTest::TEST_CLASS_HASH.try_into().unwrap(), 0, ArrayTrait::new().span(), false - ) - .unwrap(); - set_caller_address(signer1); + + let calldata = ArrayTrait::new(); + + let (test_address, _) = declare("MockMultisigTarget").unwrap().deploy(@calldata).unwrap(); + + // let (test_address, _) = deploy_syscall( + // MultisigTest::TEST_CLASS_HASH.try_into().unwrap(), 0, ArrayTrait::new().span(), false + // ) + // .unwrap(); + + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); let increment_calldata = array![42, 100]; MultisigImpl::submit_transaction( ref state, @@ -300,7 +303,8 @@ fn test_execute() { calldata: increment_calldata, ); MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(signer2); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); MultisigImpl::confirm_transaction(ref state, nonce: 0); let response = MultisigImpl::execute_transaction(ref state, nonce: 0); @@ -318,7 +322,8 @@ fn test_execute_not_signer() { let signer2 = contract_address_const::<2>(); let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -326,96 +331,122 @@ fn test_execute_not_signer() { calldata: sample_calldata(), ); MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(signer2); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(contract_address_const::<3>()); + start_cheat_caller_address_global(contract_address_const::<3>()); + // set_caller_address(contract_address_const::<3>()); MultisigImpl::execute_transaction(ref state, nonce: 0); } #[test] #[should_panic(expected: ('transaction invalid',))] fn test_execute_after_set_signers() { - let mut state = STATE(); - let contract_address = contract_address_const::<100>(); - set_contract_address(contract_address); let signer1 = contract_address_const::<1>(); let signer2 = contract_address_const::<2>(); let signer3 = contract_address_const::<3>(); let signers = array![signer1, signer2]; - Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); - MultisigImpl::submit_transaction( - ref state, - to: contract_address_const::<42>(), - function_selector: 10, - calldata: sample_calldata(), - ); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(signer2); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(contract_address); - let new_signers = array![signer2, signer3]; - MultisigImpl::set_signers(ref state, new_signers); + let init_threshold: usize = 2; - set_caller_address(signer2); - MultisigImpl::execute_transaction(ref state, nonce: 0); + let mut deploy_calldata = ArrayTrait::new(); + Serde::serialize(@signers, ref deploy_calldata); + Serde::serialize(@init_threshold, ref deploy_calldata); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + let multisig = IMultisigDispatcher { contract_address: multisig_address }; + + // Multisig::constructor(ref state, :signers, threshold: 2); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); + multisig + .submit_transaction( + to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), + ); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(multisig_address); + // set_caller_address(contract_address); + let new_signers = array![signer2, signer3]; + multisig.set_signers(new_signers); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); + multisig.execute_transaction(nonce: 0); } #[test] #[should_panic(expected: ('transaction invalid',))] fn test_execute_after_set_signers_and_threshold() { - let mut state = STATE(); - let contract_address = contract_address_const::<100>(); - set_contract_address(contract_address); + // set_contract_address(contract_address); let signer1 = contract_address_const::<1>(); let signer2 = contract_address_const::<2>(); let signer3 = contract_address_const::<3>(); let signers = array![signer1, signer2]; - Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); - MultisigImpl::submit_transaction( - ref state, - to: contract_address_const::<42>(), - function_selector: 10, - calldata: sample_calldata(), - ); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(signer2); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(contract_address); - let new_signers = array![signer2, signer3]; - MultisigImpl::set_signers_and_threshold(ref state, new_signers, 1); + let init_threshold: usize = 2; - set_caller_address(signer2); - MultisigImpl::execute_transaction(ref state, nonce: 0); + let mut deploy_calldata = ArrayTrait::new(); + Serde::serialize(@signers, ref deploy_calldata); + Serde::serialize(@init_threshold, ref deploy_calldata); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + let multisig = IMultisigDispatcher { contract_address: multisig_address }; + + // Multisig::constructor(ref state, :signers, threshold: 2); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); + multisig + .submit_transaction( + to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), + ); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(multisig_address); + // set_caller_address(contract_address); + let new_signers = array![signer2, signer3]; + multisig.set_signers_and_threshold(new_signers, 1); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); + multisig.execute_transaction(nonce: 0); } #[test] #[should_panic(expected: ('transaction invalid',))] fn test_execute_after_set_threshold() { - let mut state = STATE(); - let contract_address = contract_address_const::<100>(); - set_contract_address(contract_address); let signer1 = contract_address_const::<1>(); let signer2 = contract_address_const::<2>(); let signers = array![signer1, signer2]; - Multisig::constructor(ref state, :signers, threshold: 2); - set_caller_address(signer1); - MultisigImpl::submit_transaction( - ref state, - to: contract_address_const::<42>(), - function_selector: 10, - calldata: sample_calldata(), - ); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(signer2); - MultisigImpl::confirm_transaction(ref state, nonce: 0); - set_caller_address(contract_address); - MultisigImpl::set_threshold(ref state, 1); + let init_threshold: usize = 2; - set_caller_address(signer1); - MultisigImpl::execute_transaction(ref state, nonce: 0); + let mut deploy_calldata = ArrayTrait::new(); + Serde::serialize(@signers, ref deploy_calldata); + Serde::serialize(@init_threshold, ref deploy_calldata); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + let multisig = IMultisigDispatcher { contract_address: multisig_address }; + + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); + multisig + .submit_transaction( + to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), + ); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(signer2); + // set_caller_address(signer2); + multisig.confirm_transaction(nonce: 0); + start_cheat_caller_address_global(multisig_address); + // set_caller_address(contract_address); + multisig.set_threshold(1); + start_cheat_caller_address_global(signer1); + // set_caller_address(signer1); + multisig.execute_transaction(nonce: 0); } // test set_threshold (non-recursive) @@ -431,14 +462,18 @@ fn test_set_threshold() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@signers, ref deploy_calldata); Serde::serialize(@init_threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); let multisig = IMultisigDispatcher { contract_address: multisig_address }; assert(multisig.get_threshold() == init_threshold, 'invalid init threshold'); - set_contract_address(multisig_address); + start_cheat_caller_address_global(multisig_address); + // set_contract_address(multisig_address); multisig.set_threshold(new_threshold); assert(multisig.get_threshold() == new_threshold, 'threshold was not updated'); } @@ -457,10 +492,13 @@ fn test_recursive_set_threshold() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@signers, ref deploy_calldata); Serde::serialize(@init_threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -472,19 +510,26 @@ fn test_recursive_set_threshold() { // contract. let mut set_threshold_calldata = ArrayTrait::new(); Serde::serialize(@new_threshold, ref set_threshold_calldata); - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig .submit_transaction(multisig_address, selector!("set_threshold"), set_threshold_calldata); // Signer 1 confirms the transaction - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction - set_contract_address(s2); + start_cheat_caller_address_global(s2); + // set_contract_address(s2); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction - set_contract_address(s1); + // cheat only once because we want the multisig to execute the recursive tx + cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); + // start_cheat_caller_address_global(s1); + // set_contract_address(s1); + multisig.execute_transaction(0); // Now we check that the threshold was actually updated @@ -503,10 +548,13 @@ fn test_set_signers() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@init_signers, ref deploy_calldata); Serde::serialize(@threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -516,7 +564,8 @@ fn test_set_signers() { assert(*returned_signers.at(1) == s2, 'should match signer 2'); assert(multisig.get_threshold() == 2, 'wrong init threshold'); - set_contract_address(multisig_address); + start_cheat_caller_address_global(multisig_address); + // set_contract_address(multisig_address); multisig.set_signers(new_signers); let updated_signers = multisig.get_signers(); @@ -539,10 +588,13 @@ fn test_recursive_set_signers() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@init_signers, ref deploy_calldata); Serde::serialize(@init_threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -559,19 +611,23 @@ fn test_recursive_set_signers() { // contract. let mut set_signers_calldata = ArrayTrait::new(); Serde::serialize(@new_signers, ref set_signers_calldata); - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig.submit_transaction(multisig_address, selector!("set_signers"), set_signers_calldata); // Signer 1 confirms the transaction - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction - set_contract_address(s2); + start_cheat_caller_address_global(s2); + // set_contract_address(s2); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction - set_contract_address(s1); + cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); + // set_contract_address(s1); multisig.execute_transaction(0); // Now we check that the signers were actually updated @@ -595,10 +651,13 @@ fn test_set_signers_and_threshold() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@init_signers, ref deploy_calldata); Serde::serialize(@init_threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -609,7 +668,8 @@ fn test_set_signers_and_threshold() { assert(*returned_signers.at(2) == s3, 'should match signer 3'); assert(multisig.get_threshold() == init_threshold, 'wrong init threshold'); - set_contract_address(multisig_address); + start_cheat_caller_address_global(multisig_address); + // set_contract_address(multisig_address); multisig.set_signers_and_threshold(new_signers, new_threshold); let updated_signers = multisig.get_signers(); @@ -635,10 +695,13 @@ fn test_recursive_set_signers_and_threshold() { let mut deploy_calldata = ArrayTrait::new(); Serde::serialize(@init_signers, ref deploy_calldata); Serde::serialize(@init_threshold, ref deploy_calldata); - let (multisig_address, _) = deploy_syscall( - Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - ) - .unwrap(); + + let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); + + // let (multisig_address, _) = deploy_syscall( + // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false + // ) + // .unwrap(); // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -657,7 +720,8 @@ fn test_recursive_set_signers_and_threshold() { let mut set_signers_and_threshold_calldata = ArrayTrait::new(); Serde::serialize(@new_signers, ref set_signers_and_threshold_calldata); Serde::serialize(@new_threshold, ref set_signers_and_threshold_calldata); - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig .submit_transaction( multisig_address, @@ -666,19 +730,23 @@ fn test_recursive_set_signers_and_threshold() { ); // Signer 1 confirms the transaction - set_contract_address(s1); + start_cheat_caller_address_global(s1); + // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction - set_contract_address(s2); + start_cheat_caller_address_global(s2); + // set_contract_address(s2); multisig.confirm_transaction(0); // Signer 3 confirms the transaction - set_contract_address(s3); + start_cheat_caller_address_global(s3); + // set_contract_address(s3); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction - set_contract_address(s1); + cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); + // set_contract_address(s1); multisig.execute_transaction(0); // Now we check that the signers were actually updated diff --git a/contracts/src/tests/test_ownable.cairo b/contracts/src/tests/test_ownable.cairo index 40e86c1c5..a651f4372 100644 --- a/contracts/src/tests/test_ownable.cairo +++ b/contracts/src/tests/test_ownable.cairo @@ -8,6 +8,10 @@ use openzeppelin::access::ownable::interface::{ IOwnableTwoStep, IOwnableTwoStepDispatcher, IOwnableTwoStepDispatcherTrait }; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + // // General ownable contract tests // @@ -20,13 +24,15 @@ fn should_implement_ownable(contract_addr: ContractAddress, owner: ContractAddre assert(owner == contract.owner(), 'owner does not match'); // transfer ownership - check owner unchanged and proposed owner set correctly - set_contract_address(owner); // required to call contract as owner + start_cheat_caller_address_global(owner); + // set_contract_address(owner); // required to call contract as owner contract.transfer_ownership(acc2); assert(owner == contract.owner(), 'owner should remain unchanged'); assert(acc2 == contract.pending_owner(), 'acc2 should be proposed owner'); // accept ownership - check owner changed and proposed owner set to zero - set_contract_address(acc2); // required to call function as acc2 + start_cheat_caller_address_global(acc2); + // set_contract_address(acc2); // required to call function as acc2 contract.accept_ownership(); assert(contract.owner() == acc2, 'failed to change ownership'); assert(contract.pending_owner().is_zero(), 'proposed owner should be zero'); diff --git a/contracts/src/tests/test_sequencer_uptime_feed.cairo b/contracts/src/tests/test_sequencer_uptime_feed.cairo index 51dabbd25..a3bea486e 100644 --- a/contracts/src/tests/test_sequencer_uptime_feed.cairo +++ b/contracts/src/tests/test_sequencer_uptime_feed.cairo @@ -29,6 +29,11 @@ use chainlink::emergency::sequencer_uptime_feed::{ ISequencerUptimeFeed, ISequencerUptimeFeedDispatcher, ISequencerUptimeFeedDispatcherTrait }; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + + fn PROXY() -> AggregatorProxy::ContractState { AggregatorProxy::contract_state_for_testing() } @@ -39,16 +44,21 @@ fn STATE() -> SequencerUptimeFeed::ContractState { fn setup() -> (ContractAddress, ContractAddress, ISequencerUptimeFeedDispatcher) { let account: ContractAddress = contract_address_const::<777>(); - set_caller_address(account); + + start_cheat_caller_address_global(account); + // set_caller_address(account); // Deploy seqeuencer uptime feed let calldata = array![0, // initial status account.into() // owner ]; - let (sequencerFeedAddr, _) = deploy_syscall( - SequencerUptimeFeed::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (sequencerFeedAddr, _) = declare("SequencerUptimeFeed").unwrap().deploy(@calldata).unwrap(); + + // let (sequencerFeedAddr, _) = deploy_syscall( + // SequencerUptimeFeed::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let sequencerUptimeFeed = ISequencerUptimeFeedDispatcher { contract_address: sequencerFeedAddr }; @@ -72,13 +82,15 @@ fn test_access_control() { #[should_panic()] fn test_set_l1_sender_not_owner() { let (_, _, sequencerUptimeFeed) = setup(); + start_cheat_caller_address_global(contract_address_const::<111>()); sequencerUptimeFeed.set_l1_sender(EthAddress { address: 789 }); } #[test] fn test_set_l1_sender() { let (owner, _, sequencerUptimeFeed) = setup(); - set_contract_address(owner); + start_cheat_caller_address_global(owner); + // set_contract_address(owner); sequencerUptimeFeed.set_l1_sender(EthAddress { address: 789 }); assert(sequencerUptimeFeed.l1_sender().address == 789, 'l1_sender should be set to 789'); } @@ -104,8 +116,8 @@ fn test_latest_answer_no_access() { #[test] fn test_aggregator_proxy_response() { let (owner, sequencerFeedAddr, _) = setup(); - - set_contract_address(owner); + start_cheat_caller_address_global(owner); + // set_contract_address(owner); let contract = IAccessControllerDispatcher { contract_address: sequencerFeedAddr }; contract.add_access(owner); diff --git a/contracts/src/tests/test_upgradeable.cairo b/contracts/src/tests/test_upgradeable.cairo index 4d4213573..1de5e755b 100644 --- a/contracts/src/tests/test_upgradeable.cairo +++ b/contracts/src/tests/test_upgradeable.cairo @@ -16,9 +16,15 @@ use chainlink::libraries::mocks::mock_non_upgradeable::{ IMockNonUpgradeableDispatcherImpl }; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + + fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); - set_caller_address(account); + start_cheat_caller_address_global(account); + // set_caller_address(account); account } @@ -27,14 +33,19 @@ fn test_upgrade_and_call() { let _ = setup(); let calldata = array![]; - let (contractAddr, _) = deploy_syscall( - MockUpgradeable::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - ) - .unwrap(); + + let (contractAddr, _) = declare("MockUpgradeable").unwrap().deploy(@calldata).unwrap(); + + // let (contractAddr, _) = deploy_syscall( + // MockUpgradeable::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false + // ) + // .unwrap(); let mockUpgradeable = IMockUpgradeableDispatcher { contract_address: contractAddr }; assert(mockUpgradeable.foo() == true, 'should call foo'); - mockUpgradeable.upgrade(MockNonUpgradeable::TEST_CLASS_HASH.try_into().unwrap()); + let contract_class = declare("MockNonUpgradeable").unwrap(); + + mockUpgradeable.upgrade(contract_class.class_hash); // now, contract should be different let mockNonUpgradeable = IMockNonUpgradeableDispatcher { contract_address: contractAddr }; diff --git a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo index 9826331de..91874ba2e 100644 --- a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo +++ b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo @@ -1,7 +1,3 @@ -use snforge_std::{ - declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global -}; - use chainlink::emergency::sequencer_uptime_feed::ISequencerUptimeFeedDispatcherTrait; use chainlink::emergency::sequencer_uptime_feed::ISequencerUptimeFeedDispatcher; use chainlink::libraries::access_control::IAccessControllerDispatcherTrait; @@ -16,6 +12,10 @@ use starknet::contract_address_const; use starknet::get_caller_address; use starknet::ContractAddress; +use snforge_std::{ + declare, ContractClassTrait, start_cheat_caller_address_global, stop_cheat_caller_address_global +}; + fn deploy_mock_aggregator(decimals: u8) -> ContractAddress { let mut calldata = ArrayTrait::new(); calldata.append(decimals.into()); From 8345c2895c71b7f2e56bcaf5a7b28cc10dc3774f Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 30 Aug 2024 12:35:10 -0400 Subject: [PATCH 10/13] remove dead comments --- .../src/tests/test_access_controller.cairo | 7 -- contracts/src/tests/test_aggregator.cairo | 31 ------- .../src/tests/test_aggregator_proxy.cairo | 21 ----- contracts/src/tests/test_erc677.cairo | 9 -- contracts/src/tests/test_link_token.cairo | 6 -- .../src/tests/test_mock_aggregator.cairo | 1 - contracts/src/tests/test_multisig.cairo | 83 ------------------- contracts/src/tests/test_ownable.cairo | 2 - .../tests/test_sequencer_uptime_feed.cairo | 7 -- contracts/src/tests/test_upgradeable.cairo | 5 -- 10 files changed, 172 deletions(-) diff --git a/contracts/src/tests/test_access_controller.cairo b/contracts/src/tests/test_access_controller.cairo index 5f20b2da2..a78715cf4 100644 --- a/contracts/src/tests/test_access_controller.cairo +++ b/contracts/src/tests/test_access_controller.cairo @@ -31,7 +31,6 @@ fn STATE() -> AccessController::ContractState { fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); start_cheat_caller_address_global(account); - // set_caller_address(account); account } @@ -53,11 +52,6 @@ fn test_access_control() { let (accessControllerAddr, _) = declare("AccessController").unwrap().deploy(@calldata).unwrap(); - // let (accessControllerAddr, _) = deploy_syscall( - // AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); - should_implement_access_control(accessControllerAddr, owner); } @@ -72,7 +66,6 @@ fn should_implement_access_control(contract_addr: ContractAddress, owner: Contra let acc2: ContractAddress = contract_address_const::<2222987765>(); start_cheat_caller_address_global(owner); - // set_contract_address(owner); // required to call contract as owner // access check is enabled by default assert(!contract.has_access(acc2, array![]), 'should not have access'); diff --git a/contracts/src/tests/test_aggregator.cairo b/contracts/src/tests/test_aggregator.cairo index cdf12e938..77719d6f9 100644 --- a/contracts/src/tests/test_aggregator.cairo +++ b/contracts/src/tests/test_aggregator.cairo @@ -84,8 +84,6 @@ fn setup() -> ( // set acc1 as default caller start_cheat_caller_address_global(acc1); - // set_caller_address(acc1); - // deploy billing access controller let calldata = array![acc1.into(), // owner = acc1; ]; @@ -95,10 +93,6 @@ fn setup() -> ( .deploy(@calldata) .unwrap(); - // let (billingAccessControllerAddr, _) = deploy_syscall( - // AccessController::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let billingAccessController = IAccessControllerDispatcher { contract_address: billingAccessControllerAddr }; @@ -110,10 +104,6 @@ fn setup() -> ( let (linkTokenAddr, _) = declare("LinkToken").unwrap().deploy(@calldata).unwrap(); - // let (linkTokenAddr, _) = deploy_syscall( - // LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let linkToken = ILinkTokenDispatcher { contract_address: linkTokenAddr }; // return accounts, billing access controller, link token @@ -136,11 +126,6 @@ fn test_ownable() { let (aggregatorAddr, _) = declare("Aggregator").unwrap().deploy(@calldata).unwrap(); - // let (aggregatorAddr, _) = deploy_syscall( - // Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); - should_implement_ownable(aggregatorAddr, account); } @@ -191,7 +176,6 @@ fn test_set_billing_access_controller_not_owner() { // set billing access controller should revert if caller is not owner start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); BillingImpl::set_billing_access_controller(ref state, billingAccessController.contract_address); } @@ -219,7 +203,6 @@ fn test_set_billing_config_no_access() { gas_per_signature: 1, }; start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); BillingImpl::set_billing(ref state, config); } @@ -261,7 +244,6 @@ fn test_set_billing_config_as_acc_with_access() { let mut state = STATE(); // grant acc2 access on access controller start_cheat_caller_address_global(owner); - // set_contract_address(owner); billingAccessController.add_access(acc2); Aggregator::constructor( @@ -283,7 +265,6 @@ fn test_set_billing_config_as_acc_with_access() { gas_per_signature: 1, }; start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); BillingImpl::set_billing(ref state, config); // check billing config @@ -308,7 +289,6 @@ fn test_set_payees_caller_not_owner() { let payees = array![PayeeConfig { transmitter: acc2, payee: acc2, },]; // set payee should revert if caller is not owner start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); PayeeManagementImpl::set_payees(ref state, payees); } @@ -322,7 +302,6 @@ fn test_set_single_payee() { let payees = array![PayeeConfig { transmitter: acc2, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); } @@ -339,7 +318,6 @@ fn test_set_multiple_payees() { PayeeConfig { transmitter: owner, payee: owner, }, ]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); } @@ -356,7 +334,6 @@ fn test_transfer_payeeship_caller_not_payee() { let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); } @@ -374,10 +351,8 @@ fn test_transfer_payeeship_to_self() { let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, acc2); } @@ -394,10 +369,8 @@ fn test_accept_payeeship_caller_not_proposed_payee() { let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); PayeeManagementImpl::accept_payeeship(ref state, transmitter); } @@ -414,13 +387,10 @@ fn test_transfer_and_accept_payeeship() { let payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); start_cheat_caller_address_global(acc2); - // set_caller_address(acc2); PayeeManagementImpl::transfer_payeeship(ref state, transmitter, owner); start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::accept_payeeship(ref state, transmitter); } // --- Payments and Withdrawals Tests --- @@ -442,7 +412,6 @@ fn test_owed_payment_no_rounds() { let mut payees = array![PayeeConfig { transmitter: transmitter, payee: acc2, },]; start_cheat_caller_address_global(owner); - // set_caller_address(owner); PayeeManagementImpl::set_payees(ref state, payees); let owed = BillingImpl::owed_payment(@state, transmitter); diff --git a/contracts/src/tests/test_aggregator_proxy.cairo b/contracts/src/tests/test_aggregator_proxy.cairo index 2a2c091d4..eb7e350bc 100644 --- a/contracts/src/tests/test_aggregator_proxy.cairo +++ b/contracts/src/tests/test_aggregator_proxy.cairo @@ -44,7 +44,6 @@ fn setup() -> ( let account: ContractAddress = contract_address_const::<1>(); start_cheat_caller_address_global(account); - // set_caller_address(account); // Deploy mock aggregator 1 let mut calldata = ArrayTrait::new(); @@ -54,10 +53,6 @@ fn setup() -> ( let (mockAggregatorAddr1, _) = contract_class.deploy(@calldata).unwrap(); - // let (mockAggregatorAddr1, _) = deploy_syscall( - // MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let mockAggregator1 = IMockAggregatorDispatcher { contract_address: mockAggregatorAddr1 }; // Deploy mock aggregator 2 @@ -68,10 +63,6 @@ fn setup() -> ( let (mockAggregatorAddr2, _) = contract_class.deploy(@calldata).unwrap(); - // let (mockAggregatorAddr2, _) = deploy_syscall( - // MockAggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata2.span(), false - // ) - // .unwrap(); let mockAggregator2 = IMockAggregatorDispatcher { contract_address: mockAggregatorAddr2 }; // Return account, mock aggregator address and mock aggregator contract @@ -86,11 +77,6 @@ fn test_ownable() { mockAggregatorAddr.into(),]; let (aggregatorProxyAddr, _) = declare("AggregatorProxy").unwrap().deploy(@calldata).unwrap(); - // let (aggregatorProxyAddr, _) = deploy_syscall( - // AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); - should_implement_ownable(aggregatorProxyAddr, account); } @@ -103,11 +89,6 @@ fn test_access_control() { let (aggregatorProxyAddr, _) = declare("AggregatorProxy").unwrap().deploy(@calldata).unwrap(); - // let (aggregatorProxyAddr, _) = deploy_syscall( - // AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); - should_implement_access_control(aggregatorProxyAddr, account); } @@ -154,7 +135,6 @@ fn test_query_latest_round_data_without_access() { mockAggregator.set_latest_round_data(10, 1, 9, 8); // set caller to non-owner address with no read access start_cheat_caller_address_global(contract_address_const::<2>()); - // set_caller_address(contract_address_const::<2>()); // query latest round AggregatorProxyImpl::latest_round_data(@state); } @@ -171,7 +151,6 @@ fn test_query_latest_answer_without_access() { mockAggregator.set_latest_round_data(10, 1, 9, 8); // set caller to non-owner address with no read access start_cheat_caller_address_global(contract_address_const::<2>()); - // set_caller_address(contract_address_const::<2>()); // query latest round AggregatorProxyImpl::latest_answer(@state); } diff --git a/contracts/src/tests/test_erc677.cairo b/contracts/src/tests/test_erc677.cairo index 796bec043..b81394a21 100644 --- a/contracts/src/tests/test_erc677.cairo +++ b/contracts/src/tests/test_erc677.cairo @@ -35,7 +35,6 @@ fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<1>(); // Set account as default caller start_cheat_caller_address_global(account); - // set_caller_address(account); account } @@ -44,10 +43,6 @@ fn setup_valid_receiver() -> (ContractAddress, MockValidReceiverDispatcher) { let (address, _) = declare("ValidReceiver").unwrap().deploy(@calldata).unwrap(); - // let (address, _) = deploy_syscall( - // ValidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let contract = MockValidReceiverDispatcher { contract_address: address }; (address, contract) } @@ -58,10 +53,6 @@ fn setup_invalid_receiver() -> (ContractAddress, MockInvalidReceiverDispatcher) let (address, _) = declare("InvalidReceiver").unwrap().deploy(@calldata).unwrap(); - // let (address, _) = deploy_syscall( - // InvalidReceiver::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let contract = MockInvalidReceiverDispatcher { contract_address: address }; (address, contract) } diff --git a/contracts/src/tests/test_link_token.cairo b/contracts/src/tests/test_link_token.cairo index 4892ea93b..1ee0f9946 100644 --- a/contracts/src/tests/test_link_token.cairo +++ b/contracts/src/tests/test_link_token.cairo @@ -33,7 +33,6 @@ fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<1>(); // Set account as default caller start_cheat_caller_address_global(account); - // set_caller_address(account); account } @@ -47,11 +46,6 @@ fn test_ownable() { let (linkAddr, _) = declare("LinkToken").unwrap().deploy(@calldata).unwrap(); - // let (linkAddr, _) = deploy_syscall( - // LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); - should_implement_ownable(linkAddr, account); } diff --git a/contracts/src/tests/test_mock_aggregator.cairo b/contracts/src/tests/test_mock_aggregator.cairo index 81cc8871e..af3387415 100644 --- a/contracts/src/tests/test_mock_aggregator.cairo +++ b/contracts/src/tests/test_mock_aggregator.cairo @@ -16,7 +16,6 @@ fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); // Set account as default caller start_cheat_caller_address_global(account); - // set_caller_address(account); account } diff --git a/contracts/src/tests/test_multisig.cairo b/contracts/src/tests/test_multisig.cairo index cd32be7ae..77eeb17b7 100644 --- a/contracts/src/tests/test_multisig.cairo +++ b/contracts/src/tests/test_multisig.cairo @@ -111,7 +111,6 @@ fn test_submit_transaction() { Multisig::constructor(ref state, :signers, threshold: 1); start_cheat_caller_address_global(signer); - // set_caller_address(signer); let to = contract_address_const::<42>(); let function_selector = 10; MultisigImpl::submit_transaction( @@ -136,7 +135,6 @@ fn test_submit_transaction_not_signer() { Multisig::constructor(ref state, :signers, threshold: 1); start_cheat_caller_address_global(contract_address_const::<3>()); - // set_caller_address(contract_address_const::<3>()); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -154,7 +152,6 @@ fn test_confirm_transaction() { Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -180,7 +177,6 @@ fn test_confirm_transaction_not_signer() { let signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 1); start_cheat_caller_address_global(signer); - // set_caller_address(signer); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -188,7 +184,6 @@ fn test_confirm_transaction_not_signer() { calldata: sample_calldata(), ); start_cheat_caller_address_global(not_signer); - // set_caller_address(not_signer); MultisigImpl::confirm_transaction(ref state, nonce: 0); } @@ -200,7 +195,6 @@ fn test_revoke_confirmation() { let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -230,7 +224,6 @@ fn test_revoke_confirmation_not_signer() { let mut signers = array![signer]; Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer); - // set_caller_address(signer); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -239,7 +232,6 @@ fn test_revoke_confirmation_not_signer() { ); MultisigImpl::confirm_transaction(ref state, nonce: 0); start_cheat_caller_address_global(not_signer); - // set_caller_address(not_signer); MultisigImpl::revoke_confirmation(ref state, nonce: 0); } @@ -253,7 +245,6 @@ fn test_execute_confirmation_below_threshold() { Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -270,8 +261,6 @@ fn test_upgrade_not_multisig() { let mut state = STATE(); let account = contract_address_const::<777>(); start_cheat_caller_address_global(account); - // set_caller_address(account); - UpgradeableImpl::upgrade(ref state, class_hash_const::<1>()) } @@ -287,13 +276,7 @@ fn test_execute() { let (test_address, _) = declare("MockMultisigTarget").unwrap().deploy(@calldata).unwrap(); - // let (test_address, _) = deploy_syscall( - // MultisigTest::TEST_CLASS_HASH.try_into().unwrap(), 0, ArrayTrait::new().span(), false - // ) - // .unwrap(); - start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); let increment_calldata = array![42, 100]; MultisigImpl::submit_transaction( ref state, @@ -304,7 +287,6 @@ fn test_execute() { ); MultisigImpl::confirm_transaction(ref state, nonce: 0); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); MultisigImpl::confirm_transaction(ref state, nonce: 0); let response = MultisigImpl::execute_transaction(ref state, nonce: 0); @@ -323,7 +305,6 @@ fn test_execute_not_signer() { let signers = array![signer1, signer2]; Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); MultisigImpl::submit_transaction( ref state, to: contract_address_const::<42>(), @@ -332,11 +313,8 @@ fn test_execute_not_signer() { ); MultisigImpl::confirm_transaction(ref state, nonce: 0); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); MultisigImpl::confirm_transaction(ref state, nonce: 0); - start_cheat_caller_address_global(contract_address_const::<3>()); - // set_caller_address(contract_address_const::<3>()); MultisigImpl::execute_transaction(ref state, nonce: 0); } @@ -357,30 +335,24 @@ fn test_execute_after_set_signers() { let multisig = IMultisigDispatcher { contract_address: multisig_address }; - // Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); multisig .submit_transaction( to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), ); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(multisig_address); - // set_caller_address(contract_address); let new_signers = array![signer2, signer3]; multisig.set_signers(new_signers); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); multisig.execute_transaction(nonce: 0); } #[test] #[should_panic(expected: ('transaction invalid',))] fn test_execute_after_set_signers_and_threshold() { - // set_contract_address(contract_address); let signer1 = contract_address_const::<1>(); let signer2 = contract_address_const::<2>(); let signer3 = contract_address_const::<3>(); @@ -397,21 +369,17 @@ fn test_execute_after_set_signers_and_threshold() { // Multisig::constructor(ref state, :signers, threshold: 2); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); multisig .submit_transaction( to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), ); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(multisig_address); - // set_caller_address(contract_address); let new_signers = array![signer2, signer3]; multisig.set_signers_and_threshold(new_signers, 1); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); multisig.execute_transaction(nonce: 0); } @@ -432,20 +400,16 @@ fn test_execute_after_set_threshold() { let multisig = IMultisigDispatcher { contract_address: multisig_address }; start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); multisig .submit_transaction( to: contract_address_const::<42>(), function_selector: 10, calldata: sample_calldata(), ); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(signer2); - // set_caller_address(signer2); multisig.confirm_transaction(nonce: 0); start_cheat_caller_address_global(multisig_address); - // set_caller_address(contract_address); multisig.set_threshold(1); start_cheat_caller_address_global(signer1); - // set_caller_address(signer1); multisig.execute_transaction(nonce: 0); } @@ -465,15 +429,9 @@ fn test_set_threshold() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - let multisig = IMultisigDispatcher { contract_address: multisig_address }; assert(multisig.get_threshold() == init_threshold, 'invalid init threshold'); start_cheat_caller_address_global(multisig_address); - // set_contract_address(multisig_address); multisig.set_threshold(new_threshold); assert(multisig.get_threshold() == new_threshold, 'threshold was not updated'); } @@ -495,11 +453,6 @@ fn test_recursive_set_threshold() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -511,24 +464,19 @@ fn test_recursive_set_threshold() { let mut set_threshold_calldata = ArrayTrait::new(); Serde::serialize(@new_threshold, ref set_threshold_calldata); start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig .submit_transaction(multisig_address, selector!("set_threshold"), set_threshold_calldata); // Signer 1 confirms the transaction start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction start_cheat_caller_address_global(s2); - // set_contract_address(s2); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction // cheat only once because we want the multisig to execute the recursive tx cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); - // start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig.execute_transaction(0); @@ -551,11 +499,6 @@ fn test_set_signers() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - let multisig = IMultisigDispatcher { contract_address: multisig_address }; let returned_signers = multisig.get_signers(); @@ -565,7 +508,6 @@ fn test_set_signers() { assert(multisig.get_threshold() == 2, 'wrong init threshold'); start_cheat_caller_address_global(multisig_address); - // set_contract_address(multisig_address); multisig.set_signers(new_signers); let updated_signers = multisig.get_signers(); @@ -591,11 +533,6 @@ fn test_recursive_set_signers() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -612,22 +549,18 @@ fn test_recursive_set_signers() { let mut set_signers_calldata = ArrayTrait::new(); Serde::serialize(@new_signers, ref set_signers_calldata); start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig.submit_transaction(multisig_address, selector!("set_signers"), set_signers_calldata); // Signer 1 confirms the transaction start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction start_cheat_caller_address_global(s2); - // set_contract_address(s2); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); - // set_contract_address(s1); multisig.execute_transaction(0); // Now we check that the signers were actually updated @@ -654,11 +587,6 @@ fn test_set_signers_and_threshold() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - let multisig = IMultisigDispatcher { contract_address: multisig_address }; let returned_signers = multisig.get_signers(); @@ -669,7 +597,6 @@ fn test_set_signers_and_threshold() { assert(multisig.get_threshold() == init_threshold, 'wrong init threshold'); start_cheat_caller_address_global(multisig_address); - // set_contract_address(multisig_address); multisig.set_signers_and_threshold(new_signers, new_threshold); let updated_signers = multisig.get_signers(); @@ -698,11 +625,6 @@ fn test_recursive_set_signers_and_threshold() { let (multisig_address, _) = declare("Multisig").unwrap().deploy(@deploy_calldata).unwrap(); - // let (multisig_address, _) = deploy_syscall( - // Multisig::TEST_CLASS_HASH.try_into().unwrap(), 0, deploy_calldata.span(), false - // ) - // .unwrap(); - // Gets a dispatcher (so we can call methods on the deployed contract) let multisig = IMultisigDispatcher { contract_address: multisig_address }; @@ -721,7 +643,6 @@ fn test_recursive_set_signers_and_threshold() { Serde::serialize(@new_signers, ref set_signers_and_threshold_calldata); Serde::serialize(@new_threshold, ref set_signers_and_threshold_calldata); start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig .submit_transaction( multisig_address, @@ -731,22 +652,18 @@ fn test_recursive_set_signers_and_threshold() { // Signer 1 confirms the transaction start_cheat_caller_address_global(s1); - // set_contract_address(s1); multisig.confirm_transaction(0); // Signer 2 confirms the transaction start_cheat_caller_address_global(s2); - // set_contract_address(s2); multisig.confirm_transaction(0); // Signer 3 confirms the transaction start_cheat_caller_address_global(s3); - // set_contract_address(s3); multisig.confirm_transaction(0); // Once we have enough confirmations, we execute the transaction cheat_caller_address(multisig_address, s1, CheatSpan::TargetCalls(1)); - // set_contract_address(s1); multisig.execute_transaction(0); // Now we check that the signers were actually updated diff --git a/contracts/src/tests/test_ownable.cairo b/contracts/src/tests/test_ownable.cairo index a651f4372..d898bbb73 100644 --- a/contracts/src/tests/test_ownable.cairo +++ b/contracts/src/tests/test_ownable.cairo @@ -25,14 +25,12 @@ fn should_implement_ownable(contract_addr: ContractAddress, owner: ContractAddre // transfer ownership - check owner unchanged and proposed owner set correctly start_cheat_caller_address_global(owner); - // set_contract_address(owner); // required to call contract as owner contract.transfer_ownership(acc2); assert(owner == contract.owner(), 'owner should remain unchanged'); assert(acc2 == contract.pending_owner(), 'acc2 should be proposed owner'); // accept ownership - check owner changed and proposed owner set to zero start_cheat_caller_address_global(acc2); - // set_contract_address(acc2); // required to call function as acc2 contract.accept_ownership(); assert(contract.owner() == acc2, 'failed to change ownership'); assert(contract.pending_owner().is_zero(), 'proposed owner should be zero'); diff --git a/contracts/src/tests/test_sequencer_uptime_feed.cairo b/contracts/src/tests/test_sequencer_uptime_feed.cairo index a3bea486e..a7b44ff24 100644 --- a/contracts/src/tests/test_sequencer_uptime_feed.cairo +++ b/contracts/src/tests/test_sequencer_uptime_feed.cairo @@ -46,7 +46,6 @@ fn setup() -> (ContractAddress, ContractAddress, ISequencerUptimeFeedDispatcher) let account: ContractAddress = contract_address_const::<777>(); start_cheat_caller_address_global(account); - // set_caller_address(account); // Deploy seqeuencer uptime feed let calldata = array![0, // initial status @@ -55,10 +54,6 @@ fn setup() -> (ContractAddress, ContractAddress, ISequencerUptimeFeedDispatcher) let (sequencerFeedAddr, _) = declare("SequencerUptimeFeed").unwrap().deploy(@calldata).unwrap(); - // let (sequencerFeedAddr, _) = deploy_syscall( - // SequencerUptimeFeed::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let sequencerUptimeFeed = ISequencerUptimeFeedDispatcher { contract_address: sequencerFeedAddr }; @@ -90,7 +85,6 @@ fn test_set_l1_sender_not_owner() { fn test_set_l1_sender() { let (owner, _, sequencerUptimeFeed) = setup(); start_cheat_caller_address_global(owner); - // set_contract_address(owner); sequencerUptimeFeed.set_l1_sender(EthAddress { address: 789 }); assert(sequencerUptimeFeed.l1_sender().address == 789, 'l1_sender should be set to 789'); } @@ -117,7 +111,6 @@ fn test_latest_answer_no_access() { fn test_aggregator_proxy_response() { let (owner, sequencerFeedAddr, _) = setup(); start_cheat_caller_address_global(owner); - // set_contract_address(owner); let contract = IAccessControllerDispatcher { contract_address: sequencerFeedAddr }; contract.add_access(owner); diff --git a/contracts/src/tests/test_upgradeable.cairo b/contracts/src/tests/test_upgradeable.cairo index 1de5e755b..fee927616 100644 --- a/contracts/src/tests/test_upgradeable.cairo +++ b/contracts/src/tests/test_upgradeable.cairo @@ -24,7 +24,6 @@ use snforge_std::{ fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<777>(); start_cheat_caller_address_global(account); - // set_caller_address(account); account } @@ -36,10 +35,6 @@ fn test_upgrade_and_call() { let (contractAddr, _) = declare("MockUpgradeable").unwrap().deploy(@calldata).unwrap(); - // let (contractAddr, _) = deploy_syscall( - // MockUpgradeable::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false - // ) - // .unwrap(); let mockUpgradeable = IMockUpgradeableDispatcher { contract_address: contractAddr }; assert(mockUpgradeable.foo() == true, 'should call foo'); From f4ab723d938d9eb08a51ec4787c5f633ec448227 Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 30 Aug 2024 13:47:44 -0400 Subject: [PATCH 11/13] remove mcms for first change --- contracts/src/lib.cairo | 1 - contracts/src/mcms.cairo | 581 --------------------------------------- 2 files changed, 582 deletions(-) delete mode 100644 contracts/src/mcms.cairo diff --git a/contracts/src/lib.cairo b/contracts/src/lib.cairo index 71ffd2336..eef049d8c 100644 --- a/contracts/src/lib.cairo +++ b/contracts/src/lib.cairo @@ -8,7 +8,6 @@ mod emergency; mod multisig; mod token; mod access_control; -mod mcms; #[cfg(test)] mod tests; diff --git a/contracts/src/mcms.cairo b/contracts/src/mcms.cairo deleted file mode 100644 index 3f3e8d613..000000000 --- a/contracts/src/mcms.cairo +++ /dev/null @@ -1,581 +0,0 @@ -use starknet::ContractAddress; -use starknet::{ - eth_signature::public_key_point_to_eth_address, EthAddress, - secp256_trait::{ - Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature - }, - secp256k1::Secp256k1Point, SyscallResult, SyscallResultTrait -}; -use alexandria_bytes::{Bytes, BytesTrait}; -use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; -use alexandria_encoding::sol_abi::encode::SolAbiEncodeTrait; - -#[starknet::interface] -trait IManyChainMultiSig { - // todo: check length of byte array is 32 - fn set_root( - ref self: TContractState, - root: u256, - valid_until: u32, - metadata: RootMetadata, - metadata_proof: Span, - // note: v is a boolean and not uint8 - signatures: Array - ); - fn execute(ref self: TContractState, op: Op, proof: Span); - // // todo: check length of group_quorums and group_parents - fn set_config( - ref self: TContractState, - signer_addresses: Span, - signer_groups: Span, - group_quorums: Span, - group_parents: Span, - clear_root: bool - ); - fn get_config(self: @TContractState) -> Config; - fn get_op_count(self: @TContractState) -> u64; - fn get_root(self: @TContractState) -> (u256, u32); - fn get_root_metadata(self: @TContractState) -> RootMetadata; -} - -#[derive(Copy, Drop, Serde, starknet::Store)] -struct Signer { - address: EthAddress, - index: u8, - group: u8 -} - -#[derive(Copy, Drop, Serde, starknet::Store)] -struct RootMetadata { - chain_id: u256, - multisig: ContractAddress, - pre_op_count: u64, - post_op_count: u64, - override_previous_root: bool -} - -// todo: maybe use copy -// todo: figure out how this works off-chain with MCMS since we have a new selector field here -#[derive(Copy, Drop, Serde)] -struct Op { - chain_id: u256, - multisig: ContractAddress, - nonce: u64, - to: ContractAddress, - selector: felt252, - data: Span -} - -// does not implement Storage trait because structs cannot support arrays or maps -#[derive(Copy, Drop, Serde)] -struct Config { - signers: Span, - group_quorums: Span, - group_parents: Span -} - -#[derive(Copy, Drop, Serde, starknet::Store)] -struct ExpiringRootAndOpCount { - root: u256, - valid_until: u32, - op_count: u64 -} - -// based of https://github.com/starkware-libs/cairo/blob/1b747da1ec7e43a6fd0c0a4cbce302616408bc72/corelib/src/starknet/eth_signature.cairo#L25 -pub fn recover_eth_ecdsa(msg_hash: u256, signature: Signature) -> Result { - if !is_signature_entry_valid::(signature.r) { - return Result::Err('Signature out of range'); - } - if !is_signature_entry_valid::(signature.s) { - return Result::Err('Signature out of range'); - } - - let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); - // calculated eth address - return Result::Ok(public_key_point_to_eth_address(:public_key_point)); -} - -pub fn to_u256(address: EthAddress) -> u256 { - let temp: felt252 = address.into(); - temp.into() -} - -pub fn verify_merkle_proof(proof: Span, root: u256, leaf: u256) -> bool { - let mut computed_hash = leaf; - - let mut i = 0; - - while i < proof.len() { - computed_hash = hash_pair(computed_hash, *proof.at(i)); - i += 1; - }; - - computed_hash == root -} - -fn hash_pair(a: u256, b: u256) -> u256 { - let (lower, higher) = if a < b { - (a, b) - } else { - (b, a) - }; - BytesTrait::new_empty().encode(lower).encode(higher).keccak() -} - -#[starknet::contract] -mod ManyChainMultiSig { - use core::array::ArrayTrait; - use core::starknet::SyscallResultTrait; - use core::array::SpanTrait; - use core::dict::Felt252Dict; - use core::traits::PanicDestruct; - use super::{ - ExpiringRootAndOpCount, Config, Signer, RootMetadata, Op, Signature, recover_eth_ecdsa, - to_u256, verify_merkle_proof - }; - use starknet::{ - EthAddress, EthAddressZeroable, EthAddressIntoFelt252, ContractAddress, - call_contract_syscall - }; - use starknet::eth_signature::is_eth_signature_valid; - use alexandria_bytes::{Bytes, BytesTrait}; - use alexandria_encoding::sol_abi::sol_bytes::SolBytesTrait; - use alexandria_encoding::sol_abi::encode::SolAbiEncodeTrait; - - const NUM_GROUPS: u8 = 32; - - const MAX_NUM_SIGNERS: u8 = 200; - // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP") - const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP: u256 = - 0x08d275622006c4ca82d03f498e90163cafd53c663a48470c3b52ac8bfbd9f52c; - // keccak256("MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA") - const MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA: u256 = - 0xe6b82be989101b4eb519770114b997b97b3c8707515286748a871717f0e4ea1c; - - #[storage] - struct Storage { - // s_signers is used to easily validate the existence of the signer by its address. - s_signers: LegacyMap, - // begin s_config (defined in storage bc Config struct cannot support maps) - _s_config_signers_len: u8, - _s_config_signers: LegacyMap, - // no _s_config_group_len because there are always 32 groups - _s_config_group_quorums: LegacyMap, - _s_config_group_parents: LegacyMap, - // end s_config - s_seen_signed_hashes: LegacyMap, - s_expiring_root_and_op_count: ExpiringRootAndOpCount, - s_root_metadata: RootMetadata - } - - #[derive(Drop, starknet::Event)] - struct NewRoot { - #[key] - root: u256, - valid_until: u32, - metadata: RootMetadata, - } - - #[derive(Drop, starknet::Event)] - struct OpExecuted { - #[key] - nonce: u64, - to: ContractAddress, - selector: felt252, - data: Span, - // no value because value is sent through ERC20 tokens, even the native STRK token - } - - #[derive(Drop, starknet::Event)] - struct ConfigSet { - config: Config, - is_root_cleared: bool, - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - NewRoot: NewRoot, - OpExecuted: OpExecuted, - ConfigSet: ConfigSet, - } - - - #[abi(embed_v0)] - impl ManyChainMultiSigImpl of super::IManyChainMultiSig { - fn set_root( - ref self: ContractState, - root: u256, - valid_until: u32, - metadata: RootMetadata, - metadata_proof: Span, - // note: v is a boolean and not uint8 - mut signatures: Array - ) { - let encoded_root: Bytes = BytesTrait::new_empty().encode(root).encode(valid_until); - - let mut eip_191_msg: Bytes = BytesTrait::new_empty(); - eip_191_msg.append_felt252('\x19Ethereum Signed Message:\n32'); - eip_191_msg.append_u256(encoded_root.keccak()); - let msg_hash = eip_191_msg.keccak(); - - assert(!self.s_seen_signed_hashes.read(msg_hash), 'signed hash already seen'); - - let prev_address = EthAddressZeroable::zero(); - let mut group_vote_counts: Felt252Dict = Default::default(); - while let Option::Some(signature) = signatures - .pop_front() { - let signer_address = match recover_eth_ecdsa(msg_hash, signature) { - Result::Ok(signer_address) => signer_address, - Result::Err(e) => panic_with_felt252(e), - }; - - assert( - to_u256(prev_address) < to_u256(signer_address), - 'signer address must increase' - ); - - let signer = self.s_signers.read(signer_address); - assert(signer.address == signer_address, 'invalid signer'); - - let mut group = signer.group; - loop { - // todo: may be unnecessary assert - assert(group < NUM_GROUPS, 'invalid group number'); - let counts = group_vote_counts.get(group.into()); - group_vote_counts.insert(group.into(), counts + 1); - if counts + 1 != self._s_config_group_quorums.read(group) { - break; - } - if group == 0 { - // reached root - break; - } - group = self._s_config_group_parents.read(group) - }; - }; - - let root_group_quorum = self._s_config_group_quorums.read(0); - assert(root_group_quorum > 0, 'missing config'); - assert(group_vote_counts.get(0) >= root_group_quorum, 'insufficient signers'); - assert( - valid_until.into() >= starknet::info::get_block_timestamp(), - 'valid until has passed' - ); - // verify metadataProof - // todo: make sure this is the right way to encode the struct - let encoded_metadata: Bytes = BytesTrait::new_empty() - .encode(MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_METADATA) - .encode(valid_until) - .encode(metadata.chain_id) - .encode(metadata.multisig) - .encode(metadata.pre_op_count) - .encode(metadata.post_op_count) - .encode(metadata.override_previous_root); - - let hashed_leaf = encoded_metadata.keccak(); - assert( - verify_merkle_proof(metadata_proof, root, hashed_leaf), 'proof verification failed' - ); - - // maybe move to beginning of function - assert( - starknet::info::get_tx_info().unbox().chain_id.into() == metadata.chain_id, - 'wrong chain id' - ); - assert( - starknet::info::get_contract_address() == metadata.multisig, - 'wrong multisig address' - ); - - let op_count = self.s_expiring_root_and_op_count.read().op_count; - let current_root_metadata = self.s_root_metadata.read(); - assert( - op_count == current_root_metadata.post_op_count - || current_root_metadata.override_previous_root, - 'expect pending operations' - ); - assert(op_count == metadata.pre_op_count, 'wrong pre-operation count'); - assert(metadata.pre_op_count <= metadata.post_op_count, 'wrong post-operation count'); - - self.s_seen_signed_hashes.write(msg_hash, true); - self - .s_expiring_root_and_op_count - .write( - ExpiringRootAndOpCount { - root: root, valid_until: valid_until, op_count: metadata.pre_op_count - } - ); - - self - .emit( - Event::NewRoot( - NewRoot { root: root, valid_until: valid_until, metadata: metadata, } - ) - ); - } - - fn execute(ref self: ContractState, op: Op, proof: Span) { - let current_expiring_root_and_op_count = self.s_expiring_root_and_op_count.read(); - - assert( - self - .s_root_metadata - .read() - .post_op_count > current_expiring_root_and_op_count - .op_count, - 'post-operation count reached' - ); - - assert( - starknet::info::get_tx_info().unbox().chain_id.into() == op.chain_id, - 'wrong chain id' - ); - - assert(starknet::info::get_contract_address() == op.multisig, 'wrong multisig address'); - - assert( - current_expiring_root_and_op_count - .valid_until - .into() >= starknet::info::get_block_timestamp(), - 'root has expired' - ); - - assert(op.nonce == current_expiring_root_and_op_count.op_count, 'wrong nonce'); - - // verify op exists in merkle tree - let mut encoded_leaf: Bytes = BytesTrait::new_empty() - .encode(MANY_CHAIN_MULTI_SIG_DOMAIN_SEPARATOR_OP) - .encode(op.chain_id) - .encode(op.multisig) - .encode(op.nonce) - .encode(op.to) - .encode(op.selector); - // encode the data field by looping through - let mut i = 0; - while i < op.data.len() { - encoded_leaf = encoded_leaf.encode(*op.data.at(i)); - i += 1; - }; - let hashed_leaf = encoded_leaf.keccak(); - - assert( - verify_merkle_proof(proof, current_expiring_root_and_op_count.root, hashed_leaf), - 'proof verification failed' - ); - - let mut new_expiring_root_and_op_count = current_expiring_root_and_op_count; - new_expiring_root_and_op_count.op_count += 1; - - self.s_expiring_root_and_op_count.write(new_expiring_root_and_op_count); - // todo: execute - self._execute(op.to, op.selector, op.data); - - self - .emit( - Event::OpExecuted( - OpExecuted { - nonce: op.nonce, to: op.to, selector: op.selector, data: op.data - } - ) - ); - } - - // todo: make onlyOwner - fn set_config( - ref self: ContractState, - signer_addresses: Span, - signer_groups: Span, - group_quorums: Span, - group_parents: Span, - clear_root: bool - ) { - assert( - signer_addresses.len() != 0 && signer_addresses.len() <= MAX_NUM_SIGNERS.into(), - 'out of bound signers len' - ); - - assert(signer_addresses.len() == signer_groups.len(), 'signer groups len mismatch'); - - assert( - group_quorums.len() == NUM_GROUPS.into() - && group_quorums.len() == group_parents.len(), - 'group quorums/parents mismatch' - ); - - let mut group_children_counts: Felt252Dict = Default::default(); - let mut i = 0; - while i < signer_groups - .len() { - let group = *signer_groups.at(i); - assert(group < NUM_GROUPS, 'out of bounds group'); - // increment count for each group - group_children_counts - .insert(group.into(), group_children_counts.get(group.into()) + 1); - i += 1; - }; - - let mut j = 0; - while j < NUM_GROUPS { - // iterate backwards: i is the group # - let i = NUM_GROUPS - 1 - j; - let group_parent = (*group_parents.at(i.into())); - - assert( - (i == 0 || group_parent < i) && (i != 0 || group_parent == 0), - 'group tree malformed' - ); - - let disabled = *group_quorums.at(i.into()) == 0; - - if disabled { - assert(group_children_counts.get(i.into()) == 0, 'signer in disabled group'); - } else { - assert( - group_children_counts.get(i.into()) >= *group_quorums.at(i.into()), - 'quorum impossible' - ); - - group_children_counts - .insert( - group_parent.into(), group_children_counts.get(group_parent.into()) + 1 - ); - // the above line clobbers group_children_counts[0] in last iteration, don't use it after the loop ends - } - j += 1; - }; - - // remove any old signer addresses - let mut i: u8 = 0; - let signers_len = self._s_config_signers_len.read(); - - while i < signers_len { - let mut old_signer = self._s_config_signers.read(i); - let empty_signer = Signer { - address: EthAddressZeroable::zero(), index: 0, group: 0 - }; - // reset s_signers - self.s_signers.write(old_signer.address, empty_signer); - // reset _s_config_signers - self._s_config_signers.write(i.into(), empty_signer); - }; - // reset _s_config_signers_len - self._s_config_signers_len.write(0); - - let mut i: u8 = 0; - while i < NUM_GROUPS { - self._s_config_group_quorums.write(i, *group_quorums.at(i.into())); - self._s_config_group_parents.write(i, *group_parents.at(i.into())); - i += 1; - }; - - // create signers array (for event logs) - let mut signers = ArrayTrait::::new(); - let mut prev_signer_address = EthAddressZeroable::zero(); - let mut i: u8 = 0; - while i - .into() < signer_addresses - .len() { - let signer_address = *signer_addresses.at(i.into()); - assert( - to_u256(prev_signer_address) < to_u256(signer_address), - 'addresses not sorted' - ); - - let signer = Signer { - address: signer_address, index: i, group: *signer_groups.at(i.into()) - }; - - self.s_signers.write(signer_address, signer); - self._s_config_signers.write(i.into(), signer); - - signers.append(signer); - - prev_signer_address = signer_address; - i += 1; - }; - - if clear_root { - let op_count = self.s_expiring_root_and_op_count.read().op_count; - self - .s_expiring_root_and_op_count - .write(ExpiringRootAndOpCount { root: 0, valid_until: 0, op_count: op_count }); - self - .s_root_metadata - .write( - RootMetadata { - chain_id: starknet::info::get_tx_info().unbox().chain_id.into(), - multisig: starknet::info::get_contract_address(), - pre_op_count: op_count, - post_op_count: op_count, - override_previous_root: true - } - ); - } - - self - .emit( - Event::ConfigSet( - ConfigSet { - config: Config { - signers: signers.span(), - group_quorums: group_quorums, - group_parents: group_parents, - }, - is_root_cleared: clear_root - } - ) - ); - } - - fn get_config(self: @ContractState) -> Config { - let mut signers = ArrayTrait::::new(); - - let mut i = 0; - let signers_len = self._s_config_signers_len.read(); - while i < signers_len { - let signer = self._s_config_signers.read(i); - signers.append(signer); - i += 1 - }; - - let mut group_quorums = ArrayTrait::::new(); - let mut group_parents = ArrayTrait::::new(); - - let mut i = 0; - while i < NUM_GROUPS { - group_quorums.append(self._s_config_group_quorums.read(i)); - group_parents.append(self._s_config_group_parents.read(i)); - }; - - Config { - signers: signers.span(), - group_quorums: group_quorums.span(), - group_parents: group_parents.span() - } - } - - fn get_op_count(self: @ContractState) -> u64 { - self.s_expiring_root_and_op_count.read().op_count - } - - fn get_root(self: @ContractState) -> (u256, u32) { - let current = self.s_expiring_root_and_op_count.read(); - (current.root, current.valid_until) - } - - fn get_root_metadata(self: @ContractState) -> RootMetadata { - self.s_root_metadata.read() - } - } - - #[generate_trait] - impl InternalFunctions of InternalFunctionsTrait { - fn _execute( - ref self: ContractState, target: ContractAddress, selector: felt252, data: Span - ) { - let _response = call_contract_syscall(target, selector, data).unwrap_syscall(); - } - } -} - From 6b3fd7afaec4d7c75ef3667c30ccf9a70132586e Mon Sep 17 00:00:00 2001 From: Augustus Chang Date: Fri, 30 Aug 2024 13:54:43 -0400 Subject: [PATCH 12/13] update ci/cd --- .github/actions/install-starknet-foundry/action.yml | 3 +-- .github/workflows/contracts.yml | 3 +++ contracts/Scarb.toml | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/actions/install-starknet-foundry/action.yml b/.github/actions/install-starknet-foundry/action.yml index 02b9b59dc..8e4360edf 100644 --- a/.github/actions/install-starknet-foundry/action.yml +++ b/.github/actions/install-starknet-foundry/action.yml @@ -4,7 +4,7 @@ description: A composite action that installs the snforge and sncast binaries inputs: starknet_foundry_version: description: Starknet Foundry release version - default: "0.21.0" + default: "0.27.0" required: false runs: @@ -16,4 +16,3 @@ runs: run: | curl -L https://raw.githubusercontent.com/foundry-rs/starknet-foundry/master/scripts/install.sh | sh snfoundryup -v ${{ inputs.starknet_foundry_version }} - diff --git a/.github/workflows/contracts.yml b/.github/workflows/contracts.yml index 58e06e4bc..d10fdfdc7 100644 --- a/.github/workflows/contracts.yml +++ b/.github/workflows/contracts.yml @@ -41,5 +41,8 @@ jobs: - name: Install Cairo uses: ./.github/actions/install-cairo + - name: Install Starknet Foundry + uses: ./.github/actions/install-starknet-foundry + - name: Test run: nix develop -c make test-cairo-contracts diff --git a/contracts/Scarb.toml b/contracts/Scarb.toml index 261bc54b7..a84f65f83 100644 --- a/contracts/Scarb.toml +++ b/contracts/Scarb.toml @@ -7,7 +7,7 @@ homepage = "https://github.com/smartcontractkit/chainlink-starknet" [scripts] sierra = "cairo-compile . -r" -# test = "snforge test" +test = "snforge test" # Add your own custom commands and run them with scarb run # Uncomment if you want to use dependencies From 61803537e82b1de23933ea12bf18ddf86d4a49b6 Mon Sep 17 00:00:00 2001 From: Augustus <14297860+augustbleeds@users.noreply.github.com> Date: Fri, 30 Aug 2024 14:01:16 -0400 Subject: [PATCH 13/13] Delete contracts/src/tests/test_mcms.cairo --- contracts/src/tests/test_mcms.cairo | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 contracts/src/tests/test_mcms.cairo diff --git a/contracts/src/tests/test_mcms.cairo b/contracts/src/tests/test_mcms.cairo deleted file mode 100644 index 7233889b2..000000000 --- a/contracts/src/tests/test_mcms.cairo +++ /dev/null @@ -1,15 +0,0 @@ -// set_config tests - -// 1. test if lena(signer_address) = 0 => revert -// 2. test if lena(signer_address) > MAX_NUM_SIGNERS => revert -// 3. test if signer addresses and signer groups not same size -// 4. test if group_quorum and group_parents not same size -// 5. test if one of signer_group #'s is out of bounds NUM_GROUPS -// 6. test if group_parents[i] is greater than or equal to i (when not 0) there is revert -// 7. test if i is 0 and group_parents[i] != 0 and revert -// 8. test if there is a signer in a group where group_quorum[i] == 0 => revert -// 9. test if there are not enough signers to meet a quorum => revert -// 10. test if signer addresses are not in ascending order -// 11. successful => test without clearing root. test the state of storage variables and that event was emitted - -