From 30a25672b8ab56816582dcfba5172acd4c3219d0 Mon Sep 17 00:00:00 2001 From: andrew Date: Tue, 13 Aug 2024 15:54:03 -0500 Subject: [PATCH 1/2] replace erc20 mock with local mock, remove token dep from account package, update tests --- Scarb.lock | 1 - packages/account/Scarb.toml | 1 - packages/account/src/tests/mocks.cairo | 2 +- .../account/src/tests/mocks/erc20_mocks.cairo | 40 ----------- .../account/src/tests/mocks/simple_mock.cairo | 25 +++++++ packages/account/src/tests/test_account.cairo | 63 +++++++++-------- .../account/src/tests/test_eth_account.cairo | 70 ++++++++++--------- 7 files changed, 96 insertions(+), 106 deletions(-) delete mode 100644 packages/account/src/tests/mocks/erc20_mocks.cairo create mode 100644 packages/account/src/tests/mocks/simple_mock.cairo diff --git a/Scarb.lock b/Scarb.lock index 1e5777347..e7bd29063 100644 --- a/Scarb.lock +++ b/Scarb.lock @@ -37,7 +37,6 @@ dependencies = [ "openzeppelin_introspection", "openzeppelin_test_common", "openzeppelin_testing", - "openzeppelin_token", "openzeppelin_utils", "snforge_std", ] diff --git a/packages/account/Scarb.toml b/packages/account/Scarb.toml index a615534e8..6b1734a83 100644 --- a/packages/account/Scarb.toml +++ b/packages/account/Scarb.toml @@ -19,7 +19,6 @@ fmt.workspace = true starknet.workspace = true openzeppelin_introspection = { path = "../introspection" } openzeppelin_utils = { path = "../utils" } -openzeppelin_token = { path = "../token" } [dev-dependencies] snforge_std.workspace = true diff --git a/packages/account/src/tests/mocks.cairo b/packages/account/src/tests/mocks.cairo index 8b7ce956f..5fd0e4c04 100644 --- a/packages/account/src/tests/mocks.cairo +++ b/packages/account/src/tests/mocks.cairo @@ -1,4 +1,4 @@ pub(crate) mod account_mocks; -pub(crate) mod erc20_mocks; pub(crate) mod eth_account_mocks; pub(crate) mod non_implementing_mock; +pub(crate) mod simple_mock; diff --git a/packages/account/src/tests/mocks/erc20_mocks.cairo b/packages/account/src/tests/mocks/erc20_mocks.cairo deleted file mode 100644 index 8e24ff5f9..000000000 --- a/packages/account/src/tests/mocks/erc20_mocks.cairo +++ /dev/null @@ -1,40 +0,0 @@ -#[starknet::contract] -pub(crate) mod DualCaseERC20Mock { - use openzeppelin_token::erc20::{ERC20Component, ERC20HooksEmptyImpl}; - use starknet::ContractAddress; - - component!(path: ERC20Component, storage: erc20, event: ERC20Event); - - #[abi(embed_v0)] - impl ERC20Impl = ERC20Component::ERC20Impl; - #[abi(embed_v0)] - impl ERC20MetadataImpl = ERC20Component::ERC20MetadataImpl; - #[abi(embed_v0)] - impl ERC20CamelOnlyImpl = ERC20Component::ERC20CamelOnlyImpl; - impl InternalImpl = ERC20Component::InternalImpl; - - #[storage] - struct Storage { - #[substorage(v0)] - erc20: ERC20Component::Storage - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - #[flat] - ERC20Event: ERC20Component::Event - } - - #[constructor] - fn constructor( - ref self: ContractState, - name: ByteArray, - symbol: ByteArray, - initial_supply: u256, - recipient: ContractAddress - ) { - self.erc20.initializer(name, symbol); - self.erc20.mint(recipient, initial_supply); - } -} diff --git a/packages/account/src/tests/mocks/simple_mock.cairo b/packages/account/src/tests/mocks/simple_mock.cairo new file mode 100644 index 000000000..afd4d53fb --- /dev/null +++ b/packages/account/src/tests/mocks/simple_mock.cairo @@ -0,0 +1,25 @@ +#[starknet::interface] +pub(crate) trait ISimpleMock { + fn increase_balance(ref self: TContractState, amount: felt252) -> bool; + fn get_balance(self: @TContractState) -> felt252; +} + +#[starknet::contract] +pub(crate) mod SimpleMock { + #[storage] + struct Storage { + balance: felt252, + } + + #[abi(embed_v0)] + impl SimpleMockImpl of super::ISimpleMock { + fn increase_balance(ref self: ContractState, amount: felt252) -> bool { + self.balance.write(self.balance.read() + amount); + true + } + + fn get_balance(self: @ContractState) -> felt252 { + self.balance.read() + } + } +} diff --git a/packages/account/src/tests/test_account.cairo b/packages/account/src/tests/test_account.cairo index 8cb7d8cda..c1afd5c62 100644 --- a/packages/account/src/tests/test_account.cairo +++ b/packages/account/src/tests/test_account.cairo @@ -6,17 +6,19 @@ use openzeppelin_account::AccountComponent; use openzeppelin_account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin_account::interface::{ISRC6, ISRC6_ID}; use openzeppelin_account::tests::mocks::account_mocks::DualCaseAccountMock; +use openzeppelin_account::tests::mocks::simple_mock::SimpleMock; +use openzeppelin_account::tests::mocks::simple_mock::{ + ISimpleMockDispatcher, ISimpleMockDispatcherTrait +}; use openzeppelin_introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin_test_common::account::{AccountSpyHelpers, SignedTransactionData}; use openzeppelin_test_common::account::{SIGNED_TX_DATA, get_accept_ownership_signature}; -use openzeppelin_test_common::erc20::deploy_erc20; use openzeppelin_testing as utils; use openzeppelin_testing::constants::stark::{KEY_PAIR, KEY_PAIR_2}; use openzeppelin_testing::constants::{ SALT, ZERO, OTHER, CALLER, RECIPIENT, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION }; use openzeppelin_testing::signing::StarkKeyPair; -use openzeppelin_token::erc20::interface::IERC20DispatcherTrait; use openzeppelin_utils::selectors; use openzeppelin_utils::serde::SerializedAppend; use snforge_std::{ @@ -196,16 +198,19 @@ fn test_validate_declare_empty_signature() { fn test_execute_with_version(version: Option) { 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 = RECIPIENT(); + + // Deploy target contract + let calldata = array![]; + let address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address: address }; // Craft call and add to calls array - let mut calldata = array![]; - let amount: u256 = 200; - calldata.append_serde(recipient); - calldata.append_serde(amount); + let amount = 200; + let calldata = array![amount]; let call = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata.span() }; let calls = array![call]; @@ -217,9 +222,8 @@ fn test_execute_with_version(version: Option) { // Execute let ret = account.__execute__(calls); - // Assert that the transfer was successful - assert_eq!(erc20.balance_of(account.contract_address), 800, "Should have remainder"); - assert_eq!(erc20.balance_of(recipient), amount, "Should have transferred"); + // Assert that the call was successful + assert_eq!(simple_mock.get_balance(), amount); // Test return value let mut call_serialized_retval = *ret.at(0); @@ -285,36 +289,37 @@ fn test_validate_invalid() { fn test_multicall() { 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 = RECIPIENT(); - let recipient2 = OTHER(); + + // Deploy target contract + let calldata = array![]; + let address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address: address }; // Craft call1 - let mut calldata1 = array![]; - let amount1: u256 = 300; - calldata1.append_serde(recipient1); - calldata1.append_serde(amount1); + let amount1 = 300; + let calldata1 = array![amount1]; let call1 = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata1.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata1.span() }; // Craft call2 - let mut calldata2 = array![]; - let amount2: u256 = 500; - calldata2.append_serde(recipient2); - calldata2.append_serde(amount2); + let amount2 = 500; + let calldata2 = array![amount2]; let call2 = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata2.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata2.span() }; // Bundle calls and execute let calls = array![call1, call2]; let ret = account.__execute__(calls); - // Assert that the transfers were successful - assert_eq!(erc20.balance_of(account.contract_address), 200, "Should have remainder"); - assert_eq!(erc20.balance_of(recipient1), 300, "Should have transferred from call1"); - assert_eq!(erc20.balance_of(recipient2), 500, "Should have transferred from call2"); + // Assert that the txs were successful + let total_balance = amount1 + amount2; + assert_eq!(simple_mock.get_balance(), total_balance); // Test return values let mut call1_serialized_retval = *ret.at(0); diff --git a/packages/account/src/tests/test_eth_account.cairo b/packages/account/src/tests/test_eth_account.cairo index 02f9d451e..9bf2b8671 100644 --- a/packages/account/src/tests/test_eth_account.cairo +++ b/packages/account/src/tests/test_eth_account.cairo @@ -4,10 +4,13 @@ use openzeppelin_account::EthAccountComponent; use openzeppelin_account::interface::{EthAccountABIDispatcherTrait, EthAccountABIDispatcher}; use openzeppelin_account::interface::{ISRC6, ISRC6_ID}; use openzeppelin_account::tests::mocks::eth_account_mocks::DualCaseEthAccountMock; +use openzeppelin_account::tests::mocks::simple_mock::SimpleMock; +use openzeppelin_account::tests::mocks::simple_mock::{ + ISimpleMockDispatcher, ISimpleMockDispatcherTrait +}; use openzeppelin_account::utils::secp256k1::{DebugSecp256k1Point, Secp256k1PointPartialEq}; use openzeppelin_account::utils::signature::EthSignature; use openzeppelin_introspection::interface::{ISRC5, ISRC5_ID}; -use openzeppelin_test_common::erc20::deploy_erc20; use openzeppelin_test_common::eth_account::EthAccountSpyHelpers; use openzeppelin_test_common::eth_account::{ SIGNED_TX_DATA, SignedTransactionData, get_accept_ownership_signature @@ -18,7 +21,6 @@ use openzeppelin_testing::constants::{ ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO, OTHER, RECIPIENT, CALLER, QUERY_VERSION, MIN_TRANSACTION_VERSION }; -use openzeppelin_token::erc20::interface::IERC20DispatcherTrait; use openzeppelin_utils::selectors; use openzeppelin_utils::serde::SerializedAppend; use snforge_std::{ @@ -218,19 +220,21 @@ 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 erc20 = deploy_erc20(account.contract_address, 1000); - let recipient = RECIPIENT(); + + // Deploy target contract + let calldata = array![]; + let address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address: address }; // Craft call and add to calls array - let mut calldata = array![]; - let amount: u256 = 200; - calldata.append_serde(recipient); - calldata.append_serde(amount); + let amount = 200; + let calldata = array![amount]; let call = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata.span() }; - let mut calls = array![]; - calls.append(call); + let calls = array![call]; // Handle version for test if let Option::Some(version) = version { @@ -240,9 +244,8 @@ fn test_execute_with_version(version: Option) { // Execute let ret = account.__execute__(calls); - // Assert that the transfer was successful - assert_eq!(erc20.balance_of(account.contract_address), 800, "Should have remainder"); - assert_eq!(erc20.balance_of(recipient), amount, "Should have transferred"); + // Assert that the call was successful + assert_eq!(simple_mock.get_balance(), amount); // Test return value let mut call_serialized_retval = *ret.at(0); @@ -289,38 +292,37 @@ fn test_validate_invalid() { #[test] fn test_multicall() { let (account, _) = setup_dispatcher(Option::Some(@SIGNED_TX_DATA(KEY_PAIR()))); - let erc20 = deploy_erc20(account.contract_address, 1000); - let recipient1 = RECIPIENT(); - let recipient2 = OTHER(); - let mut calls = array![]; + + // Deploy target contract + let calldata = array![]; + let address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address: address }; // Craft call1 - let mut calldata1 = array![]; - let amount1: u256 = 300; - calldata1.append_serde(recipient1); - calldata1.append_serde(amount1); + let amount1 = 300; + let calldata1 = array![amount1]; let call1 = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata1.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata1.span() }; // Craft call2 - let mut calldata2 = array![]; - let amount2: u256 = 500; - calldata2.append_serde(recipient2); - calldata2.append_serde(amount2); + let amount2 = 500; + let calldata2 = array![amount2]; let call2 = Call { - to: erc20.contract_address, selector: selectors::transfer, calldata: calldata2.span() + to: simple_mock.contract_address, + selector: selector!("increase_balance"), + calldata: calldata2.span() }; // Bundle calls and execute - calls.append(call1); - calls.append(call2); + let calls = array![call1, call2]; let ret = account.__execute__(calls); - // Assert that the transfers were successful - assert_eq!(erc20.balance_of(account.contract_address), 200, "Should have remainder"); - assert_eq!(erc20.balance_of(recipient1), 300, "Should have transferred in call 1"); - assert_eq!(erc20.balance_of(recipient2), 500, "Should have transferred in call 2"); + // Assert that the txs were successful + let total_balance = amount1 + amount2; + assert_eq!(simple_mock.get_balance(), total_balance); // Test return value let mut call1_serialized_retval = *ret.at(0); From 8375a884846f4a6e6a58098db13a30af8d58cfce Mon Sep 17 00:00:00 2001 From: andrew Date: Tue, 13 Aug 2024 16:22:06 -0500 Subject: [PATCH 2/2] remove unused append serde --- packages/account/src/tests/test_account.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/account/src/tests/test_account.cairo b/packages/account/src/tests/test_account.cairo index c1afd5c62..7af00ef4a 100644 --- a/packages/account/src/tests/test_account.cairo +++ b/packages/account/src/tests/test_account.cairo @@ -20,7 +20,6 @@ use openzeppelin_testing::constants::{ }; use openzeppelin_testing::signing::StarkKeyPair; use openzeppelin_utils::selectors; -use openzeppelin_utils::serde::SerializedAppend; use snforge_std::{ cheat_signature_global, cheat_transaction_version_global, cheat_transaction_hash_global };