Refactor ETH Account tests

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 {

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);
 }