Refactor ETH Account tests (#1098)

* Migrate erc20 tests (#1022) * feat: migrate event tests * feat: update workflow * feat: add foundry utils * feat: finish erc20 and erc20votes migration * feat: remove dual case tests * refactor: format files * refactor: variable name * Update src/tests/token.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/utils/foundry.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20/test_erc20_votes.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Snforge utils and some common changes (#1030) * Update gitignore with snfoundry cache dir * Delete foundry utils file * Add snfoundry-related test utils, comment out previous utils * Update ERC20 event helpers * Update ERC20 tests * Update ERC20Votes tests * Run linter * Add util function to drop single event * Fix review issues * Make Github workflow run on every PR * Fix review issues * Bump snforge version * Support snforge 0.26 in test utils * Update ERC20 tests to support snforge upgrade * Run linter * Re-add dual20 tests (#1028) * feat: migrate event tests * feat: update workflow * feat: add foundry utils * feat: finish erc20 and erc20votes migration * feat: remove dual case tests * refactor: format files * refactor: variable name * Update src/tests/token.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/utils/foundry.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20/test_erc20_votes.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates * feat: readd dual20 tests * refactor: remove unnecessary import * feat: add dual20 import --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Migrate security tests (#1034) * migrate pausable and initializable tests * migrate reentrancy guard tests * clean up tests * fix fmt * add event trait to pausable, fix tests * Apply suggestions from code review Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * fix fmt --------- Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * Migrate erc1155 tests (#1037) * feat: update common module * feat: migrate dual modules * feat: finish component test migration * refactor: remove unnecessary imports * feat: apply review updates * Migrate erc721 tests (#1027) * feat: migrate event tests * feat: update workflow * feat: add foundry utils * feat: finish erc20 and erc20votes migration * feat: remove dual case tests * refactor: format files * refactor: variable name * Update src/tests/token.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/utils/foundry.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/token/erc20/test_erc20_votes.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates * feat: migrate erc721 components tests * refactor: remove extra line * feat: apply review updates * feat: update utilities * feat: apply review updates * fix: ignore tests --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Check ignored tests (#1049) * test: check ignored tests and add reason * feat: apply review updates * Migrate Ownable tests (#1033) * Update ERC20Votes tests * Run linter * Add test helpers for Ownable tests * Migrate Ownable tests * Migrate OwnableTwoStep tests * Migrate Ownable Dual Dispatcher tests * Support event changes in snforge 0.26 * Resolve review issues * Address review comments * Migrate Upgrades tests (#1051) * Migrate Upgrades tests to Foundry * Update src/tests/upgrades/test_upgradeable.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Update src/tests/upgrades/test_upgradeable.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> --------- Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Migrate erc20 preset tests (#1055) * feat: include erc20 preset tests * feat: tests updated * feat: remove unused imports * feat: add deployments * Update src/tests/utils/common.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: apply review updates --------- Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * feat: update ignored tests messages (#1056) * Migrate erc1155 preset tests (#1057) * feat: update tests * refactor: remove unnecessary import * Migrate cryptography and UDC tests (#1059) * fix import name * migrate cryptography tests * migrate udc tests * fix fmt * fix impl name * fix event assertions * re-add class hash helper * Migrate eth account tests (#1058) * feat: update dual_eth_account tests * feat: update eth_account tests * refactor: remove unused helpers * refactor: some inconsistencies * Update src/tests/account/ethereum/test_eth_account.cairo Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> * feat: apply review updates * refactor: format files * Update src/tests/account/ethereum/test_eth_account.cairo Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> --------- Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> * Migrate eth account preset tests (#1060) * feat: update dual_eth_account tests * feat: update eth_account tests * refactor: remove unused helpers * feat: update eth account preset tests * Update src/tests/account/ethereum/test_eth_account.cairo Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> * Update src/tests/presets/test_eth_account.cairo Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> * feat: apply review updates --------- Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> * Migrate AccessControl Tests (#1044) * Migrate AccessControl tests * Migrate AccessControl Dual Dispatcher tests * Fix imports * Address review comments * Update error messages * Bring back separator line * Fix ignore reasons in access module * Fix review issues * Migrate Starknet Account tests (#1050) * Update test utils and helpers * Update signature tests * Update account tests * Update dual account tests * Run linter * Run linter * Fix review issues * Update ignore reason messages * Run linter * Support eth account tests changes * Run linter * Improve setup functions, remove unused imports * Remove unnecessary accept_ownership step, make use of serialized_sign fn * Migrate Starknet Account Preset tests (#1069) * Update test utils and helpers * Update signature tests * Update account tests * Update dual account tests * Run linter * Run linter * Fix review issues * Update ignore reason messages * Run linter * Support eth account tests changes * Run linter * Improve setup functions, remove unused imports * Remove unnecessary accept_ownership step, make use of serialized_sign fn * Migrate Starknet Account Preset tests, make some code improvements * Run linter * Remove unnecessary clone * Update the comment for assert_entrypoint_not_found_error function * Minor review fixes * Refactor Eth Account tests * Fix review issues * Fix account tests after merge * Fix ETH account tests * Remove tests dir at wrong location * Fix var name in test * Fix import --------- Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com> Co-authored-by: Andrew Fleming <fleming-andrew@protonmail.com>
OpenZeppelin · Aug 15, 2024 · 9dcc3f5 · 9dcc3f5
1 parent 486699f
commit 9dcc3f5
Show file tree

Hide file tree

Showing 7 changed files with 334 additions and 339 deletions.
diff --git a/packages/account/src/tests/test_account.cairo b/packages/account/src/tests/test_account.cairo
@@ -294,7 +294,7 @@ fn test_multicall() {
     let address = utils::declare_and_deploy("SimpleMock", calldata);
     let simple_mock = ISimpleMockDispatcher { contract_address: address };
 
-    // Craft call1
+    // Craft 1st call
     let amount1 = 300;
     let calldata1 = array![amount1];
     let call1 = Call {
@@ -303,7 +303,7 @@ fn test_multicall() {
         calldata: calldata1.span()
     };
 
-    // Craft call2
+    // Craft 2nd call
     let amount2 = 500;
     let calldata2 = array![amount2];
     let call2 = Call {

diff --git a/packages/account/src/tests/test_dual_eth_account.cairo b/packages/account/src/tests/test_dual_eth_account.cairo
@@ -1,46 +1,40 @@
-use openzeppelin_account::dual_eth_account::{DualCaseEthAccountABI, DualCaseEthAccount};
+use openzeppelin_account::dual_eth_account::{DualCaseEthAccount, DualCaseEthAccountABI};
 use openzeppelin_account::interface::{EthAccountABIDispatcherTrait, EthAccountABIDispatcher};
 use openzeppelin_account::utils::secp256k1::{DebugSecp256k1Point, Secp256k1PointPartialEq};
 use openzeppelin_introspection::interface::ISRC5_ID;
 
 use openzeppelin_test_common::eth_account::get_accept_ownership_signature;
 use openzeppelin_testing as utils;
-use openzeppelin_testing::constants::secp256k1::KEY_PAIR;
-use openzeppelin_testing::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY, TRANSACTION_HASH};
-use openzeppelin_testing::signing::Secp256k1SerializedSigning;
+use openzeppelin_testing::constants::TRANSACTION_HASH;
+use openzeppelin_testing::constants::secp256k1::{KEY_PAIR, KEY_PAIR_2};
+use openzeppelin_testing::signing::{Secp256k1KeyPair, Secp256k1SerializedSigning};
 use openzeppelin_utils::serde::SerializedAppend;
 use snforge_std::start_cheat_caller_address;
 
 //
 // Setup
 //
 
-fn setup_snake() -> (DualCaseEthAccount, EthAccountABIDispatcher) {
+fn setup_snake(key_pair: Secp256k1KeyPair) -> (DualCaseEthAccount, EthAccountABIDispatcher) {
     let mut calldata = array![];
-    calldata.append_serde(ETH_PUBKEY());
+    calldata.append_serde(key_pair.public_key);
 
-    let target = utils::declare_and_deploy("SnakeEthAccountMock", calldata);
-    (
-        DualCaseEthAccount { contract_address: target },
-        EthAccountABIDispatcher { contract_address: target }
-    )
+    let contract_address = utils::declare_and_deploy("SnakeEthAccountMock", calldata);
+    (DualCaseEthAccount { contract_address }, EthAccountABIDispatcher { contract_address })
 }
 
-fn setup_camel() -> (DualCaseEthAccount, EthAccountABIDispatcher) {
+fn setup_camel(key_pair: Secp256k1KeyPair) -> (DualCaseEthAccount, EthAccountABIDispatcher) {
     let mut calldata = array![];
-    calldata.append_serde(ETH_PUBKEY());
+    calldata.append_serde(key_pair.public_key);
 
-    let target = utils::declare_and_deploy("CamelEthAccountMock", calldata);
-    (
-        DualCaseEthAccount { contract_address: target },
-        EthAccountABIDispatcher { contract_address: target }
-    )
+    let contract_address = utils::declare_and_deploy("CamelEthAccountMock", calldata);
+    (DualCaseEthAccount { contract_address }, EthAccountABIDispatcher { contract_address })
 }
 
 fn setup_non_account() -> DualCaseEthAccount {
     let calldata = array![];
-    let target = utils::declare_and_deploy("NonImplementingMock", calldata);
-    DualCaseEthAccount { contract_address: target }
+    let contract_address = utils::declare_and_deploy("NonImplementingMock", calldata);
+    DualCaseEthAccount { contract_address }
 }
 
 fn setup_account_panic() -> (DualCaseEthAccount, DualCaseEthAccount) {
@@ -58,37 +52,40 @@ fn setup_account_panic() -> (DualCaseEthAccount, DualCaseEthAccount) {
 
 #[test]
 fn test_dual_set_public_key() {
-    let (snake_dispatcher, target) = setup_snake();
+    let key_pair = KEY_PAIR();
+    let (snake_dispatcher, target) = setup_snake(key_pair);
     let contract_address = snake_dispatcher.contract_address;
 
+    let new_key_pair = KEY_PAIR_2();
     start_cheat_caller_address(contract_address, contract_address);
+    let signature = get_accept_ownership_signature(
+        contract_address, key_pair.public_key, new_key_pair
+    );
+    snake_dispatcher.set_public_key(new_key_pair.public_key, signature);
 
-    let key_pair = KEY_PAIR();
-    let signature = get_accept_ownership_signature(contract_address, ETH_PUBKEY(), key_pair);
-
-    snake_dispatcher.set_public_key(key_pair.public_key, signature);
-    assert_eq!(target.get_public_key(), key_pair.public_key);
+    assert_eq!(target.get_public_key(), new_key_pair.public_key);
 }
 
 #[test]
 #[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();
-    dispatcher.set_public_key(NEW_ETH_PUBKEY(), array![].span());
+    dispatcher.set_public_key(KEY_PAIR().public_key, array![].span());
 }
 
 #[test]
 #[should_panic(expected: ("Some error",))]
 fn test_dual_set_public_key_exists_and_panics() {
     let (dispatcher, _) = setup_account_panic();
-    dispatcher.set_public_key(NEW_ETH_PUBKEY(), array![].span());
+    dispatcher.set_public_key(KEY_PAIR().public_key, array![].span());
 }
 
 #[test]
 fn test_dual_get_public_key() {
-    let (snake_dispatcher, _) = setup_snake();
-    assert_eq!(snake_dispatcher.get_public_key(), ETH_PUBKEY());
+    let key_pair = KEY_PAIR();
+    let (snake_dispatcher, _) = setup_snake(key_pair);
+    assert_eq!(snake_dispatcher.get_public_key(), key_pair.public_key);
 }
 
 #[test]
@@ -108,16 +105,10 @@ fn test_dual_get_public_key_exists_and_panics() {
 
 #[test]
 fn test_dual_is_valid_signature() {
-    let (snake_dispatcher, target) = setup_snake();
-    let contract_address = snake_dispatcher.contract_address;
-
-    start_cheat_caller_address(contract_address, contract_address);
     let key_pair = KEY_PAIR();
-    let signature = get_accept_ownership_signature(contract_address, ETH_PUBKEY(), key_pair);
+    let (snake_dispatcher, _) = setup_snake(key_pair);
 
-    target.set_public_key(key_pair.public_key, signature);
     let serialized_signature = key_pair.serialized_sign(TRANSACTION_HASH.into());
-
     let is_valid = snake_dispatcher.is_valid_signature(TRANSACTION_HASH, serialized_signature);
     assert_eq!(is_valid, starknet::VALIDATED);
 }
@@ -126,26 +117,24 @@ 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 (dispatcher, _) = setup_account_panic();
-    dispatcher.is_valid_signature(hash, signature);
+    dispatcher.is_valid_signature(TRANSACTION_HASH, signature);
 }
 
 #[test]
 fn test_dual_supports_interface() {
-    let (snake_dispatcher, _) = setup_snake();
+    let (snake_dispatcher, _) = setup_snake(KEY_PAIR());
     assert!(snake_dispatcher.supports_interface(ISRC5_ID), "Should implement ISRC5");
 }
 
@@ -171,31 +160,34 @@ 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 key_pair = KEY_PAIR();
+    let (camel_dispatcher, target) = setup_camel(key_pair);
     let contract_address = camel_dispatcher.contract_address;
 
     start_cheat_caller_address(contract_address, contract_address);
+    let new_key_pair = KEY_PAIR_2();
+    let signature = get_accept_ownership_signature(
+        contract_address, key_pair.public_key, new_key_pair
+    );
 
-    let key_pair = KEY_PAIR();
-    let signature = get_accept_ownership_signature(contract_address, ETH_PUBKEY(), key_pair);
-
-    camel_dispatcher.set_public_key(key_pair.public_key, signature);
-    assert_eq!(target.getPublicKey(), key_pair.public_key);
+    camel_dispatcher.set_public_key(new_key_pair.public_key, signature);
+    assert_eq!(target.getPublicKey(), new_key_pair.public_key);
 }
 
 #[test]
 #[ignore] // REASON: foundry entrypoint_not_found error message inconsistent with mainnet.
 #[should_panic(expected: ("Some error",))]
 fn test_dual_setPublicKey_exists_and_panics() {
     let (_, dispatcher) = setup_account_panic();
-    dispatcher.set_public_key(NEW_ETH_PUBKEY(), array![].span());
+    dispatcher.set_public_key(KEY_PAIR_2().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();
-    assert_eq!(camel_dispatcher.get_public_key(), ETH_PUBKEY());
+    let key_pair = KEY_PAIR();
+    let (camel_dispatcher, _) = setup_camel(key_pair);
+    assert_eq!(camel_dispatcher.get_public_key(), key_pair.public_key);
 }
 
 #[test]
@@ -209,17 +201,10 @@ 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 contract_address = camel_dispatcher.contract_address;
-
-    start_cheat_caller_address(contract_address, contract_address);
-
     let key_pair = KEY_PAIR();
-    let signature = get_accept_ownership_signature(contract_address, ETH_PUBKEY(), key_pair);
+    let (camel_dispatcher, _) = setup_camel(key_pair);
 
-    target.setPublicKey(key_pair.public_key, signature);
     let serialized_signature = key_pair.serialized_sign(TRANSACTION_HASH.into());
-
     let is_valid = camel_dispatcher.is_valid_signature(TRANSACTION_HASH, serialized_signature);
     assert_eq!(is_valid, starknet::VALIDATED);
 }
@@ -228,9 +213,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 (_, dispatcher) = setup_account_panic();
-    dispatcher.is_valid_signature(hash, signature);
+    dispatcher.is_valid_signature(TRANSACTION_HASH, signature);
 }