From 944e730ad23ade786dad00eed889dad28a26ba90 Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 19:48:45 +0200 Subject: [PATCH 01/13] Update test utils and helpers --- src/tests.cairo | 4 +- src/tests/account/ethereum.cairo | 5 +- src/tests/account/ethereum/common.cairo | 62 +++++++++---------- src/tests/account/starknet/common.cairo | 80 ++++++++++++++----------- src/tests/utils/signing.cairo | 46 +++++++++++--- 5 files changed, 120 insertions(+), 77 deletions(-) diff --git a/src/tests.cairo b/src/tests.cairo index eceac55c6..d84011ca9 100644 --- a/src/tests.cairo +++ b/src/tests.cairo @@ -1,7 +1,7 @@ // #[cfg(test)] // mod access; -// #[cfg(test)] -// mod account; +#[cfg(test)] +mod account; // #[cfg(test)] // mod cryptography; // #[cfg(test)] diff --git a/src/tests/account/ethereum.cairo b/src/tests/account/ethereum.cairo index d6fcf488b..014854b34 100644 --- a/src/tests/account/ethereum.cairo +++ b/src/tests/account/ethereum.cairo @@ -1,4 +1,3 @@ +// mod test_dual_eth_account; +// mod test_eth_account; pub(crate) mod common; - -mod test_dual_eth_account; -mod test_eth_account; diff --git a/src/tests/account/ethereum/common.cairo b/src/tests/account/ethereum/common.cairo index faf88a25c..130fe35ea 100644 --- a/src/tests/account/ethereum/common.cairo +++ b/src/tests/account/ethereum/common.cairo @@ -6,9 +6,12 @@ use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::signature::EthSignature; use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; use openzeppelin::tests::utils::constants::{NAME, SYMBOL}; +use openzeppelin::tests::utils::events::EventSpyExt; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::serde::SerializedAppend; +use snforge_std::EventSpy; +use snforge_std::cheatcodes::events::EventSpyAssertionsTrait; use starknet::ContractAddress; use starknet::SyscallResultTrait; use starknet::secp256_trait::Secp256Trait; @@ -18,7 +21,7 @@ use starknet::secp256k1::Secp256k1Point; pub(crate) struct SignedTransactionData { pub(crate) private_key: u256, pub(crate) public_key: EthPublicKey, - pub(crate) transaction_hash: felt252, + pub(crate) tx_hash: felt252, pub(crate) signature: EthSignature } @@ -27,7 +30,7 @@ pub(crate) fn SIGNED_TX_DATA() -> SignedTransactionData { SignedTransactionData { private_key: 0x45397ee6ca34cb49060f1c303c6cb7ee2d6123e617601ef3e31ccf7bf5bef1f9, public_key: NEW_ETH_PUBKEY(), - transaction_hash: 0x008f882c63d0396d216d57529fe29ad5e70b6cd51b47bd2458b0a4ccb2ba0957, + tx_hash: 0x008f882c63d0396d216d57529fe29ad5e70b6cd51b47bd2458b0a4ccb2ba0957, signature: EthSignature { r: 0x82bb3efc0554ec181405468f273b0dbf935cca47182b22da78967d0770f7dcc3, s: 0x6719fef30c11c74add873e4da0e1234deb69eae6a6bd4daa44b816dc199f3e86, @@ -54,7 +57,7 @@ pub(crate) fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> calldata.append_serde(initial_supply); calldata.append_serde(recipient); - let address = utils::deploy(DualCaseERC20Mock::TEST_CLASS_HASH, calldata); + let address = utils::declare_and_deploy("DualCaseERC20Mock", calldata); IERC20Dispatcher { contract_address: address } } @@ -70,35 +73,34 @@ pub(crate) fn get_points() -> (Secp256k1Point, Secp256k1Point) { (point_1, point_2) } -pub(crate) fn assert_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { - let event = utils::pop_log::(contract).unwrap(); - let new_owner_guid = get_guid_from_public_key(public_key); - let expected = EthAccountComponent::Event::OwnerAdded(OwnerAdded { new_owner_guid }); - assert!(event == expected); - - // Check indexed keys - let mut indexed_keys = array![]; - indexed_keys.append_serde(selector!("OwnerAdded")); - indexed_keys.append_serde(new_owner_guid); - utils::assert_indexed_keys(event, indexed_keys.span()); -} - -pub(crate) fn assert_only_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { - assert_event_owner_added(contract, public_key); - utils::assert_no_events_left(contract); -} +#[generate_trait] +pub(crate) impl AccountSpyHelpersImpl of AccountSpyHelpers { + fn assert_event_owner_added( + ref self: EventSpy, contract: ContractAddress, public_key: EthPublicKey + ) { + let new_owner_guid = get_guid_from_public_key(public_key); + let expected = EthAccountComponent::Event::OwnerAdded(OwnerAdded { new_owner_guid }); + self.assert_emitted_single(contract, expected); + // TODO: Add check for indexed keys + } -pub(crate) fn assert_event_owner_removed(contract: ContractAddress, public_key: EthPublicKey) { - let event = utils::pop_log::(contract).unwrap(); - let removed_owner_guid = get_guid_from_public_key(public_key); - let expected = EthAccountComponent::Event::OwnerRemoved(OwnerRemoved { removed_owner_guid }); - assert!(event == expected); + fn assert_only_event_owner_added( + ref self: EventSpy, contract: ContractAddress, public_key: EthPublicKey + ) { + self.assert_event_owner_added(contract, public_key); + self.assert_no_events_left_from(contract); + } - // Check indexed keys - let mut indexed_keys = array![]; - indexed_keys.append_serde(selector!("OwnerRemoved")); - indexed_keys.append_serde(removed_owner_guid); - utils::assert_indexed_keys(event, indexed_keys.span()); + fn assert_event_owner_removed( + ref self: EventSpy, contract: ContractAddress, public_key: EthPublicKey + ) { + let removed_owner_guid = get_guid_from_public_key(public_key); + let expected = EthAccountComponent::Event::OwnerRemoved( + OwnerRemoved { removed_owner_guid } + ); + self.assert_emitted_single(contract, expected); + // TODO: Add check for indexed keys + } } fn get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { diff --git a/src/tests/account/starknet/common.cairo b/src/tests/account/starknet/common.cairo index 91905d856..80e959ad5 100644 --- a/src/tests/account/starknet/common.cairo +++ b/src/tests/account/starknet/common.cairo @@ -1,28 +1,31 @@ +use core::hash::{HashStateTrait, HashStateExTrait}; +use core::poseidon::PoseidonTrait; use openzeppelin::account::AccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::AccountComponent; -use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; -use openzeppelin::tests::utils::constants::{NAME, SYMBOL, NEW_PUBKEY}; +use openzeppelin::tests::utils::constants::{NAME, SYMBOL}; +use openzeppelin::tests::utils::events::EventSpyExt; +use openzeppelin::tests::utils::signing::stark::StarkKeyPair; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::serde::SerializedAppend; +use snforge_std::EventSpy; +use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; use starknet::ContractAddress; #[derive(Drop)] pub(crate) struct SignedTransactionData { pub(crate) private_key: felt252, pub(crate) public_key: felt252, - pub(crate) transaction_hash: felt252, + pub(crate) tx_hash: felt252, pub(crate) r: felt252, - pub(crate) s: felt252 + pub(crate) s: felt252, } -pub(crate) fn SIGNED_TX_DATA() -> SignedTransactionData { +pub(crate) fn SIGNED_TX_DATA(key_pair: StarkKeyPair) -> SignedTransactionData { + let tx_hash = 'TRANSACTION_HASH'; + let (r, s) = key_pair.sign(tx_hash).unwrap(); SignedTransactionData { - private_key: 1234, - public_key: NEW_PUBKEY, - transaction_hash: 0x601d3d2e265c10ff645e1554c435e72ce6721f0ba5fc96f0c650bfc6231191a, - r: 0x6bc22689efcaeacb9459577138aff9f0af5b77ee7894cdc8efabaf760f6cf6e, - s: 0x295989881583b9325436851934334faa9d639a2094cd1e2f8691c8a71cd4cdf + private_key: key_pair.secret_key, public_key: key_pair.public_key, tx_hash, r, s } } @@ -34,36 +37,43 @@ pub(crate) fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> calldata.append_serde(initial_supply); calldata.append_serde(recipient); - let address = utils::deploy(DualCaseERC20Mock::TEST_CLASS_HASH, calldata); + let address = utils::declare_and_deploy("DualCaseERC20Mock", calldata); IERC20Dispatcher { contract_address: address } } - -pub(crate) fn assert_event_owner_removed(contract: ContractAddress, removed_owner_guid: felt252) { - let event = utils::pop_log::(contract).unwrap(); - let expected = AccountComponent::Event::OwnerRemoved(OwnerRemoved { removed_owner_guid }); - assert!(event == expected); - - // Check indexed keys - let mut indexed_keys = array![]; - indexed_keys.append_serde(selector!("OwnerRemoved")); - indexed_keys.append_serde(removed_owner_guid); - utils::assert_indexed_keys(event, indexed_keys.span()); +pub(crate) fn get_accept_ownership_signature( + account_address: ContractAddress, current_public_key: felt252, new_key_pair: StarkKeyPair +) -> Span { + let msg_hash = PoseidonTrait::new() + .update_with('StarkNet Message') + .update_with('accept_ownership') + .update_with(account_address) + .update_with(current_public_key) + .finalize(); + let (sig_r, sig_s) = new_key_pair.sign(msg_hash).unwrap(); + array![sig_r, sig_s].span() } -pub(crate) fn assert_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { - let event = utils::pop_log::(contract).unwrap(); - let expected = AccountComponent::Event::OwnerAdded(OwnerAdded { new_owner_guid }); - assert!(event == expected); +#[generate_trait] +pub(crate) impl AccountSpyHelpersImpl of AccountSpyHelpers { + fn assert_event_owner_removed( + ref self: EventSpy, contract: ContractAddress, removed_owner_guid: felt252 + ) { + let expected = AccountComponent::Event::OwnerRemoved(OwnerRemoved { removed_owner_guid }); + self.assert_emitted_single(contract, expected); + } - // Check indexed keys - let mut indexed_keys = array![]; - indexed_keys.append_serde(selector!("OwnerAdded")); - indexed_keys.append_serde(new_owner_guid); - utils::assert_indexed_keys(event, indexed_keys.span()); -} + fn assert_event_owner_added( + ref self: EventSpy, contract: ContractAddress, new_owner_guid: felt252 + ) { + let expected = AccountComponent::Event::OwnerAdded(OwnerAdded { new_owner_guid }); + self.assert_emitted_single(contract, expected); + } -pub(crate) fn assert_only_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { - assert_event_owner_added(contract, new_owner_guid); - utils::assert_no_events_left(contract); + fn assert_only_event_owner_added( + ref self: EventSpy, contract: ContractAddress, new_owner_guid: felt252 + ) { + self.assert_event_owner_added(contract, new_owner_guid); + self.assert_no_events_left_from(contract); + } } diff --git a/src/tests/utils/signing.cairo b/src/tests/utils/signing.cairo index 90be99ed5..f160445ae 100644 --- a/src/tests/utils/signing.cairo +++ b/src/tests/utils/signing.cairo @@ -1,11 +1,43 @@ -use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; -use snforge_std::signature::{KeyPair, KeyPairTrait}; +pub mod stark { + use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; + use snforge_std::signature::{KeyPair, KeyPairTrait}; -pub type StarkKeyPair = KeyPair; + pub type StarkKeyPair = KeyPair; -pub fn KEY_PAIR() -> StarkKeyPair { - KeyPairTrait::from_secret_key('SECRET_KEY') + pub const PRIVATE_KEY: felt252 = 'PRIVATE_KEY'; + pub const PUBKEY: felt252 = 0x454c2645c42b23ea47717675e972e3fdcc1865a40ada320286e33b5a921ecd3; + pub const KEY_PAIR: StarkKeyPair = StarkKeyPair { secret_key: PRIVATE_KEY, public_key: PUBKEY }; + + pub const PRIVATE_KEY_2: felt252 = 'PRIVATE_KEY_2'; + pub const PUBKEY_2: felt252 = 0x3611882107b0c824d5a0f3d1dd9468d71bc5a8584ee30b2e166e532a7cd8eda; + pub const KEY_PAIR_2: StarkKeyPair = + StarkKeyPair { secret_key: PRIVATE_KEY_2, public_key: PUBKEY_2 }; + + pub fn key_pair_from(private_key: felt252) -> StarkKeyPair { + StarkCurveKeyPairImpl::from_secret_key(private_key) + } } -pub fn KEY_PAIR_2() -> StarkKeyPair { - KeyPairTrait::from_secret_key('SECRET_KEY_2') + +pub mod secp256k1 { + use snforge_std::signature::secp256k1_curve::{ + Secp256k1CurveSignerImpl, Secp256k1CurveKeyPairImpl + }; + use snforge_std::signature::{KeyPair, KeyPairTrait}; + use starknet::secp256k1::Secp256k1Point; + + pub type Secp256k1KeyPair = KeyPair; + + pub const PRIVATE_KEY: u256 = u256 { low: 'PRIVATE_LOW', high: 'PRIVATE_HIGH' }; + pub fn KEY_PAIR() -> Secp256k1KeyPair { + Secp256k1CurveKeyPairImpl::from_secret_key(PRIVATE_KEY) + } + + pub const PRIVATE_KEY_2: u256 = u256 { low: 'PRIVATE_LOW_2', high: 'PRIVATE_HIGH_2' }; + pub fn KEY_PAIR_2() -> Secp256k1KeyPair { + Secp256k1CurveKeyPairImpl::from_secret_key(PRIVATE_KEY_2) + } + + pub fn key_pair_from(private_key: u256) -> Secp256k1KeyPair { + Secp256k1CurveKeyPairImpl::from_secret_key(private_key) + } } From b2560c37fe83fe9230dd7c84bcf2fb360ec89e72 Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 19:48:57 +0200 Subject: [PATCH 02/13] Update signature tests --- src/tests/account/test_signature.cairo | 36 +++++++++++--------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/tests/account/test_signature.cairo b/src/tests/account/test_signature.cairo index ffdb2ce74..ac533187a 100644 --- a/src/tests/account/test_signature.cairo +++ b/src/tests/account/test_signature.cairo @@ -1,6 +1,7 @@ use openzeppelin::account::utils::signature::{is_valid_stark_signature, is_valid_eth_signature}; use starknet::secp256_trait::Secp256Trait; use starknet::secp256k1::Secp256k1Point; +use openzeppelin::tests::utils::signing::{stark, secp256k1}; use super::ethereum::common::SIGNED_TX_DATA as eth_signature_data; use super::starknet::common::SIGNED_TX_DATA as stark_signature_data; @@ -11,34 +12,31 @@ use super::starknet::common::SIGNED_TX_DATA as stark_signature_data; #[test] fn test_is_valid_stark_signature_good_sig() { - let data = stark_signature_data(); - let hash = data.transaction_hash; + let data = stark_signature_data(stark::KEY_PAIR); let mut good_signature = array![data.r, data.s].span(); - let is_valid = is_valid_stark_signature(hash, data.public_key, good_signature); + let is_valid = is_valid_stark_signature(data.tx_hash, data.public_key, good_signature); assert!(is_valid); } #[test] fn test_is_valid_stark_signature_bad_sig() { - let data = stark_signature_data(); - let hash = data.transaction_hash; + let data = stark_signature_data(stark::KEY_PAIR); - let mut bad_signature = array![0x987, 0x564].span(); + let bad_signature = array!['BAD', 'SIGNATURE'].span(); - let is_invalid = !is_valid_stark_signature(hash, data.public_key, bad_signature); + let is_invalid = !is_valid_stark_signature(data.tx_hash, data.public_key, bad_signature); assert!(is_invalid); } #[test] fn test_is_valid_stark_signature_invalid_len_sig() { - let data = stark_signature_data(); - let hash = data.transaction_hash; + let data = stark_signature_data(stark::KEY_PAIR); let mut bad_signature = array![0x987].span(); - let is_invalid = !is_valid_stark_signature(hash, data.public_key, bad_signature); + let is_invalid = !is_valid_stark_signature(data.tx_hash, data.public_key, bad_signature); assert!(is_invalid); } @@ -49,20 +47,19 @@ fn test_is_valid_stark_signature_invalid_len_sig() { #[test] fn test_is_valid_eth_signature_good_sig() { let data = eth_signature_data(); - let hash = data.transaction_hash; - let mut serialized_good_signature = array![]; data.signature.serialize(ref serialized_good_signature); - let is_valid = is_valid_eth_signature(hash, data.public_key, serialized_good_signature.span()); + let is_valid = is_valid_eth_signature( + data.tx_hash, data.public_key, serialized_good_signature.span() + ); assert!(is_valid); } #[test] fn test_is_valid_eth_signature_bad_sig() { let data = eth_signature_data(); - let hash = data.transaction_hash; let mut bad_signature = data.signature; bad_signature.r += 1; @@ -72,7 +69,7 @@ fn test_is_valid_eth_signature_bad_sig() { bad_signature.serialize(ref serialized_bad_signature); let is_invalid = !is_valid_eth_signature( - hash, data.public_key, serialized_bad_signature.span() + data.tx_hash, data.public_key, serialized_bad_signature.span() ); assert!(is_invalid); } @@ -81,17 +78,15 @@ fn test_is_valid_eth_signature_bad_sig() { #[should_panic(expected: ('Signature: Invalid format.',))] fn test_is_valid_eth_signature_invalid_format_sig() { let data = eth_signature_data(); - let hash = data.transaction_hash; let mut serialized_bad_signature = array![0x1]; - is_valid_eth_signature(hash, data.public_key, serialized_bad_signature.span()); + is_valid_eth_signature(data.tx_hash, data.public_key, serialized_bad_signature.span()); } #[test] fn test_signature_r_out_of_range() { let data = eth_signature_data(); - let hash = data.transaction_hash; let mut bad_signature = data.signature; let curve_size = Secp256Trait::::get_curve_size(); @@ -103,7 +98,7 @@ fn test_signature_r_out_of_range() { bad_signature.serialize(ref serialized_bad_signature); let is_invalid = !is_valid_eth_signature( - hash, data.public_key, serialized_bad_signature.span() + data.tx_hash, data.public_key, serialized_bad_signature.span() ); assert!(is_invalid); } @@ -111,7 +106,6 @@ fn test_signature_r_out_of_range() { #[test] fn test_signature_s_out_of_range() { let data = eth_signature_data(); - let hash = data.transaction_hash; let mut bad_signature = data.signature; let curve_size = Secp256Trait::::get_curve_size(); @@ -123,7 +117,7 @@ fn test_signature_s_out_of_range() { bad_signature.serialize(ref serialized_bad_signature); let is_invalid = !is_valid_eth_signature( - hash, data.public_key, serialized_bad_signature.span() + data.tx_hash, data.public_key, serialized_bad_signature.span() ); assert!(is_invalid); } From 766fc31fefccbc72ce9d34fb2affe14856eae49c Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 19:49:04 +0200 Subject: [PATCH 03/13] Update account tests --- src/tests/account/starknet/test_account.cairo | 301 ++++++++---------- 1 file changed, 130 insertions(+), 171 deletions(-) diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index ed4dc81ed..e22cc0d72 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -1,4 +1,5 @@ use core::num::traits::Zero; +use core::starknet::SyscallResultTrait; use openzeppelin::account::AccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; use openzeppelin::account::AccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; use openzeppelin::account::AccountComponent; @@ -7,31 +8,25 @@ use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::account_mocks::DualCaseAccountMock; use openzeppelin::tests::utils::constants::{ - PUBKEY, NEW_PUBKEY, SALT, ZERO, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION + SALT, ZERO, OTHER, CALLER, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION +}; +use openzeppelin::tests::utils::signing::stark::{ + KEY_PAIR, KEY_PAIR_2, PUBKEY, PUBKEY_2, PRIVATE_KEY }; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; -use starknet::ContractAddress; +use snforge_std::signature::SignerTrait; +use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; +use snforge_std::{EventSpy, spy_events, declare, test_address, start_cheat_caller_address}; +use snforge_std::{cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global}; use starknet::account::Call; use starknet::contract_address_const; -use starknet::testing; - -use super::common::{assert_only_event_owner_added, assert_event_owner_removed}; -use super::common::{deploy_erc20, SIGNED_TX_DATA, SignedTransactionData}; - -// -// Constants -// +use starknet::{ContractAddress, ClassHash}; -fn CLASS_HASH() -> felt252 { - DualCaseAccountMock::TEST_CLASS_HASH -} - -fn ACCOUNT_ADDRESS() -> ContractAddress { - contract_address_const::<0x111111>() -} +use super::common::{AccountSpyHelpers, SignedTransactionData}; +use super::common::{deploy_erc20, SIGNED_TX_DATA, get_accept_ownership_signature}; // // Setup @@ -50,25 +45,26 @@ fn COMPONENT_STATE() -> ComponentState { fn setup() -> ComponentState { let mut state = COMPONENT_STATE(); state.initializer(PUBKEY); - utils::drop_event(ZERO()); state } -fn setup_dispatcher(data: Option<@SignedTransactionData>) -> AccountABIDispatcher { - testing::set_version(MIN_TRANSACTION_VERSION); +fn setup_dispatcher(data: Option<@SignedTransactionData>) -> (AccountABIDispatcher, felt252) { + let contract_class = declare("DualCaseAccountMock").unwrap_syscall(); + let calldata = if let Option::Some(data) = data { + cheat_signature_global(array![*data.r, *data.s].span()); + cheat_transaction_hash_global(*data.tx_hash); + array![*data.public_key] + } else { + array![PUBKEY] + }; - let mut calldata = array![]; - if data.is_some() { - let data = data.unwrap(); - testing::set_signature(array![*data.r, *data.s].span()); - testing::set_transaction_hash(*data.transaction_hash); + let address = utils::deploy(contract_class, calldata); + let dispatcher = AccountABIDispatcher { contract_address: address }; - calldata.append(*data.public_key); - } else { - calldata.append(PUBKEY); - } - let address = utils::deploy(CLASS_HASH(), calldata); - AccountABIDispatcher { contract_address: address } + cheat_transaction_version_global(MIN_TRANSACTION_VERSION); + start_cheat_caller_address(address, ZERO()); + + (dispatcher, contract_class.class_hash.into()) } // @@ -78,36 +74,33 @@ fn setup_dispatcher(data: Option<@SignedTransactionData>) -> AccountABIDispatche #[test] fn test_is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; + let data = SIGNED_TX_DATA(KEY_PAIR); let mut good_signature = array![data.r, data.s]; let mut bad_signature = array![0x987, 0x564]; state._set_public_key(data.public_key); - let is_valid = state.is_valid_signature(hash, good_signature); + let is_valid = state.is_valid_signature(data.tx_hash, good_signature); assert_eq!(is_valid, starknet::VALIDATED); - let is_valid = state.is_valid_signature(hash, bad_signature); + let is_valid = state.is_valid_signature(data.tx_hash, bad_signature); assert!(is_valid.is_zero(), "Should reject invalid signature"); } #[test] fn test_isValidSignature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; - - let mut good_signature = array![data.r, data.s]; - let mut bad_signature = array![0x987, 0x564]; + let data = SIGNED_TX_DATA(KEY_PAIR); state._set_public_key(data.public_key); - let is_valid = state.isValidSignature(hash, good_signature); + let good_signature = array![data.r, data.s]; + let is_valid = state.isValidSignature(data.tx_hash, good_signature); assert_eq!(is_valid, starknet::VALIDATED); - let is_valid = state.isValidSignature(hash, bad_signature); + let bad_signature = array!['BAD', 'SIGNATURE']; + let is_valid = state.isValidSignature(data.tx_hash, bad_signature); assert!(is_valid.is_zero(), "Should reject invalid signature"); } @@ -117,94 +110,92 @@ fn test_isValidSignature() { #[test] fn test_validate_deploy() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); // `__validate_deploy__` does not directly use the passed arguments. Their // values are already integrated in the tx hash. The passed arguments in this // testing context are decoupled from the signature and have no effect on the test. - let is_valid = account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); + let is_valid = account.__validate_deploy__(class_hash, SALT, PUBKEY); assert_eq!(is_valid, starknet::VALIDATED); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_invalid_signature_data() { - let mut data = SIGNED_TX_DATA(); - data.transaction_hash += 1; - let account = setup_dispatcher(Option::Some(@data)); + let mut data = SIGNED_TX_DATA(KEY_PAIR); + data.tx_hash += 1; + let (account, class_hash) = setup_dispatcher(Option::Some(@data)); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); + account.__validate_deploy__(class_hash, SALT, PUBKEY); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_invalid_signature_length() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); - let mut signature = array![]; - - signature.append(0x1); - testing::set_signature(signature.span()); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let signature = array![0x1]; + cheat_signature_global(signature.span()); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); + account.__validate_deploy__(class_hash, SALT, PUBKEY); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_empty_signature() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); let empty_sig = array![]; - testing::set_signature(empty_sig.span()); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); + cheat_signature_global(empty_sig.span()); + account.__validate_deploy__(class_hash, SALT, PUBKEY); } #[test] fn test_validate_declare() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); // `__validate_declare__` does not directly use the class_hash argument. Its // value is already integrated in the tx hash. The class_hash argument in this // testing context is decoupled from the signature and has no effect on the test. - let is_valid = account.__validate_declare__(CLASS_HASH()); + let is_valid = account.__validate_declare__(class_hash); assert_eq!(is_valid, starknet::VALIDATED); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_data() { - let mut data = SIGNED_TX_DATA(); - data.transaction_hash += 1; - let account = setup_dispatcher(Option::Some(@data)); + let mut data = SIGNED_TX_DATA(KEY_PAIR); + data.tx_hash += 1; + let (account, class_hash) = setup_dispatcher(Option::Some(@data)); - account.__validate_declare__(CLASS_HASH()); + account.__validate_declare__(class_hash); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_length() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); let mut signature = array![]; signature.append(0x1); - testing::set_signature(signature.span()); + cheat_signature_global(signature.span()); - account.__validate_declare__(CLASS_HASH()); + account.__validate_declare__(class_hash); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_empty_signature() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); let empty_sig = array![]; - testing::set_signature(empty_sig.span()); + cheat_signature_global(empty_sig.span()); - account.__validate_declare__(CLASS_HASH()); + account.__validate_declare__(class_hash); } fn test_execute_with_version(version: Option) { - let data = SIGNED_TX_DATA(); - let account = setup_dispatcher(Option::Some(@data)); + let data = SIGNED_TX_DATA(KEY_PAIR); + let (account, _) = setup_dispatcher(Option::Some(@data)); let erc20 = deploy_erc20(account.contract_address, 1000); let recipient = contract_address_const::<0x123>(); @@ -216,12 +207,11 @@ fn test_execute_with_version(version: Option) { let call = Call { to: erc20.contract_address, selector: selectors::transfer, calldata: calldata.span() }; - let mut calls = array![]; - calls.append(call); + let calls = array![call]; // Handle version for test - if version.is_some() { - testing::set_version(version.unwrap()); + if let Option::Some(version) = version { + cheat_transaction_version_global(version); } // Execute @@ -239,7 +229,7 @@ fn test_execute_with_version(version: Option) { #[test] fn test_execute() { - test_execute_with_version(Option::None(())); + test_execute_with_version(Option::None); } #[test] @@ -253,7 +243,7 @@ fn test_execute_query_version() { } #[test] -#[should_panic(expected: ('Account: invalid tx version', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid tx version',))] fn test_execute_invalid_query_version() { test_execute_with_version(Option::Some(QUERY_OFFSET)); } @@ -264,7 +254,7 @@ fn test_execute_future_query_version() { } #[test] -#[should_panic(expected: ('Account: invalid tx version', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid tx version',))] fn test_execute_invalid_version() { test_execute_with_version(Option::Some(MIN_TRANSACTION_VERSION - 1)); } @@ -272,30 +262,29 @@ fn test_execute_invalid_version() { #[test] fn test_validate() { let calls = array![]; - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); let is_valid = account.__validate__(calls); assert_eq!(is_valid, starknet::VALIDATED); } #[test] -#[should_panic(expected: ('Account: invalid signature', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Account: invalid signature',))] fn test_validate_invalid() { let calls = array![]; - let mut data = SIGNED_TX_DATA(); - data.transaction_hash += 1; - let account = setup_dispatcher(Option::Some(@data)); + let mut data = SIGNED_TX_DATA(KEY_PAIR); + data.tx_hash += 1; + let (account, _) = setup_dispatcher(Option::Some(@data)); account.__validate__(calls); } #[test] fn test_multicall() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); let erc20 = deploy_erc20(account.contract_address, 1000); let recipient1 = contract_address_const::<0x123>(); let recipient2 = contract_address_const::<0x456>(); - let mut calls = array![]; // Craft call1 let mut calldata1 = array![]; @@ -316,8 +305,7 @@ fn test_multicall() { }; // Bundle calls and exeute - calls.append(call1); - calls.append(call2); + let calls = array![call1, call2]; let ret = account.__execute__(calls); // Assert that the transfers were successful @@ -338,8 +326,8 @@ fn test_multicall() { #[test] fn test_multicall_zero_calls() { - let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); - let mut calls = array![]; + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let calls = array![]; let response = account.__execute__(calls); assert!(response.is_empty()); @@ -350,10 +338,9 @@ fn test_multicall_zero_calls() { fn test_account_called_from_contract() { let state = setup(); let calls = array![]; - let caller = contract_address_const::<0x123>(); + let account_address = test_address(); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(caller); + start_cheat_caller_address(account_address, CALLER()); state.__execute__(calls); } @@ -365,33 +352,33 @@ fn test_account_called_from_contract() { #[test] fn test_public_key_setter_and_getter() { let mut state = COMPONENT_STATE(); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(ACCOUNT_ADDRESS()); + let account_address = test_address(); + start_cheat_caller_address(account_address, account_address); state._set_public_key(PUBKEY); - utils::drop_event(ACCOUNT_ADDRESS()); let public_key = state.get_public_key(); assert_eq!(public_key, PUBKEY); + let mut spy = spy_events(); // Set key - state.set_public_key(NEW_PUBKEY, get_accept_ownership_signature()); + let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); + state.set_public_key(PUBKEY_2, signature); - assert_event_owner_removed(ACCOUNT_ADDRESS(), PUBKEY); - assert_only_event_owner_added(ACCOUNT_ADDRESS(), NEW_PUBKEY); + spy.assert_event_owner_removed(account_address, PUBKEY); + spy.assert_only_event_owner_added(account_address, PUBKEY_2); let public_key = state.get_public_key(); - assert_eq!(public_key, NEW_PUBKEY); + assert_eq!(public_key, PUBKEY_2); } #[test] #[should_panic(expected: ('Account: unauthorized',))] fn test_public_key_setter_different_account() { let mut state = COMPONENT_STATE(); - let caller = contract_address_const::<0x123>(); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(caller); + let account_address = test_address(); + start_cheat_caller_address(account_address, CALLER()); - state.set_public_key(NEW_PUBKEY, array![].span()); + state.set_public_key(PUBKEY_2, array![].span()); } // @@ -401,33 +388,33 @@ fn test_public_key_setter_different_account() { #[test] fn test_public_key_setter_and_getter_camel() { let mut state = COMPONENT_STATE(); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(ACCOUNT_ADDRESS()); + let account_address = test_address(); + start_cheat_caller_address(account_address, account_address); state._set_public_key(PUBKEY); - utils::drop_event(ACCOUNT_ADDRESS()); let public_key = state.getPublicKey(); assert_eq!(public_key, PUBKEY); + let mut spy = spy_events(); // Set key - state.setPublicKey(NEW_PUBKEY, get_accept_ownership_signature()); + let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); + state.setPublicKey(PUBKEY_2, signature); - assert_event_owner_removed(ACCOUNT_ADDRESS(), PUBKEY); - assert_only_event_owner_added(ACCOUNT_ADDRESS(), NEW_PUBKEY); + spy.assert_event_owner_removed(account_address, PUBKEY); + spy.assert_only_event_owner_added(account_address, PUBKEY_2); let public_key = state.getPublicKey(); - assert_eq!(public_key, NEW_PUBKEY); + assert_eq!(public_key, PUBKEY_2); } #[test] #[should_panic(expected: ('Account: unauthorized',))] fn test_public_key_setter_different_account_camel() { let mut state = COMPONENT_STATE(); - let caller = contract_address_const::<0x123>(); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(caller); + let account_address = test_address(); + start_cheat_caller_address(account_address, CALLER()); - state.setPublicKey(NEW_PUBKEY, array![].span()); + state.setPublicKey(PUBKEY_2, array![].span()); } // @@ -438,9 +425,11 @@ fn test_public_key_setter_different_account_camel() { fn test_initializer() { let mut state = COMPONENT_STATE(); let mock_state = CONTRACT_STATE(); + let account_address = test_address(); + let mut spy = spy_events(); state.initializer(PUBKEY); - assert_only_event_owner_added(ZERO(), PUBKEY); + spy.assert_only_event_owner_added(account_address, PUBKEY); let public_key = state.get_public_key(); assert_eq!(public_key, PUBKEY); @@ -455,9 +444,9 @@ fn test_initializer() { #[test] fn test_assert_only_self_true() { let mut state = COMPONENT_STATE(); + let account_address = test_address(); + start_cheat_caller_address(account_address, account_address); - testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(ACCOUNT_ADDRESS()); state.assert_only_self(); } @@ -465,19 +454,19 @@ fn test_assert_only_self_true() { #[should_panic(expected: ('Account: unauthorized',))] fn test_assert_only_self_false() { let mut state = COMPONENT_STATE(); + let account_address = test_address(); + start_cheat_caller_address(account_address, OTHER()); - testing::set_contract_address(ACCOUNT_ADDRESS()); - let other = contract_address_const::<0x4567>(); - testing::set_caller_address(other); state.assert_only_self(); } #[test] fn test_assert_valid_new_owner() { let state = setup(); + let account_address = test_address(); + let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); - testing::set_contract_address(ACCOUNT_ADDRESS()); - state.assert_valid_new_owner(PUBKEY, NEW_PUBKEY, get_accept_ownership_signature()); + state.assert_valid_new_owner(PUBKEY, PUBKEY_2, signature); } @@ -485,68 +474,38 @@ fn test_assert_valid_new_owner() { #[should_panic(expected: ('Account: invalid signature',))] fn test_assert_valid_new_owner_invalid_signature() { let state = setup(); + let bad_signature = array!['BAD', 'SIGNATURE']; - testing::set_contract_address(ACCOUNT_ADDRESS()); - let bad_signature = array![ - 0x2ce8fbcf8a793ee5b2a57254dc96863b696698943e3bc7845285f3851336318, - 0x6009f5720649ff1ceb0aba44f85bec3572c81aecb9d2dada7c0cc70b791debe - ]; - state.assert_valid_new_owner(PUBKEY, NEW_PUBKEY, bad_signature.span()); + state.assert_valid_new_owner(PUBKEY, PUBKEY_2, bad_signature.span()); } #[test] fn test__is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; - - let mut good_signature = array![data.r, data.s]; - let mut bad_signature = array![0x987, 0x564]; - let mut invalid_length_signature = array![0x987]; + let data = SIGNED_TX_DATA(KEY_PAIR); state._set_public_key(data.public_key); - let is_valid = state._is_valid_signature(hash, good_signature.span()); - assert!(is_valid); + let good_signature = array![data.r, data.s]; + assert!(state._is_valid_signature(data.tx_hash, good_signature.span())); - let is_not_valid = !state._is_valid_signature(hash, bad_signature.span()); - assert!(is_not_valid); + let bad_signature = array!['BAD', 'SIGNATURE']; + assert!(!state._is_valid_signature(data.tx_hash, bad_signature.span())); - let is_not_valid = !state._is_valid_signature(hash, invalid_length_signature.span()); - assert!(is_not_valid); + let invalid_length_signature = array!['SINGLE_ELEMENT']; + assert!(!state._is_valid_signature(data.tx_hash, invalid_length_signature.span())); } #[test] fn test__set_public_key() { let mut state = COMPONENT_STATE(); + let mut spy = spy_events(); + let account_address = test_address(); + state._set_public_key(PUBKEY); - assert_only_event_owner_added(ZERO(), PUBKEY); + spy.assert_only_event_owner_added(account_address, PUBKEY); let public_key = state.get_public_key(); assert_eq!(public_key, PUBKEY); } - -// -// Helpers -// - -fn get_accept_ownership_signature() -> Span { - // 0x7b3d2ce38c132a36e692f6e809b518276d091513af3baf0e94ce2abceee3632 = - // PoseidonTrait::new() - // .update_with('StarkNet Message') - // .update_with('accept_ownership') - // .update_with(ACCOUNT_ADDRESS()) - // .update_with(PUBKEY) - // .finalize(); - - // This signature was computed using starknet js sdk from the following values: - // - private_key: '1234' - // - public_key: 0x26da8d11938b76025862be14fdb8b28438827f73e75e86f7bfa38b196951fa7 - // - msg_hash: 0x7b3d2ce38c132a36e692f6e809b518276d091513af3baf0e94ce2abceee3632 - array![ - 0x1ce8fbcf8a793ee5b2a57254dc96863b696698943e3bc7845285f3851336318, - 0x6009f5720649ff1ceb0aba44f85bec3572c81aecb9d2dada7c0cc70b791debe - ] - .span() -} From b6435b4216335350d39d5cae71e24121953e73a4 Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 19:49:16 +0200 Subject: [PATCH 04/13] Update dual account tests --- .../account/starknet/test_dual_account.cairo | 159 ++++++++---------- 1 file changed, 68 insertions(+), 91 deletions(-) diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index 003075b1f..5e0589cc8 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -6,17 +6,20 @@ use openzeppelin::tests::mocks::account_mocks::{ CamelAccountPanicMock, CamelAccountMock, SnakeAccountMock, SnakeAccountPanicMock }; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; -use openzeppelin::tests::utils::constants::{PUBKEY, NEW_PUBKEY}; +use openzeppelin::tests::utils::constants::ZERO; +use openzeppelin::tests::utils::signing::stark::{KEY_PAIR, KEY_PAIR_2, PUBKEY, PUBKEY_2}; use openzeppelin::tests::utils; -use starknet::testing; +use snforge_std::{EventSpy, declare, start_cheat_caller_address}; + +use super::common::get_accept_ownership_signature; // // Setup // fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { - let mut calldata = array![PUBKEY]; - let target = utils::deploy(SnakeAccountMock::TEST_CLASS_HASH, calldata); + let calldata = array![PUBKEY]; + let target = utils::declare_and_deploy("SnakeAccountMock", calldata); ( DualCaseAccount { contract_address: target }, AccountABIDispatcher { contract_address: target } @@ -24,8 +27,8 @@ fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { } fn setup_camel() -> (DualCaseAccount, AccountABIDispatcher) { - let mut calldata = array![PUBKEY]; - let target = utils::deploy(CamelAccountMock::TEST_CLASS_HASH, calldata); + let calldata = array![PUBKEY]; + let target = utils::declare_and_deploy("CamelAccountMock", calldata); ( DualCaseAccount { contract_address: target }, AccountABIDispatcher { contract_address: target } @@ -34,13 +37,13 @@ fn setup_camel() -> (DualCaseAccount, AccountABIDispatcher) { fn setup_non_account() -> DualCaseAccount { let calldata = array![]; - let target = utils::deploy(NonImplementingMock::TEST_CLASS_HASH, calldata); + let target = utils::declare_and_deploy("NonImplementingMock", calldata); DualCaseAccount { contract_address: target } } fn setup_account_panic() -> (DualCaseAccount, DualCaseAccount) { - let snake_target = utils::deploy(SnakeAccountPanicMock::TEST_CLASS_HASH, array![]); - let camel_target = utils::deploy(CamelAccountPanicMock::TEST_CLASS_HASH, array![]); + let snake_target = utils::declare_and_deploy("SnakeAccountPanicMock", array![]); + let camel_target = utils::declare_and_deploy("CamelAccountPanicMock", array![]); ( DualCaseAccount { contract_address: snake_target }, DualCaseAccount { contract_address: camel_target } @@ -51,19 +54,24 @@ fn setup_account_panic() -> (DualCaseAccount, DualCaseAccount) { // snake_case target // +const NEW_PUBKEY: felt252 = PUBKEY_2; + #[test] fn test_dual_set_public_key() { let (snake_dispatcher, target) = setup_snake(); + let signature = get_accept_ownership_signature( + snake_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + ); + start_cheat_caller_address(target.contract_address, target.contract_address); - testing::set_contract_address(snake_dispatcher.contract_address); - - snake_dispatcher.set_public_key(NEW_PUBKEY, get_accept_ownership_signature_snake()); + snake_dispatcher.set_public_key(NEW_PUBKEY, signature); let public_key = target.get_public_key(); assert_eq!(public_key, NEW_PUBKEY); } #[test] +#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_set_public_key() { let dispatcher = setup_non_account(); @@ -71,10 +79,10 @@ fn test_dual_no_set_public_key() { } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[should_panic(expected: ("Some error",))] fn test_dual_set_public_key_exists_and_panics() { - let (dispatcher, _) = setup_account_panic(); - dispatcher.set_public_key(NEW_PUBKEY, array![].span()); + let (snake_dispatcher, _) = setup_account_panic(); + snake_dispatcher.set_public_key(NEW_PUBKEY, array![].span()); } #[test] @@ -85,6 +93,7 @@ fn test_dual_get_public_key() { } #[test] +#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_get_public_key() { let dispatcher = setup_non_account(); @@ -92,28 +101,28 @@ fn test_dual_no_get_public_key() { } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[should_panic(expected: ("Some error",))] fn test_dual_get_public_key_exists_and_panics() { - let (dispatcher, _) = setup_account_panic(); - dispatcher.get_public_key(); + let (snake_dispatcher, _) = setup_account_panic(); + snake_dispatcher.get_public_key(); } #[test] fn test_dual_is_valid_signature() { let (snake_dispatcher, target) = setup_snake(); + let data = SIGNED_TX_DATA(KEY_PAIR_2); + let signature = get_accept_ownership_signature(target.contract_address, PUBKEY, KEY_PAIR_2); + start_cheat_caller_address(target.contract_address, target.contract_address); - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; - let mut signature = array![data.r, data.s]; + target.set_public_key(data.public_key, signature); - testing::set_contract_address(snake_dispatcher.contract_address); - target.set_public_key(data.public_key, get_accept_ownership_signature_snake()); - - let is_valid = snake_dispatcher.is_valid_signature(hash, signature); + let signature = array![data.r, data.s]; + let is_valid = snake_dispatcher.is_valid_signature(data.tx_hash, signature); assert_eq!(is_valid, starknet::VALIDATED); } #[test] +#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_is_valid_signature() { let hash = 0x0; @@ -124,13 +133,13 @@ fn test_dual_no_is_valid_signature() { } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[should_panic(expected: ("Some error",))] fn test_dual_is_valid_signature_exists_and_panics() { let hash = 0x0; let signature = array![]; - let (dispatcher, _) = setup_account_panic(); - dispatcher.is_valid_signature(hash, signature); + let (snake_dispatcher, _) = setup_account_panic(); + snake_dispatcher.is_valid_signature(hash, signature); } #[test] @@ -141,6 +150,7 @@ fn test_dual_supports_interface() { } #[test] +#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_supports_interface() { let dispatcher = setup_non_account(); @@ -148,10 +158,10 @@ fn test_dual_no_supports_interface() { } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[should_panic(expected: ("Some error",))] fn test_dual_supports_interface_exists_and_panics() { - let (dispatcher, _) = setup_account_panic(); - dispatcher.supports_interface(ISRC5_ID); + let (snake_dispatcher, _) = setup_account_panic(); + snake_dispatcher.supports_interface(ISRC5_ID); } // @@ -159,24 +169,30 @@ fn test_dual_supports_interface_exists_and_panics() { // #[test] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail fn test_dual_setPublicKey() { let (camel_dispatcher, target) = setup_camel(); + let signature = get_accept_ownership_signature( + camel_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + ); + start_cheat_caller_address(target.contract_address, target.contract_address); - testing::set_contract_address(camel_dispatcher.contract_address); - camel_dispatcher.set_public_key(NEW_PUBKEY, get_accept_ownership_signature_camel()); + camel_dispatcher.set_public_key(NEW_PUBKEY, signature); let public_key = target.getPublicKey(); assert_eq!(public_key, NEW_PUBKEY); } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[should_panic(expected: ("Some error",))] fn test_dual_setPublicKey_exists_and_panics() { - let (_, dispatcher) = setup_account_panic(); - dispatcher.set_public_key(NEW_PUBKEY, array![].span()); + let (_, camel_dispatcher) = setup_account_panic(); + camel_dispatcher.set_public_key(NEW_PUBKEY, array![].span()); } #[test] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); let public_key = camel_dispatcher.get_public_key(); @@ -184,77 +200,38 @@ fn test_dual_getPublicKey() { } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[should_panic(expected: ("Some error",))] fn test_dual_getPublicKey_exists_and_panics() { - let (_, dispatcher) = setup_account_panic(); - dispatcher.get_public_key(); + let (_, camel_dispatcher) = setup_account_panic(); + camel_dispatcher.get_public_key(); } #[test] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail fn test_dual_isValidSignature() { let (camel_dispatcher, target) = setup_camel(); - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; + let data = SIGNED_TX_DATA(KEY_PAIR_2); let signature = array![data.r, data.s]; + start_cheat_caller_address(target.contract_address, target.contract_address); + let accept_signature = get_accept_ownership_signature( + camel_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + ); - testing::set_contract_address(camel_dispatcher.contract_address); - target.setPublicKey(data.public_key, get_accept_ownership_signature_camel()); + target.setPublicKey(data.public_key, accept_signature); - let is_valid = camel_dispatcher.is_valid_signature(hash, signature); + let is_valid = camel_dispatcher.is_valid_signature(data.tx_hash, signature); assert_eq!(is_valid, starknet::VALIDATED); } #[test] -#[should_panic(expected: ("Some error", 'ENTRYPOINT_FAILED',))] +#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[should_panic(expected: ("Some error",))] fn test_dual_isValidSignature_exists_and_panics() { let hash = 0x0; let signature = array![]; - let (_, dispatcher) = setup_account_panic(); - dispatcher.is_valid_signature(hash, signature); -} - -// -// Helpers -// - -fn get_accept_ownership_signature_snake() -> Span { - // 0x26a14ae81fad8adcb81337e49fd68ac44d508cfca09c4a6167c71e85759e98d = - // PoseidonTrait::new() - // .update_with('StarkNet Message') - // .update_with('accept_ownership') - // .update_with(snake_dispatcher.contract_address) - // .update_with(PUBKEY) - // .finalize(); - - // This signature was computed using starknet js sdk from the following values: - // - private_key: '1234' - // - public_key: 0x26da8d11938b76025862be14fdb8b28438827f73e75e86f7bfa38b196951fa7 - // - msg_hash: 0x26a14ae81fad8adcb81337e49fd68ac44d508cfca09c4a6167c71e85759e98d - array![ - 0x14de5d99a3a43bc7e8e0ddf8ff72fab172798ac3dd7bd858d4f6f489a2a4bcb, - 0x88dd5cb7f27a8932a5c6d226b486830e952e3d9e78996e7ca2315afa9c0be2 - ] - .span() -} - -fn get_accept_ownership_signature_camel() -> Span { - // 0x1e2acd787c2778bebfbf4d8770fdc0834db1576b5fd190f35afae5a6f7f469f = - // PoseidonTrait::new() - // .update_with('StarkNet Message') - // .update_with('accept_ownership') - // .update_with(camel_dispatcher.contract_address) - // .update_with(PUBKEY) - // .finalize(); - - // This signature was computed using starknet js sdk from the following values: - // - private_key: '1234' - // - public_key: 0x26da8d11938b76025862be14fdb8b28438827f73e75e86f7bfa38b196951fa7 - // - msg_hash: 0x1e2acd787c2778bebfbf4d8770fdc0834db1576b5fd190f35afae5a6f7f469f - array![ - 0xd0eab69db72a46ee0fb39c833cfaec035ca809eb884285d0e603262f15e0dd, - 0x768e984a96d5f731c3cf1660c5a00f508f3d99d8240bf3ba4c3e69561a545d1 - ] - .span() + let (_, camel_dispatcher) = setup_account_panic(); + camel_dispatcher.is_valid_signature(hash, signature); } From 48332410c6280db3960840286681773f80c7341a Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 20:12:55 +0200 Subject: [PATCH 05/13] Run linter --- src/tests/account/starknet/test_account.cairo | 4 +++- src/tests/account/test_signature.cairo | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index e22cc0d72..b661acbd2 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -20,7 +20,9 @@ use openzeppelin::utils::serde::SerializedAppend; use snforge_std::signature::SignerTrait; use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; use snforge_std::{EventSpy, spy_events, declare, test_address, start_cheat_caller_address}; -use snforge_std::{cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global}; +use snforge_std::{ + cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global +}; use starknet::account::Call; use starknet::contract_address_const; use starknet::{ContractAddress, ClassHash}; diff --git a/src/tests/account/test_signature.cairo b/src/tests/account/test_signature.cairo index ac533187a..1d7560942 100644 --- a/src/tests/account/test_signature.cairo +++ b/src/tests/account/test_signature.cairo @@ -1,7 +1,7 @@ use openzeppelin::account::utils::signature::{is_valid_stark_signature, is_valid_eth_signature}; +use openzeppelin::tests::utils::signing::{stark, secp256k1}; use starknet::secp256_trait::Secp256Trait; use starknet::secp256k1::Secp256k1Point; -use openzeppelin::tests::utils::signing::{stark, secp256k1}; use super::ethereum::common::SIGNED_TX_DATA as eth_signature_data; use super::starknet::common::SIGNED_TX_DATA as stark_signature_data; From 943a101d9cd1f592358863e492dcb94f13a8180a Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 12 Jul 2024 20:31:27 +0200 Subject: [PATCH 06/13] Run linter --- src/tests/account/starknet/test_account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index b661acbd2..477f90f0a 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -19,10 +19,10 @@ use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; use snforge_std::signature::SignerTrait; use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; -use snforge_std::{EventSpy, spy_events, declare, test_address, start_cheat_caller_address}; use snforge_std::{ cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global }; +use snforge_std::{EventSpy, spy_events, declare, test_address, start_cheat_caller_address}; use starknet::account::Call; use starknet::contract_address_const; use starknet::{ContractAddress, ClassHash}; From d764b56742bfd2767964460d1a121f77adc6f106 Mon Sep 17 00:00:00 2001 From: immrsd Date: Tue, 23 Jul 2024 15:43:53 +0200 Subject: [PATCH 07/13] Fix review issues --- src/tests/account/ethereum/common.cairo | 2 -- src/tests/account/starknet/test_dual_account.cairo | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/tests/account/ethereum/common.cairo b/src/tests/account/ethereum/common.cairo index 130fe35ea..e4caf0839 100644 --- a/src/tests/account/ethereum/common.cairo +++ b/src/tests/account/ethereum/common.cairo @@ -81,7 +81,6 @@ pub(crate) impl AccountSpyHelpersImpl of AccountSpyHelpers { let new_owner_guid = get_guid_from_public_key(public_key); let expected = EthAccountComponent::Event::OwnerAdded(OwnerAdded { new_owner_guid }); self.assert_emitted_single(contract, expected); - // TODO: Add check for indexed keys } fn assert_only_event_owner_added( @@ -99,7 +98,6 @@ pub(crate) impl AccountSpyHelpersImpl of AccountSpyHelpers { OwnerRemoved { removed_owner_guid } ); self.assert_emitted_single(contract, expected); - // TODO: Add check for indexed keys } } diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index 5e0589cc8..251b39a20 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -6,10 +6,9 @@ use openzeppelin::tests::mocks::account_mocks::{ CamelAccountPanicMock, CamelAccountMock, SnakeAccountMock, SnakeAccountPanicMock }; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; -use openzeppelin::tests::utils::constants::ZERO; use openzeppelin::tests::utils::signing::stark::{KEY_PAIR, KEY_PAIR_2, PUBKEY, PUBKEY_2}; use openzeppelin::tests::utils; -use snforge_std::{EventSpy, declare, start_cheat_caller_address}; +use snforge_std::{declare, start_cheat_caller_address}; use super::common::get_accept_ownership_signature; From 965f8a7d3d23d3948a48c84825cde23b150ce78d Mon Sep 17 00:00:00 2001 From: immrsd Date: Tue, 23 Jul 2024 15:46:33 +0200 Subject: [PATCH 08/13] Update ignore reason messages --- .../account/starknet/test_dual_account.cairo | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index 251b39a20..32cf7c2e5 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -70,7 +70,7 @@ fn test_dual_set_public_key() { } #[test] -#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message +#[ignore] // REASON: should_panic attribute not fit for complex panic messages. #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_set_public_key() { let dispatcher = setup_non_account(); @@ -92,7 +92,7 @@ fn test_dual_get_public_key() { } #[test] -#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message +#[ignore] // REASON: should_panic attribute not fit for complex panic messages. #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_get_public_key() { let dispatcher = setup_non_account(); @@ -121,7 +121,7 @@ fn test_dual_is_valid_signature() { } #[test] -#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message +#[ignore] // REASON: should_panic attribute not fit for complex panic messages. #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_is_valid_signature() { let hash = 0x0; @@ -149,7 +149,7 @@ fn test_dual_supports_interface() { } #[test] -#[ignore] // REASON: inconsistent ENTRYPOINT_NOT_FOUND panic message +#[ignore] // REASON: should_panic attribute not fit for complex panic messages. #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_supports_interface() { let dispatcher = setup_non_account(); @@ -168,7 +168,7 @@ fn test_dual_supports_interface_exists_and_panics() { // #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_setPublicKey() { let (camel_dispatcher, target) = setup_camel(); let signature = get_accept_ownership_signature( @@ -183,7 +183,7 @@ fn test_dual_setPublicKey() { } #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. #[should_panic(expected: ("Some error",))] fn test_dual_setPublicKey_exists_and_panics() { let (_, camel_dispatcher) = setup_account_panic(); @@ -191,7 +191,7 @@ fn test_dual_setPublicKey_exists_and_panics() { } #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); let public_key = camel_dispatcher.get_public_key(); @@ -199,7 +199,7 @@ fn test_dual_getPublicKey() { } #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. #[should_panic(expected: ("Some error",))] fn test_dual_getPublicKey_exists_and_panics() { let (_, camel_dispatcher) = setup_account_panic(); @@ -207,7 +207,7 @@ fn test_dual_getPublicKey_exists_and_panics() { } #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_isValidSignature() { let (camel_dispatcher, target) = setup_camel(); @@ -225,7 +225,7 @@ fn test_dual_isValidSignature() { } #[test] -#[ignore] // REASON: lack of error handling causes try_selector_with_fallback to fail +#[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. #[should_panic(expected: ("Some error",))] fn test_dual_isValidSignature_exists_and_panics() { let hash = 0x0; From f81ebf03997a845c567d280b2e3d1b0ee1061277 Mon Sep 17 00:00:00 2001 From: immrsd Date: Tue, 23 Jul 2024 15:47:44 +0200 Subject: [PATCH 09/13] Run linter --- src/tests.cairo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests.cairo b/src/tests.cairo index 1a4166a74..fab03b247 100644 --- a/src/tests.cairo +++ b/src/tests.cairo @@ -1,8 +1,8 @@ #[cfg(test)] -mod account; -#[cfg(test)] mod access; #[cfg(test)] +mod account; +#[cfg(test)] mod cryptography; #[cfg(test)] mod introspection; From 29035c50fd3ed984aa40e2a34541622f5f4dc5f9 Mon Sep 17 00:00:00 2001 From: immrsd Date: Wed, 24 Jul 2024 18:30:04 +0200 Subject: [PATCH 10/13] Support eth account tests changes --- src/tests/account/starknet.cairo | 6 +- src/tests/account/starknet/common.cairo | 10 +- src/tests/account/starknet/test_account.cairo | 145 +++++++++--------- .../account/starknet/test_dual_account.cairo | 78 ++++++---- 4 files changed, 127 insertions(+), 112 deletions(-) diff --git a/src/tests/account/starknet.cairo b/src/tests/account/starknet.cairo index f426692e3..8c3e80c69 100644 --- a/src/tests/account/starknet.cairo +++ b/src/tests/account/starknet.cairo @@ -1,5 +1,3 @@ pub(crate) mod common; -// mod test_account; -// mod test_dual_account; - - +mod test_account; +mod test_dual_account; diff --git a/src/tests/account/starknet/common.cairo b/src/tests/account/starknet/common.cairo index 80e959ad5..a47594b14 100644 --- a/src/tests/account/starknet/common.cairo +++ b/src/tests/account/starknet/common.cairo @@ -2,14 +2,14 @@ use core::hash::{HashStateTrait, HashStateExTrait}; use core::poseidon::PoseidonTrait; use openzeppelin::account::AccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::AccountComponent; -use openzeppelin::tests::utils::constants::{NAME, SYMBOL}; +use openzeppelin::tests::utils::constants::{NAME, SYMBOL, TRANSACTION_HASH}; use openzeppelin::tests::utils::events::EventSpyExt; -use openzeppelin::tests::utils::signing::stark::StarkKeyPair; +use openzeppelin::tests::utils::signing::StarkKeyPair; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::serde::SerializedAppend; use snforge_std::EventSpy; -use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; +use snforge_std::signature::stark_curve::StarkCurveSignerImpl; use starknet::ContractAddress; #[derive(Drop)] @@ -18,11 +18,11 @@ pub(crate) struct SignedTransactionData { pub(crate) public_key: felt252, pub(crate) tx_hash: felt252, pub(crate) r: felt252, - pub(crate) s: felt252, + pub(crate) s: felt252 } pub(crate) fn SIGNED_TX_DATA(key_pair: StarkKeyPair) -> SignedTransactionData { - let tx_hash = 'TRANSACTION_HASH'; + let tx_hash = TRANSACTION_HASH; let (r, s) = key_pair.sign(tx_hash).unwrap(); SignedTransactionData { private_key: key_pair.secret_key, public_key: key_pair.public_key, tx_hash, r, s diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index 477f90f0a..524f4dffd 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -10,9 +10,7 @@ use openzeppelin::tests::mocks::account_mocks::DualCaseAccountMock; use openzeppelin::tests::utils::constants::{ SALT, ZERO, OTHER, CALLER, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION }; -use openzeppelin::tests::utils::signing::stark::{ - KEY_PAIR, KEY_PAIR_2, PUBKEY, PUBKEY_2, PRIVATE_KEY -}; +use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; @@ -46,7 +44,8 @@ fn COMPONENT_STATE() -> ComponentState { fn setup() -> ComponentState { let mut state = COMPONENT_STATE(); - state.initializer(PUBKEY); + let key_pair = KEY_PAIR(); + state.initializer(key_pair.public_key); state } @@ -57,7 +56,7 @@ fn setup_dispatcher(data: Option<@SignedTransactionData>) -> (AccountABIDispatch cheat_transaction_hash_global(*data.tx_hash); array![*data.public_key] } else { - array![PUBKEY] + array![KEY_PAIR().public_key] }; let address = utils::deploy(contract_class, calldata); @@ -76,16 +75,15 @@ fn setup_dispatcher(data: Option<@SignedTransactionData>) -> (AccountABIDispatch #[test] fn test_is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR); - - let mut good_signature = array![data.r, data.s]; - let mut bad_signature = array![0x987, 0x564]; + let data = SIGNED_TX_DATA(KEY_PAIR()); state._set_public_key(data.public_key); + let good_signature = array![data.r, data.s]; let is_valid = state.is_valid_signature(data.tx_hash, good_signature); assert_eq!(is_valid, starknet::VALIDATED); + let bad_signature = array!['BAD', 'SIGNATURE']; let is_valid = state.is_valid_signature(data.tx_hash, bad_signature); assert!(is_valid.is_zero(), "Should reject invalid signature"); } @@ -93,7 +91,7 @@ fn test_is_valid_signature() { #[test] fn test_isValidSignature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR); + let data = SIGNED_TX_DATA(KEY_PAIR()); state._set_public_key(data.public_key); @@ -112,48 +110,52 @@ fn test_isValidSignature() { #[test] fn test_validate_deploy() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); // `__validate_deploy__` does not directly use the passed arguments. Their // values are already integrated in the tx hash. The passed arguments in this // testing context are decoupled from the signature and have no effect on the test. - let is_valid = account.__validate_deploy__(class_hash, SALT, PUBKEY); + let is_valid = account.__validate_deploy__(class_hash, SALT, key_pair.public_key); assert_eq!(is_valid, starknet::VALIDATED); } #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_invalid_signature_data() { - let mut data = SIGNED_TX_DATA(KEY_PAIR); + let key_pair = KEY_PAIR(); + let mut data = SIGNED_TX_DATA(key_pair); data.tx_hash += 1; let (account, class_hash) = setup_dispatcher(Option::Some(@data)); - account.__validate_deploy__(class_hash, SALT, PUBKEY); + account.__validate_deploy__(class_hash, SALT, key_pair.public_key); } #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_invalid_signature_length() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); - let signature = array![0x1]; - cheat_signature_global(signature.span()); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); + let short_sig = array!['SHORT_SIGNATURE']; + cheat_signature_global(short_sig.span()); - account.__validate_deploy__(class_hash, SALT, PUBKEY); + account.__validate_deploy__(class_hash, SALT, key_pair.public_key); } #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_empty_signature() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); let empty_sig = array![]; cheat_signature_global(empty_sig.span()); - account.__validate_deploy__(class_hash, SALT, PUBKEY); + account.__validate_deploy__(class_hash, SALT, key_pair.public_key); } #[test] fn test_validate_declare() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); // `__validate_declare__` does not directly use the class_hash argument. Its // value is already integrated in the tx hash. The class_hash argument in this @@ -165,7 +167,7 @@ fn test_validate_declare() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_data() { - let mut data = SIGNED_TX_DATA(KEY_PAIR); + let mut data = SIGNED_TX_DATA(KEY_PAIR()); data.tx_hash += 1; let (account, class_hash) = setup_dispatcher(Option::Some(@data)); @@ -175,11 +177,9 @@ fn test_validate_declare_invalid_signature_data() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_length() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); - let mut signature = array![]; - - signature.append(0x1); - cheat_signature_global(signature.span()); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); + let short_sig = array!['SHORT_SIGNATURE']; + cheat_signature_global(short_sig.span()); account.__validate_declare__(class_hash); } @@ -187,16 +187,15 @@ fn test_validate_declare_invalid_signature_length() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_empty_signature() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); let empty_sig = array![]; - cheat_signature_global(empty_sig.span()); account.__validate_declare__(class_hash); } fn test_execute_with_version(version: Option) { - let data = SIGNED_TX_DATA(KEY_PAIR); + let data = SIGNED_TX_DATA(KEY_PAIR()); let (account, _) = setup_dispatcher(Option::Some(@data)); let erc20 = deploy_erc20(account.contract_address, 1000); let recipient = contract_address_const::<0x123>(); @@ -264,7 +263,7 @@ fn test_execute_invalid_version() { #[test] fn test_validate() { let calls = array![]; - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); let is_valid = account.__validate__(calls); assert_eq!(is_valid, starknet::VALIDATED); @@ -274,7 +273,7 @@ fn test_validate() { #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_invalid() { let calls = array![]; - let mut data = SIGNED_TX_DATA(KEY_PAIR); + let mut data = SIGNED_TX_DATA(KEY_PAIR()); data.tx_hash += 1; let (account, _) = setup_dispatcher(Option::Some(@data)); @@ -283,7 +282,7 @@ fn test_validate_invalid() { #[test] fn test_multicall() { - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); let erc20 = deploy_erc20(account.contract_address, 1000); let recipient1 = contract_address_const::<0x123>(); let recipient2 = contract_address_const::<0x456>(); @@ -328,7 +327,7 @@ fn test_multicall() { #[test] fn test_multicall_zero_calls() { - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR))); + let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); let calls = array![]; let response = account.__execute__(calls); @@ -355,22 +354,23 @@ fn test_account_called_from_contract() { fn test_public_key_setter_and_getter() { let mut state = COMPONENT_STATE(); let account_address = test_address(); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let new_public_key = new_key_pair.public_key; start_cheat_caller_address(account_address, account_address); - state._set_public_key(PUBKEY); - let public_key = state.get_public_key(); - assert_eq!(public_key, PUBKEY); - let mut spy = spy_events(); + state._set_public_key(public_key); + assert_eq!(state.get_public_key(), public_key); // Set key - let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); - state.set_public_key(PUBKEY_2, signature); + let mut spy = spy_events(); + let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); + state.set_public_key(new_public_key, signature); - spy.assert_event_owner_removed(account_address, PUBKEY); - spy.assert_only_event_owner_added(account_address, PUBKEY_2); + spy.assert_event_owner_removed(account_address, public_key); + spy.assert_only_event_owner_added(account_address, new_public_key); - let public_key = state.get_public_key(); - assert_eq!(public_key, PUBKEY_2); + assert_eq!(state.get_public_key(), new_public_key); } #[test] @@ -378,9 +378,10 @@ fn test_public_key_setter_and_getter() { fn test_public_key_setter_different_account() { let mut state = COMPONENT_STATE(); let account_address = test_address(); + let new_public_key = KEY_PAIR_2().public_key; start_cheat_caller_address(account_address, CALLER()); - state.set_public_key(PUBKEY_2, array![].span()); + state.set_public_key(new_public_key, array![].span()); } // @@ -391,22 +392,23 @@ fn test_public_key_setter_different_account() { fn test_public_key_setter_and_getter_camel() { let mut state = COMPONENT_STATE(); let account_address = test_address(); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let new_public_key = new_key_pair.public_key; start_cheat_caller_address(account_address, account_address); - state._set_public_key(PUBKEY); - let public_key = state.getPublicKey(); - assert_eq!(public_key, PUBKEY); - let mut spy = spy_events(); + state._set_public_key(public_key); + assert_eq!(state.getPublicKey(), public_key); // Set key - let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); - state.setPublicKey(PUBKEY_2, signature); + let mut spy = spy_events(); + let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); + state.setPublicKey(new_public_key, signature); - spy.assert_event_owner_removed(account_address, PUBKEY); - spy.assert_only_event_owner_added(account_address, PUBKEY_2); + spy.assert_event_owner_removed(account_address, public_key); + spy.assert_only_event_owner_added(account_address, new_public_key); - let public_key = state.getPublicKey(); - assert_eq!(public_key, PUBKEY_2); + assert_eq!(state.getPublicKey(), new_public_key); } #[test] @@ -414,9 +416,10 @@ fn test_public_key_setter_and_getter_camel() { fn test_public_key_setter_different_account_camel() { let mut state = COMPONENT_STATE(); let account_address = test_address(); + let new_public_key = KEY_PAIR_2().public_key; start_cheat_caller_address(account_address, CALLER()); - state.setPublicKey(PUBKEY_2, array![].span()); + state.setPublicKey(new_public_key, array![].span()); } // @@ -428,13 +431,13 @@ fn test_initializer() { let mut state = COMPONENT_STATE(); let mock_state = CONTRACT_STATE(); let account_address = test_address(); + let public_key = KEY_PAIR().public_key; let mut spy = spy_events(); - state.initializer(PUBKEY); - spy.assert_only_event_owner_added(account_address, PUBKEY); + state.initializer(public_key); + spy.assert_only_event_owner_added(account_address, public_key); - let public_key = state.get_public_key(); - assert_eq!(public_key, PUBKEY); + assert_eq!(state.get_public_key(), public_key); let supports_isrc5 = mock_state.supports_interface(ISRC5_ID); assert!(supports_isrc5); @@ -466,9 +469,12 @@ fn test_assert_only_self_false() { fn test_assert_valid_new_owner() { let state = setup(); let account_address = test_address(); - let signature = get_accept_ownership_signature(account_address, PUBKEY, KEY_PAIR_2); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let new_public_key = new_key_pair.public_key; + let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); - state.assert_valid_new_owner(PUBKEY, PUBKEY_2, signature); + state.assert_valid_new_owner(public_key, new_public_key, signature); } @@ -476,15 +482,17 @@ fn test_assert_valid_new_owner() { #[should_panic(expected: ('Account: invalid signature',))] fn test_assert_valid_new_owner_invalid_signature() { let state = setup(); + let public_key = KEY_PAIR().public_key; + let new_public_key = KEY_PAIR_2().public_key; let bad_signature = array!['BAD', 'SIGNATURE']; - state.assert_valid_new_owner(PUBKEY, PUBKEY_2, bad_signature.span()); + state.assert_valid_new_owner(public_key, new_public_key, bad_signature.span()); } #[test] fn test__is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR); + let data = SIGNED_TX_DATA(KEY_PAIR()); state._set_public_key(data.public_key); @@ -502,12 +510,11 @@ fn test__is_valid_signature() { fn test__set_public_key() { let mut state = COMPONENT_STATE(); let mut spy = spy_events(); + let public_key = KEY_PAIR().public_key; let account_address = test_address(); - state._set_public_key(PUBKEY); - - spy.assert_only_event_owner_added(account_address, PUBKEY); + state._set_public_key(public_key); - let public_key = state.get_public_key(); - assert_eq!(public_key, PUBKEY); + spy.assert_only_event_owner_added(account_address, public_key); + assert_eq!(state.get_public_key(), public_key); } diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index 32cf7c2e5..78e964677 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -6,7 +6,8 @@ use openzeppelin::tests::mocks::account_mocks::{ CamelAccountPanicMock, CamelAccountMock, SnakeAccountMock, SnakeAccountPanicMock }; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; -use openzeppelin::tests::utils::signing::stark::{KEY_PAIR, KEY_PAIR_2, PUBKEY, PUBKEY_2}; +use openzeppelin::tests::utils::constants::TRANSACTION_HASH; +use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin::tests::utils; use snforge_std::{declare, start_cheat_caller_address}; @@ -17,7 +18,8 @@ use super::common::get_accept_ownership_signature; // fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { - let calldata = array![PUBKEY]; + let key_pair = KEY_PAIR(); + let calldata = array![key_pair.public_key]; let target = utils::declare_and_deploy("SnakeAccountMock", calldata); ( DualCaseAccount { contract_address: target }, @@ -26,7 +28,8 @@ fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { } fn setup_camel() -> (DualCaseAccount, AccountABIDispatcher) { - let calldata = array![PUBKEY]; + let key_pair = KEY_PAIR(); + let calldata = array![key_pair.public_key]; let target = utils::declare_and_deploy("CamelAccountMock", calldata); ( DualCaseAccount { contract_address: target }, @@ -53,20 +56,20 @@ fn setup_account_panic() -> (DualCaseAccount, DualCaseAccount) { // snake_case target // -const NEW_PUBKEY: felt252 = PUBKEY_2; - #[test] fn test_dual_set_public_key() { let (snake_dispatcher, target) = setup_snake(); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let new_public_key = new_key_pair.public_key; let signature = get_accept_ownership_signature( - snake_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + snake_dispatcher.contract_address, public_key, new_key_pair ); start_cheat_caller_address(target.contract_address, target.contract_address); - snake_dispatcher.set_public_key(NEW_PUBKEY, signature); + snake_dispatcher.set_public_key(new_public_key, signature); - let public_key = target.get_public_key(); - assert_eq!(public_key, NEW_PUBKEY); + assert_eq!(target.get_public_key(), new_public_key); } #[test] @@ -74,21 +77,23 @@ fn test_dual_set_public_key() { #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_set_public_key() { let dispatcher = setup_non_account(); - dispatcher.set_public_key(NEW_PUBKEY, array![].span()); + let new_public_key = KEY_PAIR_2().public_key; + dispatcher.set_public_key(new_public_key, array![].span()); } #[test] #[should_panic(expected: ("Some error",))] fn test_dual_set_public_key_exists_and_panics() { let (snake_dispatcher, _) = setup_account_panic(); - snake_dispatcher.set_public_key(NEW_PUBKEY, array![].span()); + let new_public_key = KEY_PAIR_2().public_key; + snake_dispatcher.set_public_key(new_public_key, array![].span()); } #[test] fn test_dual_get_public_key() { let (snake_dispatcher, _) = setup_snake(); - let public_key = snake_dispatcher.get_public_key(); - assert_eq!(public_key, PUBKEY); + let expected_public_key = KEY_PAIR().public_key; + assert_eq!(snake_dispatcher.get_public_key(), expected_public_key); } #[test] @@ -109,12 +114,16 @@ fn test_dual_get_public_key_exists_and_panics() { #[test] fn test_dual_is_valid_signature() { let (snake_dispatcher, target) = setup_snake(); - let data = SIGNED_TX_DATA(KEY_PAIR_2); - let signature = get_accept_ownership_signature(target.contract_address, PUBKEY, KEY_PAIR_2); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let data = SIGNED_TX_DATA(new_key_pair); start_cheat_caller_address(target.contract_address, target.contract_address); + let accept_signature = get_accept_ownership_signature( + snake_dispatcher.contract_address, public_key, new_key_pair + ); - target.set_public_key(data.public_key, signature); - + target.set_public_key(data.public_key, accept_signature); + let signature = array![data.r, data.s]; let is_valid = snake_dispatcher.is_valid_signature(data.tx_hash, signature); assert_eq!(is_valid, starknet::VALIDATED); @@ -124,21 +133,19 @@ fn test_dual_is_valid_signature() { #[ignore] // REASON: should_panic attribute not fit for complex panic messages. #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_is_valid_signature() { - let hash = 0x0; let signature = array![]; let dispatcher = setup_non_account(); - dispatcher.is_valid_signature(hash, signature); + dispatcher.is_valid_signature(TRANSACTION_HASH, signature); } #[test] #[should_panic(expected: ("Some error",))] fn test_dual_is_valid_signature_exists_and_panics() { - let hash = 0x0; let signature = array![]; - let (snake_dispatcher, _) = setup_account_panic(); - snake_dispatcher.is_valid_signature(hash, signature); + + snake_dispatcher.is_valid_signature(TRANSACTION_HASH, signature); } #[test] @@ -171,15 +178,17 @@ fn test_dual_supports_interface_exists_and_panics() { #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_setPublicKey() { let (camel_dispatcher, target) = setup_camel(); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let new_public_key = new_key_pair.public_key; let signature = get_accept_ownership_signature( - camel_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + camel_dispatcher.contract_address, public_key, new_key_pair ); start_cheat_caller_address(target.contract_address, target.contract_address); - camel_dispatcher.set_public_key(NEW_PUBKEY, signature); + camel_dispatcher.set_public_key(new_public_key, signature); - let public_key = target.getPublicKey(); - assert_eq!(public_key, NEW_PUBKEY); + assert_eq!(target.getPublicKey(), new_public_key); } #[test] @@ -187,15 +196,16 @@ fn test_dual_setPublicKey() { #[should_panic(expected: ("Some error",))] fn test_dual_setPublicKey_exists_and_panics() { let (_, camel_dispatcher) = setup_account_panic(); - camel_dispatcher.set_public_key(NEW_PUBKEY, array![].span()); + let new_public_key = KEY_PAIR_2().public_key; + camel_dispatcher.set_public_key(new_public_key, array![].span()); } #[test] #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); - let public_key = camel_dispatcher.get_public_key(); - assert_eq!(public_key, PUBKEY); + let expected_public_key = KEY_PAIR().public_key; + assert_eq!(camel_dispatcher.get_public_key(), expected_public_key); } #[test] @@ -210,12 +220,13 @@ fn test_dual_getPublicKey_exists_and_panics() { #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_isValidSignature() { let (camel_dispatcher, target) = setup_camel(); - - let data = SIGNED_TX_DATA(KEY_PAIR_2); + let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); + let data = SIGNED_TX_DATA(new_key_pair); let signature = array![data.r, data.s]; start_cheat_caller_address(target.contract_address, target.contract_address); let accept_signature = get_accept_ownership_signature( - camel_dispatcher.contract_address, PUBKEY, KEY_PAIR_2 + camel_dispatcher.contract_address, public_key, new_key_pair ); target.setPublicKey(data.public_key, accept_signature); @@ -228,9 +239,8 @@ fn test_dual_isValidSignature() { #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. #[should_panic(expected: ("Some error",))] fn test_dual_isValidSignature_exists_and_panics() { - let hash = 0x0; let signature = array![]; let (_, camel_dispatcher) = setup_account_panic(); - camel_dispatcher.is_valid_signature(hash, signature); + camel_dispatcher.is_valid_signature(TRANSACTION_HASH, signature); } From 60e87294520293a3b2f6218bf340d56bd2d8dfdf Mon Sep 17 00:00:00 2001 From: immrsd Date: Wed, 24 Jul 2024 18:36:56 +0200 Subject: [PATCH 11/13] Run linter --- src/tests/account/starknet/test_account.cairo | 2 +- src/tests/account/starknet/test_dual_account.cairo | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index 524f4dffd..ed27c4a57 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -7,10 +7,10 @@ use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDisp use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::account_mocks::DualCaseAccountMock; +use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin::tests::utils::constants::{ SALT, ZERO, OTHER, CALLER, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION }; -use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index 78e964677..f81b9eedb 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -123,7 +123,7 @@ fn test_dual_is_valid_signature() { ); target.set_public_key(data.public_key, accept_signature); - + let signature = array![data.r, data.s]; let is_valid = snake_dispatcher.is_valid_signature(data.tx_hash, signature); assert_eq!(is_valid, starknet::VALIDATED); From e063b90c1c01a5db5e182da6c4bebf96bd4ff809 Mon Sep 17 00:00:00 2001 From: immrsd Date: Thu, 25 Jul 2024 09:27:55 +0200 Subject: [PATCH 12/13] Improve setup functions, remove unused imports --- src/tests/account/starknet/common.cairo | 9 +- src/tests/account/starknet/test_account.cairo | 165 ++++++++++-------- .../account/starknet/test_dual_account.cairo | 80 ++++----- src/tests/account/test_signature.cairo | 15 +- 4 files changed, 134 insertions(+), 135 deletions(-) diff --git a/src/tests/account/starknet/common.cairo b/src/tests/account/starknet/common.cairo index a47594b14..26554ba33 100644 --- a/src/tests/account/starknet/common.cairo +++ b/src/tests/account/starknet/common.cairo @@ -6,7 +6,7 @@ use openzeppelin::tests::utils::constants::{NAME, SYMBOL, TRANSACTION_HASH}; use openzeppelin::tests::utils::events::EventSpyExt; use openzeppelin::tests::utils::signing::StarkKeyPair; use openzeppelin::tests::utils; -use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; +use openzeppelin::token::erc20::interface::IERC20Dispatcher; use openzeppelin::utils::serde::SerializedAppend; use snforge_std::EventSpy; use snforge_std::signature::stark_curve::StarkCurveSignerImpl; @@ -14,8 +14,6 @@ use starknet::ContractAddress; #[derive(Drop)] pub(crate) struct SignedTransactionData { - pub(crate) private_key: felt252, - pub(crate) public_key: felt252, pub(crate) tx_hash: felt252, pub(crate) r: felt252, pub(crate) s: felt252 @@ -24,14 +22,11 @@ pub(crate) struct SignedTransactionData { pub(crate) fn SIGNED_TX_DATA(key_pair: StarkKeyPair) -> SignedTransactionData { let tx_hash = TRANSACTION_HASH; let (r, s) = key_pair.sign(tx_hash).unwrap(); - SignedTransactionData { - private_key: key_pair.secret_key, public_key: key_pair.public_key, tx_hash, r, s - } + SignedTransactionData { tx_hash, r, s } } pub(crate) fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> IERC20Dispatcher { let mut calldata = array![]; - calldata.append_serde(NAME()); calldata.append_serde(SYMBOL()); calldata.append_serde(initial_supply); diff --git a/src/tests/account/starknet/test_account.cairo b/src/tests/account/starknet/test_account.cairo index ed27c4a57..13dd7f36e 100644 --- a/src/tests/account/starknet/test_account.cairo +++ b/src/tests/account/starknet/test_account.cairo @@ -9,21 +9,19 @@ use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::account_mocks::DualCaseAccountMock; use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin::tests::utils::constants::{ - SALT, ZERO, OTHER, CALLER, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION + SALT, ZERO, OTHER, CALLER, RECIPIENT, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION }; +use openzeppelin::tests::utils::signing::StarkKeyPair; use openzeppelin::tests::utils; -use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; +use openzeppelin::token::erc20::interface::IERC20DispatcherTrait; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; -use snforge_std::signature::SignerTrait; -use snforge_std::signature::stark_curve::{StarkCurveSignerImpl, StarkCurveKeyPairImpl}; use snforge_std::{ cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global }; -use snforge_std::{EventSpy, spy_events, declare, test_address, start_cheat_caller_address}; +use snforge_std::{spy_events, declare, test_address, start_cheat_caller_address}; use starknet::account::Call; -use starknet::contract_address_const; -use starknet::{ContractAddress, ClassHash}; +use starknet::{contract_address_const, ContractAddress, ClassHash}; use super::common::{AccountSpyHelpers, SignedTransactionData}; use super::common::{deploy_erc20, SIGNED_TX_DATA, get_accept_ownership_signature}; @@ -42,26 +40,22 @@ fn COMPONENT_STATE() -> ComponentState { AccountComponent::component_state_for_testing() } -fn setup() -> ComponentState { +fn setup(key_pair: StarkKeyPair) -> ComponentState { let mut state = COMPONENT_STATE(); - let key_pair = KEY_PAIR(); state.initializer(key_pair.public_key); state } -fn setup_dispatcher(data: Option<@SignedTransactionData>) -> (AccountABIDispatcher, felt252) { +fn setup_dispatcher( + key_pair: StarkKeyPair, data: SignedTransactionData +) -> (AccountABIDispatcher, felt252) { let contract_class = declare("DualCaseAccountMock").unwrap_syscall(); - let calldata = if let Option::Some(data) = data { - cheat_signature_global(array![*data.r, *data.s].span()); - cheat_transaction_hash_global(*data.tx_hash); - array![*data.public_key] - } else { - array![KEY_PAIR().public_key] - }; - + let calldata = array![key_pair.public_key]; let address = utils::deploy(contract_class, calldata); let dispatcher = AccountABIDispatcher { contract_address: address }; + cheat_signature_global(array![data.r, data.s].span()); + cheat_transaction_hash_global(data.tx_hash); cheat_transaction_version_global(MIN_TRANSACTION_VERSION); start_cheat_caller_address(address, ZERO()); @@ -75,9 +69,10 @@ fn setup_dispatcher(data: Option<@SignedTransactionData>) -> (AccountABIDispatch #[test] fn test_is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR()); + let key_pair = KEY_PAIR(); + let data = SIGNED_TX_DATA(key_pair); - state._set_public_key(data.public_key); + state._set_public_key(key_pair.public_key); let good_signature = array![data.r, data.s]; let is_valid = state.is_valid_signature(data.tx_hash, good_signature); @@ -91,9 +86,10 @@ fn test_is_valid_signature() { #[test] fn test_isValidSignature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR()); + let key_pair = KEY_PAIR(); + let data = SIGNED_TX_DATA(key_pair); - state._set_public_key(data.public_key); + state._set_public_key(key_pair.public_key); let good_signature = array![data.r, data.s]; let is_valid = state.isValidSignature(data.tx_hash, good_signature); @@ -111,7 +107,7 @@ fn test_isValidSignature() { #[test] fn test_validate_deploy() { let key_pair = KEY_PAIR(); - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); // `__validate_deploy__` does not directly use the passed arguments. Their // values are already integrated in the tx hash. The passed arguments in this @@ -126,7 +122,7 @@ fn test_validate_deploy_invalid_signature_data() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); data.tx_hash += 1; - let (account, class_hash) = setup_dispatcher(Option::Some(@data)); + let (account, class_hash) = setup_dispatcher(key_pair, data); account.__validate_deploy__(class_hash, SALT, key_pair.public_key); } @@ -135,9 +131,9 @@ fn test_validate_deploy_invalid_signature_data() { #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_invalid_signature_length() { let key_pair = KEY_PAIR(); - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); - let short_sig = array!['SHORT_SIGNATURE']; - cheat_signature_global(short_sig.span()); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); + let invalid_len_sig = array!['INVALID_LEN_SIG']; + cheat_signature_global(invalid_len_sig.span()); account.__validate_deploy__(class_hash, SALT, key_pair.public_key); } @@ -146,7 +142,7 @@ fn test_validate_deploy_invalid_signature_length() { #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_deploy_empty_signature() { let key_pair = KEY_PAIR(); - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(key_pair))); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let empty_sig = array![]; cheat_signature_global(empty_sig.span()); @@ -155,7 +151,8 @@ fn test_validate_deploy_empty_signature() { #[test] fn test_validate_declare() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); // `__validate_declare__` does not directly use the class_hash argument. Its // value is already integrated in the tx hash. The class_hash argument in this @@ -167,9 +164,10 @@ fn test_validate_declare() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_data() { - let mut data = SIGNED_TX_DATA(KEY_PAIR()); + let key_pair = KEY_PAIR(); + let mut data = SIGNED_TX_DATA(key_pair); data.tx_hash += 1; - let (account, class_hash) = setup_dispatcher(Option::Some(@data)); + let (account, class_hash) = setup_dispatcher(key_pair, data); account.__validate_declare__(class_hash); } @@ -177,9 +175,10 @@ fn test_validate_declare_invalid_signature_data() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_invalid_signature_length() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); - let short_sig = array!['SHORT_SIGNATURE']; - cheat_signature_global(short_sig.span()); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); + let invalid_len_sig = array!['INVALID_LEN_SIG']; + cheat_signature_global(invalid_len_sig.span()); account.__validate_declare__(class_hash); } @@ -187,7 +186,8 @@ fn test_validate_declare_invalid_signature_length() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_declare_empty_signature() { - let (account, class_hash) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); + let key_pair = KEY_PAIR(); + let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let empty_sig = array![]; cheat_signature_global(empty_sig.span()); @@ -195,10 +195,10 @@ fn test_validate_declare_empty_signature() { } fn test_execute_with_version(version: Option) { - let data = SIGNED_TX_DATA(KEY_PAIR()); - let (account, _) = setup_dispatcher(Option::Some(@data)); + let key_pair = KEY_PAIR(); + let (account, _) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let erc20 = deploy_erc20(account.contract_address, 1000); - let recipient = contract_address_const::<0x123>(); + let recipient = RECIPIENT(); // Craft call and add to calls array let mut calldata = array![]; @@ -262,8 +262,9 @@ fn test_execute_invalid_version() { #[test] fn test_validate() { + let key_pair = KEY_PAIR(); + let (account, _) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let calls = array![]; - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); let is_valid = account.__validate__(calls); assert_eq!(is_valid, starknet::VALIDATED); @@ -272,20 +273,22 @@ fn test_validate() { #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_validate_invalid() { - let calls = array![]; - let mut data = SIGNED_TX_DATA(KEY_PAIR()); + let key_pair = KEY_PAIR(); + let mut data = SIGNED_TX_DATA(key_pair); data.tx_hash += 1; - let (account, _) = setup_dispatcher(Option::Some(@data)); + let (account, _) = setup_dispatcher(key_pair, data); + let calls = array![]; account.__validate__(calls); } #[test] fn test_multicall() { - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); + let key_pair = KEY_PAIR(); + let (account, _) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let erc20 = deploy_erc20(account.contract_address, 1000); - let recipient1 = contract_address_const::<0x123>(); - let recipient2 = contract_address_const::<0x456>(); + let recipient1 = RECIPIENT(); + let recipient2 = OTHER(); // Craft call1 let mut calldata1 = array![]; @@ -327,7 +330,8 @@ fn test_multicall() { #[test] fn test_multicall_zero_calls() { - let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); + let key_pair = KEY_PAIR(); + let (account, _) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); let calls = array![]; let response = account.__execute__(calls); @@ -337,9 +341,9 @@ fn test_multicall_zero_calls() { #[test] #[should_panic(expected: ('Account: invalid caller',))] fn test_account_called_from_contract() { - let state = setup(); - let calls = array![]; + let state = setup(KEY_PAIR()); let account_address = test_address(); + let calls = array![]; start_cheat_caller_address(account_address, CALLER()); @@ -354,23 +358,24 @@ fn test_account_called_from_contract() { fn test_public_key_setter_and_getter() { let mut state = COMPONENT_STATE(); let account_address = test_address(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); let new_key_pair = KEY_PAIR_2(); - let new_public_key = new_key_pair.public_key; start_cheat_caller_address(account_address, account_address); - state._set_public_key(public_key); - assert_eq!(state.get_public_key(), public_key); + state._set_public_key(key_pair.public_key); + assert_eq!(state.get_public_key(), key_pair.public_key); // Set key let mut spy = spy_events(); - let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); - state.set_public_key(new_public_key, signature); + let signature = get_accept_ownership_signature( + account_address, key_pair.public_key, new_key_pair + ); + state.set_public_key(new_key_pair.public_key, signature); - spy.assert_event_owner_removed(account_address, public_key); - spy.assert_only_event_owner_added(account_address, new_public_key); + spy.assert_event_owner_removed(account_address, key_pair.public_key); + spy.assert_only_event_owner_added(account_address, new_key_pair.public_key); - assert_eq!(state.get_public_key(), new_public_key); + assert_eq!(state.get_public_key(), new_key_pair.public_key); } #[test] @@ -392,23 +397,24 @@ fn test_public_key_setter_different_account() { fn test_public_key_setter_and_getter_camel() { let mut state = COMPONENT_STATE(); let account_address = test_address(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); let new_key_pair = KEY_PAIR_2(); - let new_public_key = new_key_pair.public_key; start_cheat_caller_address(account_address, account_address); - state._set_public_key(public_key); - assert_eq!(state.getPublicKey(), public_key); + state._set_public_key(key_pair.public_key); + assert_eq!(state.getPublicKey(), key_pair.public_key); // Set key let mut spy = spy_events(); - let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); - state.setPublicKey(new_public_key, signature); + let signature = get_accept_ownership_signature( + account_address, key_pair.public_key, new_key_pair + ); + state.setPublicKey(new_key_pair.public_key, signature); - spy.assert_event_owner_removed(account_address, public_key); - spy.assert_only_event_owner_added(account_address, new_public_key); + spy.assert_event_owner_removed(account_address, key_pair.public_key); + spy.assert_only_event_owner_added(account_address, new_key_pair.public_key); - assert_eq!(state.getPublicKey(), new_public_key); + assert_eq!(state.getPublicKey(), new_key_pair.public_key); } #[test] @@ -467,34 +473,39 @@ fn test_assert_only_self_false() { #[test] fn test_assert_valid_new_owner() { - let state = setup(); + let key_pair = KEY_PAIR(); + let state = setup(key_pair); let account_address = test_address(); - let public_key = KEY_PAIR().public_key; + let new_key_pair = KEY_PAIR_2(); - let new_public_key = new_key_pair.public_key; - let signature = get_accept_ownership_signature(account_address, public_key, new_key_pair); + let signature = get_accept_ownership_signature( + account_address, key_pair.public_key, new_key_pair + ); - state.assert_valid_new_owner(public_key, new_public_key, signature); + state.assert_valid_new_owner(key_pair.public_key, new_key_pair.public_key, signature); } #[test] #[should_panic(expected: ('Account: invalid signature',))] fn test_assert_valid_new_owner_invalid_signature() { - let state = setup(); - let public_key = KEY_PAIR().public_key; - let new_public_key = KEY_PAIR_2().public_key; + let key_pair = KEY_PAIR(); + let state = setup(key_pair); + + let new_key_pair = KEY_PAIR_2(); let bad_signature = array!['BAD', 'SIGNATURE']; - state.assert_valid_new_owner(public_key, new_public_key, bad_signature.span()); + state + .assert_valid_new_owner(key_pair.public_key, new_key_pair.public_key, bad_signature.span()); } #[test] fn test__is_valid_signature() { let mut state = COMPONENT_STATE(); - let data = SIGNED_TX_DATA(KEY_PAIR()); + let key_pair = KEY_PAIR(); + let data = SIGNED_TX_DATA(key_pair); - state._set_public_key(data.public_key); + state._set_public_key(key_pair.public_key); let good_signature = array![data.r, data.s]; assert!(state._is_valid_signature(data.tx_hash, good_signature.span())); diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index f81b9eedb..da457d8e4 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -2,12 +2,9 @@ use openzeppelin::account::dual_account::{DualCaseAccountABI, DualCaseAccount}; use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::starknet::common::SIGNED_TX_DATA; -use openzeppelin::tests::mocks::account_mocks::{ - CamelAccountPanicMock, CamelAccountMock, SnakeAccountMock, SnakeAccountPanicMock -}; -use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; use openzeppelin::tests::utils::constants::TRANSACTION_HASH; use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; +use openzeppelin::tests::utils::signing::StarkKeyPair; use openzeppelin::tests::utils; use snforge_std::{declare, start_cheat_caller_address}; @@ -17,30 +14,22 @@ use super::common::get_accept_ownership_signature; // Setup // -fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { - let key_pair = KEY_PAIR(); +fn setup_snake(key_pair: StarkKeyPair) -> (DualCaseAccount, AccountABIDispatcher) { let calldata = array![key_pair.public_key]; - let target = utils::declare_and_deploy("SnakeAccountMock", calldata); - ( - DualCaseAccount { contract_address: target }, - AccountABIDispatcher { contract_address: target } - ) + let contract_address = utils::declare_and_deploy("SnakeAccountMock", calldata); + (DualCaseAccount { contract_address }, AccountABIDispatcher { contract_address }) } -fn setup_camel() -> (DualCaseAccount, AccountABIDispatcher) { - let key_pair = KEY_PAIR(); +fn setup_camel(key_pair: StarkKeyPair) -> (DualCaseAccount, AccountABIDispatcher) { let calldata = array![key_pair.public_key]; - let target = utils::declare_and_deploy("CamelAccountMock", calldata); - ( - DualCaseAccount { contract_address: target }, - AccountABIDispatcher { contract_address: target } - ) + let contract_address = utils::declare_and_deploy("CamelAccountMock", calldata); + (DualCaseAccount { contract_address }, AccountABIDispatcher { contract_address }) } fn setup_non_account() -> DualCaseAccount { let calldata = array![]; - let target = utils::declare_and_deploy("NonImplementingMock", calldata); - DualCaseAccount { contract_address: target } + let contract_address = utils::declare_and_deploy("NonImplementingMock", calldata); + DualCaseAccount { contract_address } } fn setup_account_panic() -> (DualCaseAccount, DualCaseAccount) { @@ -58,18 +47,17 @@ fn setup_account_panic() -> (DualCaseAccount, DualCaseAccount) { #[test] fn test_dual_set_public_key() { - let (snake_dispatcher, target) = setup_snake(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (snake_dispatcher, target) = setup_snake(key_pair); let new_key_pair = KEY_PAIR_2(); - let new_public_key = new_key_pair.public_key; let signature = get_accept_ownership_signature( - snake_dispatcher.contract_address, public_key, new_key_pair + snake_dispatcher.contract_address, key_pair.public_key, new_key_pair ); start_cheat_caller_address(target.contract_address, target.contract_address); - snake_dispatcher.set_public_key(new_public_key, signature); + snake_dispatcher.set_public_key(new_key_pair.public_key, signature); - assert_eq!(target.get_public_key(), new_public_key); + assert_eq!(target.get_public_key(), new_key_pair.public_key); } #[test] @@ -91,8 +79,9 @@ fn test_dual_set_public_key_exists_and_panics() { #[test] fn test_dual_get_public_key() { - let (snake_dispatcher, _) = setup_snake(); - let expected_public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (snake_dispatcher, _) = setup_snake(key_pair); + let expected_public_key = key_pair.public_key; assert_eq!(snake_dispatcher.get_public_key(), expected_public_key); } @@ -113,16 +102,17 @@ fn test_dual_get_public_key_exists_and_panics() { #[test] fn test_dual_is_valid_signature() { - let (snake_dispatcher, target) = setup_snake(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (snake_dispatcher, target) = setup_snake(key_pair); + let new_key_pair = KEY_PAIR_2(); let data = SIGNED_TX_DATA(new_key_pair); start_cheat_caller_address(target.contract_address, target.contract_address); let accept_signature = get_accept_ownership_signature( - snake_dispatcher.contract_address, public_key, new_key_pair + snake_dispatcher.contract_address, key_pair.public_key, new_key_pair ); - target.set_public_key(data.public_key, accept_signature); + target.set_public_key(new_key_pair.public_key, accept_signature); let signature = array![data.r, data.s]; let is_valid = snake_dispatcher.is_valid_signature(data.tx_hash, signature); @@ -150,7 +140,7 @@ fn test_dual_is_valid_signature_exists_and_panics() { #[test] fn test_dual_supports_interface() { - let (snake_dispatcher, _) = setup_snake(); + let (snake_dispatcher, _) = setup_snake(KEY_PAIR()); let supports_isrc5 = snake_dispatcher.supports_interface(ISRC5_ID); assert!(supports_isrc5); } @@ -177,18 +167,17 @@ fn test_dual_supports_interface_exists_and_panics() { #[test] #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_setPublicKey() { - let (camel_dispatcher, target) = setup_camel(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (camel_dispatcher, target) = setup_camel(key_pair); let new_key_pair = KEY_PAIR_2(); - let new_public_key = new_key_pair.public_key; let signature = get_accept_ownership_signature( - camel_dispatcher.contract_address, public_key, new_key_pair + camel_dispatcher.contract_address, key_pair.public_key, new_key_pair ); start_cheat_caller_address(target.contract_address, target.contract_address); - camel_dispatcher.set_public_key(new_public_key, signature); + camel_dispatcher.set_public_key(new_key_pair.public_key, signature); - assert_eq!(target.getPublicKey(), new_public_key); + assert_eq!(target.getPublicKey(), new_key_pair.public_key); } #[test] @@ -203,8 +192,9 @@ fn test_dual_setPublicKey_exists_and_panics() { #[test] #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_getPublicKey() { - let (camel_dispatcher, _) = setup_camel(); - let expected_public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (camel_dispatcher, _) = setup_camel(key_pair); + let expected_public_key = key_pair.public_key; assert_eq!(camel_dispatcher.get_public_key(), expected_public_key); } @@ -219,17 +209,17 @@ fn test_dual_getPublicKey_exists_and_panics() { #[test] #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_isValidSignature() { - let (camel_dispatcher, target) = setup_camel(); - let public_key = KEY_PAIR().public_key; + let key_pair = KEY_PAIR(); + let (camel_dispatcher, target) = setup_camel(key_pair); let new_key_pair = KEY_PAIR_2(); let data = SIGNED_TX_DATA(new_key_pair); let signature = array![data.r, data.s]; start_cheat_caller_address(target.contract_address, target.contract_address); let accept_signature = get_accept_ownership_signature( - camel_dispatcher.contract_address, public_key, new_key_pair + camel_dispatcher.contract_address, key_pair.public_key, new_key_pair ); - target.setPublicKey(data.public_key, accept_signature); + target.setPublicKey(new_key_pair.public_key, accept_signature); let is_valid = camel_dispatcher.is_valid_signature(data.tx_hash, signature); assert_eq!(is_valid, starknet::VALIDATED); diff --git a/src/tests/account/test_signature.cairo b/src/tests/account/test_signature.cairo index 604da2d4c..548afb817 100644 --- a/src/tests/account/test_signature.cairo +++ b/src/tests/account/test_signature.cairo @@ -12,28 +12,31 @@ use super::starknet::common::SIGNED_TX_DATA as stark_signature_data; #[test] fn test_is_valid_stark_signature_good_sig() { - let data = stark_signature_data(stark::KEY_PAIR()); + let key_pair = stark::KEY_PAIR(); + let data = stark_signature_data(key_pair); let good_signature = array![data.r, data.s].span(); - let is_valid = is_valid_stark_signature(data.tx_hash, data.public_key, good_signature); + let is_valid = is_valid_stark_signature(data.tx_hash, key_pair.public_key, good_signature); assert!(is_valid); } #[test] fn test_is_valid_stark_signature_bad_sig() { - let data = stark_signature_data(stark::KEY_PAIR()); + let key_pair = stark::KEY_PAIR(); + let data = stark_signature_data(key_pair); let bad_signature = array!['BAD', 'SIGNATURE'].span(); - let is_invalid = !is_valid_stark_signature(data.tx_hash, data.public_key, bad_signature); + let is_invalid = !is_valid_stark_signature(data.tx_hash, key_pair.public_key, bad_signature); assert!(is_invalid); } #[test] fn test_is_valid_stark_signature_invalid_len_sig() { - let data = stark_signature_data(stark::KEY_PAIR()); + let key_pair = stark::KEY_PAIR(); + let data = stark_signature_data(key_pair); let bad_signature = array!['BAD_SIGNATURE'].span(); - let is_invalid = !is_valid_stark_signature(data.tx_hash, data.public_key, bad_signature); + let is_invalid = !is_valid_stark_signature(data.tx_hash, key_pair.public_key, bad_signature); assert!(is_invalid); } From 76403931243f85abae5c2a494aa8e8909db94ce8 Mon Sep 17 00:00:00 2001 From: immrsd Date: Fri, 26 Jul 2024 10:49:42 +0200 Subject: [PATCH 13/13] Remove unnecessary accept_ownership step, make use of serialized_sign fn --- .../account/starknet/test_dual_account.cairo | 33 +++++-------------- src/tests/utils/signing.cairo | 8 +++++ 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/tests/account/starknet/test_dual_account.cairo b/src/tests/account/starknet/test_dual_account.cairo index da457d8e4..ab0887896 100644 --- a/src/tests/account/starknet/test_dual_account.cairo +++ b/src/tests/account/starknet/test_dual_account.cairo @@ -4,7 +4,7 @@ use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::starknet::common::SIGNED_TX_DATA; use openzeppelin::tests::utils::constants::TRANSACTION_HASH; use openzeppelin::tests::utils::constants::stark::{KEY_PAIR, KEY_PAIR_2}; -use openzeppelin::tests::utils::signing::StarkKeyPair; +use openzeppelin::tests::utils::signing::{StarkKeyPair, StarkKeyPairExt}; use openzeppelin::tests::utils; use snforge_std::{declare, start_cheat_caller_address}; @@ -103,19 +103,11 @@ fn test_dual_get_public_key_exists_and_panics() { #[test] fn test_dual_is_valid_signature() { let key_pair = KEY_PAIR(); - let (snake_dispatcher, target) = setup_snake(key_pair); - - let new_key_pair = KEY_PAIR_2(); - let data = SIGNED_TX_DATA(new_key_pair); - start_cheat_caller_address(target.contract_address, target.contract_address); - let accept_signature = get_accept_ownership_signature( - snake_dispatcher.contract_address, key_pair.public_key, new_key_pair - ); - - target.set_public_key(new_key_pair.public_key, accept_signature); + let (snake_dispatcher, _) = setup_snake(key_pair); + let tx_hash = TRANSACTION_HASH; + let serialized_signature = key_pair.serialized_sign(tx_hash); - let signature = array![data.r, data.s]; - let is_valid = snake_dispatcher.is_valid_signature(data.tx_hash, signature); + let is_valid = snake_dispatcher.is_valid_signature(tx_hash, serialized_signature); assert_eq!(is_valid, starknet::VALIDATED); } @@ -210,18 +202,11 @@ fn test_dual_getPublicKey_exists_and_panics() { #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet. fn test_dual_isValidSignature() { let key_pair = KEY_PAIR(); - let (camel_dispatcher, target) = setup_camel(key_pair); - let new_key_pair = KEY_PAIR_2(); - let data = SIGNED_TX_DATA(new_key_pair); - let signature = array![data.r, data.s]; - start_cheat_caller_address(target.contract_address, target.contract_address); - let accept_signature = get_accept_ownership_signature( - camel_dispatcher.contract_address, key_pair.public_key, new_key_pair - ); - - target.setPublicKey(new_key_pair.public_key, accept_signature); + let (camel_dispatcher, _) = setup_camel(key_pair); + let tx_hash = TRANSACTION_HASH; + let serialized_signature = key_pair.serialized_sign(tx_hash); - let is_valid = camel_dispatcher.is_valid_signature(data.tx_hash, signature); + let is_valid = camel_dispatcher.is_valid_signature(tx_hash, serialized_signature); assert_eq!(is_valid, starknet::VALIDATED); } diff --git a/src/tests/utils/signing.cairo b/src/tests/utils/signing.cairo index ee596c8d0..8b1e7ca44 100644 --- a/src/tests/utils/signing.cairo +++ b/src/tests/utils/signing.cairo @@ -14,6 +14,14 @@ pub fn get_secp256k1_keys_from(private_key: u256) -> Secp256k1KeyPair { Secp256k1CurveKeyPairImpl::from_secret_key(private_key) } +#[generate_trait] +pub impl StarkKeyPairExt of StarkKeyPairExtTrait { + fn serialized_sign(self: StarkKeyPair, msg: felt252) -> Array { + let (r, s) = self.sign(msg).unwrap(); + array![r, s] + } +} + #[generate_trait] pub impl Secp256k1KeyPairExt of Secp256k1KeyPairExtTrait { fn serialized_sign(self: Secp256k1KeyPair, msg: u256) -> Array {