From cd135797db14b807de3cad08d3bf9e8fa6d5a661 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 12 Dec 2023 13:18:36 +0100 Subject: [PATCH 01/24] feat: add eth_account main logic --- src/account.cairo | 8 +- src/account/account.cairo | 259 +----------------- src/account/account/account.cairo | 230 ++++++++++++++++ src/account/{ => account}/dual_account.cairo | 8 +- src/account/{ => account}/interface.cairo | 2 +- src/account/eth_account.cairo | 5 + .../eth_account/dual_eth_account.cairo | 72 +++++ src/account/eth_account/eth_account.cairo | 243 ++++++++++++++++ src/account/eth_account/interface.cairo | 65 +++++ src/account/utils.cairo | 23 ++ src/account/utils/signature_validator.cairo | 42 +++ src/presets.cairo | 2 + src/presets/eth_account.cairo | 58 ++++ src/tests/account/test_account.cairo | 2 +- src/tests/account/test_dual_account.cairo | 2 +- src/tests/presets/test_account.cairo | 2 +- 16 files changed, 756 insertions(+), 267 deletions(-) create mode 100644 src/account/account/account.cairo rename src/account/{ => account}/dual_account.cairo (91%) rename src/account/{ => account}/interface.cairo (96%) create mode 100644 src/account/eth_account.cairo create mode 100644 src/account/eth_account/dual_eth_account.cairo create mode 100644 src/account/eth_account/eth_account.cairo create mode 100644 src/account/eth_account/interface.cairo create mode 100644 src/account/utils.cairo create mode 100644 src/account/utils/signature_validator.cairo create mode 100644 src/presets/eth_account.cairo diff --git a/src/account.cairo b/src/account.cairo index 60ee4bcad..e50ac6eb3 100644 --- a/src/account.cairo +++ b/src/account.cairo @@ -1,7 +1,7 @@ mod account; -mod dual_account; -mod interface; +mod eth_account; +mod utils; use account::AccountComponent; -use interface::AccountABIDispatcher; -use interface::AccountABIDispatcherTrait; +use account::dual_account; +use account::interface; diff --git a/src/account/account.cairo b/src/account/account.cairo index 8223ae999..da35ae9f4 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -1,256 +1,5 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/account.cairo) +mod account; +mod dual_account; +mod interface; -/// # Account Component -/// -/// The Account component enables contracts to behave as accounts. -#[starknet::component] -mod AccountComponent { - use ecdsa::check_ecdsa_signature; - use openzeppelin::account::interface; - use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; - use openzeppelin::introspection::src5::SRC5Component; - use starknet::account::Call; - use starknet::get_caller_address; - use starknet::get_contract_address; - use starknet::get_tx_info; - - const TRANSACTION_VERSION: felt252 = 1; - // 2**128 + TRANSACTION_VERSION - const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; - - #[storage] - struct Storage { - Account_public_key: felt252 - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - OwnerAdded: OwnerAdded, - OwnerRemoved: OwnerRemoved - } - - #[derive(Drop, starknet::Event)] - struct OwnerAdded { - new_owner_guid: felt252 - } - - #[derive(Drop, starknet::Event)] - struct OwnerRemoved { - removed_owner_guid: felt252 - } - - mod Errors { - const INVALID_CALLER: felt252 = 'Account: invalid caller'; - const INVALID_SIGNATURE: felt252 = 'Account: invalid signature'; - const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version'; - const UNAUTHORIZED: felt252 = 'Account: unauthorized'; - } - - #[embeddable_as(SRC6Impl)] - impl SRC6< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6> { - /// Executes a list of calls from the account. - fn __execute__( - self: @ComponentState, mut calls: Array - ) -> Array> { - // Avoid calls from other contracts - // https://github.com/OpenZeppelin/cairo-contracts/issues/344 - let sender = get_caller_address(); - assert(sender.is_zero(), Errors::INVALID_CALLER); - - // Check tx version - let tx_info = get_tx_info().unbox(); - let version = tx_info.version; - if version != TRANSACTION_VERSION { - assert(version == QUERY_VERSION, Errors::INVALID_TX_VERSION); - } - - _execute_calls(calls) - } - - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `invoke` transactions. - fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { - self.validate_transaction() - } - - /// Verifies that the given signature is valid for the given hash. - fn is_valid_signature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - if self._is_valid_signature(hash, signature.span()) { - starknet::VALIDATED - } else { - 0 - } - } - } - - #[embeddable_as(DeclarerImpl)] - impl Declarer< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeclarer> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `declare` transactions. - fn __validate_declare__( - self: @ComponentState, class_hash: felt252 - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(DeployableImpl)] - impl Deployable< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeployable> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `deploy_account` transactions. - fn __validate_deploy__( - self: @ComponentState, - class_hash: felt252, - contract_address_salt: felt252, - public_key: felt252 - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(PublicKeyImpl)] - impl PublicKey< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKey> { - /// Returns the current public key of the account. - fn get_public_key(self: @ComponentState) -> felt252 { - self.Account_public_key.read() - } - - /// Sets the public key of the account to `new_public_key`. - fn set_public_key(ref self: ComponentState, new_public_key: felt252) { - self.assert_only_self(); - self.emit(OwnerRemoved { removed_owner_guid: self.Account_public_key.read() }); - self._set_public_key(new_public_key); - } - } - - /// Adds camelCase support for `ISRC6`. - #[embeddable_as(SRC6CamelOnlyImpl)] - impl SRC6CamelOnly< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6CamelOnly> { - fn isValidSignature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - self.is_valid_signature(hash, signature) - } - } - - /// Adds camelCase support for `PublicKeyTrait`. - #[embeddable_as(PublicKeyCamelImpl)] - impl PublicKeyCamel< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKeyCamel> { - fn getPublicKey(self: @ComponentState) -> felt252 { - self.Account_public_key.read() - } - - fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { - self.set_public_key(newPublicKey); - } - } - - #[generate_trait] - impl InternalImpl< - TContractState, - +HasComponent, - impl SRC5: SRC5Component::HasComponent, - +Drop - > of InternalTrait { - /// Initializes the account by setting the initial public key - /// and registering the ISRC6 interface Id. - fn initializer(ref self: ComponentState, public_key: felt252) { - let mut src5_component = get_dep_component_mut!(ref self, SRC5); - src5_component.register_interface(interface::ISRC6_ID); - self._set_public_key(public_key); - } - - /// Validates that the caller is the account itself. Otherwise it reverts. - fn assert_only_self(self: @ComponentState) { - let caller = get_caller_address(); - let self = get_contract_address(); - assert(self == caller, Errors::UNAUTHORIZED); - } - - /// Validates the signature for the current transaction. - /// Returns the short string `VALID` if valid, otherwise it reverts. - fn validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); - let tx_hash = tx_info.transaction_hash; - let signature = tx_info.signature; - assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); - starknet::VALIDATED - } - - /// Sets the public key without validating the caller. - /// The usage of this method outside the `set_public_key` function is discouraged. - fn _set_public_key(ref self: ComponentState, new_public_key: felt252) { - self.Account_public_key.write(new_public_key); - self.emit(OwnerAdded { new_owner_guid: new_public_key }); - } - - /// Returns whether the given signature is valid for the given hash - /// using the account's current public key. - fn _is_valid_signature( - self: @ComponentState, hash: felt252, signature: Span - ) -> bool { - let valid_length = signature.len() == 2_u32; - - if valid_length { - check_ecdsa_signature( - hash, self.Account_public_key.read(), *signature.at(0_u32), *signature.at(1_u32) - ) - } else { - false - } - } - } - - fn _execute_calls(mut calls: Array) -> Array> { - let mut res = ArrayTrait::new(); - loop { - match calls.pop_front() { - Option::Some(call) => { - let _res = _execute_single_call(call); - res.append(_res); - }, - Option::None(_) => { break (); }, - }; - }; - res - } - - fn _execute_single_call(call: Call) -> Span { - let Call{to, selector, calldata } = call; - starknet::call_contract_syscall(to, selector, calldata.span()).unwrap() - } -} +use account::AccountComponent; diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo new file mode 100644 index 000000000..d67765df0 --- /dev/null +++ b/src/account/account/account.cairo @@ -0,0 +1,230 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/account.cairo) + +/// # Account Component +/// +/// The Account component enables contracts to behave as accounts. +#[starknet::component] +mod AccountComponent { + use openzeppelin::account::interface; + use openzeppelin::account::utils::{execute_calls, is_valid_signature}; + use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::account::Call; + use starknet::get_caller_address; + use starknet::get_contract_address; + use starknet::get_tx_info; + + const TRANSACTION_VERSION: felt252 = 1; + // 2**128 + TRANSACTION_VERSION + const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; + + #[storage] + struct Storage { + Account_public_key: felt252 + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + OwnerAdded: OwnerAdded, + OwnerRemoved: OwnerRemoved + } + + #[derive(Drop, starknet::Event)] + struct OwnerAdded { + new_owner_guid: felt252 + } + + #[derive(Drop, starknet::Event)] + struct OwnerRemoved { + removed_owner_guid: felt252 + } + + mod Errors { + const INVALID_CALLER: felt252 = 'Account: invalid caller'; + const INVALID_SIGNATURE: felt252 = 'Account: invalid signature'; + const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version'; + const UNAUTHORIZED: felt252 = 'Account: unauthorized'; + } + + #[embeddable_as(SRC6Impl)] + impl SRC6< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6> { + /// Executes a list of calls from the account. + fn __execute__( + self: @ComponentState, mut calls: Array + ) -> Array> { + // Avoid calls from other contracts + // https://github.com/OpenZeppelin/cairo-contracts/issues/344 + let sender = get_caller_address(); + assert(sender.is_zero(), Errors::INVALID_CALLER); + + // Check tx version + let tx_info = get_tx_info().unbox(); + let version = tx_info.version; + if version != TRANSACTION_VERSION { + assert(version == QUERY_VERSION, Errors::INVALID_TX_VERSION); + } + + execute_calls(calls) + } + + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `invoke` transactions. + fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + self.validate_transaction() + } + + /// Verifies that the given signature is valid for the given hash. + fn is_valid_signature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + if self._is_valid_signature(hash, signature.span()) { + starknet::VALIDATED + } else { + 0 + } + } + } + + #[embeddable_as(DeclarerImpl)] + impl Declarer< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeclarer> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `declare` transactions. + fn __validate_declare__( + self: @ComponentState, class_hash: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(DeployableImpl)] + impl Deployable< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeployable> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `deploy_account` transactions. + fn __validate_deploy__( + self: @ComponentState, + class_hash: felt252, + contract_address_salt: felt252, + public_key: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(PublicKeyImpl)] + impl PublicKey< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKey> { + /// Returns the current public key of the account. + fn get_public_key(self: @ComponentState) -> felt252 { + self.Account_public_key.read() + } + + /// Sets the public key of the account to `new_public_key`. + fn set_public_key(ref self: ComponentState, new_public_key: felt252) { + self.assert_only_self(); + self.emit(OwnerRemoved { removed_owner_guid: self.Account_public_key.read() }); + self._set_public_key(new_public_key); + } + } + + /// Adds camelCase support for `ISRC6`. + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6CamelOnly> { + fn isValidSignature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + self.is_valid_signature(hash, signature) + } + } + + /// Adds camelCase support for `PublicKeyTrait`. + #[embeddable_as(PublicKeyCamelImpl)] + impl PublicKeyCamel< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKeyCamel> { + fn getPublicKey(self: @ComponentState) -> felt252 { + self.Account_public_key.read() + } + + fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { + self.set_public_key(newPublicKey); + } + } + + #[generate_trait] + impl InternalImpl< + TContractState, + +HasComponent, + impl SRC5: SRC5Component::HasComponent, + +Drop + > of InternalTrait { + /// Initializes the account by setting the initial public key + /// and registering the ISRC6 interface Id. + fn initializer(ref self: ComponentState, public_key: felt252) { + let mut src5_component = get_dep_component_mut!(ref self, SRC5); + src5_component.register_interface(interface::ISRC6_ID); + self._set_public_key(public_key); + } + + /// Validates that the caller is the account itself. Otherwise it reverts. + fn assert_only_self(self: @ComponentState) { + let caller = get_caller_address(); + let self = get_contract_address(); + assert(self == caller, Errors::UNAUTHORIZED); + } + + /// Validates the signature for the current transaction. + /// Returns the short string `VALID` if valid, otherwise it reverts. + fn validate_transaction(self: @ComponentState) -> felt252 { + let tx_info = get_tx_info().unbox(); + let tx_hash = tx_info.transaction_hash; + let signature = tx_info.signature; + assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); + starknet::VALIDATED + } + + /// Sets the public key without validating the caller. + /// The usage of this method outside the `set_public_key` function is discouraged. + fn _set_public_key(ref self: ComponentState, new_public_key: felt252) { + self.Account_public_key.write(new_public_key); + self.emit(OwnerAdded { new_owner_guid: new_public_key }); + } + + /// Returns whether the given signature is valid for the given hash + /// using the account's current public key. + fn _is_valid_signature( + self: @ComponentState, hash: felt252, signature: Span + ) -> bool { + let public_key = self.Account_public_key.read(); + is_valid_signature(hash, public_key, signature) + } + } +} diff --git a/src/account/dual_account.cairo b/src/account/account/dual_account.cairo similarity index 91% rename from src/account/dual_account.cairo rename to src/account/account/dual_account.cairo index 6576dce52..139b1596e 100644 --- a/src/account/dual_account.cairo +++ b/src/account/account/dual_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/dual_account.cairo) +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/dual_account.cairo) use openzeppelin::utils::UnwrapAndCast; use openzeppelin::utils::selectors; @@ -24,7 +24,7 @@ trait DualCaseAccountABI { impl DualCaseAccountImpl of DualCaseAccountABI { fn set_public_key(self: @DualCaseAccount, new_public_key: felt252) { - let mut args = array![new_public_key]; + let args = array![new_public_key]; try_selector_with_fallback( *self.contract_address, selectors::set_public_key, selectors::setPublicKey, args.span() @@ -33,7 +33,7 @@ impl DualCaseAccountImpl of DualCaseAccountABI { } fn get_public_key(self: @DualCaseAccount) -> felt252 { - let mut args = array![]; + let args = array![]; try_selector_with_fallback( *self.contract_address, selectors::get_public_key, selectors::getPublicKey, args.span() @@ -57,7 +57,7 @@ impl DualCaseAccountImpl of DualCaseAccountABI { } fn supports_interface(self: @DualCaseAccount, interface_id: felt252) -> bool { - let mut args = array![interface_id]; + let args = array![interface_id]; try_selector_with_fallback( *self.contract_address, diff --git a/src/account/interface.cairo b/src/account/account/interface.cairo similarity index 96% rename from src/account/interface.cairo rename to src/account/account/interface.cairo index 3dbc596ba..14087f856 100644 --- a/src/account/interface.cairo +++ b/src/account/account/interface.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/interface.cairo) +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/interface.cairo) use starknet::ContractAddress; use starknet::account::Call; diff --git a/src/account/eth_account.cairo b/src/account/eth_account.cairo new file mode 100644 index 000000000..6818ef0c5 --- /dev/null +++ b/src/account/eth_account.cairo @@ -0,0 +1,5 @@ +mod dual_eth_account; +mod eth_account; +mod interface; + +use eth_account::EthAccountComponent; diff --git a/src/account/eth_account/dual_eth_account.cairo b/src/account/eth_account/dual_eth_account.cairo new file mode 100644 index 000000000..77265f044 --- /dev/null +++ b/src/account/eth_account/dual_eth_account.cairo @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/dual_eth_account.cairo) + +use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::utils::UnwrapAndCast; +use openzeppelin::utils::selectors; +use openzeppelin::utils::serde::SerializedAppend; +use openzeppelin::utils::try_selector_with_fallback; +use starknet::ContractAddress; +use starknet::SyscallResultTrait; + +#[derive(Copy, Drop)] +struct DualCaseEthAccount { + contract_address: ContractAddress +} + +trait DualCaseEthAccountABI { + fn set_public_key(self: @DualCaseEthAccount, new_public_key: EthPublicKey); + fn get_public_key(self: @DualCaseEthAccount) -> EthPublicKey; + fn is_valid_signature( + self: @DualCaseEthAccount, hash: felt252, signature: Array + ) -> felt252; + fn supports_interface(self: @DualCaseEthAccount, interface_id: felt252) -> bool; +} + +impl DualCaseEthAccountImpl of DualCaseEthAccountABI { + fn set_public_key(self: @DualCaseEthAccount, new_public_key: EthPublicKey) { + let mut args = array![]; + new_public_key.serialize(ref args); + + try_selector_with_fallback( + *self.contract_address, selectors::set_public_key, selectors::setPublicKey, args.span() + ) + .unwrap_syscall(); + } + + fn get_public_key(self: @DualCaseEthAccount) -> EthPublicKey { + let args = array![]; + + try_selector_with_fallback( + *self.contract_address, selectors::get_public_key, selectors::getPublicKey, args.span() + ) + .unwrap_and_cast() + } + + fn is_valid_signature( + self: @DualCaseEthAccount, hash: felt252, signature: Array + ) -> felt252 { + let mut args = array![hash]; + args.append_serde(signature); + + try_selector_with_fallback( + *self.contract_address, + selectors::is_valid_signature, + selectors::isValidSignature, + args.span() + ) + .unwrap_and_cast() + } + + fn supports_interface(self: @DualCaseEthAccount, interface_id: felt252) -> bool { + let args = array![interface_id]; + + try_selector_with_fallback( + *self.contract_address, + selectors::supports_interface, + selectors::supportsInterface, + args.span() + ) + .unwrap_and_cast() + } +} diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo new file mode 100644 index 000000000..37c47442a --- /dev/null +++ b/src/account/eth_account/eth_account.cairo @@ -0,0 +1,243 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/eth_account.cairo) + +/// # EthAccount Component +/// +/// The EthAccount component enables contracts to behave as accounts signing with Ethereum keys. +#[starknet::component] +mod EthAccountComponent { + use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::eth_account::interface; + use openzeppelin::account::utils::{execute_calls, is_valid_eth_signature}; + use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; + use openzeppelin::introspection::src5::SRC5Component; + use poseidon::poseidon_hash_span; + use starknet::account::Call; + use starknet::get_caller_address; + use starknet::get_contract_address; + use starknet::get_tx_info; + + + const TRANSACTION_VERSION: felt252 = 1; + // 2**128 + TRANSACTION_VERSION + const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; + + #[storage] + struct Storage { + EthAccount_public_key: EthPublicKey + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + OwnerAdded: OwnerAdded, + OwnerRemoved: OwnerRemoved + } + + #[derive(Drop, starknet::Event)] + struct OwnerAdded { + new_owner_guid: felt252 + } + + #[derive(Drop, starknet::Event)] + struct OwnerRemoved { + removed_owner_guid: felt252 + } + + mod Errors { + const INVALID_CALLER: felt252 = 'EthAccount: invalid caller'; + const INVALID_SIGNATURE: felt252 = 'EthAccount: invalid signature'; + const INVALID_TX_VERSION: felt252 = 'EthAccount: invalid tx version'; + const UNAUTHORIZED: felt252 = 'EthAccount: unauthorized'; + } + + #[embeddable_as(SRC6Impl)] + impl SRC6< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6> { + /// Executes a list of calls from the account. + fn __execute__( + self: @ComponentState, mut calls: Array + ) -> Array> { + // Avoid calls from other contracts + // https://github.com/OpenZeppelin/cairo-contracts/issues/344 + let sender = get_caller_address(); + assert(sender.is_zero(), Errors::INVALID_CALLER); + + // Check tx version + let tx_info = get_tx_info().unbox(); + let version = tx_info.version; + if version != TRANSACTION_VERSION { + assert(version == QUERY_VERSION, Errors::INVALID_TX_VERSION); + } + + execute_calls(calls) + } + + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `invoke` transactions. + fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + self.validate_transaction() + } + + /// Verifies that the given signature is valid for the given hash. + fn is_valid_signature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + if self._is_valid_signature(hash, signature.span()) { + starknet::VALIDATED + } else { + 0 + } + } + } + + #[embeddable_as(DeclarerImpl)] + impl Declarer< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeclarer> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `declare` transactions. + fn __validate_declare__( + self: @ComponentState, class_hash: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(DeployableImpl)] + impl Deployable< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeployable> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `deploy_account` transactions. + fn __validate_deploy__( + self: @ComponentState, + class_hash: felt252, + contract_address_salt: felt252, + public_key: EthPublicKey + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(PublicKeyImpl)] + impl PublicKey< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKey> { + /// Returns the current public key of the account. + fn get_public_key(self: @ComponentState) -> EthPublicKey { + self.EthAccount_public_key.read() + } + + /// Sets the public key of the account to `new_public_key`. + fn set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { + self.assert_only_self(); + + let current_public_key: EthPublicKey = self.EthAccount_public_key.read(); + let removed_owner_guid = _get_guid_from_public_key(current_public_key); + + self.emit(OwnerRemoved { removed_owner_guid }); + self._set_public_key(new_public_key); + } + } + + /// Adds camelCase support for `ISRC6`. + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6CamelOnly> { + fn isValidSignature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + self.is_valid_signature(hash, signature) + } + } + + /// Adds camelCase support for `PublicKeyTrait`. + #[embeddable_as(PublicKeyCamelImpl)] + impl PublicKeyCamel< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKeyCamel> { + fn getPublicKey(self: @ComponentState) -> EthPublicKey { + self.EthAccount_public_key.read() + } + + fn setPublicKey(ref self: ComponentState, newPublicKey: EthPublicKey) { + self.set_public_key(newPublicKey); + } + } + + #[generate_trait] + impl InternalImpl< + TContractState, + +HasComponent, + impl SRC5: SRC5Component::HasComponent, + +Drop + > of InternalTrait { + /// Initializes the account by setting the initial public key + /// and registering the ISRC6 interface Id. + fn initializer(ref self: ComponentState, public_key: EthPublicKey) { + let mut src5_component = get_dep_component_mut!(ref self, SRC5); + src5_component.register_interface(interface::ISRC6_ID); + self._set_public_key(public_key); + } + + /// Validates that the caller is the account itself. Otherwise it reverts. + fn assert_only_self(self: @ComponentState) { + let caller = get_caller_address(); + let self = get_contract_address(); + assert(self == caller, Errors::UNAUTHORIZED); + } + + /// Validates the signature for the current transaction. + /// Returns the short string `VALID` if valid, otherwise it reverts. + fn validate_transaction(self: @ComponentState) -> felt252 { + let tx_info = get_tx_info().unbox(); + let tx_hash = tx_info.transaction_hash; + let signature = tx_info.signature; + assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); + starknet::VALIDATED + } + + /// Sets the public key without validating the caller. + /// The usage of this method outside the `set_public_key` function is discouraged. + fn _set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { + self.EthAccount_public_key.write(new_public_key); + let new_owner_guid = _get_guid_from_public_key(new_public_key); + self.emit(OwnerAdded { new_owner_guid }); + } + + /// Returns whether the given signature is valid for the given hash + /// using the account's current public key. + fn _is_valid_signature( + self: @ComponentState, hash: felt252, signature: Span + ) -> bool { + let public_key: EthPublicKey = self.EthAccount_public_key.read(); + is_valid_eth_signature(hash, public_key, signature) + } + } + + fn _get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { + let (x, y) = public_key; + poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) + } +} diff --git a/src/account/eth_account/interface.cairo b/src/account/eth_account/interface.cairo new file mode 100644 index 000000000..06a6d04e3 --- /dev/null +++ b/src/account/eth_account/interface.cairo @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/interface.cairo) + +use openzeppelin::account::interface::{ISRC6, ISRC6CamelOnly, IDeclarer, ISRC6_ID}; +use starknet::ContractAddress; +use starknet::account::Call; + +type EthPublicKey = (u256, u256); + +#[starknet::interface] +trait IDeployable { + fn __validate_deploy__( + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey + ) -> felt252; +} + +#[starknet::interface] +trait IPublicKey { + fn get_public_key(self: @TState) -> EthPublicKey; + fn set_public_key(ref self: TState, new_public_key: EthPublicKey); +} + + +#[starknet::interface] +trait IPublicKeyCamel { + fn getPublicKey(self: @TState) -> EthPublicKey; + fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); +} + +// +// EthAccount ABI +// + +#[starknet::interface] +trait EthAccountABI { + // ISRC6 + fn __execute__(self: @TState, calls: Array) -> Array>; + fn __validate__(self: @TState, calls: Array) -> felt252; + fn is_valid_signature(self: @TState, hash: felt252, signature: Array) -> felt252; + + // ISRC5 + fn supports_interface(self: @TState, interface_id: felt252) -> bool; + + // IDeclarer + fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; + + // IDeployable + fn __validate_deploy__( + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey + ) -> felt252; + + // IPublicKey + fn get_public_key(self: @TState) -> EthPublicKey; + fn set_public_key(ref self: TState, new_public_key: EthPublicKey); + + // ISRC6CamelOnly + fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; + + // ISRC5Camel + fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; + + // IPublicKeyCamel + fn getPublicKey(self: @TState) -> EthPublicKey; + fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); +} diff --git a/src/account/utils.cairo b/src/account/utils.cairo new file mode 100644 index 000000000..1cc920a3d --- /dev/null +++ b/src/account/utils.cairo @@ -0,0 +1,23 @@ +mod signature_validator; + +use signature_validator::{is_valid_signature, is_valid_eth_signature}; +use starknet::account::Call; + +fn execute_calls(mut calls: Array) -> Array> { + let mut res = ArrayTrait::new(); + loop { + match calls.pop_front() { + Option::Some(call) => { + let _res = execute_single_call(call); + res.append(_res); + }, + Option::None(_) => { break (); }, + }; + }; + res +} + +fn execute_single_call(call: Call) -> Span { + let Call{to, selector, calldata } = call; + starknet::call_contract_syscall(to, selector, calldata.span()).unwrap() +} diff --git a/src/account/utils/signature_validator.cairo b/src/account/utils/signature_validator.cairo new file mode 100644 index 000000000..8533d29a5 --- /dev/null +++ b/src/account/utils/signature_validator.cairo @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/signature_validator.cairo) + +use ecdsa::check_ecdsa_signature; +use openzeppelin::account::eth_account::interface::EthPublicKey; +use starknet::eth_signature::{Signature, is_eth_signature_valid}; +use starknet::secp256_trait::{Secp256PointTrait, is_signature_entry_valid, recover_public_key}; +use starknet::secp256k1::Secp256k1Point; + + +fn is_valid_signature(msg_hash: felt252, public_key: felt252, signature: Span) -> bool { + let valid_length = signature.len() == 2; + + if valid_length { + check_ecdsa_signature(msg_hash, public_key, *signature.at(0_u32), *signature.at(1_u32)) + } else { + false + } +} + +fn is_valid_eth_signature( + msg_hash: felt252, public_key: EthPublicKey, signature: Span +) -> bool { + let mut signature = signature; + let signature: Signature = Serde::deserialize(ref signature).unwrap(); + + // Signature out of range + if !is_signature_entry_valid::(signature.r) { + return false; + } + if !is_signature_entry_valid::(signature.s) { + return false; + } + + let public_key_point: Secp256k1Point = recover_public_key(msg_hash.into(), signature).unwrap(); + + if public_key_point.get_coordinates().unwrap() != public_key { + // Invalid signature + return false; + } + true +} diff --git a/src/presets.cairo b/src/presets.cairo index 6d3ddc6f6..07faa0fc7 100644 --- a/src/presets.cairo +++ b/src/presets.cairo @@ -1,7 +1,9 @@ mod account; mod erc20; mod erc721; +mod eth_account; use account::Account; use erc20::ERC20; use erc721::ERC721; +use eth_account::EthAccount; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo new file mode 100644 index 000000000..ee62ce149 --- /dev/null +++ b/src/presets/eth_account.cairo @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (presets/eth_account.cairo) + +/// # EthAccount Preset +/// +/// OpenZeppelin's account which can change its public key and declare, +/// deploy, or call contracts, using Ethereum signing keys. +#[starknet::contract] +mod EthAccount { + use openzeppelin::account::eth_account::EthAccountComponent; + use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::introspection::src5::SRC5Component; + + component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6Impl = EthAccountComponent::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = EthAccountComponent::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyImpl = EthAccountComponent::PublicKeyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = + EthAccountComponent::PublicKeyCamelImpl; + #[abi(embed_v0)] + impl DeclarerImpl = EthAccountComponent::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = EthAccountComponent::DeployableImpl; + impl AccountInternalImpl = EthAccountComponent::InternalImpl; + + // SRC5 + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + + #[storage] + struct Storage { + #[substorage(v0)] + eth_account: EthAccountComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + EthAccountEvent: EthAccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: EthPublicKey) { + self.eth_account.initializer(public_key); + } +} diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index a5b1f0a40..0ab544291 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -2,8 +2,8 @@ use openzeppelin::account::AccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; use openzeppelin::account::AccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::AccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; use openzeppelin::account::AccountComponent::{TRANSACTION_VERSION, QUERY_VERSION}; +use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; -use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::account_mocks::DualCaseAccountMock; use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20; diff --git a/src/tests/account/test_dual_account.cairo b/src/tests/account/test_dual_account.cairo index ef7ee88c5..763adba12 100644 --- a/src/tests/account/test_dual_account.cairo +++ b/src/tests/account/test_dual_account.cairo @@ -1,5 +1,5 @@ use openzeppelin::account::dual_account::{DualCaseAccountABI, DualCaseAccount}; -use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; +use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::test_account::SIGNED_TX_DATA; use openzeppelin::tests::mocks::account_mocks::{ diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index 080b48d41..ed6359f4b 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -1,7 +1,7 @@ use openzeppelin::account::AccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::AccountComponent::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::interface::ISRC6_ID; -use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; +use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::presets::Account; use openzeppelin::tests::account::test_account::{ From 463d6637c89a153b9f577ec3dfc4987b1fe9b8ca Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 15 Dec 2023 16:35:26 +0100 Subject: [PATCH 02/24] feat: add tests --- Scarb.toml | 5 +- src/account/utils.cairo | 7 +- ...nature_validator.cairo => signature.cairo} | 3 +- src/tests/account.cairo | 2 + src/tests/account/test_account.cairo | 20 +- src/tests/account/test_dual_eth_account.cairo | 237 +++++++ src/tests/account/test_eth_account.cairo | 581 ++++++++++++++++++ src/tests/mocks.cairo | 1 + src/tests/mocks/eth_account_mocks.cairo | 211 +++++++ src/tests/utils/constants.cairo | 8 + 10 files changed, 1059 insertions(+), 16 deletions(-) rename src/account/utils/{signature_validator.cairo => signature.cairo} (98%) create mode 100644 src/tests/account/test_dual_eth_account.cairo create mode 100644 src/tests/account/test_eth_account.cairo create mode 100644 src/tests/mocks/eth_account_mocks.cairo diff --git a/Scarb.toml b/Scarb.toml index d684990ec..64a02997c 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -1,7 +1,8 @@ [package] name = "openzeppelin" version = "0.8.0" -cairo-version = "2.3.1" +edition = "2023_01" +cairo-version = "2.4.0" authors = ["OpenZeppelin Community "] description = "OpenZeppelin Contracts written in Cairo for StarkNet, a decentralized ZK Rollup" documentation = "https://docs.openzeppelin.com/contracts-cairo" @@ -11,7 +12,7 @@ license-file = "LICENSE" keywords = ["openzeppelin", "starknet", "cairo", "contracts", "security", "standards"] [dependencies] -starknet = "2.3.1" +starknet = "2.4.0" [lib] diff --git a/src/account/utils.cairo b/src/account/utils.cairo index 1cc920a3d..92b081c67 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -1,6 +1,9 @@ -mod signature_validator; +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils.cairo) -use signature_validator::{is_valid_signature, is_valid_eth_signature}; +mod signature; + +use signature::{is_valid_signature, is_valid_eth_signature}; use starknet::account::Call; fn execute_calls(mut calls: Array) -> Array> { diff --git a/src/account/utils/signature_validator.cairo b/src/account/utils/signature.cairo similarity index 98% rename from src/account/utils/signature_validator.cairo rename to src/account/utils/signature.cairo index 8533d29a5..fae140ca2 100644 --- a/src/account/utils/signature_validator.cairo +++ b/src/account/utils/signature.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/signature_validator.cairo) +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/signature.cairo) use ecdsa::check_ecdsa_signature; use openzeppelin::account::eth_account::interface::EthPublicKey; @@ -33,7 +33,6 @@ fn is_valid_eth_signature( } let public_key_point: Secp256k1Point = recover_public_key(msg_hash.into(), signature).unwrap(); - if public_key_point.get_coordinates().unwrap() != public_key { // Invalid signature return false; diff --git a/src/tests/account.cairo b/src/tests/account.cairo index 1708ec912..356504143 100644 --- a/src/tests/account.cairo +++ b/src/tests/account.cairo @@ -1,2 +1,4 @@ mod test_account; mod test_dual_account; +mod test_dual_eth_account; +mod test_eth_account; diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 13944115f..4b3006a9d 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -27,6 +27,16 @@ struct SignedTransactionData { s: felt252 } +fn SIGNED_TX_DATA() -> SignedTransactionData { + SignedTransactionData { + private_key: 1234, + public_key: 0x1f3c942d7f492a37608cde0d77b884a5aa9e11d2919225968557370ddb5a5aa, + transaction_hash: 0x601d3d2e265c10ff645e1554c435e72ce6721f0ba5fc96f0c650bfc6231191a, + r: 0x6c8be1fb0fb5c730fbd7abaecbed9d980376ff2e660dfcd157e158d2b026891, + s: 0x76b4669998eb933f44a59eace12b41328ab975ceafddf92602b21eb23e22e35 + } +} + // // Constants // @@ -39,16 +49,6 @@ fn ACCOUNT_ADDRESS() -> ContractAddress { contract_address_const::<0x111111>() } -fn SIGNED_TX_DATA() -> SignedTransactionData { - SignedTransactionData { - private_key: 1234, - public_key: 0x1f3c942d7f492a37608cde0d77b884a5aa9e11d2919225968557370ddb5a5aa, - transaction_hash: 0x601d3d2e265c10ff645e1554c435e72ce6721f0ba5fc96f0c650bfc6231191a, - r: 0x6c8be1fb0fb5c730fbd7abaecbed9d980376ff2e660dfcd157e158d2b026891, - s: 0x76b4669998eb933f44a59eace12b41328ab975ceafddf92602b21eb23e22e35 - } -} - // // Setup // diff --git a/src/tests/account/test_dual_eth_account.cairo b/src/tests/account/test_dual_eth_account.cairo new file mode 100644 index 000000000..95c2e0287 --- /dev/null +++ b/src/tests/account/test_dual_eth_account.cairo @@ -0,0 +1,237 @@ +use openzeppelin::account::eth_account::dual_eth_account::{ + DualCaseEthAccountABI, DualCaseEthAccount +}; +use openzeppelin::account::eth_account::interface::{ + EthAccountABIDispatcherTrait, EthAccountABIDispatcher +}; +use openzeppelin::introspection::interface::ISRC5_ID; +use openzeppelin::tests::account::test_eth_account::SIGNED_TX_DATA; +use openzeppelin::tests::mocks::eth_account_mocks::{ + CamelEthAccountPanicMock, CamelEthAccountMock, SnakeEthAccountMock, SnakeEthAccountPanicMock +}; +use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; +use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY}; +use openzeppelin::tests::utils; +use openzeppelin::utils::serde::SerializedAppend; +use starknet::eth_signature::Signature; +use starknet::testing; + +// +// Setup +// + +fn setup_snake() -> (DualCaseEthAccount, EthAccountABIDispatcher) { + let mut calldata = array![]; + calldata.append_serde(ETH_PUBKEY()); + + let target = utils::deploy(SnakeEthAccountMock::TEST_CLASS_HASH, calldata); + ( + DualCaseEthAccount { contract_address: target }, + EthAccountABIDispatcher { contract_address: target } + ) +} + +fn setup_camel() -> (DualCaseEthAccount, EthAccountABIDispatcher) { + let mut calldata = array![]; + calldata.append_serde(ETH_PUBKEY()); + + let target = utils::deploy(CamelEthAccountMock::TEST_CLASS_HASH, calldata); + ( + DualCaseEthAccount { contract_address: target }, + EthAccountABIDispatcher { contract_address: target } + ) +} + +fn setup_non_account() -> DualCaseEthAccount { + let calldata = array![]; + let target = utils::deploy(NonImplementingMock::TEST_CLASS_HASH, calldata); + DualCaseEthAccount { contract_address: target } +} + +fn setup_account_panic() -> (DualCaseEthAccount, DualCaseEthAccount) { + let snake_target = utils::deploy(SnakeEthAccountPanicMock::TEST_CLASS_HASH, array![]); + let camel_target = utils::deploy(CamelEthAccountPanicMock::TEST_CLASS_HASH, array![]); + ( + DualCaseEthAccount { contract_address: snake_target }, + DualCaseEthAccount { contract_address: camel_target } + ) +} + +// +// snake_case target +// + +#[test] +fn test_dual_set_public_key() { + let (snake_dispatcher, target) = setup_snake(); + + testing::set_contract_address(snake_dispatcher.contract_address); + + let new_public_key = NEW_ETH_PUBKEY(); + snake_dispatcher.set_public_key(new_public_key); + assert(target.get_public_key() == new_public_key, 'Should return NEW_ETH_PUBKEY'); +} + +#[test] +#[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()); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_set_public_key_exists_and_panics() { + let (dispatcher, _) = setup_account_panic(); + dispatcher.set_public_key(NEW_ETH_PUBKEY()); +} + +#[test] +fn test_dual_get_public_key() { + let (snake_dispatcher, _) = setup_snake(); + assert(snake_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); +} + +#[test] +#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] +fn test_dual_no_get_public_key() { + let dispatcher = setup_non_account(); + dispatcher.get_public_key(); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_get_public_key_exists_and_panics() { + let (dispatcher, _) = setup_account_panic(); + dispatcher.get_public_key(); +} + +#[test] +fn test_dual_is_valid_signature() { + let (snake_dispatcher, target) = setup_snake(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut serialized_signature = array![]; + data.signature.serialize(ref serialized_signature); + + testing::set_contract_address(snake_dispatcher.contract_address); + target.set_public_key(data.public_key); + + let is_valid = snake_dispatcher.is_valid_signature(hash, serialized_signature); + assert(is_valid == 'VALID', 'Should accept valid signature'); +} + +#[test] +#[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); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +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); +} + +#[test] +fn test_dual_supports_interface() { + let (snake_dispatcher, target) = setup_snake(); + assert(snake_dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); +} + +#[test] +#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] +fn test_dual_no_supports_interface() { + let dispatcher = setup_non_account(); + dispatcher.supports_interface(ISRC5_ID); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_supports_interface_exists_and_panics() { + let (dispatcher, _) = setup_account_panic(); + dispatcher.supports_interface(ISRC5_ID); +} + +// +// camelCase target +// + +#[test] +fn test_dual_setPublicKey() { + let (camel_dispatcher, target) = setup_camel(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(camel_dispatcher.contract_address); + + camel_dispatcher.set_public_key(new_public_key); + assert(target.getPublicKey() == new_public_key, 'Should return NEW_ETH_PUBKEY'); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_setPublicKey_exists_and_panics() { + let (_, dispatcher) = setup_account_panic(); + dispatcher.set_public_key(NEW_ETH_PUBKEY()); +} + +#[test] +fn test_dual_getPublicKey() { + let (camel_dispatcher, _) = setup_camel(); + assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_getPublicKey_exists_and_panics() { + let (_, dispatcher) = setup_account_panic(); + dispatcher.get_public_key(); +} + +#[test] +fn test_dual_isValidSignature() { + let (camel_dispatcher, target) = setup_camel(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut serialized_signature = array![]; + data.signature.serialize(ref serialized_signature); + + testing::set_contract_address(camel_dispatcher.contract_address); + target.setPublicKey(data.public_key); + + let is_valid = camel_dispatcher.is_valid_signature(hash, serialized_signature); + assert(is_valid == 'VALID', 'Should accept valid signature'); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_isValidSignature_exists_and_panics() { + let hash = 0x0; + let signature = array![]; + + let (_, dispatcher) = setup_account_panic(); + dispatcher.is_valid_signature(hash, signature); +} + +#[test] +fn test_dual_supportsInterface() { + let (camel_dispatcher, _) = setup_camel(); + assert(camel_dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); +} + +#[test] +#[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] +fn test_dual_supportsInterface_exists_and_panics() { + let (_, dispatcher) = setup_account_panic(); + dispatcher.supports_interface(ISRC5_ID); +} diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo new file mode 100644 index 000000000..7fc75388a --- /dev/null +++ b/src/tests/account/test_eth_account.cairo @@ -0,0 +1,581 @@ +use openzeppelin::account::eth_account::EthAccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; +use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::eth_account::EthAccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; +use openzeppelin::account::eth_account::EthAccountComponent::{TRANSACTION_VERSION, QUERY_VERSION}; +use openzeppelin::account::eth_account::EthAccountComponent; +use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::eth_account::interface::{ + EthAccountABIDispatcherTrait, EthAccountABIDispatcher +}; +use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; +use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; +use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; +use openzeppelin::tests::mocks::eth_account_mocks::DualCaseEthAccountMock; +use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO}; +use openzeppelin::tests::utils; +use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; +use openzeppelin::utils::selectors; +use openzeppelin::utils::serde::SerializedAppend; +use poseidon::poseidon_hash_span; +use starknet::ContractAddress; +use starknet::account::Call; +use starknet::contract_address_const; +use starknet::eth_signature::Signature; +use starknet::testing; + +#[derive(Drop)] +struct SignedTransactionData { + private_key: u256, + public_key: EthPublicKey, + transaction_hash: felt252, + signature: Signature +} + +/// This signature was computed using ethers.js. +fn SIGNED_TX_DATA() -> SignedTransactionData { + SignedTransactionData { + private_key: 0x45397ee6ca34cb49060f1c303c6cb7ee2d6123e617601ef3e31ccf7bf5bef1f9, + public_key: ( + 0x829307f82a1883c2414503ba85fc85037f22c6fc6f80910801f6b01a4131da1e, + 0x2a23f7bddf3715d11767b1247eccc68c89e11b926e2615268db6ad1af8d8da96 + ), + transaction_hash: 0x008f882c63d0396d216d57529fe29ad5e70b6cd51b47bd2458b0a4ccb2ba0957, + signature: Signature { + r: 0x82bb3efc0554ec181405468f273b0dbf935cca47182b22da78967d0770f7dcc3, + s: 0x6719fef30c11c74add873e4da0e1234deb69eae6a6bd4daa44b816dc199f3e86, + y_parity: true + } + } +} + +// +// Constants +// + +fn CLASS_HASH() -> felt252 { + DualCaseEthAccountMock::TEST_CLASS_HASH +} + +fn ACCOUNT_ADDRESS() -> ContractAddress { + Zeroable::zero() +} + +// +// Setup +// + +type ComponentState = EthAccountComponent::ComponentState; + +fn CONTRACT_STATE() -> DualCaseEthAccountMock::ContractState { + DualCaseEthAccountMock::contract_state_for_testing() +} + +fn COMPONENT_STATE() -> ComponentState { + EthAccountComponent::component_state_for_testing() +} + +fn setup() -> ComponentState { + let mut state = COMPONENT_STATE(); + state.initializer(ETH_PUBKEY()); + utils::drop_event(ZERO()); + state +} + +fn setup_dispatcher(data: Option<@SignedTransactionData>) -> EthAccountABIDispatcher { + testing::set_version(TRANSACTION_VERSION); + + let mut calldata = array![]; + if data.is_some() { + let data = data.unwrap(); + let mut serialized_signature = array![]; + data.signature.serialize(ref serialized_signature); + + testing::set_signature(serialized_signature.span()); + testing::set_transaction_hash(*data.transaction_hash); + + calldata.append_serde(*data.public_key); + } else { + calldata.append_serde(ETH_PUBKEY()); + } + let address = utils::deploy(CLASS_HASH(), calldata); + EthAccountABIDispatcher { contract_address: address } +} + +fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> IERC20Dispatcher { + let name = 0; + let symbol = 0; + let mut calldata = array![]; + + calldata.append_serde(name); + calldata.append_serde(symbol); + calldata.append_serde(initial_supply); + calldata.append_serde(recipient); + + let address = utils::deploy(DualCaseERC20Mock::TEST_CLASS_HASH, calldata); + IERC20Dispatcher { contract_address: address } +} + +// +// is_valid_signature & isValidSignature +// + +#[test] +fn test_is_valid_signature() { + let mut state = COMPONENT_STATE(); + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut bad_signature = data.signature; + + bad_signature.r += 1; + + let mut serialized_good_signature = array![]; + let mut serialized_bad_signature = array![]; + + data.signature.serialize(ref serialized_good_signature); + bad_signature.serialize(ref serialized_bad_signature); + + state.set_public_key(data.public_key); + + let is_valid = state.is_valid_signature(hash, serialized_good_signature); + assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); + + let is_valid = state.is_valid_signature(hash, serialized_bad_signature); + assert(is_valid == 0, '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 bad_signature = data.signature; + + bad_signature.r += 1; + + let mut serialized_good_signature = array![]; + let mut serialized_bad_signature = array![]; + + data.signature.serialize(ref serialized_good_signature); + bad_signature.serialize(ref serialized_bad_signature); + + state.set_public_key(data.public_key); + + let is_valid = state.isValidSignature(hash, serialized_good_signature); + assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); + + let is_valid = state.isValidSignature(hash, serialized_bad_signature); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +// +// Entry points +// + +#[test] +fn test_validate_deploy() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + + // `__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. + assert( + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_deploy_invalid_signature_data() { + let mut data = SIGNED_TX_DATA(); + data.transaction_hash += 1; + let account = setup_dispatcher(Option::Some(@data)); + + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +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()); + + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +fn test_validate_deploy_empty_signature() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let empty_sig = array![]; + + testing::set_signature(empty_sig.span()); + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +fn test_validate_declare() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + + // `__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. + assert( + account.__validate_declare__(CLASS_HASH()) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_invalid_signature_data() { + let mut data = SIGNED_TX_DATA(); + data.transaction_hash += 1; + let account = setup_dispatcher(Option::Some(@data)); + + account.__validate_declare__(CLASS_HASH()); +} + +#[test] +#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_invalid_signature_length() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let mut signature = array![]; + + signature.append(0x1); + testing::set_signature(signature.span()); + + account.__validate_declare__(CLASS_HASH()); +} + +#[test] +#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_empty_signature() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let empty_sig = array![]; + + testing::set_signature(empty_sig.span()); + + 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 erc20 = deploy_erc20(account.contract_address, 1000); + let recipient = contract_address_const::<0x123>(); + + // Craft call and add to calls array + let mut calldata = array![]; + let amount: u256 = 200; + calldata.append_serde(recipient); + calldata.append_serde(amount); + let call = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata + }; + let mut calls = array![]; + calls.append(call); + + // Handle version for test + if version.is_some() { + testing::set_version(version.unwrap()); + } + + // Execute + let ret = account.__execute__(calls); + + // Assert that the transfer was successful + assert(erc20.balance_of(account.contract_address) == 800, 'Should have remainder'); + assert(erc20.balance_of(recipient) == amount, 'Should have transferred'); + + // Test return value + let mut call_serialized_retval = *ret.at(0); + let call_retval = Serde::::deserialize(ref call_serialized_retval); + assert(call_retval.unwrap(), 'Should have succeeded'); +} + +#[test] +fn test_execute() { + test_execute_with_version(Option::None(())); +} + +#[test] +fn test_execute_query_version() { + test_execute_with_version(Option::Some(QUERY_VERSION)); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid tx version', 'ENTRYPOINT_FAILED'))] +fn test_execute_invalid_version() { + test_execute_with_version(Option::Some(TRANSACTION_VERSION - 1)); +} + +#[test] +fn test_validate() { + let calls = array![]; + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + + assert(account.__validate__(calls) == starknet::VALIDATED, 'Should validate correctly'); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_invalid() { + let calls = array![]; + let mut data = SIGNED_TX_DATA(); + data.transaction_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 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![]; + let amount1: u256 = 300; + calldata1.append_serde(recipient1); + calldata1.append_serde(amount1); + let call1 = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata1 + }; + + // Craft call2 + let mut calldata2 = array![]; + let amount2: u256 = 500; + calldata2.append_serde(recipient2); + calldata2.append_serde(amount2); + let call2 = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata2 + }; + + // Bundle calls and exeute + calls.append(call1); + calls.append(call2); + let ret = account.__execute__(calls); + + // Assert that the transfers were successful + assert(erc20.balance_of(account.contract_address) == 200, 'Should have remainder'); + assert(erc20.balance_of(recipient1) == 300, 'Should have transferred'); + assert(erc20.balance_of(recipient2) == 500, 'Should have transferred'); + + // Test return value + let mut call1_serialized_retval = *ret.at(0); + let mut call2_serialized_retval = *ret.at(1); + let call1_retval = Serde::::deserialize(ref call1_serialized_retval); + let call2_retval = Serde::::deserialize(ref call2_serialized_retval); + assert(call1_retval.unwrap(), 'Should have succeeded'); + assert(call2_retval.unwrap(), 'Should have succeeded'); +} + +#[test] +fn test_multicall_zero_calls() { + let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); + let mut calls = array![]; + + let ret = account.__execute__(calls); + + // Test return value + assert(ret.len() == 0, 'Should have an empty response'); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid caller',))] +fn test_account_called_from_contract() { + let state = setup(); + let calls = array![]; + let caller = contract_address_const::<0x123>(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + testing::set_caller_address(caller); + + state.__execute__(calls); +} + +// +// set_public_key & get_public_key +// + +#[test] +fn test_public_key_setter_and_getter() { + let mut state = COMPONENT_STATE(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + testing::set_caller_address(ACCOUNT_ADDRESS()); + + // Check default + let public_key = state.get_public_key(); + assert(public_key == (0, 0), 'Should be zero'); + + // Set key + state.set_public_key(new_public_key); + + assert_event_owner_removed(ACCOUNT_ADDRESS(), (0, 0)); + assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); + + let public_key = state.get_public_key(); + assert(public_key == new_public_key, 'Should update key'); +} + +#[test] +#[should_panic(expected: ('EthAccount: 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); + + state.set_public_key(NEW_ETH_PUBKEY()); +} + +// +// setPublicKey & getPublicKey +// + +#[test] +fn test_public_key_setter_and_getter_camel() { + let mut state = COMPONENT_STATE(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + testing::set_caller_address(ACCOUNT_ADDRESS()); + + let public_key = state.getPublicKey(); + assert(public_key == (0, 0), 'Should be zero'); + + state.setPublicKey(new_public_key); + + assert_event_owner_removed(ACCOUNT_ADDRESS(), (0, 0)); + assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); + + let public_key = state.getPublicKey(); + assert(public_key == new_public_key, 'Should update key'); +} + +#[test] +#[should_panic(expected: ('EthAccount: 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); + + state.setPublicKey(NEW_ETH_PUBKEY()); +} + +// +// Test internals +// + +#[test] +fn test_initializer() { + let mut state = COMPONENT_STATE(); + let mock_state = CONTRACT_STATE(); + let public_key = ETH_PUBKEY(); + + state.initializer(public_key); + + assert_only_event_owner_added(ZERO(), public_key); + + assert(state.get_public_key() == public_key, 'Should return public_key'); + + let supports_default_interface = mock_state.supports_interface(ISRC5_ID); + assert(supports_default_interface, 'Should support base interface'); + + let supports_account_interface = mock_state.supports_interface(ISRC6_ID); + assert(supports_account_interface, 'Should support account id'); +} + +#[test] +fn test_assert_only_self_true() { + let mut state = COMPONENT_STATE(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + testing::set_caller_address(ACCOUNT_ADDRESS()); + state.assert_only_self(); +} + +#[test] +#[should_panic(expected: ('EthAccount: unauthorized',))] +fn test_assert_only_self_false() { + let mut state = COMPONENT_STATE(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + let other = contract_address_const::<0x4567>(); + testing::set_caller_address(other); + state.assert_only_self(); +} + +#[test] +fn test__is_valid_signature() { + let mut state = COMPONENT_STATE(); + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + + let mut bad_signature = data.signature; + + bad_signature.r += 1; + + let mut serialized_good_signature = array![]; + let mut serialized_bad_signature = array![]; + + data.signature.serialize(ref serialized_good_signature); + bad_signature.serialize(ref serialized_bad_signature); + + state.set_public_key(data.public_key); + + let is_valid = state._is_valid_signature(hash, serialized_good_signature.span()); + assert(is_valid, 'Should accept valid signature'); + + let is_valid = state._is_valid_signature(hash, serialized_bad_signature.span()); + assert(!is_valid, 'Should reject invalid signature'); +} + +#[test] +fn test__set_public_key() { + let mut state = COMPONENT_STATE(); + let public_key = ETH_PUBKEY(); + state._set_public_key(public_key); + + assert_only_event_owner_added(ZERO(), public_key); + + let public_key = state.get_public_key(); + assert(public_key == public_key, 'Should update key'); +} + +// +// Helpers +// + +fn assert_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { + let event = utils::pop_log::(contract).unwrap(); + let guid = get_guid_from_public_key(public_key); + assert(event.new_owner_guid == guid, 'Invalid `new_owner_guid`'); +} + +fn assert_only_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { + assert_event_owner_added(contract, public_key); + utils::assert_no_events_left(contract); +} + +fn assert_event_owner_removed(contract: ContractAddress, public_key: EthPublicKey) { + let event = utils::pop_log::(contract).unwrap(); + let guid = get_guid_from_public_key(public_key); + assert(event.removed_owner_guid == guid, 'Invalid `removed_owner_guid`'); +} + +fn get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { + let (x, y) = public_key; + poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) +} diff --git a/src/tests/mocks.cairo b/src/tests/mocks.cairo index 4bc03de22..710c4be90 100644 --- a/src/tests/mocks.cairo +++ b/src/tests/mocks.cairo @@ -3,6 +3,7 @@ mod account_mocks; mod erc20_mocks; mod erc721_mocks; mod erc721_receiver_mocks; +mod eth_account_mocks; mod initializable_mock; mod non_implementing_mock; mod ownable_mocks; diff --git a/src/tests/mocks/eth_account_mocks.cairo b/src/tests/mocks/eth_account_mocks.cairo new file mode 100644 index 000000000..009b85f41 --- /dev/null +++ b/src/tests/mocks/eth_account_mocks.cairo @@ -0,0 +1,211 @@ +#[starknet::contract] +mod DualCaseEthAccountMock { + use openzeppelin::account::eth_account::EthAccountComponent; + use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::introspection::src5::SRC5Component; + + component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6Impl = EthAccountComponent::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = EthAccountComponent::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl DeclarerImpl = EthAccountComponent::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = EthAccountComponent::DeployableImpl; + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + eth_account: EthAccountComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + EthAccountEvent: EthAccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: EthPublicKey) { + self.eth_account.initializer(public_key); + } +} + +#[starknet::contract] +mod SnakeEthAccountMock { + use openzeppelin::account::eth_account::EthAccountComponent; + use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::introspection::src5::SRC5Component; + + component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6Impl = EthAccountComponent::SRC6Impl; + #[abi(embed_v0)] + impl PublicKeyImpl = EthAccountComponent::PublicKeyImpl; + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + eth_account: EthAccountComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + EthAccountEvent: EthAccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: EthPublicKey) { + self.eth_account.initializer(public_key); + } +} + +#[starknet::contract] +mod CamelEthAccountMock { + use openzeppelin::account::eth_account::EthAccountComponent; + use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::account::Call; + + component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); + component!(path: SRC5Component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = EthAccountComponent::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = + EthAccountComponent::PublicKeyCamelImpl; + #[abi(embed_v0)] + impl SRC5Impl = SRC5Component::SRC5Impl; + impl SRC6Impl = EthAccountComponent::SRC6Impl; + impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + eth_account: EthAccountComponent::Storage, + #[substorage(v0)] + src5: SRC5Component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + EthAccountEvent: EthAccountComponent::Event, + #[flat] + SRC5Event: SRC5Component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, publicKey: EthPublicKey) { + self.eth_account.initializer(publicKey); + } + + #[external(v0)] + #[generate_trait] + impl ExternalImpl of ExternalTrait { + fn __execute__(self: @ContractState, mut calls: Array) -> Array> { + self.eth_account.__execute__(calls) + } + + fn __validate__(self: @ContractState, mut calls: Array) -> felt252 { + self.eth_account.__validate__(calls) + } + } +} + +// Although these modules are designed to panic, functions +// still need a valid return value. We chose: +// +// 3 for felt252 +// false for bool + +#[starknet::contract] +mod SnakeEthAccountPanicMock { + use openzeppelin::account::eth_account::interface::EthPublicKey; + + #[storage] + struct Storage {} + + #[external(v0)] + fn set_public_key(ref self: ContractState, new_public_key: EthPublicKey) { + panic_with_felt252('Some error'); + } + + #[external(v0)] + fn get_public_key(self: @ContractState) -> EthPublicKey { + panic_with_felt252('Some error'); + (3, 3) + } + + #[external(v0)] + fn is_valid_signature( + self: @ContractState, hash: felt252, signature: Array + ) -> felt252 { + panic_with_felt252('Some error'); + 3 + } + + #[external(v0)] + fn supports_interface(self: @ContractState, interface_id: felt252) -> bool { + panic_with_felt252('Some error'); + false + } +} + +#[starknet::contract] +mod CamelEthAccountPanicMock { + use openzeppelin::account::eth_account::interface::EthPublicKey; + + #[storage] + struct Storage {} + + #[external(v0)] + fn setPublicKey(ref self: ContractState, newPublicKey: EthPublicKey) { + panic_with_felt252('Some error'); + } + + #[external(v0)] + fn getPublicKey(self: @ContractState) -> EthPublicKey { + panic_with_felt252('Some error'); + (3, 3) + } + + #[external(v0)] + fn isValidSignature(self: @ContractState, hash: felt252, signature: Array) -> felt252 { + panic_with_felt252('Some error'); + 3 + } + + #[external(v0)] + fn supportsInterface(self: @ContractState, interfaceId: felt252) -> bool { + panic_with_felt252('Some error'); + false + } +} diff --git a/src/tests/utils/constants.cairo b/src/tests/utils/constants.cairo index 846a899fc..808afd23a 100644 --- a/src/tests/utils/constants.cairo +++ b/src/tests/utils/constants.cairo @@ -18,6 +18,14 @@ const SALT: felt252 = 'SALT'; const SUCCESS: felt252 = 123123; const FAILURE: felt252 = 456456; +fn ETH_PUBKEY() -> (u256, u256) { + ('X', 'Y') +} + +fn NEW_ETH_PUBKEY() -> (u256, u256) { + ('NEW_X', 'NEW_Y') +} + fn ADMIN() -> ContractAddress { contract_address_const::<'ADMIN'>() } From 6ce5a08e033354a5de4fd69afe73fe6ff39f2ab1 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 15 Dec 2023 16:36:44 +0100 Subject: [PATCH 03/24] fix: workflow --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 117bdaedb..e20aa8fa3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: - uses: actions/checkout@v3 - uses: software-mansion/setup-scarb@v1 with: - scarb-version: "2.3.1" + scarb-version: "2.4.0" - name: Markdown lint uses: DavidAnson/markdownlint-cli2-action@5b7c9f74fec47e6b15667b2cc23c63dff11e449e # v9 with: From e2ee2186793cf371fa5cee4539bb45e7fdc5cbf1 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 19 Dec 2023 16:25:27 +0100 Subject: [PATCH 04/24] feat: change EthPublicKey to Secp256k1Point --- .../eth_account/dual_eth_account.cairo | 1 + src/account/eth_account/eth_account.cairo | 4 +- src/account/eth_account/interface.cairo | 3 +- src/account/utils.cairo | 1 + src/account/utils/secp256k1.cairo | 57 ++++++++++++++++++ src/account/utils/signature.cairo | 3 +- src/presets/eth_account.cairo | 1 + src/tests/account/test_dual_eth_account.cairo | 3 +- src/tests/account/test_eth_account.cairo | 60 ++++++++++++++----- src/tests/mocks/eth_account_mocks.cairo | 11 +++- src/tests/utils/constants.cairo | 10 ++-- 11 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 src/account/utils/secp256k1.cairo diff --git a/src/account/eth_account/dual_eth_account.cairo b/src/account/eth_account/dual_eth_account.cairo index 77265f044..313780f17 100644 --- a/src/account/eth_account/dual_eth_account.cairo +++ b/src/account/eth_account/dual_eth_account.cairo @@ -2,6 +2,7 @@ // OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/dual_eth_account.cairo) use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::utils::UnwrapAndCast; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo index 28dfa5073..47582750e 100644 --- a/src/account/eth_account/eth_account.cairo +++ b/src/account/eth_account/eth_account.cairo @@ -6,8 +6,10 @@ /// The EthAccount component enables contracts to behave as accounts signing with Ethereum keys. #[starknet::component] mod EthAccountComponent { + use core::starknet::secp256_trait::Secp256PointTrait; use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::eth_account::interface; + use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointStorePacking}; use openzeppelin::account::utils::{execute_calls, is_valid_eth_signature}; use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin::introspection::src5::SRC5Component; @@ -250,7 +252,7 @@ mod EthAccountComponent { } fn _get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { - let (x, y) = public_key; + let (x, y) = public_key.get_coordinates().unwrap(); poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) } } diff --git a/src/account/eth_account/interface.cairo b/src/account/eth_account/interface.cairo index 06a6d04e3..555a80502 100644 --- a/src/account/eth_account/interface.cairo +++ b/src/account/eth_account/interface.cairo @@ -2,10 +2,11 @@ // OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/interface.cairo) use openzeppelin::account::interface::{ISRC6, ISRC6CamelOnly, IDeclarer, ISRC6_ID}; +use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use starknet::ContractAddress; use starknet::account::Call; -type EthPublicKey = (u256, u256); +type EthPublicKey = starknet::secp256k1::Secp256k1Point; #[starknet::interface] trait IDeployable { diff --git a/src/account/utils.cairo b/src/account/utils.cairo index 92b081c67..167145cd9 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -1,3 +1,4 @@ +mod secp256k1; // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils.cairo) diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo new file mode 100644 index 000000000..4e15f6fbc --- /dev/null +++ b/src/account/utils/secp256k1.cairo @@ -0,0 +1,57 @@ +use starknet::SyscallResultTrait; +use starknet::secp256_trait::Secp256PointTrait; +use starknet::secp256k1::{ + Secp256k1Point, secp256k1_get_point_from_x_syscall, secp256k1_new_syscall +}; + +/// Packs a Secp256k1Point into a (felt252, felt252). +/// +/// The packing is done as follows: +/// - First felt contains x.low (being x the x-coordinate of the point). +/// - Second felt contains x.high and the parity bit, at the less significant bits (2 * x.high + parity). +impl Secp256k1PointStorePacking of starknet::StorePacking { + fn pack(value: Secp256k1Point) -> (felt252, felt252) { + let (x, y) = value.get_coordinates().unwrap(); + + let parity = y % 2; + let xhigh_and_parity = 2 * x.high.into() + parity.try_into().unwrap(); + + (x.low.into(), xhigh_and_parity) + } + + fn unpack(value: (felt252, felt252)) -> Secp256k1Point { + let (xlow, xhigh_and_parity) = value; + + let xhigh_and_parity: u256 = xhigh_and_parity.into(); + + let x = u256 { + low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap(), + }; + let parity = xhigh_and_parity % 2 == 1; + + // Expects parity odd to be true + secp256k1_get_point_from_x_syscall(x, parity).unwrap_syscall().unwrap() + } +} + +impl Secp256k1PointSerde of Serde { + fn serialize(self: @Secp256k1Point, ref output: Array) { + let point = (*self).get_coordinates().unwrap(); + point.serialize(ref output) + } + fn deserialize(ref serialized: Span) -> Option { + let (x, y) = Serde::<(u256, u256)>::deserialize(ref serialized)?; + secp256k1_new_syscall(x, y).unwrap_syscall() + } +} + +impl Secp256k1PointPartialEq of PartialEq { + #[inline(always)] + fn eq(lhs: @Secp256k1Point, rhs: @Secp256k1Point) -> bool { + (*lhs).get_coordinates().unwrap() == (*rhs).get_coordinates().unwrap() + } + #[inline(always)] + fn ne(lhs: @Secp256k1Point, rhs: @Secp256k1Point) -> bool { + !(lhs == rhs) + } +} diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index fae140ca2..df1cc509a 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -3,6 +3,7 @@ use ecdsa::check_ecdsa_signature; use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::utils::secp256k1::Secp256k1PointPartialEq; use starknet::eth_signature::{Signature, is_eth_signature_valid}; use starknet::secp256_trait::{Secp256PointTrait, is_signature_entry_valid, recover_public_key}; use starknet::secp256k1::Secp256k1Point; @@ -33,7 +34,7 @@ fn is_valid_eth_signature( } let public_key_point: Secp256k1Point = recover_public_key(msg_hash.into(), signature).unwrap(); - if public_key_point.get_coordinates().unwrap() != public_key { + if public_key_point != public_key { // Invalid signature return false; } diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index ee62ce149..05984d54e 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -9,6 +9,7 @@ mod EthAccount { use openzeppelin::account::eth_account::EthAccountComponent; use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); diff --git a/src/tests/account/test_dual_eth_account.cairo b/src/tests/account/test_dual_eth_account.cairo index 95c2e0287..8fba18d43 100644 --- a/src/tests/account/test_dual_eth_account.cairo +++ b/src/tests/account/test_dual_eth_account.cairo @@ -4,6 +4,7 @@ use openzeppelin::account::eth_account::dual_eth_account::{ use openzeppelin::account::eth_account::interface::{ EthAccountABIDispatcherTrait, EthAccountABIDispatcher }; +use openzeppelin::account::utils::secp256k1::{Secp256k1PointPartialEq, Secp256k1PointSerde}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::test_eth_account::SIGNED_TX_DATA; use openzeppelin::tests::mocks::eth_account_mocks::{ @@ -187,7 +188,7 @@ fn test_dual_setPublicKey_exists_and_panics() { #[test] fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); - assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); +// assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); } #[test] diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index 7fc75388a..0fd043d80 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -1,3 +1,4 @@ +use core::starknet::secp256_trait::Secp256PointTrait; use openzeppelin::account::eth_account::EthAccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::eth_account::EthAccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; @@ -8,6 +9,7 @@ use openzeppelin::account::eth_account::interface::{ EthAccountABIDispatcherTrait, EthAccountABIDispatcher }; use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; +use openzeppelin::account::utils::secp256k1::{Secp256k1PointPartialEq, Secp256k1PointSerde}; use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; use openzeppelin::tests::mocks::eth_account_mocks::DualCaseEthAccountMock; @@ -21,6 +23,7 @@ use starknet::ContractAddress; use starknet::account::Call; use starknet::contract_address_const; use starknet::eth_signature::Signature; +use starknet::secp256k1::secp256k1_new_syscall; use starknet::testing; #[derive(Drop)] @@ -35,10 +38,12 @@ struct SignedTransactionData { fn SIGNED_TX_DATA() -> SignedTransactionData { SignedTransactionData { private_key: 0x45397ee6ca34cb49060f1c303c6cb7ee2d6123e617601ef3e31ccf7bf5bef1f9, - public_key: ( + public_key: secp256k1_new_syscall( 0x829307f82a1883c2414503ba85fc85037f22c6fc6f80910801f6b01a4131da1e, 0x2a23f7bddf3715d11767b1247eccc68c89e11b926e2615268db6ad1af8d8da96 - ), + ) + .unwrap() + .unwrap(), transaction_hash: 0x008f882c63d0396d216d57529fe29ad5e70b6cd51b47bd2458b0a4ccb2ba0957, signature: Signature { r: 0x82bb3efc0554ec181405468f273b0dbf935cca47182b22da78967d0770f7dcc3, @@ -134,7 +139,7 @@ fn test_is_valid_signature() { data.signature.serialize(ref serialized_good_signature); bad_signature.serialize(ref serialized_bad_signature); - state.set_public_key(data.public_key); + state.initializer(data.public_key); let is_valid = state.is_valid_signature(hash, serialized_good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); @@ -159,7 +164,7 @@ fn test_isValidSignature() { data.signature.serialize(ref serialized_good_signature); bad_signature.serialize(ref serialized_bad_signature); - state.set_public_key(data.public_key); + state.initializer(data.public_key); let is_valid = state.isValidSignature(hash, serialized_good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); @@ -406,26 +411,49 @@ fn test_account_called_from_contract() { // set_public_key & get_public_key // +#[test] +#[should_panic(expected: ('Option::unwrap failed.',))] +fn test_cannot_get_without_initialize() { + let mut state = COMPONENT_STATE(); + state.get_public_key(); +} + +#[test] +#[should_panic(expected: ('Option::unwrap failed.',))] +fn test_cannot_set_without_initialize() { + let mut state = COMPONENT_STATE(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(ACCOUNT_ADDRESS()); + testing::set_caller_address(ACCOUNT_ADDRESS()); + + state.set_public_key(new_public_key); +} + #[test] fn test_public_key_setter_and_getter() { let mut state = COMPONENT_STATE(); + let public_key = ETH_PUBKEY(); let new_public_key = NEW_ETH_PUBKEY(); testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(ACCOUNT_ADDRESS()); + state.initializer(public_key); + utils::drop_event(ACCOUNT_ADDRESS()); + // Check default - let public_key = state.get_public_key(); - assert(public_key == (0, 0), 'Should be zero'); + let current = state.get_public_key(); + assert(current == public_key, 'Should be public_key'); // Set key state.set_public_key(new_public_key); - assert_event_owner_removed(ACCOUNT_ADDRESS(), (0, 0)); + assert_event_owner_removed(ACCOUNT_ADDRESS(), current); assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); let public_key = state.get_public_key(); - assert(public_key == new_public_key, 'Should update key'); + assert(public_key == new_public_key, 'Should be new_public_key'); } #[test] @@ -446,21 +474,25 @@ fn test_public_key_setter_different_account() { #[test] fn test_public_key_setter_and_getter_camel() { let mut state = COMPONENT_STATE(); + let public_key = ETH_PUBKEY(); let new_public_key = NEW_ETH_PUBKEY(); testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(ACCOUNT_ADDRESS()); - let public_key = state.getPublicKey(); - assert(public_key == (0, 0), 'Should be zero'); + state.initializer(public_key); + utils::drop_event(ACCOUNT_ADDRESS()); + + let current = state.getPublicKey(); + assert(current == public_key, 'Should be public_key'); state.setPublicKey(new_public_key); - assert_event_owner_removed(ACCOUNT_ADDRESS(), (0, 0)); + assert_event_owner_removed(ACCOUNT_ADDRESS(), public_key); assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); let public_key = state.getPublicKey(); - assert(public_key == new_public_key, 'Should update key'); + assert(public_key == new_public_key, 'Should be new_public_key'); } #[test] @@ -533,7 +565,7 @@ fn test__is_valid_signature() { data.signature.serialize(ref serialized_good_signature); bad_signature.serialize(ref serialized_bad_signature); - state.set_public_key(data.public_key); + state.initializer(data.public_key); let is_valid = state._is_valid_signature(hash, serialized_good_signature.span()); assert(is_valid, 'Should accept valid signature'); @@ -576,6 +608,6 @@ fn assert_event_owner_removed(contract: ContractAddress, public_key: EthPublicKe } fn get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { - let (x, y) = public_key; + let (x, y) = public_key.get_coordinates().unwrap(); poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) } diff --git a/src/tests/mocks/eth_account_mocks.cairo b/src/tests/mocks/eth_account_mocks.cairo index 009b85f41..2ce13cff9 100644 --- a/src/tests/mocks/eth_account_mocks.cairo +++ b/src/tests/mocks/eth_account_mocks.cairo @@ -2,6 +2,7 @@ mod DualCaseEthAccountMock { use openzeppelin::account::eth_account::EthAccountComponent; use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); @@ -47,6 +48,7 @@ mod DualCaseEthAccountMock { mod SnakeEthAccountMock { use openzeppelin::account::eth_account::EthAccountComponent; use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); @@ -88,6 +90,7 @@ mod SnakeEthAccountMock { mod CamelEthAccountMock { use openzeppelin::account::eth_account::EthAccountComponent; use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; use starknet::account::Call; @@ -149,6 +152,8 @@ mod CamelEthAccountMock { #[starknet::contract] mod SnakeEthAccountPanicMock { use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; + use starknet::secp256k1::secp256k1_new_syscall; #[storage] struct Storage {} @@ -161,7 +166,7 @@ mod SnakeEthAccountPanicMock { #[external(v0)] fn get_public_key(self: @ContractState) -> EthPublicKey { panic_with_felt252('Some error'); - (3, 3) + secp256k1_new_syscall(3, 3).unwrap().unwrap() } #[external(v0)] @@ -182,6 +187,8 @@ mod SnakeEthAccountPanicMock { #[starknet::contract] mod CamelEthAccountPanicMock { use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; + use starknet::secp256k1::secp256k1_new_syscall; #[storage] struct Storage {} @@ -194,7 +201,7 @@ mod CamelEthAccountPanicMock { #[external(v0)] fn getPublicKey(self: @ContractState) -> EthPublicKey { panic_with_felt252('Some error'); - (3, 3) + secp256k1_new_syscall(3, 3).unwrap().unwrap() } #[external(v0)] diff --git a/src/tests/utils/constants.cairo b/src/tests/utils/constants.cairo index 808afd23a..9d2d44e66 100644 --- a/src/tests/utils/constants.cairo +++ b/src/tests/utils/constants.cairo @@ -1,7 +1,9 @@ +use openzeppelin::account::eth_account::interface::EthPublicKey; use starknet::ClassHash; use starknet::ContractAddress; use starknet::class_hash_const; use starknet::contract_address_const; +use starknet::secp256k1::secp256k1_get_point_from_x_syscall; const NAME: felt252 = 'NAME'; const SYMBOL: felt252 = 'SYMBOL'; @@ -18,12 +20,12 @@ const SALT: felt252 = 'SALT'; const SUCCESS: felt252 = 123123; const FAILURE: felt252 = 456456; -fn ETH_PUBKEY() -> (u256, u256) { - ('X', 'Y') +fn ETH_PUBKEY() -> EthPublicKey { + secp256k1_get_point_from_x_syscall(3, false).unwrap().unwrap() } -fn NEW_ETH_PUBKEY() -> (u256, u256) { - ('NEW_X', 'NEW_Y') +fn NEW_ETH_PUBKEY() -> EthPublicKey { + secp256k1_get_point_from_x_syscall(4, false).unwrap().unwrap() } fn ADMIN() -> ContractAddress { From 3fdaf157ffa98345d7848d266ee9db48e588afb6 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 20 Dec 2023 13:13:57 +0100 Subject: [PATCH 05/24] feat: add more tests --- src/account/utils/secp256k1.cairo | 145 ++++++++++++++++++ src/tests/account/test_dual_eth_account.cairo | 2 +- 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 4e15f6fbc..75e8104da 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -55,3 +55,148 @@ impl Secp256k1PointPartialEq of PartialEq { !(lhs == rhs) } } + +#[cfg(test)] +mod test { + use starknet::secp256_trait::Secp256PointTrait; + use starknet::secp256k1::Secp256k1Impl; + use super::{ + SyscallResultTrait, Secp256k1Point, Secp256k1PointSerde, Secp256k1PointPartialEq, + Secp256k1PointStorePacking as StorePacking + }; + + + #[test] + fn test_pack_big_secp256k1_points() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); + let xhigh_and_parity: u256 = xhigh_and_parity.into(); + + let x = u256 { + low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() + }; + let parity = xhigh_and_parity % 2 == 1; + + assert(x == curve_size, 'Invalid x'); + assert(parity == true, 'Invalid parity'); + + // Check point 2 + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); + let xhigh_and_parity: u256 = xhigh_and_parity.into(); + + let x = u256 { + low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() + }; + let parity = xhigh_and_parity % 2 == 1; + + assert(x == curve_size, 'Invalid x'); + assert(parity == false, 'Invalid parity'); + } + + #[test] + fn test_unpack_big_secp256k1_points() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let (expected_x, expected_y) = big_point_1.get_coordinates().unwrap(); + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); + let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); + + assert(x == expected_x, 'Invalid x'); + assert(y == expected_y, 'Invalid y'); + + // Check point 2 + + let (expected_x, expected_y) = big_point_2.get_coordinates().unwrap(); + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); + let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); + + assert(x == expected_x, 'Invalid x'); + } + + #[test] + fn test_secp256k1_serialization() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + let mut serialized_point = array![]; + let mut expected_serialization = array![]; + + // Check point 1 + + big_point_1.serialize(ref serialized_point); + big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); + + assert(serialized_point == expected_serialization, 'Invalid serialization'); + + // Check point 2 + + big_point_2.serialize(ref serialized_point); + big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); + + assert(serialized_point == expected_serialization, 'Invalid serialization'); + } + + #[test] + fn test_secp256k1_deserialization() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let mut expected_serialization = array![]; + + big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); + let mut expected_serialization = expected_serialization.span(); + let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization) + .unwrap(); + + assert(big_point_1 == deserialized_point, 'Invalid deserialization'); + + // Check point 2 + + let mut expected_serialization = array![]; + + big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); + let mut expected_serialization = expected_serialization.span(); + let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization) + .unwrap(); + + assert(big_point_2 == deserialized_point, 'Invalid deserialization'); + } + + #[test] + fn test_partial_eq() { + let (big_point_1, big_point_2) = get_points(); + + assert(big_point_1 == big_point_1, 'Invalid equality'); + assert(big_point_2 == big_point_2, 'Invalid equality'); + assert(big_point_1 != big_point_2, 'Invalid equality'); + assert(big_point_2 != big_point_1, 'Invalid equality'); + } + + // + // Helpers + // + + fn get_points() -> (Secp256k1Point, Secp256k1Point) { + let curve_size = Secp256k1Impl::get_curve_size(); + let point_1 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, true) + .unwrap_syscall() + .unwrap(); + let point_2 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, false) + .unwrap_syscall() + .unwrap(); + + (point_1, point_2) + } +} diff --git a/src/tests/account/test_dual_eth_account.cairo b/src/tests/account/test_dual_eth_account.cairo index 8fba18d43..5fa1f61bc 100644 --- a/src/tests/account/test_dual_eth_account.cairo +++ b/src/tests/account/test_dual_eth_account.cairo @@ -188,7 +188,7 @@ fn test_dual_setPublicKey_exists_and_panics() { #[test] fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); -// assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); + assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); } #[test] From 16427fca65319cc2b489dadce5840f012749be1f Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 20 Dec 2023 13:17:58 +0100 Subject: [PATCH 06/24] feat: add entry to CHANGELOG --- CHANGELOG.md | 1 + src/account/utils/signature.cairo | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d657f5e8..3aab65b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Usage docs (#823) - Utilities documentation (#825) - Documentation for presets (#832) +- EthAccount component and preset (#853) ### Changed diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index df1cc509a..4f8ae97d0 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -4,8 +4,8 @@ use ecdsa::check_ecdsa_signature; use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointPartialEq; -use starknet::eth_signature::{Signature, is_eth_signature_valid}; -use starknet::secp256_trait::{Secp256PointTrait, is_signature_entry_valid, recover_public_key}; +use starknet::eth_signature::Signature; +use starknet::secp256_trait::{is_signature_entry_valid, recover_public_key}; use starknet::secp256k1::Secp256k1Point; From 135e8f126ccfca4192f4a36a7b029b540abe22d7 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 20 Dec 2023 14:10:24 +0100 Subject: [PATCH 07/24] feat: add tests for preset --- src/account/utils/secp256k1.cairo | 4 +- src/account/utils/signature.cairo | 3 +- src/presets.cairo | 2 +- src/presets/eth_account.cairo | 25 +- src/tests/account/test_eth_account.cairo | 12 +- src/tests/presets.cairo | 1 + src/tests/presets/test_account.cairo | 20 +- src/tests/presets/test_eth_account.cairo | 418 +++++++++++++++++++++++ 8 files changed, 463 insertions(+), 22 deletions(-) create mode 100644 src/tests/presets/test_eth_account.cairo diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 75e8104da..9f5a6c8d1 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -30,7 +30,9 @@ impl Secp256k1PointStorePacking of starknet::StorePacking ) -> bool { let mut signature = signature; - let signature: Signature = Serde::deserialize(ref signature).unwrap(); + let signature: Signature = Serde::deserialize(ref signature) + .expect('Signature: Invalid format.'); // Signature out of range if !is_signature_entry_valid::(signature.r) { diff --git a/src/presets.cairo b/src/presets.cairo index 07faa0fc7..1a0804706 100644 --- a/src/presets.cairo +++ b/src/presets.cairo @@ -6,4 +6,4 @@ mod eth_account; use account::Account; use erc20::ERC20; use erc721::ERC721; -use eth_account::EthAccount; +use eth_account::EthAccountUpgradeable; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index 05984d54e..cad8527a9 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -6,14 +6,18 @@ /// OpenZeppelin's account which can change its public key and declare, /// deploy, or call contracts, using Ethereum signing keys. #[starknet::contract] -mod EthAccount { +mod EthAccountUpgradeable { use openzeppelin::account::eth_account::EthAccountComponent; use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; + use openzeppelin::upgrades::UpgradeableComponent; + use openzeppelin::upgrades::interface::IUpgradeable; + use starknet::ClassHash; component!(path: EthAccountComponent, storage: eth_account, event: EthAccountEvent); component!(path: SRC5Component, storage: src5, event: SRC5Event); + component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); // Account #[abi(embed_v0)] @@ -35,12 +39,17 @@ mod EthAccount { #[abi(embed_v0)] impl SRC5Impl = SRC5Component::SRC5Impl; + // Upgradeable + impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl; + #[storage] struct Storage { #[substorage(v0)] eth_account: EthAccountComponent::Storage, #[substorage(v0)] - src5: SRC5Component::Storage + src5: SRC5Component::Storage, + #[substorage(v0)] + upgradeable: UpgradeableComponent::Storage } #[event] @@ -49,11 +58,21 @@ mod EthAccount { #[flat] EthAccountEvent: EthAccountComponent::Event, #[flat] - SRC5Event: SRC5Component::Event + SRC5Event: SRC5Component::Event, + #[flat] + UpgradeableEvent: UpgradeableComponent::Event } #[constructor] fn constructor(ref self: ContractState, public_key: EthPublicKey) { self.eth_account.initializer(public_key); } + + #[external(v0)] + impl UpgradeableImpl of IUpgradeable { + fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { + self.eth_account.assert_only_self(); + self.upgradeable._upgrade(new_class_hash); + } + } } diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index 0fd043d80..fb06f7c62 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -201,7 +201,7 @@ fn test_validate_deploy_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] fn test_validate_deploy_invalid_signature_length() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); let mut signature = array![]; @@ -213,7 +213,7 @@ fn test_validate_deploy_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] fn test_validate_deploy_empty_signature() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); let empty_sig = array![]; @@ -246,7 +246,7 @@ fn test_validate_declare_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] fn test_validate_declare_invalid_signature_length() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); let mut signature = array![]; @@ -258,7 +258,7 @@ fn test_validate_declare_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Option::unwrap failed.', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] fn test_validate_declare_empty_signature() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); let empty_sig = array![]; @@ -412,14 +412,14 @@ fn test_account_called_from_contract() { // #[test] -#[should_panic(expected: ('Option::unwrap failed.',))] +#[should_panic(expected: ('Secp256k1Point: Invalid point.',))] fn test_cannot_get_without_initialize() { let mut state = COMPONENT_STATE(); state.get_public_key(); } #[test] -#[should_panic(expected: ('Option::unwrap failed.',))] +#[should_panic(expected: ('Secp256k1Point: Invalid point.',))] fn test_cannot_set_without_initialize() { let mut state = COMPONENT_STATE(); let new_public_key = NEW_ETH_PUBKEY(); diff --git a/src/tests/presets.cairo b/src/tests/presets.cairo index 15f206215..91046a8a8 100644 --- a/src/tests/presets.cairo +++ b/src/tests/presets.cairo @@ -1,3 +1,4 @@ mod test_account; mod test_erc20; mod test_erc721; +mod test_eth_account; diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index ed6359f4b..a73a01615 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -26,7 +26,7 @@ fn CLASS_HASH() -> felt252 { // fn setup_dispatcher() -> AccountABIDispatcher { - let mut calldata = array![PUBKEY]; + let calldata = array![PUBKEY]; let target = utils::deploy(CLASS_HASH(), calldata); utils::drop_event(target); @@ -60,7 +60,7 @@ fn test_constructor() { let mut state = Account::contract_state_for_testing(); Account::constructor(ref state, PUBKEY); - assert_only_event_owner_added(PUBKEY, ZERO()); + assert_only_event_owner_added(ZERO(), PUBKEY); assert(Account::PublicKeyImpl::get_public_key(@state) == PUBKEY, 'Should return PUBKEY'); assert(Account::SRC5Impl::supports_interface(@state, ISRC5_ID), 'Should implement ISRC5'); @@ -81,8 +81,8 @@ fn test_public_key_setter_and_getter() { dispatcher.set_public_key(NEW_PUBKEY); assert(dispatcher.get_public_key() == NEW_PUBKEY, 'Should return NEW_PUBKEY'); - assert_event_owner_removed(PUBKEY, dispatcher.contract_address); - assert_only_event_owner_added(NEW_PUBKEY, dispatcher.contract_address); + assert_event_owner_removed(dispatcher.contract_address, PUBKEY); + assert_only_event_owner_added(dispatcher.contract_address, NEW_PUBKEY); } #[test] @@ -95,8 +95,8 @@ fn test_public_key_setter_and_getter_camel() { dispatcher.setPublicKey(NEW_PUBKEY); assert(dispatcher.getPublicKey() == NEW_PUBKEY, 'Should return NEW_PUBKEY'); - assert_event_owner_removed(PUBKEY, dispatcher.contract_address); - assert_only_event_owner_added(NEW_PUBKEY, dispatcher.contract_address); + assert_event_owner_removed(dispatcher.contract_address, PUBKEY); + assert_only_event_owner_added(dispatcher.contract_address, NEW_PUBKEY); } #[test] @@ -438,17 +438,17 @@ fn test_account_called_from_contract() { // Helpers // -fn assert_event_owner_removed(removed_owner_guid: felt252, contract: ContractAddress) { +fn assert_event_owner_removed(contract: ContractAddress, removed_owner_guid: felt252) { let event = utils::pop_log::(contract).unwrap(); assert(event.removed_owner_guid == removed_owner_guid, 'Invalid `removed_owner_guid`'); } -fn assert_event_owner_added(new_owner_guid: felt252, contract: ContractAddress) { +fn assert_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { let event = utils::pop_log::(contract).unwrap(); assert(event.new_owner_guid == new_owner_guid, 'Invalid `new_owner_guid`'); } -fn assert_only_event_owner_added(new_owner_guid: felt252, contract: ContractAddress) { - assert_event_owner_added(new_owner_guid, contract); +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); } diff --git a/src/tests/presets/test_eth_account.cairo b/src/tests/presets/test_eth_account.cairo new file mode 100644 index 000000000..e79c559d2 --- /dev/null +++ b/src/tests/presets/test_eth_account.cairo @@ -0,0 +1,418 @@ +use core::serde::Serde; +use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::eth_account::EthAccountComponent::{TRANSACTION_VERSION, QUERY_VERSION}; +use openzeppelin::account::eth_account::interface::ISRC6_ID; +use openzeppelin::account::eth_account::interface::{ + EthAccountABIDispatcherTrait, EthAccountABIDispatcher +}; +use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointPartialEq}; +use openzeppelin::introspection::interface::ISRC5_ID; +use openzeppelin::presets::EthAccountUpgradeable; +use openzeppelin::tests::account::test_eth_account::{ + assert_only_event_owner_added, assert_event_owner_removed +}; +use openzeppelin::tests::account::test_eth_account::{ + deploy_erc20, SIGNED_TX_DATA, SignedTransactionData +}; +use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO, RECIPIENT}; +use openzeppelin::tests::utils; +use openzeppelin::token::erc20::interface::IERC20DispatcherTrait; +use openzeppelin::utils::selectors; +use openzeppelin::utils::serde::SerializedAppend; +use starknet::account::Call; +use starknet::contract_address_const; +use starknet::testing; + +fn CLASS_HASH() -> felt252 { + EthAccountUpgradeable::TEST_CLASS_HASH +} + +// +// Setup +// + +fn setup_dispatcher() -> EthAccountABIDispatcher { + let mut calldata = array![]; + calldata.append_serde(ETH_PUBKEY()); + + let target = utils::deploy(CLASS_HASH(), calldata); + utils::drop_event(target); + + EthAccountABIDispatcher { contract_address: target } +} + +fn setup_dispatcher_with_data(data: Option<@SignedTransactionData>) -> EthAccountABIDispatcher { + testing::set_version(TRANSACTION_VERSION); + + let mut calldata = array![]; + if data.is_some() { + let data = data.unwrap(); + let mut serialized_signature = array![]; + data.signature.serialize(ref serialized_signature); + + testing::set_signature(serialized_signature.span()); + testing::set_transaction_hash(*data.transaction_hash); + + calldata.append_serde(*data.public_key); + } else { + calldata.append_serde(ETH_PUBKEY()); + } + let address = utils::deploy(CLASS_HASH(), calldata); + EthAccountABIDispatcher { contract_address: address } +} + +// +// constructor +// + +#[test] +fn test_constructor() { + let mut state = EthAccountUpgradeable::contract_state_for_testing(); + let public_key = ETH_PUBKEY(); + + EthAccountUpgradeable::constructor(ref state, public_key); + + assert_only_event_owner_added(ZERO(), public_key); + assert( + EthAccountUpgradeable::PublicKeyImpl::get_public_key(@state) == public_key, + 'Should return public_key' + ); + assert( + EthAccountUpgradeable::SRC5Impl::supports_interface(@state, ISRC5_ID), + 'Should implement ISRC5' + ); + assert( + EthAccountUpgradeable::SRC5Impl::supports_interface(@state, ISRC6_ID), + 'Should implement ISRC6' + ); +} + +// +// set_public_key & setPublicKey +// + +#[test] +fn test_public_key_setter_and_getter() { + let dispatcher = setup_dispatcher(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(dispatcher.contract_address); + + dispatcher.set_public_key(new_public_key); + assert(dispatcher.get_public_key() == new_public_key, 'Should return new_public_key'); + + assert_event_owner_removed(dispatcher.contract_address, ETH_PUBKEY()); + assert_only_event_owner_added(dispatcher.contract_address, new_public_key); +} + +#[test] +fn test_public_key_setter_and_getter_camel() { + let dispatcher = setup_dispatcher(); + let new_public_key = NEW_ETH_PUBKEY(); + + testing::set_contract_address(dispatcher.contract_address); + + dispatcher.setPublicKey(new_public_key); + assert(dispatcher.getPublicKey() == new_public_key, 'Should return new_public_key'); + + assert_event_owner_removed(dispatcher.contract_address, ETH_PUBKEY()); + assert_only_event_owner_added(dispatcher.contract_address, new_public_key); +} + +#[test] +#[should_panic(expected: ('EthAccount: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_set_public_key_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.set_public_key(NEW_ETH_PUBKEY()); +} + +#[test] +#[should_panic(expected: ('EthAccount: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_setPublicKey_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.setPublicKey(NEW_ETH_PUBKEY()); +} + +// +// is_valid_signature & isValidSignature +// + +fn is_valid_sig_dispatcher() -> (EthAccountABIDispatcher, felt252, Array) { + let dispatcher = setup_dispatcher(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut serialized_signature = array![]; + data.signature.serialize(ref serialized_signature); + + testing::set_contract_address(dispatcher.contract_address); + dispatcher.set_public_key(data.public_key); + + (dispatcher, hash, serialized_signature) +} + +#[test] +fn test_is_valid_signature() { + let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); + + let is_valid = dispatcher.is_valid_signature(hash, signature); + assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); +} + +#[test] +fn test_is_valid_signature_bad_sig() { + let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); + + let is_valid = dispatcher.is_valid_signature(hash + 1, signature); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +#[test] +fn test_isValidSignature() { + let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); + + let is_valid = dispatcher.isValidSignature(hash, signature); + assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); +} + +#[test] +fn test_isValidSignature_bad_sig() { + let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); + + let is_valid = dispatcher.isValidSignature(hash + 1, signature); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +// +// supports_interface +// + +#[test] +fn test_supports_interface() { + let dispatcher = setup_dispatcher(); + assert(dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); + assert(dispatcher.supports_interface(ISRC6_ID), 'Should implement ISRC6'); + assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); +} + +// +// Entry points +// + +#[test] +fn test_validate_deploy() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + + // `__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. + assert( + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_deploy_invalid_signature_data() { + let mut data = SIGNED_TX_DATA(); + data.transaction_hash += 1; + let account = setup_dispatcher_with_data(Option::Some(@data)); + + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] +fn test_validate_deploy_invalid_signature_length() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let mut signature = array![0x1]; + + testing::set_signature(signature.span()); + + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] +fn test_validate_deploy_empty_signature() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let empty_sig = array![]; + + testing::set_signature(empty_sig.span()); + account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); +} + +#[test] +fn test_validate_declare() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + + // `__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. + assert( + account.__validate_declare__(CLASS_HASH()) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_invalid_signature_data() { + let mut data = SIGNED_TX_DATA(); + data.transaction_hash += 1; + let account = setup_dispatcher_with_data(Option::Some(@data)); + + account.__validate_declare__(CLASS_HASH()); +} + +#[test] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_invalid_signature_length() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let mut signature = array![0x1]; + + testing::set_signature(signature.span()); + + account.__validate_declare__(CLASS_HASH()); +} + +#[test] +#[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] +fn test_validate_declare_empty_signature() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let empty_sig = array![]; + + testing::set_signature(empty_sig.span()); + + account.__validate_declare__(CLASS_HASH()); +} + +fn test_execute_with_version(version: Option) { + let data = SIGNED_TX_DATA(); + let account = setup_dispatcher_with_data(Option::Some(@data)); + let erc20 = deploy_erc20(account.contract_address, 1000); + + let amount: u256 = 200; + + let mut calldata = array![]; + calldata.append_serde(RECIPIENT()); + calldata.append_serde(amount); + + let call = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata + }; + let mut calls = array![]; + calls.append(call); + + if version.is_some() { + testing::set_version(version.unwrap()); + } + + let ret = account.__execute__(calls); + + assert(erc20.balance_of(account.contract_address) == 800, 'Should have remainder'); + assert(erc20.balance_of(RECIPIENT()) == amount, 'Should have transferred'); + + let mut call_serialized_retval = *ret.at(0); + let call_retval = Serde::::deserialize(ref call_serialized_retval); + assert(call_retval.unwrap(), 'Should have succeeded'); +} + +#[test] +fn test_execute() { + test_execute_with_version(Option::None(())); +} + +#[test] +fn test_execute_query_version() { + test_execute_with_version(Option::Some(QUERY_VERSION)); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid tx version', 'ENTRYPOINT_FAILED'))] +fn test_execute_invalid_version() { + test_execute_with_version(Option::Some(TRANSACTION_VERSION - 1)); +} + +#[test] +fn test_validate() { + let calls = array![]; + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + + assert(account.__validate__(calls) == starknet::VALIDATED, 'Should validate correctly'); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid signature', 'ENTRYPOINT_FAILED'))] +fn test_validate_invalid() { + let calls = array![]; + let mut data = SIGNED_TX_DATA(); + data.transaction_hash += 1; + let account = setup_dispatcher_with_data(Option::Some(@data)); + + account.__validate__(calls); +} + +#[test] +fn test_multicall() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let erc20 = deploy_erc20(account.contract_address, 1000); + let recipient1 = contract_address_const::<0x123>(); + let recipient2 = contract_address_const::<0x456>(); + let mut calls = array![]; + + let mut calldata1 = array![]; + let amount1: u256 = 300; + calldata1.append_serde(recipient1); + calldata1.append_serde(amount1); + let call1 = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata1 + }; + + let mut calldata2 = array![]; + let amount2: u256 = 500; + calldata2.append_serde(recipient2); + calldata2.append_serde(amount2); + let call2 = Call { + to: erc20.contract_address, selector: selectors::transfer, calldata: calldata2 + }; + + calls.append(call1); + calls.append(call2); + let ret = account.__execute__(calls); + + assert(erc20.balance_of(account.contract_address) == 200, 'Should have remainder'); + assert(erc20.balance_of(recipient1) == 300, 'Should have transferred'); + assert(erc20.balance_of(recipient2) == 500, 'Should have transferred'); + + let mut call1_serialized_retval = *ret.at(0); + let mut call2_serialized_retval = *ret.at(1); + let call1_retval = Serde::::deserialize(ref call1_serialized_retval); + let call2_retval = Serde::::deserialize(ref call2_serialized_retval); + assert(call1_retval.unwrap(), 'Should have succeeded'); + assert(call2_retval.unwrap(), 'Should have succeeded'); +} + +#[test] +fn test_multicall_zero_calls() { + let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); + let mut calls = array![]; + + let ret = account.__execute__(calls); + + assert(ret.len() == 0, 'Should have an empty response'); +} + +#[test] +#[should_panic(expected: ('EthAccount: invalid caller', 'ENTRYPOINT_FAILED'))] +fn test_account_called_from_contract() { + let account = setup_dispatcher(); + let calls = array![]; + let caller = contract_address_const::<0x123>(); + + testing::set_contract_address(account.contract_address); + testing::set_caller_address(caller); + + account.__execute__(calls); +} From f9c811a8bf1cb2cadecde26708d1e90d9bb3da81 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 20 Dec 2023 15:54:38 +0100 Subject: [PATCH 08/24] docs: add API Reference entries --- docs/modules/ROOT/pages/api/account.adoc | 272 +++++++++++++++++- docs/modules/ROOT/pages/presets.adoc | 10 +- .../ROOT/pages/utils/_class_hashes.adoc | 1 + src/account/utils/secp256k1.cairo | 3 + 4 files changed, 282 insertions(+), 4 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 03b6d6dd3..b55bc31d6 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -63,7 +63,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract] [[AccountComponent]] -=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/account/account.cairo#L27[{github-icon},role=heading-link] +=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/account/account/account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:AccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:AccountComponent-OwnerRemoved[OwnerRemoved] @@ -75,6 +75,8 @@ Account component implementing xref:ISRC6[`ISRC6`]. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. +NOTE: Every function involving signature verification, verifies with the account's current public key, using the Starknet curve. + [.contract-index#AccountComponent-Embeddable-Impls] .Embeddable Implementations -- @@ -260,6 +262,209 @@ Emitted when a `public_key` is added. Emitted when a `public_key` is removed. +[.contract] +[[EthAccountComponent]] +=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] + +:OwnerAdded: xref:EthAccountComponent-OwnerAdded[OwnerAdded] +:OwnerRemoved: xref:EthAccountComponent-OwnerRemoved[OwnerRemoved] + +```javascript +use openzeppelin::account::eth_account::EthAccountComponent; +``` +Account component implementing xref:ISRC6[`ISRC6`], but leveraging public key and signatures over the Secp256k1 curve. + +NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. + +NOTE: Every function involving signature verification, verifies with the account's current public key, using the Secp256k1 curve. + +[.contract-index#EthAccountComponent-Embeddable-Impls] +.Embeddable Implementations +-- +.SRC6Impl + +* xref:#EthAccountComponent-\\__execute__[`++__execute__(self, calls)++`] +* xref:#EthAccountComponent-\\__validate__[`++__validate__(self, calls)++`] +* xref:#EthAccountComponent-is_valid_signature[`++is_valid_signature(self, hash, signature)++`] + +.DeclarerImpl + +* xref:#EthAccountComponent-\\__validate_declare__[`++__validate_declare__(self, class_hash)++`] + +.DeployableImpl + +* xref:#EthAccountComponent-\\__validate_deploy__[`++__validate_deploy__(self, hash, signature)++`] + +.PublicKeyImpl + +* xref:#EthAccountComponent-get_public_key[`++get_public_key(self)++`] +* xref:#EthAccountComponent-set_public_key[`++set_public_key(self, new_public_key)++`] +-- + +[.contract-index#EthAccountComponent-Embeddable-Impls-camelCase] +.Embeddable Implementations (camelCase) +-- +.SRC6CamelOnlyImpl + +* xref:#EthAccountComponent-isValidSignature[`++isValidSignature(self, hash, signature)++`] + +.PublicKeyCamelImpl + +* xref:#EthAccountComponent-getPublicKey[`++getPublicKey(self)++`] +* xref:#EthAccountComponent-setPublicKey[`++setPublicKey(self, newPublicKey)++`] +-- + +[.contract-index] +.Internal Implementations +-- +.InternalImpl + +* xref:#EthAccountComponent-initializer[`++initializer(self, public_key)++`] +* xref:#EthAccountComponent-assert_only_self[`++assert_only_self(self)++`] +* xref:#EthAccountComponent-validate_transaction[`++validate_transaction(self)++`] +* xref:#EthAccountComponent-_set_public_key[`++_set_public_key(self, new_public_key)++`] +* xref:#EthAccountComponent-_is_valid_signature[`++_is_valid_signature(self, hash, signature)++`] +-- + +[.contract-index] +.Events +-- +* xref:#EthAccountComponent-OwnerAdded[`++OwnerAdded(new_owner_guid)++`] +* xref:#EthAccountComponent-OwnerRemoved[`++OwnerRemoved(removed_owner_guid)++`] +-- + +[#EthAccountComponent-Embeddable-Functions] +==== Embeddable Functions + +[.contract-item] +[[EthAccountComponent-__execute__]] +==== `[.contract-item-name]#++__execute__++#++(self: @ContractState, calls: Array) → Array>++` [.item-kind]#external# + +See xref:ISRC6-\\__execute__[ISRC6::\\__execute__]. + +[.contract-item] +[[EthAccountComponent-__validate__]] +==== `[.contract-item-name]#++__validate__++#++(self: @ContractState, calls: Array) → felt252++` [.item-kind]#external# + +See xref:ISRC6-\\__validate__[ISRC6::\\__validate__]. + +[.contract-item] +[[EthAccountComponent-is_valid_signature]] +==== `[.contract-item-name]#++is_valid_signature++#++(self: @ContractState, hash: felt252, signature: Array) → felt252++` [.item-kind]#external# + +See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. + +[.contract-item] +[[EthAccountComponent-__validate_declare__]] +==== `[.contract-item-name]#++__validate_declare__++#++(self: @ContractState, class_hash: felt252) → felt252++` [.item-kind]#external# + +Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Network_Architecture/Blocks/transactions/#declare-transaction[`Declare` transaction]. + +Returns the short string `'VALID'` if valid, otherwise it reverts. + +[.contract-item] +[[EthAccountComponent-__validate_deploy__]] +==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, public_key: Secp256k1Point) → felt252++` [.item-kind]#external# + +Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Network_Architecture/Blocks/transactions/#deploy_account_transaction[`DeployAccount` transaction]. +See xref:/guides/deployment.adoc[Counterfactual Deployments]. + +Returns the short string `'VALID'` if valid, otherwise it reverts. + +[.contract-item] +[[EthAccountComponent-get_public_key]] +==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → Secp256k1Point` [.item-kind]#external# + +Returns the current public key of the account. + +[.contract-item] +[[EthAccountComponent-set_public_key]] +==== `[.contract-item-name]#++set_public_key++#++(ref self: ContractState, new_public_key: Secp256k1Point)++` [.item-kind]#external# + +Sets a new public key for the account. Only accesible by the account calling itself through `\\__execute__`. + +Emits both an {OwnerRemoved} and an {OwnerAdded} event. + +[#EthAccountComponent-camelCase-Support] +==== camelCase Support + +[.contract-item] +[[EthAccountComponent-isValidSignature]] +==== `[.contract-item-name]#++isValidSignature++#++(self: @ContractState, hash: felt252, signature: Array) → felt252++` [.item-kind]#external# + +See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. + +[.contract-item] +[[EthAccountComponent-getPublicKey]] +==== `[.contract-item-name]#++getPublicKey++#++(self: @ContractState)++ → Secp256k1Point` [.item-kind]#external# + +See xref:EthAccountComponent-get_public_key[get_public_key]. + +[.contract-item] +[[EthAccountComponent-setPublicKey]] +==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: Secp256k1Point)++` [.item-kind]#external# + +See xref:EthAccountComponent-set_public_key[set_public_key]. + +[#EthAccountComponent-Internal-Functions] +==== Internal Functions + +[.contract-item] +[[EthAccountComponent-initializer]] +==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: Secp256k1Point)++` [.item-kind]#internal# + +Initializes the account with the given public key, and registers the ISRC6 interface ID. + +Emits an {OwnerAdded} event. + +[.contract-item] +[[EthAccountComponent-assert_only_self]] +==== `[.contract-item-name]#++assert_only_self++#++(self: @ComponentState)++` [.item-kind]#internal# + +Validates that the caller is the account itself. Otherwise it reverts. + +[.contract-item] +[[EthAccountComponent-validate_transaction]] +==== `[.contract-item-name]#++validate_transaction++#++(self: @ComponentState)++ → felt252` [.item-kind]#internal# + +Validates a transaction signature from the +https://github.com/starkware-libs/cairo/blob/main/corelib/src/starknet/info.cairo#L61[global context]. + +Returns the short string `'VALID'` if valid, otherwise it reverts. + +[.contract-item] +[[EthAccountComponent-_set_public_key]] +==== `[.contract-item-name]#++_set_public_key++#++(ref self: ComponentState, new_public_key: Secp256k1Point)++` [.item-kind]#internal# + +Set the public key without validating the caller. + +Emits an {OwnerAdded} event. + +CAUTION: The usage of this method outside the `set_public_key` function is discouraged. + +[.contract-item] +[[EthAccountComponent-_is_valid_signature]] +==== `[.contract-item-name]#++_is_valid_signature++#++(self: @ComponentState, hash: felt252, signature: Span)++ → bool` [.item-kind]#internal# + +Validates the provided `signature` for the `hash`, using the account's current public key. + +[#EthAccountComponent-Events] +==== Events + +NOTE: The `guid` is computed as the hash of the public key, using the poseidon hash function. + +[.contract-item] +[[EthAccountComponent-OwnerAdded]] +==== `[.contract-item-name]#++OwnerAdded++#++(new_owner_guid: felt252)++` [.item-kind]#event# + +Emitted when a `public_key` is added. + +[.contract-item] +[[EthAccountComponent-OwnerRemoved]] +==== `[.contract-item-name]#++OwnerRemoved++#++(removed_owner_guid: felt252)++` [.item-kind]#event# + +Emitted when a `public_key` is removed. + == Presets [.contract] @@ -311,3 +516,68 @@ include::../utils/_class_hashes.adoc[] ==== `[.contract-item-name]#++constructor++#++(ref self: ContractState, public_key: felt252)++` [.item-kind]#constructor# Sets the account `public_key` and registers the interfaces the contract supports. + +[.contract] +[[EthAccountUpgradeable]] +=== `++EthAccountUpgradeable++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/presets/eth_account.cairo[{github-icon},role=heading-link] + +```javascript +use openzeppelin::presets::EthAccountUpgradeable; +``` + +Account contract leveraging xref:#EthAccountComponent[EthAccountComponent]. + +include::../utils/_class_hashes.adoc[] + +[.contract-index] +.{presets-page} +-- +{eth-account-upgradeable-class-hash} +-- + +[.contract-index] +.Constructor +-- +* xref:#EthAccountUpgradeable-constructor[`++constructor(self, public_key)++`] +-- + +[.contract-index] +.Embedded Implementations +-- +.EthAccountComponent + +* xref:#EthAccountComponent-Embeddable-Impls[`++SRC6Impl++`] +* xref:#EthAccountComponent-Embeddable-Impls[`++PublicKeyImpl++`] +* xref:#EthAccountComponent-Embeddable-Impls[`++DeclarerImpl++`] +* xref:#EthAccountComponent-Embeddable-Impls[`++DeployableImpl++`] +* xref:#EthAccountComponent-Embeddable-Impls-camelCase[`++SRC6CamelOnlyImpl++`] +* xref:#EthAccountComponent-Embeddable-Impls-camelCase[`++PublicKeyCamelImpl++`] + +.SRC5Component + +* xref:api/introspection.adoc#SRC5Component-Embeddable-Impls[`++SRC5Impl++`] +-- + +[.contract-index] +.External Functions +-- +* xref:#EthAccountUpgradeable-upgrade[`++upgrade(self, new_class_hash)++`] +-- + +[#EthAccountUpgradeable-constructor-section] +==== Constructor + +[.contract-item] +[[EthAccountUpgradeable-constructor]] +==== `[.contract-item-name]#++constructor++#++(ref self: ContractState, public_key: Secp256k1Point)++` [.item-kind]#constructor# + +Sets the account `public_key` and registers the interfaces the contract supports. + +[#EthAccountUpgradeable-external-functions] +==== External Functions + +[.contract-item] +[[EthAccountUpgradeable-upgrade]] +==== `[.contract-item-name]#++upgrade++#++(ref self: ContractState, new_class_hash: ClassHash)++` [.item-kind]#external# + +Upgrades the contract to a new implementation given by `new_class_hash`. diff --git a/docs/modules/ROOT/pages/presets.adoc b/docs/modules/ROOT/pages/presets.adoc index 02a3f442d..0426d75d1 100644 --- a/docs/modules/ROOT/pages/presets.adoc +++ b/docs/modules/ROOT/pages/presets.adoc @@ -6,9 +6,10 @@ include::utils/_class_hashes.adoc[] List of "ready-to-deploy" presets available and their corresponding class hashes. -:account: xref:/api/account.adoc#Account[Account.cairo] -:erc20: xref:/api/erc20.adoc#ERC20[ERC20.cairo] -:erc721: xref:/api/erc721.adoc#ERC721[ERC721.cairo] +:account: xref:/api/account.adoc#Account[Account] +:eth-account-upgradeable: xref:/api/account.adoc#EthAccountUpgradeable[EthAccountUpgradeable] +:erc20: xref:/api/erc20.adoc#ERC20[ERC20] +:erc721: xref:/api/erc721.adoc#ERC721[ERC721] :cairo-and-sierra: https://docs.starknet.io/documentation/architecture_and_concepts/Smart_Contracts/cairo-and-sierra/[Cairo and Sierra] NOTE: Class hashes were computed using {class-hash-cairo-version}. @@ -19,6 +20,9 @@ NOTE: Class hashes were computed using {class-hash-cairo-version}. | `{account}` | `{account-class-hash}` +| `{eth-account-upgradeable}` +| `{eth-account-upgradeable-class-hash}` + | `{erc20}` | `{erc20-class-hash}` diff --git a/docs/modules/ROOT/pages/utils/_class_hashes.adoc b/docs/modules/ROOT/pages/utils/_class_hashes.adoc index 81c6225bc..2602d8737 100644 --- a/docs/modules/ROOT/pages/utils/_class_hashes.adoc +++ b/docs/modules/ROOT/pages/utils/_class_hashes.adoc @@ -3,6 +3,7 @@ // Class Hashes :account-class-hash: 0x016c6081eb34ad1e0c5513234ed0c025b3c7f305902d291bad534cd6474c85bc +:eth-account-upgradeable-class-hash: TBD :erc20-class-hash: 0x01d7085cee580ba542cdb5709bda80ebfe8bdede664261a3e6af92994d9a1de7 :erc721-class-hash: 0x04376782cbcd20a05f8e742e96150757383dab737ab3e791b8505ad856756907 diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 9f5a6c8d1..3410c8952 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -1,3 +1,6 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/secp256k1.cairo) + use starknet::SyscallResultTrait; use starknet::secp256_trait::Secp256PointTrait; use starknet::secp256k1::{ From b883c6bddec0271fa8ba04d1e940edc8ef4a2f30 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 21 Dec 2023 14:28:48 +0100 Subject: [PATCH 09/24] Update src/account/eth_account/eth_account.cairo Co-authored-by: Andrew Fleming --- src/account/eth_account/eth_account.cairo | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo index 47582750e..bcc0d16a1 100644 --- a/src/account/eth_account/eth_account.cairo +++ b/src/account/eth_account/eth_account.cairo @@ -18,8 +18,6 @@ mod EthAccountComponent { use starknet::get_caller_address; use starknet::get_contract_address; use starknet::get_tx_info; - - const TRANSACTION_VERSION: felt252 = 1; // 2**128 + TRANSACTION_VERSION const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; From 3c396dee142e988532898c7ced54e04f43d691e8 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 21 Dec 2023 15:15:55 +0100 Subject: [PATCH 10/24] feat: apply review updates --- CHANGELOG.md | 1 + src/account/account/account.cairo | 7 ++-- src/account/eth_account/eth_account.cairo | 8 ++-- src/account/utils.cairo | 6 ++- src/account/utils/secp256k1.cairo | 6 +-- src/account/utils/signature.cairo | 1 - src/presets/eth_account.cairo | 4 +- src/tests/account/test_account.cairo | 51 +++++++++++++++-------- src/tests/account/test_eth_account.cairo | 34 ++++++++------- src/tests/mocks/eth_account_mocks.cairo | 19 ++++----- src/tests/presets/test_account.cairo | 20 ++++++--- 11 files changed, 90 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3aab65b78..1651eaace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,3 +19,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use ComponentState in tests (#836) - Docsite navbar (#838) +- Account events indexed keys (#853) diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo index bd4f8b5c8..39ba73a3f 100644 --- a/src/account/account/account.cairo +++ b/src/account/account/account.cairo @@ -7,6 +7,7 @@ #[starknet::component] mod AccountComponent { use openzeppelin::account::interface; + use openzeppelin::account::utils::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::utils::{execute_calls, is_valid_signature}; use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin::introspection::src5::SRC5Component; @@ -15,10 +16,6 @@ mod AccountComponent { use starknet::get_contract_address; use starknet::get_tx_info; - const TRANSACTION_VERSION: felt252 = 1; - // 2**128 + TRANSACTION_VERSION - const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; - #[storage] struct Storage { Account_public_key: felt252 @@ -33,11 +30,13 @@ mod AccountComponent { #[derive(Drop, starknet::Event)] struct OwnerAdded { + #[key] new_owner_guid: felt252 } #[derive(Drop, starknet::Event)] struct OwnerRemoved { + #[key] removed_owner_guid: felt252 } diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo index 47582750e..1be4faa3d 100644 --- a/src/account/eth_account/eth_account.cairo +++ b/src/account/eth_account/eth_account.cairo @@ -10,6 +10,7 @@ mod EthAccountComponent { use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::eth_account::interface; use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointStorePacking}; + use openzeppelin::account::utils::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::utils::{execute_calls, is_valid_eth_signature}; use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin::introspection::src5::SRC5Component; @@ -19,11 +20,6 @@ mod EthAccountComponent { use starknet::get_contract_address; use starknet::get_tx_info; - - const TRANSACTION_VERSION: felt252 = 1; - // 2**128 + TRANSACTION_VERSION - const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; - #[storage] struct Storage { EthAccount_public_key: EthPublicKey @@ -38,11 +34,13 @@ mod EthAccountComponent { #[derive(Drop, starknet::Event)] struct OwnerAdded { + #[key] new_owner_guid: felt252 } #[derive(Drop, starknet::Event)] struct OwnerRemoved { + #[key] removed_owner_guid: felt252 } diff --git a/src/account/utils.cairo b/src/account/utils.cairo index 167145cd9..d65396cf4 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -1,12 +1,16 @@ -mod secp256k1; // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils.cairo) +mod secp256k1; mod signature; use signature::{is_valid_signature, is_valid_eth_signature}; use starknet::account::Call; +const TRANSACTION_VERSION: felt252 = 1; +// 2**128 + TRANSACTION_VERSION +const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; + fn execute_calls(mut calls: Array) -> Array> { let mut res = ArrayTrait::new(); loop { diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 3410c8952..d3f40395c 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -10,8 +10,8 @@ use starknet::secp256k1::{ /// Packs a Secp256k1Point into a (felt252, felt252). /// /// The packing is done as follows: -/// - First felt contains x.low (being x the x-coordinate of the point). -/// - Second felt contains x.high and the parity bit, at the less significant bits (2 * x.high + parity). +/// - First felt contains x.low (x being the x-coordinate of the point). +/// - Second felt contains x.high and the parity bit, at the least significant bits (2 * x.high + parity). impl Secp256k1PointStorePacking of starknet::StorePacking { fn pack(value: Secp256k1Point) -> (felt252, felt252) { let (x, y) = value.get_coordinates().unwrap(); @@ -24,7 +24,6 @@ impl Secp256k1PointStorePacking of starknet::StorePacking Secp256k1Point { let (xlow, xhigh_and_parity) = value; - let xhigh_and_parity: u256 = xhigh_and_parity.into(); let x = u256 { @@ -70,7 +69,6 @@ mod test { Secp256k1PointStorePacking as StorePacking }; - #[test] fn test_pack_big_secp256k1_points() { let (big_point_1, big_point_2) = get_points(); diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index a3befc557..14bb6526e 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -8,7 +8,6 @@ use starknet::eth_signature::Signature; use starknet::secp256_trait::{is_signature_entry_valid, recover_public_key}; use starknet::secp256k1::Secp256k1Point; - fn is_valid_signature(msg_hash: felt252, public_key: felt252, signature: Span) -> bool { let valid_length = signature.len() == 2; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index cad8527a9..4bef4ab29 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -19,7 +19,7 @@ mod EthAccountUpgradeable { component!(path: SRC5Component, storage: src5, event: SRC5Event); component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent); - // Account + // EthAccount #[abi(embed_v0)] impl SRC6Impl = EthAccountComponent::SRC6Impl; #[abi(embed_v0)] @@ -33,7 +33,7 @@ mod EthAccountUpgradeable { impl DeclarerImpl = EthAccountComponent::DeclarerImpl; #[abi(embed_v0)] impl DeployableImpl = EthAccountComponent::DeployableImpl; - impl AccountInternalImpl = EthAccountComponent::InternalImpl; + impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; // SRC5 #[abi(embed_v0)] diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 4b3006a9d..5c1c660f8 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -411,12 +411,8 @@ fn test_public_key_setter_and_getter() { // Set key state.set_public_key(NEW_PUBKEY); - let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); - assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - - let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); - assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); - utils::assert_no_events_left(ACCOUNT_ADDRESS()); + assert_event_owner_removed(ACCOUNT_ADDRESS(), 0); + assert_only_event_owner_added(ACCOUNT_ADDRESS(), NEW_PUBKEY); let public_key = state.get_public_key(); assert(public_key == NEW_PUBKEY, 'Should update key'); @@ -452,12 +448,8 @@ fn test_public_key_setter_and_getter_camel() { // Set key state.setPublicKey(NEW_PUBKEY); - let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); - assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - - let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); - assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); - utils::assert_no_events_left(ACCOUNT_ADDRESS()); + assert_event_owner_removed(ACCOUNT_ADDRESS(), 0); + assert_only_event_owner_added(ACCOUNT_ADDRESS(), NEW_PUBKEY); let public_key = state.getPublicKey(); assert(public_key == NEW_PUBKEY, 'Should update key'); @@ -486,10 +478,8 @@ fn test_initializer() { let mock_state = CONTRACT_STATE(); state.initializer(PUBKEY); - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.new_owner_guid == PUBKEY, 'Invalid owner key'); - utils::assert_no_events_left(ZERO()); + assert_only_event_owner_added(ZERO(), PUBKEY); assert(state.get_public_key() == PUBKEY, 'Should return PUBKEY'); let supports_default_interface = mock_state.supports_interface(ISRC5_ID); @@ -550,10 +540,35 @@ fn test__set_public_key() { let mut state = COMPONENT_STATE(); state._set_public_key(PUBKEY); - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.new_owner_guid == PUBKEY, 'Invalid owner key'); - utils::assert_no_events_left(ZERO()); + assert_only_event_owner_added(ZERO(), PUBKEY); let public_key = state.get_public_key(); assert(public_key == PUBKEY, 'Should update key'); } + +// +// Helpers +// + +fn assert_event_owner_removed(contract: ContractAddress, removed_owner_guid: felt252) { + let event = utils::pop_log::(contract).unwrap(); + assert(event.removed_owner_guid == removed_owner_guid, 'Invalid `removed_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![removed_owner_guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); +} + +fn assert_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { + let event = utils::pop_log::(contract).unwrap(); + assert(event.new_owner_guid == new_owner_guid, 'Invalid `new_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![new_owner_guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); +} + +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); +} diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index fb06f7c62..46f55d84d 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -13,7 +13,9 @@ use openzeppelin::account::utils::secp256k1::{Secp256k1PointPartialEq, Secp256k1 use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; use openzeppelin::tests::mocks::eth_account_mocks::DualCaseEthAccountMock; -use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO}; +use openzeppelin::tests::utils::constants::{ + ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO, OTHER, RECIPIENT, CALLER +}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; @@ -21,7 +23,6 @@ use openzeppelin::utils::serde::SerializedAppend; use poseidon::poseidon_hash_span; use starknet::ContractAddress; use starknet::account::Call; -use starknet::contract_address_const; use starknet::eth_signature::Signature; use starknet::secp256k1::secp256k1_new_syscall; use starknet::testing; @@ -204,9 +205,8 @@ fn test_validate_deploy_invalid_signature_data() { #[should_panic(expected: ('Signature: Invalid format.', 'ENTRYPOINT_FAILED'))] fn test_validate_deploy_invalid_signature_length() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); - let mut signature = array![]; + let signature = array![0x1]; - signature.append(0x1); testing::set_signature(signature.span()); account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); @@ -272,7 +272,7 @@ fn test_execute_with_version(version: Option) { let data = SIGNED_TX_DATA(); let account = setup_dispatcher(Option::Some(@data)); 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![]; @@ -342,8 +342,8 @@ fn test_validate_invalid() { fn test_multicall() { let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); 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(); let mut calls = array![]; // Craft call1 @@ -399,10 +399,9 @@ fn test_multicall_zero_calls() { fn test_account_called_from_contract() { let state = setup(); let calls = array![]; - let caller = contract_address_const::<0x123>(); testing::set_contract_address(ACCOUNT_ADDRESS()); - testing::set_caller_address(caller); + testing::set_caller_address(CALLER()); state.__execute__(calls); } @@ -460,9 +459,8 @@ fn test_public_key_setter_and_getter() { #[should_panic(expected: ('EthAccount: 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); + testing::set_caller_address(CALLER()); state.set_public_key(NEW_ETH_PUBKEY()); } @@ -499,9 +497,8 @@ fn test_public_key_setter_and_getter_camel() { #[should_panic(expected: ('EthAccount: 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); + testing::set_caller_address(CALLER()); state.setPublicKey(NEW_ETH_PUBKEY()); } @@ -544,8 +541,7 @@ fn test_assert_only_self_false() { let mut state = COMPONENT_STATE(); testing::set_contract_address(ACCOUNT_ADDRESS()); - let other = contract_address_const::<0x4567>(); - testing::set_caller_address(other); + testing::set_caller_address(OTHER()); state.assert_only_self(); } @@ -594,6 +590,10 @@ fn assert_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) let event = utils::pop_log::(contract).unwrap(); let guid = get_guid_from_public_key(public_key); assert(event.new_owner_guid == guid, 'Invalid `new_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_only_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { @@ -605,6 +605,10 @@ fn assert_event_owner_removed(contract: ContractAddress, public_key: EthPublicKe let event = utils::pop_log::(contract).unwrap(); let guid = get_guid_from_public_key(public_key); assert(event.removed_owner_guid == guid, 'Invalid `removed_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); } fn get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { diff --git a/src/tests/mocks/eth_account_mocks.cairo b/src/tests/mocks/eth_account_mocks.cairo index 2ce13cff9..f312e75a5 100644 --- a/src/tests/mocks/eth_account_mocks.cairo +++ b/src/tests/mocks/eth_account_mocks.cairo @@ -20,7 +20,6 @@ mod DualCaseEthAccountMock { impl SRC5Impl = SRC5Component::SRC5Impl; impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; - #[storage] struct Storage { #[substorage(v0)] @@ -62,7 +61,6 @@ mod SnakeEthAccountMock { impl SRC5Impl = SRC5Component::SRC5Impl; impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; - #[storage] struct Storage { #[substorage(v0)] @@ -107,7 +105,6 @@ mod CamelEthAccountMock { impl SRC6Impl = EthAccountComponent::SRC6Impl; impl EthAccountInternalImpl = EthAccountComponent::InternalImpl; - #[storage] struct Storage { #[substorage(v0)] @@ -160,12 +157,12 @@ mod SnakeEthAccountPanicMock { #[external(v0)] fn set_public_key(ref self: ContractState, new_public_key: EthPublicKey) { - panic_with_felt252('Some error'); + panic!("Some error"); } #[external(v0)] fn get_public_key(self: @ContractState) -> EthPublicKey { - panic_with_felt252('Some error'); + panic!("Some error"); secp256k1_new_syscall(3, 3).unwrap().unwrap() } @@ -173,13 +170,13 @@ mod SnakeEthAccountPanicMock { fn is_valid_signature( self: @ContractState, hash: felt252, signature: Array ) -> felt252 { - panic_with_felt252('Some error'); + panic!("Some error"); 3 } #[external(v0)] fn supports_interface(self: @ContractState, interface_id: felt252) -> bool { - panic_with_felt252('Some error'); + panic!("Some error"); false } } @@ -195,24 +192,24 @@ mod CamelEthAccountPanicMock { #[external(v0)] fn setPublicKey(ref self: ContractState, newPublicKey: EthPublicKey) { - panic_with_felt252('Some error'); + panic!("Some error"); } #[external(v0)] fn getPublicKey(self: @ContractState) -> EthPublicKey { - panic_with_felt252('Some error'); + panic!("Some error"); secp256k1_new_syscall(3, 3).unwrap().unwrap() } #[external(v0)] fn isValidSignature(self: @ContractState, hash: felt252, signature: Array) -> felt252 { - panic_with_felt252('Some error'); + panic!("Some error"); 3 } #[external(v0)] fn supportsInterface(self: @ContractState, interfaceId: felt252) -> bool { - panic_with_felt252('Some error'); + panic!("Some error"); false } } diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index a73a01615..4cfbcebda 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -7,14 +7,15 @@ use openzeppelin::presets::Account; use openzeppelin::tests::account::test_account::{ deploy_erc20, SIGNED_TX_DATA, SignedTransactionData }; -use openzeppelin::tests::utils::constants::{PUBKEY, NEW_PUBKEY, SALT, ZERO, RECIPIENT}; +use openzeppelin::tests::utils::constants::{ + PUBKEY, NEW_PUBKEY, SALT, ZERO, RECIPIENT, OTHER, CALLER +}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; use starknet::ContractAddress; use starknet::account::Call; -use starknet::contract_address_const; use starknet::testing; fn CLASS_HASH() -> felt252 { @@ -367,8 +368,8 @@ fn test_validate_invalid() { fn test_multicall() { let account = setup_dispatcher_with_data(Option::Some(@SIGNED_TX_DATA())); 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(); let mut calls = array![]; // Craft call1 @@ -426,10 +427,9 @@ fn test_multicall_zero_calls() { fn test_account_called_from_contract() { let account = setup_dispatcher(); let calls = array![]; - let caller = contract_address_const::<0x123>(); testing::set_contract_address(account.contract_address); - testing::set_caller_address(caller); + testing::set_caller_address(CALLER()); account.__execute__(calls); } @@ -441,11 +441,19 @@ fn test_account_called_from_contract() { fn assert_event_owner_removed(contract: ContractAddress, removed_owner_guid: felt252) { let event = utils::pop_log::(contract).unwrap(); assert(event.removed_owner_guid == removed_owner_guid, 'Invalid `removed_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![removed_owner_guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { let event = utils::pop_log::(contract).unwrap(); assert(event.new_owner_guid == new_owner_guid, 'Invalid `new_owner_guid`'); + + // Check indexed keys + let indexed_keys = array![new_owner_guid]; + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_only_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { From 34e3010bb45dfc7a750fd10469c13d5dac209464 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 21 Dec 2023 15:27:16 +0100 Subject: [PATCH 11/24] refactor: test --- src/tests/presets/test_account.cairo | 30 +++------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index 4cfbcebda..89e126819 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -4,6 +4,9 @@ use openzeppelin::account::interface::ISRC6_ID; use openzeppelin::account::interface::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::presets::Account; +use openzeppelin::tests::account::test_account::{ + assert_only_event_owner_added, assert_event_owner_removed +}; use openzeppelin::tests::account::test_account::{ deploy_erc20, SIGNED_TX_DATA, SignedTransactionData }; @@ -433,30 +436,3 @@ fn test_account_called_from_contract() { account.__execute__(calls); } - -// -// Helpers -// - -fn assert_event_owner_removed(contract: ContractAddress, removed_owner_guid: felt252) { - let event = utils::pop_log::(contract).unwrap(); - assert(event.removed_owner_guid == removed_owner_guid, 'Invalid `removed_owner_guid`'); - - // Check indexed keys - let indexed_keys = array![removed_owner_guid]; - utils::assert_indexed_keys(event, indexed_keys.span()); -} - -fn assert_event_owner_added(contract: ContractAddress, new_owner_guid: felt252) { - let event = utils::pop_log::(contract).unwrap(); - assert(event.new_owner_guid == new_owner_guid, 'Invalid `new_owner_guid`'); - - // Check indexed keys - let indexed_keys = array![new_owner_guid]; - utils::assert_indexed_keys(event, indexed_keys.span()); -} - -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); -} From b40970b67b80078fcb3cb4e26fa39e0007b69690 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 28 Dec 2023 12:55:11 +0100 Subject: [PATCH 12/24] feat: apply review updates --- docs/modules/ROOT/pages/api/account.adoc | 26 ++++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index b55bc31d6..86c8cdb51 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -67,6 +67,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. :OwnerAdded: xref:AccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:AccountComponent-OwnerRemoved[OwnerRemoved] +:starknet-curve: https://docs.starknet.io/documentation/architecture_and_concepts/Cryptography/stark-curve/[Starknet curve] ```javascript use openzeppelin::account::AccountComponent; @@ -75,7 +76,7 @@ Account component implementing xref:ISRC6[`ISRC6`]. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. -NOTE: Every function involving signature verification, verifies with the account's current public key, using the Starknet curve. +NOTE: Every function involving signature verification, verifies with the account's current public key, using the {starknet-curve}. [.contract-index#AccountComponent-Embeddable-Impls] .Embeddable Implementations @@ -268,15 +269,16 @@ Emitted when a `public_key` is removed. :OwnerAdded: xref:EthAccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:EthAccountComponent-OwnerRemoved[OwnerRemoved] +:secp256k1-curve: https://en.bitcoin.it/wiki/Secp256k1[Secp256k1 curve] ```javascript use openzeppelin::account::eth_account::EthAccountComponent; ``` -Account component implementing xref:ISRC6[`ISRC6`], but leveraging public key and signatures over the Secp256k1 curve. +Account component implementing xref:ISRC6[`ISRC6`], but leveraging public key and signatures over the {secp256k1-curve}. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. -NOTE: Every function involving signature verification, verifies with the account's current public key, using the Secp256k1 curve. +NOTE: The `EthPublicKey` type is an alias for `starknet::secp256k1::Secp256k1Point`. [.contract-index#EthAccountComponent-Embeddable-Impls] .Embeddable Implementations @@ -364,7 +366,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] [[EthAccountComponent-__validate_deploy__]] -==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, public_key: Secp256k1Point) → felt252++` [.item-kind]#external# +==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey) → felt252++` [.item-kind]#external# Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Network_Architecture/Blocks/transactions/#deploy_account_transaction[`DeployAccount` transaction]. See xref:/guides/deployment.adoc[Counterfactual Deployments]. @@ -373,13 +375,13 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] [[EthAccountComponent-get_public_key]] -==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → Secp256k1Point` [.item-kind]#external# +==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → EthPublicKey` [.item-kind]#external# Returns the current public key of the account. [.contract-item] [[EthAccountComponent-set_public_key]] -==== `[.contract-item-name]#++set_public_key++#++(ref self: ContractState, new_public_key: Secp256k1Point)++` [.item-kind]#external# +==== `[.contract-item-name]#++set_public_key++#++(ref self: ContractState, new_public_key: EthPublicKey)++` [.item-kind]#external# Sets a new public key for the account. Only accesible by the account calling itself through `\\__execute__`. @@ -396,13 +398,13 @@ See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. [.contract-item] [[EthAccountComponent-getPublicKey]] -==== `[.contract-item-name]#++getPublicKey++#++(self: @ContractState)++ → Secp256k1Point` [.item-kind]#external# +==== `[.contract-item-name]#++getPublicKey++#++(self: @ContractState)++ → EthPublicKey` [.item-kind]#external# See xref:EthAccountComponent-get_public_key[get_public_key]. [.contract-item] [[EthAccountComponent-setPublicKey]] -==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: Secp256k1Point)++` [.item-kind]#external# +==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: EthPublicKey)++` [.item-kind]#external# See xref:EthAccountComponent-set_public_key[set_public_key]. @@ -411,7 +413,7 @@ See xref:EthAccountComponent-set_public_key[set_public_key]. [.contract-item] [[EthAccountComponent-initializer]] -==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: Secp256k1Point)++` [.item-kind]#internal# +==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: EthPublicKey)++` [.item-kind]#internal# Initializes the account with the given public key, and registers the ISRC6 interface ID. @@ -434,7 +436,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] [[EthAccountComponent-_set_public_key]] -==== `[.contract-item-name]#++_set_public_key++#++(ref self: ComponentState, new_public_key: Secp256k1Point)++` [.item-kind]#internal# +==== `[.contract-item-name]#++_set_public_key++#++(ref self: ComponentState, new_public_key: EthPublicKey)++` [.item-kind]#internal# Set the public key without validating the caller. @@ -527,6 +529,8 @@ use openzeppelin::presets::EthAccountUpgradeable; Account contract leveraging xref:#EthAccountComponent[EthAccountComponent]. +NOTE: The `EthPublicKey` type is an alias for `starknet::secp256k1::Secp256k1Point`. + include::../utils/_class_hashes.adoc[] [.contract-index] @@ -569,7 +573,7 @@ include::../utils/_class_hashes.adoc[] [.contract-item] [[EthAccountUpgradeable-constructor]] -==== `[.contract-item-name]#++constructor++#++(ref self: ContractState, public_key: Secp256k1Point)++` [.item-kind]#constructor# +==== `[.contract-item-name]#++constructor++#++(ref self: ContractState, public_key: EthPublicKey)++` [.item-kind]#constructor# Sets the account `public_key` and registers the interfaces the contract supports. From e960a4ab8257d607a67e04a3c8df157a1ccb9f79 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 3 Jan 2024 12:22:59 +0100 Subject: [PATCH 13/24] feat: update error messages --- .github/workflows/test.yml | 2 +- Scarb.toml | 4 +- docs/modules/ROOT/pages/api/account.adoc | 2 +- src/account/account/account.cairo | 4 +- src/account/utils.cairo | 2 +- src/account/utils/secp256k1.cairo | 146 +----------------- src/account/utils/signature.cairo | 4 +- src/tests/account.cairo | 1 + src/tests/account/test_dual_eth_account.cairo | 20 +-- src/tests/account/test_eth_account.cairo | 68 ++++---- src/tests/account/test_secp256k1.cairo | 138 +++++++++++++++++ 11 files changed, 197 insertions(+), 194 deletions(-) create mode 100644 src/tests/account/test_secp256k1.cairo diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e20aa8fa3..99a9e2fbf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: - uses: actions/checkout@v3 - uses: software-mansion/setup-scarb@v1 with: - scarb-version: "2.4.0" + scarb-version: "2.4.2" - name: Markdown lint uses: DavidAnson/markdownlint-cli2-action@5b7c9f74fec47e6b15667b2cc23c63dff11e449e # v9 with: diff --git a/Scarb.toml b/Scarb.toml index 64a02997c..5baba8b97 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -2,7 +2,7 @@ name = "openzeppelin" version = "0.8.0" edition = "2023_01" -cairo-version = "2.4.0" +cairo-version = "2.4.2" authors = ["OpenZeppelin Community "] description = "OpenZeppelin Contracts written in Cairo for StarkNet, a decentralized ZK Rollup" documentation = "https://docs.openzeppelin.com/contracts-cairo" @@ -12,7 +12,7 @@ license-file = "LICENSE" keywords = ["openzeppelin", "starknet", "cairo", "contracts", "security", "standards"] [dependencies] -starknet = "2.4.0" +starknet = "2.4.2" [lib] diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 86c8cdb51..c947f89f6 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -76,7 +76,7 @@ Account component implementing xref:ISRC6[`ISRC6`]. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. -NOTE: Every function involving signature verification, verifies with the account's current public key, using the {starknet-curve}. +NOTE: Every function involving signature verification verifies with the account's current public key using the {starknet-curve}. [.contract-index#AccountComponent-Embeddable-Impls] .Embeddable Implementations diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo index 39ba73a3f..43cda3716 100644 --- a/src/account/account/account.cairo +++ b/src/account/account/account.cairo @@ -8,7 +8,7 @@ mod AccountComponent { use openzeppelin::account::interface; use openzeppelin::account::utils::{TRANSACTION_VERSION, QUERY_VERSION}; - use openzeppelin::account::utils::{execute_calls, is_valid_signature}; + use openzeppelin::account::utils::{execute_calls, is_valid_stark_signature}; use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin::introspection::src5::SRC5Component; use starknet::account::Call; @@ -236,7 +236,7 @@ mod AccountComponent { self: @ComponentState, hash: felt252, signature: Span ) -> bool { let public_key = self.Account_public_key.read(); - is_valid_signature(hash, public_key, signature) + is_valid_stark_signature(hash, public_key, signature) } } } diff --git a/src/account/utils.cairo b/src/account/utils.cairo index d65396cf4..259519fcb 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -4,7 +4,7 @@ mod secp256k1; mod signature; -use signature::{is_valid_signature, is_valid_eth_signature}; +use signature::{is_valid_stark_signature, is_valid_eth_signature}; use starknet::account::Call; const TRANSACTION_VERSION: felt252 = 1; diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index d3f40395c..1a1d678d8 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/secp256k1.cairo) +use core::fmt::{Debug, Formatter, Error}; use starknet::SyscallResultTrait; use starknet::secp256_trait::Secp256PointTrait; use starknet::secp256k1::{ @@ -60,146 +61,9 @@ impl Secp256k1PointPartialEq of PartialEq { } } -#[cfg(test)] -mod test { - use starknet::secp256_trait::Secp256PointTrait; - use starknet::secp256k1::Secp256k1Impl; - use super::{ - SyscallResultTrait, Secp256k1Point, Secp256k1PointSerde, Secp256k1PointPartialEq, - Secp256k1PointStorePacking as StorePacking - }; - - #[test] - fn test_pack_big_secp256k1_points() { - let (big_point_1, big_point_2) = get_points(); - let curve_size = Secp256k1Impl::get_curve_size(); - - // Check point 1 - - let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); - let xhigh_and_parity: u256 = xhigh_and_parity.into(); - - let x = u256 { - low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() - }; - let parity = xhigh_and_parity % 2 == 1; - - assert(x == curve_size, 'Invalid x'); - assert(parity == true, 'Invalid parity'); - - // Check point 2 - - let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); - let xhigh_and_parity: u256 = xhigh_and_parity.into(); - - let x = u256 { - low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() - }; - let parity = xhigh_and_parity % 2 == 1; - - assert(x == curve_size, 'Invalid x'); - assert(parity == false, 'Invalid parity'); - } - - #[test] - fn test_unpack_big_secp256k1_points() { - let (big_point_1, big_point_2) = get_points(); - let curve_size = Secp256k1Impl::get_curve_size(); - - // Check point 1 - - let (expected_x, expected_y) = big_point_1.get_coordinates().unwrap(); - - let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); - let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); - - assert(x == expected_x, 'Invalid x'); - assert(y == expected_y, 'Invalid y'); - - // Check point 2 - - let (expected_x, expected_y) = big_point_2.get_coordinates().unwrap(); - - let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); - let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); - - assert(x == expected_x, 'Invalid x'); - } - - #[test] - fn test_secp256k1_serialization() { - let (big_point_1, big_point_2) = get_points(); - let curve_size = Secp256k1Impl::get_curve_size(); - - let mut serialized_point = array![]; - let mut expected_serialization = array![]; - - // Check point 1 - - big_point_1.serialize(ref serialized_point); - big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); - - assert(serialized_point == expected_serialization, 'Invalid serialization'); - - // Check point 2 - - big_point_2.serialize(ref serialized_point); - big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); - - assert(serialized_point == expected_serialization, 'Invalid serialization'); - } - - #[test] - fn test_secp256k1_deserialization() { - let (big_point_1, big_point_2) = get_points(); - let curve_size = Secp256k1Impl::get_curve_size(); - - // Check point 1 - - let mut expected_serialization = array![]; - - big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); - let mut expected_serialization = expected_serialization.span(); - let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization) - .unwrap(); - - assert(big_point_1 == deserialized_point, 'Invalid deserialization'); - - // Check point 2 - - let mut expected_serialization = array![]; - - big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); - let mut expected_serialization = expected_serialization.span(); - let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization) - .unwrap(); - - assert(big_point_2 == deserialized_point, 'Invalid deserialization'); - } - - #[test] - fn test_partial_eq() { - let (big_point_1, big_point_2) = get_points(); - - assert(big_point_1 == big_point_1, 'Invalid equality'); - assert(big_point_2 == big_point_2, 'Invalid equality'); - assert(big_point_1 != big_point_2, 'Invalid equality'); - assert(big_point_2 != big_point_1, 'Invalid equality'); - } - - // - // Helpers - // - - fn get_points() -> (Secp256k1Point, Secp256k1Point) { - let curve_size = Secp256k1Impl::get_curve_size(); - let point_1 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, true) - .unwrap_syscall() - .unwrap(); - let point_2 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, false) - .unwrap_syscall() - .unwrap(); - - (point_1, point_2) +impl DebugSecp256k1Point of core::fmt::Debug { + fn fmt(self: @Secp256k1Point, ref f: Formatter) -> Result<(), Error> { + let (x, y) = (*self).get_coordinates().unwrap(); + write!(f, "({x:?},{y:?})") } } diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index 14bb6526e..505ff2cb0 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -8,7 +8,9 @@ use starknet::eth_signature::Signature; use starknet::secp256_trait::{is_signature_entry_valid, recover_public_key}; use starknet::secp256k1::Secp256k1Point; -fn is_valid_signature(msg_hash: felt252, public_key: felt252, signature: Span) -> bool { +fn is_valid_stark_signature( + msg_hash: felt252, public_key: felt252, signature: Span +) -> bool { let valid_length = signature.len() == 2; if valid_length { diff --git a/src/tests/account.cairo b/src/tests/account.cairo index 356504143..8ce73bca5 100644 --- a/src/tests/account.cairo +++ b/src/tests/account.cairo @@ -2,3 +2,4 @@ mod test_account; mod test_dual_account; mod test_dual_eth_account; mod test_eth_account; +mod test_secp256k1; diff --git a/src/tests/account/test_dual_eth_account.cairo b/src/tests/account/test_dual_eth_account.cairo index 282e8595b..46af544b9 100644 --- a/src/tests/account/test_dual_eth_account.cairo +++ b/src/tests/account/test_dual_eth_account.cairo @@ -4,7 +4,9 @@ use openzeppelin::account::eth_account::dual_eth_account::{ use openzeppelin::account::eth_account::interface::{ EthAccountABIDispatcherTrait, EthAccountABIDispatcher }; -use openzeppelin::account::utils::secp256k1::{Secp256k1PointPartialEq, Secp256k1PointSerde}; +use openzeppelin::account::utils::secp256k1::{ + DebugSecp256k1Point, Secp256k1PointPartialEq, Secp256k1PointSerde +}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::test_eth_account::SIGNED_TX_DATA; use openzeppelin::tests::mocks::eth_account_mocks::{ @@ -70,7 +72,7 @@ fn test_dual_set_public_key() { let new_public_key = NEW_ETH_PUBKEY(); snake_dispatcher.set_public_key(new_public_key); - assert(target.get_public_key() == new_public_key, 'Should return NEW_ETH_PUBKEY'); + assert_eq!(target.get_public_key(), new_public_key); } #[test] @@ -90,7 +92,7 @@ fn test_dual_set_public_key_exists_and_panics() { #[test] fn test_dual_get_public_key() { let (snake_dispatcher, _) = setup_snake(); - assert(snake_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); + assert_eq!(snake_dispatcher.get_public_key(), ETH_PUBKEY()); } #[test] @@ -120,7 +122,7 @@ fn test_dual_is_valid_signature() { target.set_public_key(data.public_key); let is_valid = snake_dispatcher.is_valid_signature(hash, serialized_signature); - assert(is_valid == 'VALID', 'Should accept valid signature'); + assert_eq!(is_valid, starknet::VALIDATED); } #[test] @@ -146,7 +148,7 @@ fn test_dual_is_valid_signature_exists_and_panics() { #[test] fn test_dual_supports_interface() { let (snake_dispatcher, target) = setup_snake(); - assert(snake_dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); + assert!(snake_dispatcher.supports_interface(ISRC5_ID), "Should implement ISRC5"); } #[test] @@ -175,7 +177,7 @@ fn test_dual_setPublicKey() { testing::set_contract_address(camel_dispatcher.contract_address); camel_dispatcher.set_public_key(new_public_key); - assert(target.getPublicKey() == new_public_key, 'Should return NEW_ETH_PUBKEY'); + assert_eq!(target.getPublicKey(), new_public_key); } #[test] @@ -188,7 +190,7 @@ fn test_dual_setPublicKey_exists_and_panics() { #[test] fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); - assert(camel_dispatcher.get_public_key() == ETH_PUBKEY(), 'Should return ETH_PUBKEY'); + assert_eq!(camel_dispatcher.get_public_key(), ETH_PUBKEY()); } #[test] @@ -211,7 +213,7 @@ fn test_dual_isValidSignature() { target.setPublicKey(data.public_key); let is_valid = camel_dispatcher.is_valid_signature(hash, serialized_signature); - assert(is_valid == 'VALID', 'Should accept valid signature'); + assert_eq!(is_valid, starknet::VALIDATED); } #[test] @@ -227,7 +229,7 @@ fn test_dual_isValidSignature_exists_and_panics() { #[test] fn test_dual_supportsInterface() { let (camel_dispatcher, _) = setup_camel(); - assert(camel_dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); + assert!(camel_dispatcher.supports_interface(ISRC5_ID), "Should implement ISRC5"); } #[test] diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index 46f55d84d..824947ade 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -9,7 +9,9 @@ use openzeppelin::account::eth_account::interface::{ EthAccountABIDispatcherTrait, EthAccountABIDispatcher }; use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; -use openzeppelin::account::utils::secp256k1::{Secp256k1PointPartialEq, Secp256k1PointSerde}; +use openzeppelin::account::utils::secp256k1::{ + DebugSecp256k1Point, Secp256k1PointPartialEq, Secp256k1PointSerde +}; use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID}; use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock; use openzeppelin::tests::mocks::eth_account_mocks::DualCaseEthAccountMock; @@ -143,10 +145,10 @@ fn test_is_valid_signature() { state.initializer(data.public_key); let is_valid = state.is_valid_signature(hash, serialized_good_signature); - assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); + assert_eq!(is_valid, starknet::VALIDATED); let is_valid = state.is_valid_signature(hash, serialized_bad_signature); - assert(is_valid == 0, 'Should reject invalid signature'); + assert_eq!(is_valid, 0, "Should reject invalid signature"); } #[test] @@ -168,10 +170,10 @@ fn test_isValidSignature() { state.initializer(data.public_key); let is_valid = state.isValidSignature(hash, serialized_good_signature); - assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); + assert_eq!(is_valid, starknet::VALIDATED); let is_valid = state.isValidSignature(hash, serialized_bad_signature); - assert(is_valid == 0, 'Should reject invalid signature'); + assert_eq!(is_valid, 0, "Should reject invalid signature"); } // @@ -185,10 +187,7 @@ fn test_validate_deploy() { // `__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. - assert( - account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()) == starknet::VALIDATED, - 'Should validate correctly' - ); + assert_eq!(account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()), starknet::VALIDATED); } #[test] @@ -229,10 +228,7 @@ fn test_validate_declare() { // `__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. - assert( - account.__validate_declare__(CLASS_HASH()) == starknet::VALIDATED, - 'Should validate correctly' - ); + assert_eq!(account.__validate_declare__(CLASS_HASH()), starknet::VALIDATED); } #[test] @@ -294,13 +290,13 @@ fn test_execute_with_version(version: Option) { let ret = account.__execute__(calls); // Assert that the transfer was successful - assert(erc20.balance_of(account.contract_address) == 800, 'Should have remainder'); - assert(erc20.balance_of(recipient) == amount, 'Should have transferred'); + assert_eq!(erc20.balance_of(account.contract_address), 800, "Should have remainder"); + assert_eq!(erc20.balance_of(recipient), amount, "Should have transferred"); // Test return value let mut call_serialized_retval = *ret.at(0); let call_retval = Serde::::deserialize(ref call_serialized_retval); - assert(call_retval.unwrap(), 'Should have succeeded'); + assert!(call_retval.unwrap()); } #[test] @@ -324,7 +320,7 @@ fn test_validate() { let calls = array![]; let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); - assert(account.__validate__(calls) == starknet::VALIDATED, 'Should validate correctly'); + assert_eq!(account.__validate__(calls), starknet::VALIDATED); } #[test] @@ -370,17 +366,17 @@ fn test_multicall() { let ret = account.__execute__(calls); // Assert that the transfers were successful - assert(erc20.balance_of(account.contract_address) == 200, 'Should have remainder'); - assert(erc20.balance_of(recipient1) == 300, 'Should have transferred'); - assert(erc20.balance_of(recipient2) == 500, 'Should have transferred'); + 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"); // Test return value let mut call1_serialized_retval = *ret.at(0); let mut call2_serialized_retval = *ret.at(1); let call1_retval = Serde::::deserialize(ref call1_serialized_retval); let call2_retval = Serde::::deserialize(ref call2_serialized_retval); - assert(call1_retval.unwrap(), 'Should have succeeded'); - assert(call2_retval.unwrap(), 'Should have succeeded'); + assert!(call1_retval.unwrap()); + assert!(call2_retval.unwrap()); } #[test] @@ -391,7 +387,7 @@ fn test_multicall_zero_calls() { let ret = account.__execute__(calls); // Test return value - assert(ret.len() == 0, 'Should have an empty response'); + assert_eq!(ret.len(), 0, "Should have an empty response"); } #[test] @@ -443,7 +439,7 @@ fn test_public_key_setter_and_getter() { // Check default let current = state.get_public_key(); - assert(current == public_key, 'Should be public_key'); + assert_eq!(current, public_key); // Set key state.set_public_key(new_public_key); @@ -452,7 +448,7 @@ fn test_public_key_setter_and_getter() { assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); let public_key = state.get_public_key(); - assert(public_key == new_public_key, 'Should be new_public_key'); + assert_eq!(public_key, new_public_key); } #[test] @@ -482,7 +478,7 @@ fn test_public_key_setter_and_getter_camel() { utils::drop_event(ACCOUNT_ADDRESS()); let current = state.getPublicKey(); - assert(current == public_key, 'Should be public_key'); + assert_eq!(current, public_key); state.setPublicKey(new_public_key); @@ -490,7 +486,7 @@ fn test_public_key_setter_and_getter_camel() { assert_only_event_owner_added(ACCOUNT_ADDRESS(), new_public_key); let public_key = state.getPublicKey(); - assert(public_key == new_public_key, 'Should be new_public_key'); + assert_eq!(public_key, new_public_key); } #[test] @@ -517,13 +513,13 @@ fn test_initializer() { assert_only_event_owner_added(ZERO(), public_key); - assert(state.get_public_key() == public_key, 'Should return public_key'); + assert_eq!(state.get_public_key(), public_key); let supports_default_interface = mock_state.supports_interface(ISRC5_ID); - assert(supports_default_interface, 'Should support base interface'); + assert!(supports_default_interface, "Should support ISRC5"); let supports_account_interface = mock_state.supports_interface(ISRC6_ID); - assert(supports_account_interface, 'Should support account id'); + assert!(supports_account_interface, "Should support ISRC6"); } #[test] @@ -564,10 +560,10 @@ fn test__is_valid_signature() { state.initializer(data.public_key); let is_valid = state._is_valid_signature(hash, serialized_good_signature.span()); - assert(is_valid, 'Should accept valid signature'); + assert!(is_valid); - let is_valid = state._is_valid_signature(hash, serialized_bad_signature.span()); - assert(!is_valid, 'Should reject invalid signature'); + let is_not_valid = !state._is_valid_signature(hash, serialized_bad_signature.span()); + assert!(is_not_valid); } #[test] @@ -579,7 +575,7 @@ fn test__set_public_key() { assert_only_event_owner_added(ZERO(), public_key); let public_key = state.get_public_key(); - assert(public_key == public_key, 'Should update key'); + assert_eq!(public_key, public_key); } // @@ -589,7 +585,7 @@ fn test__set_public_key() { fn assert_event_owner_added(contract: ContractAddress, public_key: EthPublicKey) { let event = utils::pop_log::(contract).unwrap(); let guid = get_guid_from_public_key(public_key); - assert(event.new_owner_guid == guid, 'Invalid `new_owner_guid`'); + assert_eq!(event.new_owner_guid, guid); // Check indexed keys let indexed_keys = array![guid]; @@ -604,7 +600,7 @@ fn assert_only_event_owner_added(contract: ContractAddress, public_key: EthPubli fn assert_event_owner_removed(contract: ContractAddress, public_key: EthPublicKey) { let event = utils::pop_log::(contract).unwrap(); let guid = get_guid_from_public_key(public_key); - assert(event.removed_owner_guid == guid, 'Invalid `removed_owner_guid`'); + assert_eq!(event.removed_owner_guid, guid); // Check indexed keys let indexed_keys = array![guid]; diff --git a/src/tests/account/test_secp256k1.cairo b/src/tests/account/test_secp256k1.cairo new file mode 100644 index 000000000..47ff2b208 --- /dev/null +++ b/src/tests/account/test_secp256k1.cairo @@ -0,0 +1,138 @@ +use openzeppelin::account::utils::secp256k1::{ + SyscallResultTrait, Secp256k1Point, DebugSecp256k1Point, Secp256k1PointSerde, + Secp256k1PointPartialEq, Secp256k1PointStorePacking as StorePacking +}; +use starknet::secp256_trait::Secp256PointTrait; +use starknet::secp256k1::Secp256k1Impl; + +#[test] +fn test_pack_big_secp256k1_points() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); + let xhigh_and_parity: u256 = xhigh_and_parity.into(); + + let x = u256 { + low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() + }; + let parity = xhigh_and_parity % 2 == 1; + + assert_eq!(x, curve_size); + assert_eq!(parity, true, "Parity should be odd"); + + // Check point 2 + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); + let xhigh_and_parity: u256 = xhigh_and_parity.into(); + + let x = u256 { + low: xlow.try_into().unwrap(), high: (xhigh_and_parity / 2).try_into().unwrap() + }; + let parity = xhigh_and_parity % 2 == 1; + + assert_eq!(x, curve_size); + assert_eq!(parity, false, "Parity should be even"); +} + +#[test] +fn test_unpack_big_secp256k1_points() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let (expected_x, expected_y) = big_point_1.get_coordinates().unwrap(); + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_1); + let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); + + assert_eq!(x, expected_x); + assert_eq!(y, expected_y); + + // Check point 2 + + let (expected_x, expected_y) = big_point_2.get_coordinates().unwrap(); + + let (xlow, xhigh_and_parity) = StorePacking::pack(big_point_2); + let (x, y) = StorePacking::unpack((xlow, xhigh_and_parity)).get_coordinates().unwrap(); + + assert_eq!(x, expected_x); +} + +#[test] +fn test_secp256k1_serialization() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + let mut serialized_point = array![]; + let mut expected_serialization = array![]; + + // Check point 1 + + big_point_1.serialize(ref serialized_point); + big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); + + assert!(serialized_point == expected_serialization); + + // Check point 2 + + big_point_2.serialize(ref serialized_point); + big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); + + assert!(serialized_point == expected_serialization); +} + +#[test] +fn test_secp256k1_deserialization() { + let (big_point_1, big_point_2) = get_points(); + let curve_size = Secp256k1Impl::get_curve_size(); + + // Check point 1 + + let mut expected_serialization = array![]; + + big_point_1.get_coordinates().unwrap().serialize(ref expected_serialization); + let mut expected_serialization = expected_serialization.span(); + let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization).unwrap(); + + assert_eq!(big_point_1, deserialized_point); + + // Check point 2 + + let mut expected_serialization = array![]; + + big_point_2.get_coordinates().unwrap().serialize(ref expected_serialization); + let mut expected_serialization = expected_serialization.span(); + let deserialized_point = Secp256k1PointSerde::deserialize(ref expected_serialization).unwrap(); + + assert_eq!(big_point_2, deserialized_point); +} + +#[test] +fn test_partial_eq() { + let (big_point_1, big_point_2) = get_points(); + + assert_eq!(big_point_1, big_point_1); + assert_eq!(big_point_2, big_point_2); + assert!(big_point_1 != big_point_2); + assert!(big_point_2 != big_point_1); +} + +// +// Helpers +// + +fn get_points() -> (Secp256k1Point, Secp256k1Point) { + let curve_size = Secp256k1Impl::get_curve_size(); + let point_1 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, true) + .unwrap_syscall() + .unwrap(); + let point_2 = Secp256k1Impl::secp256_ec_get_point_from_x_syscall(curve_size, false) + .unwrap_syscall() + .unwrap(); + + (point_1, point_2) +} From 79c64a261fdf4464aa2b6469d4d4e81de94a4d29 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 5 Jan 2024 11:27:11 +0100 Subject: [PATCH 14/24] feat: apply review updates --- src/tests/account/test_eth_account.cairo | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index 824947ade..6a478b337 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -187,7 +187,8 @@ fn test_validate_deploy() { // `__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. - assert_eq!(account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()), starknet::VALIDATED); + let is_valid = account.__validate_deploy__(CLASS_HASH(), SALT, ETH_PUBKEY()); + assert_eq!(is_valid, starknet::VALIDATED); } #[test] @@ -228,7 +229,8 @@ fn test_validate_declare() { // `__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. - assert_eq!(account.__validate_declare__(CLASS_HASH()), starknet::VALIDATED); + let is_valid = account.__validate_declare__(CLASS_HASH()); + assert_eq!(is_valid, starknet::VALIDATED); } #[test] @@ -320,7 +322,8 @@ fn test_validate() { let calls = array![]; let account = setup_dispatcher(Option::Some(@SIGNED_TX_DATA())); - assert_eq!(account.__validate__(calls), starknet::VALIDATED); + let is_valid = account.__validate__(calls); + assert_eq!(is_valid, starknet::VALIDATED); } #[test] @@ -575,7 +578,7 @@ fn test__set_public_key() { assert_only_event_owner_added(ZERO(), public_key); let public_key = state.get_public_key(); - assert_eq!(public_key, public_key); + assert_eq!(public_key, ETH_PUBKEY()); } // From facf8ab9f7de1ee9f9a9ded82f5c5b5c6de84474 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 17 Jan 2024 11:38:35 +0100 Subject: [PATCH 15/24] Update docs/modules/ROOT/pages/api/account.adoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martín Triay --- docs/modules/ROOT/pages/api/account.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index c947f89f6..b1fa8f8f6 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -274,7 +274,7 @@ Emitted when a `public_key` is removed. ```javascript use openzeppelin::account::eth_account::EthAccountComponent; ``` -Account component implementing xref:ISRC6[`ISRC6`], but leveraging public key and signatures over the {secp256k1-curve}. +Account component implementing xref:ISRC6[`ISRC6`] for signatures over the {secp256k1-curve}. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. From ebc2e9d3aa81df8532d28779f53e6097703db19e Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 17 Jan 2024 12:29:44 +0100 Subject: [PATCH 16/24] feat: add tests for upgrade --- docs/modules/ROOT/pages/api/account.adoc | 4 +- src/tests/presets/test_eth_account.cairo | 103 +++++++++++++++++++++- src/tests/upgrades/test_upgradeable.cairo | 19 +++- 3 files changed, 117 insertions(+), 9 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index c947f89f6..8bbaea85d 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -72,12 +72,10 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. ```javascript use openzeppelin::account::AccountComponent; ``` -Account component implementing xref:ISRC6[`ISRC6`]. +Account component implementing xref:ISRC6[`ISRC6`] for signatures over the {starknet-curve}. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. -NOTE: Every function involving signature verification verifies with the account's current public key using the {starknet-curve}. - [.contract-index#AccountComponent-Embeddable-Impls] .Embeddable Implementations -- diff --git a/src/tests/presets/test_eth_account.cairo b/src/tests/presets/test_eth_account.cairo index e79c559d2..8dcdeccf8 100644 --- a/src/tests/presets/test_eth_account.cairo +++ b/src/tests/presets/test_eth_account.cairo @@ -1,11 +1,14 @@ use core::serde::Serde; +use core::traits::TryInto; use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::eth_account::EthAccountComponent::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::eth_account::interface::ISRC6_ID; use openzeppelin::account::eth_account::interface::{ EthAccountABIDispatcherTrait, EthAccountABIDispatcher }; -use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointPartialEq}; +use openzeppelin::account::utils::secp256k1::{ + DebugSecp256k1Point, Secp256k1PointSerde, Secp256k1PointPartialEq +}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::presets::EthAccountUpgradeable; use openzeppelin::tests::account::test_eth_account::{ @@ -14,19 +17,30 @@ use openzeppelin::tests::account::test_eth_account::{ use openzeppelin::tests::account::test_eth_account::{ deploy_erc20, SIGNED_TX_DATA, SignedTransactionData }; -use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO, RECIPIENT}; +use openzeppelin::tests::account::test_secp256k1::get_points; +use openzeppelin::tests::mocks::eth_account_mocks::SnakeEthAccountMock; +use openzeppelin::tests::upgrades::test_upgradeable::assert_only_event_upgraded; +use openzeppelin::tests::utils::constants::{ + CLASS_HASH_ZERO, ETH_PUBKEY, NEW_ETH_PUBKEY, SALT, ZERO, RECIPIENT +}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::interface::IERC20DispatcherTrait; +use openzeppelin::upgrades::interface::{IUpgradeableDispatcherTrait, IUpgradeableDispatcher}; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; use starknet::account::Call; use starknet::contract_address_const; use starknet::testing; +use starknet::{ContractAddress, ClassHash}; fn CLASS_HASH() -> felt252 { EthAccountUpgradeable::TEST_CLASS_HASH } +fn V2_CLASS_HASH() -> ClassHash { + SnakeEthAccountMock::TEST_CLASS_HASH.try_into().unwrap() +} + // // Setup // @@ -61,6 +75,16 @@ fn setup_dispatcher_with_data(data: Option<@SignedTransactionData>) -> EthAccoun EthAccountABIDispatcher { contract_address: address } } +fn setup_upgradeable() -> IUpgradeableDispatcher { + let mut calldata = array![]; + calldata.append_serde(ETH_PUBKEY()); + + let target = utils::deploy(CLASS_HASH(), calldata); + utils::drop_event(target); + + IUpgradeableDispatcher { contract_address: target } +} + // // constructor // @@ -416,3 +440,78 @@ fn test_account_called_from_contract() { account.__execute__(calls); } + + +// +// upgrade +// + +#[test] +#[should_panic(expected: ('EthAccount: unauthorized', 'ENTRYPOINT_FAILED',))] +fn test_upgrade_access_control() { + let v1 = setup_upgradeable(); + v1.upgrade(CLASS_HASH_ZERO()); +} + +#[test] +#[should_panic(expected: ('Class hash cannot be zero', 'ENTRYPOINT_FAILED',))] +fn test_upgrade_with_class_hash_zero() { + let v1 = setup_upgradeable(); + + set_contract_and_caller(v1.contract_address); + v1.upgrade(CLASS_HASH_ZERO()); +} + +#[test] +fn test_upgraded_event() { + let v1 = setup_upgradeable(); + let v2_class_hash = V2_CLASS_HASH(); + + set_contract_and_caller(v1.contract_address); + v1.upgrade(v2_class_hash); + + assert_only_event_upgraded(v2_class_hash, v1.contract_address); +} + +#[test] +#[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] +fn test_v2_missing_camel_selector() { + let v1 = setup_upgradeable(); + let v2_class_hash = V2_CLASS_HASH(); + + set_contract_and_caller(v1.contract_address); + v1.upgrade(v2_class_hash); + + let dispatcher = EthAccountABIDispatcher { contract_address: v1.contract_address }; + dispatcher.getPublicKey(); +} + +#[test] +fn test_state_persists_after_upgrade() { + let v1 = setup_upgradeable(); + let v2_class_hash = V2_CLASS_HASH(); + + set_contract_and_caller(v1.contract_address); + let dispatcher = EthAccountABIDispatcher { contract_address: v1.contract_address }; + + let (point, _) = get_points(); + + dispatcher.set_public_key(point); + + let camel_public_key = dispatcher.getPublicKey(); + assert_eq!(camel_public_key, point); + + v1.upgrade(v2_class_hash); + let snake_public_key = dispatcher.get_public_key(); + + assert_eq!(snake_public_key, camel_public_key); +} + +// +// Helpers +// + +fn set_contract_and_caller(address: ContractAddress) { + testing::set_contract_address(address); + testing::set_caller_address(address); +} diff --git a/src/tests/upgrades/test_upgradeable.cairo b/src/tests/upgrades/test_upgradeable.cairo index 26b191f3c..3e8a95725 100644 --- a/src/tests/upgrades/test_upgradeable.cairo +++ b/src/tests/upgrades/test_upgradeable.cairo @@ -44,10 +44,7 @@ fn test_upgraded_event() { let v1 = deploy_v1(); v1.upgrade(V2_CLASS_HASH()); - let event = utils::pop_log::(v1.contract_address).unwrap(); - assert(event.class_hash == V2_CLASS_HASH(), 'Invalid class hash'); - - utils::assert_no_events_left(v1.contract_address); + assert_only_event_upgraded(V2_CLASS_HASH(), v1.contract_address); } #[test] @@ -90,3 +87,17 @@ fn test_remove_selector_fails_in_v2() { // We use the v1 dispatcher because remove_selector is not in v2 interface v1.remove_selector(); } + +// +// Helpers +// + +fn assert_event_upgraded(class_hash: ClassHash, contract: ContractAddress) { + let event = utils::pop_log::(contract).unwrap(); + assert!(event.class_hash == class_hash); +} + +fn assert_only_event_upgraded(class_hash: ClassHash, contract: ContractAddress) { + assert_event_upgraded(class_hash, contract); + utils::assert_no_events_left(ZERO()); +} From 1ef85b95a19658f81849c92191298442af01926c Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 17 Jan 2024 12:55:43 +0100 Subject: [PATCH 17/24] feat: add tests for signature --- src/tests/account.cairo | 1 + src/tests/account/test_signature.cairo | 127 +++++++++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 src/tests/account/test_signature.cairo diff --git a/src/tests/account.cairo b/src/tests/account.cairo index 8ce73bca5..56f9059dd 100644 --- a/src/tests/account.cairo +++ b/src/tests/account.cairo @@ -3,3 +3,4 @@ mod test_dual_account; mod test_dual_eth_account; mod test_eth_account; mod test_secp256k1; +mod test_signature; diff --git a/src/tests/account/test_signature.cairo b/src/tests/account/test_signature.cairo new file mode 100644 index 000000000..6a246f125 --- /dev/null +++ b/src/tests/account/test_signature.cairo @@ -0,0 +1,127 @@ +use openzeppelin::account::utils::signature::{is_valid_stark_signature, is_valid_eth_signature}; +use openzeppelin::tests::account::test_account::SIGNED_TX_DATA as stark_signature_data; +use openzeppelin::tests::account::test_eth_account::SIGNED_TX_DATA as eth_signature_data; +use starknet::secp256k1::Secp256k1Impl; + +// +// is_valid_stark_signature +// + +#[test] +fn test_is_valid_stark_signature_good_sig() { + let data = stark_signature_data(); + let hash = data.transaction_hash; + + let mut good_signature = array![data.r, data.s].span(); + + let is_valid = is_valid_stark_signature(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 mut bad_signature = array![0x987, 0x564].span(); + + let is_invalid = !is_valid_stark_signature(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 mut bad_signature = array![0x987].span(); + + let is_invalid = !is_valid_stark_signature(hash, data.public_key, bad_signature); + assert!(is_invalid); +} + +// +// is_valid_eth_signature +// + +#[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()); + 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; + + let mut serialized_bad_signature = array![]; + + bad_signature.serialize(ref serialized_bad_signature); + + let is_invalid = !is_valid_eth_signature( + hash, data.public_key, serialized_bad_signature.span() + ); + assert!(is_invalid); +} + +#[test] +#[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()); +} + +#[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 = Secp256k1Impl::get_curve_size(); + + bad_signature.r = curve_size + 1; + + let mut serialized_bad_signature = array![]; + + bad_signature.serialize(ref serialized_bad_signature); + + let is_invalid = !is_valid_eth_signature( + hash, data.public_key, serialized_bad_signature.span() + ); + assert!(is_invalid); +} + +#[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 = Secp256k1Impl::get_curve_size(); + + bad_signature.s = curve_size + 1; + + let mut serialized_bad_signature = array![]; + + bad_signature.serialize(ref serialized_bad_signature); + + let is_invalid = !is_valid_eth_signature( + hash, data.public_key, serialized_bad_signature.span() + ); + assert!(is_invalid); +} From a723d531ca5429a3826226d6c7358f8256f75b02 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 17 Jan 2024 12:58:02 +0100 Subject: [PATCH 18/24] refactor: drop X.Y.Z in favor of last released version --- docs/modules/ROOT/pages/api/account.adoc | 6 +++--- src/account/account/account.cairo | 2 +- src/account/account/dual_account.cairo | 2 +- src/account/account/interface.cairo | 2 +- src/account/eth_account/dual_eth_account.cairo | 2 +- src/account/eth_account/eth_account.cairo | 2 +- src/account/eth_account/interface.cairo | 2 +- src/account/utils.cairo | 2 +- src/account/utils/secp256k1.cairo | 2 +- src/account/utils/signature.cairo | 2 +- src/presets/eth_account.cairo | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 511e0da83..b73fe1ad8 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -63,7 +63,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract] [[AccountComponent]] -=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/account/account/account.cairo[{github-icon},role=heading-link] +=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/account/account/account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:AccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:AccountComponent-OwnerRemoved[OwnerRemoved] @@ -263,7 +263,7 @@ Emitted when a `public_key` is removed. [.contract] [[EthAccountComponent]] -=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] +=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:EthAccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:EthAccountComponent-OwnerRemoved[OwnerRemoved] @@ -519,7 +519,7 @@ Sets the account `public_key` and registers the interfaces the contract supports [.contract] [[EthAccountUpgradeable]] -=== `++EthAccountUpgradeable++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-vX.Y.Z/src/presets/eth_account.cairo[{github-icon},role=heading-link] +=== `++EthAccountUpgradeable++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/presets/eth_account.cairo[{github-icon},role=heading-link] ```javascript use openzeppelin::presets::EthAccountUpgradeable; diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo index 43cda3716..507b533bf 100644 --- a/src/account/account/account.cairo +++ b/src/account/account/account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/account/account.cairo) /// # Account Component /// diff --git a/src/account/account/dual_account.cairo b/src/account/account/dual_account.cairo index 139b1596e..899a20c3b 100644 --- a/src/account/account/dual_account.cairo +++ b/src/account/account/dual_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/dual_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/account/dual_account.cairo) use openzeppelin::utils::UnwrapAndCast; use openzeppelin::utils::selectors; diff --git a/src/account/account/interface.cairo b/src/account/account/interface.cairo index 14087f856..16612891c 100644 --- a/src/account/account/interface.cairo +++ b/src/account/account/interface.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/account/interface.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/account/interface.cairo) use starknet::ContractAddress; use starknet::account::Call; diff --git a/src/account/eth_account/dual_eth_account.cairo b/src/account/eth_account/dual_eth_account.cairo index 313780f17..c3e934f2a 100644 --- a/src/account/eth_account/dual_eth_account.cairo +++ b/src/account/eth_account/dual_eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/dual_eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/dual_eth_account.cairo) use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo index 1be4faa3d..3020601fd 100644 --- a/src/account/eth_account/eth_account.cairo +++ b/src/account/eth_account/eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/eth_account.cairo) /// # EthAccount Component /// diff --git a/src/account/eth_account/interface.cairo b/src/account/eth_account/interface.cairo index 555a80502..501e35586 100644 --- a/src/account/eth_account/interface.cairo +++ b/src/account/eth_account/interface.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/eth_account/interface.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/interface.cairo) use openzeppelin::account::interface::{ISRC6, ISRC6CamelOnly, IDeclarer, ISRC6_ID}; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; diff --git a/src/account/utils.cairo b/src/account/utils.cairo index 259519fcb..91df39fa4 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils.cairo) mod secp256k1; mod signature; diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 1a1d678d8..5eec65d59 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/secp256k1.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils/secp256k1.cairo) use core::fmt::{Debug, Formatter, Error}; use starknet::SyscallResultTrait; diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index 505ff2cb0..39636c265 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (account/utils/signature.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils/signature.cairo) use ecdsa::check_ecdsa_signature; use openzeppelin::account::eth_account::interface::EthPublicKey; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index 4bef4ab29..1fbf7ee86 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo vX.Y.Z (presets/eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0 (presets/eth_account.cairo) /// # EthAccount Preset /// From ef4128c61c6104852a1172e0e43e22fce1d33075 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 22 Jan 2024 12:19:40 +0100 Subject: [PATCH 19/24] feat: updated class hashes --- docs/modules/ROOT/pages/utils/_class_hashes.adoc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/modules/ROOT/pages/utils/_class_hashes.adoc b/docs/modules/ROOT/pages/utils/_class_hashes.adoc index 2602d8737..91b1d0f94 100644 --- a/docs/modules/ROOT/pages/utils/_class_hashes.adoc +++ b/docs/modules/ROOT/pages/utils/_class_hashes.adoc @@ -1,11 +1,11 @@ // Version -:class-hash-cairo-version: https://crates.io/crates/cairo-lang-compiler/2.3.1[cairo 2.3.1] +:class-hash-cairo-version: https://crates.io/crates/cairo-lang-compiler/2.4.4[cairo 2.4.4] // Class Hashes -:account-class-hash: 0x016c6081eb34ad1e0c5513234ed0c025b3c7f305902d291bad534cd6474c85bc -:eth-account-upgradeable-class-hash: TBD -:erc20-class-hash: 0x01d7085cee580ba542cdb5709bda80ebfe8bdede664261a3e6af92994d9a1de7 -:erc721-class-hash: 0x04376782cbcd20a05f8e742e96150757383dab737ab3e791b8505ad856756907 +:account-class-hash: 0x05e41b67aff0a188509f36697ddc7858ae45431ae7830cba38f5d29e98acf038 +:eth-account-upgradeable-class-hash: 0x045a59d97d9f59686dc083d6916115f3943568cb46675a85b62b4a121ad6d089 +:erc20-class-hash: 0x0179f43b5bcac96da911d208c52db50b10869b31e5afecd2c04f281f304d715c +:erc721-class-hash: 0x052d246bbe68953550f1bd810253f489cb43f8b5ea08a4c6af14a44cd7a1cf93 // Presets page :presets-page: xref:presets.adoc[Compiled class hash] From 42abd67a6ae06d1747a7be86a67beb9005f07cae Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 29 Jan 2024 12:28:07 +0100 Subject: [PATCH 20/24] fix: casm target --- Scarb.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Scarb.toml b/Scarb.toml index 0f5ba460f..d1fb78f7a 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -19,7 +19,7 @@ starknet = "2.4.4" [[target.starknet-contract]] allowed-libfuncs-list.name = "experimental" sierra = true -casm = true +casm = false [tool.fmt] sort-module-level-items = true From 26651b3a92e0c9dc1c5d602f3107759a73c9fdea Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 30 Jan 2024 12:19:34 +0100 Subject: [PATCH 21/24] fix: versions --- docs/modules/ROOT/pages/api/account.adoc | 4 ++-- src/account/account/account.cairo | 2 +- src/account/eth_account/dual_eth_account.cairo | 2 +- src/account/eth_account/eth_account.cairo | 2 +- src/account/eth_account/interface.cairo | 2 +- src/account/utils.cairo | 2 +- src/account/utils/secp256k1.cairo | 2 +- src/account/utils/signature.cairo | 2 +- src/presets/eth_account.cairo | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 4824adb6e..cb7896b72 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -263,7 +263,7 @@ Emitted when a `public_key` is removed. [.contract] [[EthAccountComponent]] -=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] +=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:EthAccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:EthAccountComponent-OwnerRemoved[OwnerRemoved] @@ -519,7 +519,7 @@ Sets the account `public_key` and registers the interfaces the contract supports [.contract] [[EthAccountUpgradeable]] -=== `++EthAccountUpgradeable++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0/src/presets/eth_account.cairo[{github-icon},role=heading-link] +=== `++EthAccountUpgradeable++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/presets/eth_account.cairo[{github-icon},role=heading-link] ```javascript use openzeppelin::presets::EthAccountUpgradeable; diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo index 3dc6162e2..d8b453325 100644 --- a/src/account/account/account.cairo +++ b/src/account/account/account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/account/account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/account/account.cairo) /// # Account Component /// diff --git a/src/account/eth_account/dual_eth_account.cairo b/src/account/eth_account/dual_eth_account.cairo index c3e934f2a..c6a510000 100644 --- a/src/account/eth_account/dual_eth_account.cairo +++ b/src/account/eth_account/dual_eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/dual_eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/dual_eth_account.cairo) use openzeppelin::account::eth_account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo index 9dfe86a2d..150283e60 100644 --- a/src/account/eth_account/eth_account.cairo +++ b/src/account/eth_account/eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/eth_account.cairo) /// # EthAccount Component /// diff --git a/src/account/eth_account/interface.cairo b/src/account/eth_account/interface.cairo index 501e35586..05e9b6733 100644 --- a/src/account/eth_account/interface.cairo +++ b/src/account/eth_account/interface.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/eth_account/interface.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/interface.cairo) use openzeppelin::account::interface::{ISRC6, ISRC6CamelOnly, IDeclarer, ISRC6_ID}; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; diff --git a/src/account/utils.cairo b/src/account/utils.cairo index a0a42891f..ee361528c 100644 --- a/src/account/utils.cairo +++ b/src/account/utils.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/utils.cairo) mod secp256k1; mod signature; diff --git a/src/account/utils/secp256k1.cairo b/src/account/utils/secp256k1.cairo index 5eec65d59..dc7ea4805 100644 --- a/src/account/utils/secp256k1.cairo +++ b/src/account/utils/secp256k1.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils/secp256k1.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/utils/secp256k1.cairo) use core::fmt::{Debug, Formatter, Error}; use starknet::SyscallResultTrait; diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index 39636c265..1fb5ae354 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (account/utils/signature.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/utils/signature.cairo) use ecdsa::check_ecdsa_signature; use openzeppelin::account::eth_account::interface::EthPublicKey; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index 1fbf7ee86..4d8952876 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0 (presets/eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (presets/eth_account.cairo) /// # EthAccount Preset /// From 97151fd4d252497031d9868a66b9ffc2a7d37397 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 30 Jan 2024 12:38:19 +0100 Subject: [PATCH 22/24] refactor: account directory --- docs/modules/ROOT/pages/api/account.adoc | 4 +- src/account.cairo | 6 +- src/account/account.cairo | 251 ++++++++++++++++- src/account/account/account.cairo | 248 ---------------- src/account/{account => }/dual_account.cairo | 2 +- .../{eth_account => }/dual_eth_account.cairo | 4 +- src/account/eth_account.cairo | 265 +++++++++++++++++- src/account/eth_account/eth_account.cairo | 262 ----------------- src/account/eth_account/interface.cairo | 66 ----- src/account/{account => }/interface.cairo | 70 ++++- src/account/utils/signature.cairo | 2 +- src/presets/eth_account.cairo | 4 +- src/tests/account/test_dual_eth_account.cairo | 8 +- src/tests/account/test_eth_account.cairo | 14 +- src/tests/mocks/eth_account_mocks.cairo | 16 +- src/tests/presets/test_eth_account.cairo | 8 +- src/tests/utils/constants.cairo | 2 +- 17 files changed, 609 insertions(+), 623 deletions(-) delete mode 100644 src/account/account/account.cairo rename src/account/{account => }/dual_account.cairo (96%) rename src/account/{eth_account => }/dual_eth_account.cairo (93%) delete mode 100644 src/account/eth_account/eth_account.cairo delete mode 100644 src/account/eth_account/interface.cairo rename src/account/{account => }/interface.cairo (55%) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index cb7896b72..aabb1a2e3 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -63,7 +63,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract] [[AccountComponent]] -=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/account/account/account.cairo[{github-icon},role=heading-link] +=== `++AccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/account/account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:AccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:AccountComponent-OwnerRemoved[OwnerRemoved] @@ -263,7 +263,7 @@ Emitted when a `public_key` is removed. [.contract] [[EthAccountComponent]] -=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/account/eth_account/eth_account.cairo[{github-icon},role=heading-link] +=== `++EthAccountComponent++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.1/src/account/eth_account.cairo[{github-icon},role=heading-link] :OwnerAdded: xref:EthAccountComponent-OwnerAdded[OwnerAdded] :OwnerRemoved: xref:EthAccountComponent-OwnerRemoved[OwnerRemoved] diff --git a/src/account.cairo b/src/account.cairo index e50ac6eb3..e29e379f2 100644 --- a/src/account.cairo +++ b/src/account.cairo @@ -1,7 +1,9 @@ mod account; +mod dual_account; +mod dual_eth_account; mod eth_account; +mod interface; mod utils; use account::AccountComponent; -use account::dual_account; -use account::interface; +use eth_account::EthAccountComponent; diff --git a/src/account/account.cairo b/src/account/account.cairo index da35ae9f4..d043988b8 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -1,5 +1,248 @@ -mod account; -mod dual_account; -mod interface; +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.8.1 (account/account.cairo) -use account::AccountComponent; +/// # Account Component +/// +/// The Account component enables contracts to behave as accounts. +#[starknet::component] +mod AccountComponent { + use openzeppelin::account::interface; + use openzeppelin::account::utils::{MIN_TRANSACTION_VERSION, QUERY_VERSION, QUERY_OFFSET}; + use openzeppelin::account::utils::{execute_calls, is_valid_stark_signature}; + use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; + use openzeppelin::introspection::src5::SRC5Component; + use starknet::account::Call; + use starknet::get_caller_address; + use starknet::get_contract_address; + use starknet::get_tx_info; + + #[storage] + struct Storage { + Account_public_key: felt252 + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + OwnerAdded: OwnerAdded, + OwnerRemoved: OwnerRemoved + } + + #[derive(Drop, starknet::Event)] + struct OwnerAdded { + #[key] + new_owner_guid: felt252 + } + + #[derive(Drop, starknet::Event)] + struct OwnerRemoved { + #[key] + removed_owner_guid: felt252 + } + + mod Errors { + const INVALID_CALLER: felt252 = 'Account: invalid caller'; + const INVALID_SIGNATURE: felt252 = 'Account: invalid signature'; + const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version'; + const UNAUTHORIZED: felt252 = 'Account: unauthorized'; + } + + #[embeddable_as(SRC6Impl)] + impl SRC6< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6> { + /// Executes a list of calls from the account. + /// + /// Requirements: + /// + /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. + /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be + /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. + fn __execute__( + self: @ComponentState, mut calls: Array + ) -> Array> { + // Avoid calls from other contracts + // https://github.com/OpenZeppelin/cairo-contracts/issues/344 + let sender = get_caller_address(); + assert(sender.is_zero(), Errors::INVALID_CALLER); + + // Check tx version + let tx_info = get_tx_info().unbox(); + let tx_version: u256 = tx_info.version.into(); + // Check if tx is a query + if (tx_version >= QUERY_OFFSET) { + assert( + QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION + ); + } else { + assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); + } + + execute_calls(calls) + } + + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `invoke` transactions. + fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + self.validate_transaction() + } + + /// Verifies that the given signature is valid for the given hash. + fn is_valid_signature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + if self._is_valid_signature(hash, signature.span()) { + starknet::VALIDATED + } else { + 0 + } + } + } + + #[embeddable_as(DeclarerImpl)] + impl Declarer< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeclarer> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `declare` transactions. + fn __validate_declare__( + self: @ComponentState, class_hash: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(DeployableImpl)] + impl Deployable< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeployable> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `deploy_account` transactions. + fn __validate_deploy__( + self: @ComponentState, + class_hash: felt252, + contract_address_salt: felt252, + public_key: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(PublicKeyImpl)] + impl PublicKey< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKey> { + /// Returns the current public key of the account. + fn get_public_key(self: @ComponentState) -> felt252 { + self.Account_public_key.read() + } + + /// Sets the public key of the account to `new_public_key`. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// + /// Emits an `OwnerRemoved` event. + fn set_public_key(ref self: ComponentState, new_public_key: felt252) { + self.assert_only_self(); + self.emit(OwnerRemoved { removed_owner_guid: self.Account_public_key.read() }); + self._set_public_key(new_public_key); + } + } + + /// Adds camelCase support for `ISRC6`. + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6CamelOnly> { + fn isValidSignature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + self.is_valid_signature(hash, signature) + } + } + + /// Adds camelCase support for `PublicKeyTrait`. + #[embeddable_as(PublicKeyCamelImpl)] + impl PublicKeyCamel< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IPublicKeyCamel> { + fn getPublicKey(self: @ComponentState) -> felt252 { + self.Account_public_key.read() + } + + fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { + self.set_public_key(newPublicKey); + } + } + + #[generate_trait] + impl InternalImpl< + TContractState, + +HasComponent, + impl SRC5: SRC5Component::HasComponent, + +Drop + > of InternalTrait { + /// Initializes the account by setting the initial public key + /// and registering the ISRC6 interface Id. + fn initializer(ref self: ComponentState, public_key: felt252) { + let mut src5_component = get_dep_component_mut!(ref self, SRC5); + src5_component.register_interface(interface::ISRC6_ID); + self._set_public_key(public_key); + } + + /// Validates that the caller is the account itself. Otherwise it reverts. + fn assert_only_self(self: @ComponentState) { + let caller = get_caller_address(); + let self = get_contract_address(); + assert(self == caller, Errors::UNAUTHORIZED); + } + + /// Validates the signature for the current transaction. + /// Returns the short string `VALID` if valid, otherwise it reverts. + fn validate_transaction(self: @ComponentState) -> felt252 { + let tx_info = get_tx_info().unbox(); + let tx_hash = tx_info.transaction_hash; + let signature = tx_info.signature; + assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); + starknet::VALIDATED + } + + /// Sets the public key without validating the caller. + /// The usage of this method outside the `set_public_key` function is discouraged. + /// + /// Emits an `OwnerAdded` event. + fn _set_public_key(ref self: ComponentState, new_public_key: felt252) { + self.Account_public_key.write(new_public_key); + self.emit(OwnerAdded { new_owner_guid: new_public_key }); + } + + /// Returns whether the given signature is valid for the given hash + /// using the account's current public key. + fn _is_valid_signature( + self: @ComponentState, hash: felt252, signature: Span + ) -> bool { + let public_key = self.Account_public_key.read(); + is_valid_stark_signature(hash, public_key, signature) + } + } +} diff --git a/src/account/account/account.cairo b/src/account/account/account.cairo deleted file mode 100644 index d8b453325..000000000 --- a/src/account/account/account.cairo +++ /dev/null @@ -1,248 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/account/account.cairo) - -/// # Account Component -/// -/// The Account component enables contracts to behave as accounts. -#[starknet::component] -mod AccountComponent { - use openzeppelin::account::interface; - use openzeppelin::account::utils::{MIN_TRANSACTION_VERSION, QUERY_VERSION, QUERY_OFFSET}; - use openzeppelin::account::utils::{execute_calls, is_valid_stark_signature}; - use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; - use openzeppelin::introspection::src5::SRC5Component; - use starknet::account::Call; - use starknet::get_caller_address; - use starknet::get_contract_address; - use starknet::get_tx_info; - - #[storage] - struct Storage { - Account_public_key: felt252 - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - OwnerAdded: OwnerAdded, - OwnerRemoved: OwnerRemoved - } - - #[derive(Drop, starknet::Event)] - struct OwnerAdded { - #[key] - new_owner_guid: felt252 - } - - #[derive(Drop, starknet::Event)] - struct OwnerRemoved { - #[key] - removed_owner_guid: felt252 - } - - mod Errors { - const INVALID_CALLER: felt252 = 'Account: invalid caller'; - const INVALID_SIGNATURE: felt252 = 'Account: invalid signature'; - const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version'; - const UNAUTHORIZED: felt252 = 'Account: unauthorized'; - } - - #[embeddable_as(SRC6Impl)] - impl SRC6< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6> { - /// Executes a list of calls from the account. - /// - /// Requirements: - /// - /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. - /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be - /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. - fn __execute__( - self: @ComponentState, mut calls: Array - ) -> Array> { - // Avoid calls from other contracts - // https://github.com/OpenZeppelin/cairo-contracts/issues/344 - let sender = get_caller_address(); - assert(sender.is_zero(), Errors::INVALID_CALLER); - - // Check tx version - let tx_info = get_tx_info().unbox(); - let tx_version: u256 = tx_info.version.into(); - // Check if tx is a query - if (tx_version >= QUERY_OFFSET) { - assert( - QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION - ); - } else { - assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); - } - - execute_calls(calls) - } - - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `invoke` transactions. - fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { - self.validate_transaction() - } - - /// Verifies that the given signature is valid for the given hash. - fn is_valid_signature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - if self._is_valid_signature(hash, signature.span()) { - starknet::VALIDATED - } else { - 0 - } - } - } - - #[embeddable_as(DeclarerImpl)] - impl Declarer< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeclarer> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `declare` transactions. - fn __validate_declare__( - self: @ComponentState, class_hash: felt252 - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(DeployableImpl)] - impl Deployable< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeployable> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `deploy_account` transactions. - fn __validate_deploy__( - self: @ComponentState, - class_hash: felt252, - contract_address_salt: felt252, - public_key: felt252 - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(PublicKeyImpl)] - impl PublicKey< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKey> { - /// Returns the current public key of the account. - fn get_public_key(self: @ComponentState) -> felt252 { - self.Account_public_key.read() - } - - /// Sets the public key of the account to `new_public_key`. - /// - /// Requirements: - /// - /// - The caller must be the contract itself. - /// - /// Emits an `OwnerRemoved` event. - fn set_public_key(ref self: ComponentState, new_public_key: felt252) { - self.assert_only_self(); - self.emit(OwnerRemoved { removed_owner_guid: self.Account_public_key.read() }); - self._set_public_key(new_public_key); - } - } - - /// Adds camelCase support for `ISRC6`. - #[embeddable_as(SRC6CamelOnlyImpl)] - impl SRC6CamelOnly< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6CamelOnly> { - fn isValidSignature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - self.is_valid_signature(hash, signature) - } - } - - /// Adds camelCase support for `PublicKeyTrait`. - #[embeddable_as(PublicKeyCamelImpl)] - impl PublicKeyCamel< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKeyCamel> { - fn getPublicKey(self: @ComponentState) -> felt252 { - self.Account_public_key.read() - } - - fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { - self.set_public_key(newPublicKey); - } - } - - #[generate_trait] - impl InternalImpl< - TContractState, - +HasComponent, - impl SRC5: SRC5Component::HasComponent, - +Drop - > of InternalTrait { - /// Initializes the account by setting the initial public key - /// and registering the ISRC6 interface Id. - fn initializer(ref self: ComponentState, public_key: felt252) { - let mut src5_component = get_dep_component_mut!(ref self, SRC5); - src5_component.register_interface(interface::ISRC6_ID); - self._set_public_key(public_key); - } - - /// Validates that the caller is the account itself. Otherwise it reverts. - fn assert_only_self(self: @ComponentState) { - let caller = get_caller_address(); - let self = get_contract_address(); - assert(self == caller, Errors::UNAUTHORIZED); - } - - /// Validates the signature for the current transaction. - /// Returns the short string `VALID` if valid, otherwise it reverts. - fn validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); - let tx_hash = tx_info.transaction_hash; - let signature = tx_info.signature; - assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); - starknet::VALIDATED - } - - /// Sets the public key without validating the caller. - /// The usage of this method outside the `set_public_key` function is discouraged. - /// - /// Emits an `OwnerAdded` event. - fn _set_public_key(ref self: ComponentState, new_public_key: felt252) { - self.Account_public_key.write(new_public_key); - self.emit(OwnerAdded { new_owner_guid: new_public_key }); - } - - /// Returns whether the given signature is valid for the given hash - /// using the account's current public key. - fn _is_valid_signature( - self: @ComponentState, hash: felt252, signature: Span - ) -> bool { - let public_key = self.Account_public_key.read(); - is_valid_stark_signature(hash, public_key, signature) - } - } -} diff --git a/src/account/account/dual_account.cairo b/src/account/dual_account.cairo similarity index 96% rename from src/account/account/dual_account.cairo rename to src/account/dual_account.cairo index 96d053611..eee4738d3 100644 --- a/src/account/account/dual_account.cairo +++ b/src/account/dual_account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/account/dual_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/dual_account.cairo) use openzeppelin::utils::UnwrapAndCast; use openzeppelin::utils::selectors; diff --git a/src/account/eth_account/dual_eth_account.cairo b/src/account/dual_eth_account.cairo similarity index 93% rename from src/account/eth_account/dual_eth_account.cairo rename to src/account/dual_eth_account.cairo index c6a510000..0cd4d2810 100644 --- a/src/account/eth_account/dual_eth_account.cairo +++ b/src/account/dual_eth_account.cairo @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/dual_eth_account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/dual_eth_account.cairo) -use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::utils::UnwrapAndCast; use openzeppelin::utils::selectors; diff --git a/src/account/eth_account.cairo b/src/account/eth_account.cairo index 6818ef0c5..4e551f2d3 100644 --- a/src/account/eth_account.cairo +++ b/src/account/eth_account.cairo @@ -1,5 +1,262 @@ -mod dual_eth_account; -mod eth_account; -mod interface; +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account.cairo) -use eth_account::EthAccountComponent; +/// # EthAccount Component +/// +/// The EthAccount component enables contracts to behave as accounts signing with Ethereum keys. +#[starknet::component] +mod EthAccountComponent { + use core::starknet::secp256_trait::Secp256PointTrait; + use openzeppelin::account::interface::EthPublicKey; + use openzeppelin::account::interface; + use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointStorePacking}; + use openzeppelin::account::utils::{MIN_TRANSACTION_VERSION, QUERY_VERSION, QUERY_OFFSET}; + use openzeppelin::account::utils::{execute_calls, is_valid_eth_signature}; + use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; + use openzeppelin::introspection::src5::SRC5Component; + use poseidon::poseidon_hash_span; + use starknet::account::Call; + use starknet::get_caller_address; + use starknet::get_contract_address; + use starknet::get_tx_info; + + #[storage] + struct Storage { + EthAccount_public_key: EthPublicKey + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + OwnerAdded: OwnerAdded, + OwnerRemoved: OwnerRemoved + } + + #[derive(Drop, starknet::Event)] + struct OwnerAdded { + #[key] + new_owner_guid: felt252 + } + + #[derive(Drop, starknet::Event)] + struct OwnerRemoved { + #[key] + removed_owner_guid: felt252 + } + + mod Errors { + const INVALID_CALLER: felt252 = 'EthAccount: invalid caller'; + const INVALID_SIGNATURE: felt252 = 'EthAccount: invalid signature'; + const INVALID_TX_VERSION: felt252 = 'EthAccount: invalid tx version'; + const UNAUTHORIZED: felt252 = 'EthAccount: unauthorized'; + } + + #[embeddable_as(SRC6Impl)] + impl SRC6< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6> { + /// Executes a list of calls from the account. + /// + /// Requirements: + /// + /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. + /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be + /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. + fn __execute__( + self: @ComponentState, mut calls: Array + ) -> Array> { + // Avoid calls from other contracts + // https://github.com/OpenZeppelin/cairo-contracts/issues/344 + let sender = get_caller_address(); + assert(sender.is_zero(), Errors::INVALID_CALLER); + + // Check tx version + let tx_info = get_tx_info().unbox(); + let tx_version: u256 = tx_info.version.into(); + // Check if tx is a query + if (tx_version >= QUERY_OFFSET) { + assert( + QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION + ); + } else { + assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); + } + + execute_calls(calls) + } + + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `invoke` transactions. + fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + self.validate_transaction() + } + + /// Verifies that the given signature is valid for the given hash. + fn is_valid_signature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + if self._is_valid_signature(hash, signature.span()) { + starknet::VALIDATED + } else { + 0 + } + } + } + + #[embeddable_as(DeclarerImpl)] + impl Declarer< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IDeclarer> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `declare` transactions. + fn __validate_declare__( + self: @ComponentState, class_hash: felt252 + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(DeployableImpl)] + impl Deployable< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IEthDeployable> { + /// Verifies the validity of the signature for the current transaction. + /// This function is used by the protocol to verify `deploy_account` transactions. + fn __validate_deploy__( + self: @ComponentState, + class_hash: felt252, + contract_address_salt: felt252, + public_key: EthPublicKey + ) -> felt252 { + self.validate_transaction() + } + } + + #[embeddable_as(PublicKeyImpl)] + impl PublicKey< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IEthPublicKey> { + /// Returns the current public key of the account. + fn get_public_key(self: @ComponentState) -> EthPublicKey { + self.EthAccount_public_key.read() + } + + /// Sets the public key of the account to `new_public_key`. + /// + /// Requirements: + /// + /// - The caller must be the contract itself. + /// + /// Emits an `OwnerRemoved` event. + fn set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { + self.assert_only_self(); + + let current_public_key: EthPublicKey = self.EthAccount_public_key.read(); + let removed_owner_guid = _get_guid_from_public_key(current_public_key); + + self.emit(OwnerRemoved { removed_owner_guid }); + self._set_public_key(new_public_key); + } + } + + /// Adds camelCase support for `ISRC6`. + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::ISRC6CamelOnly> { + fn isValidSignature( + self: @ComponentState, hash: felt252, signature: Array + ) -> felt252 { + self.is_valid_signature(hash, signature) + } + } + + /// Adds camelCase support for `PublicKeyTrait`. + #[embeddable_as(PublicKeyCamelImpl)] + impl PublicKeyCamel< + TContractState, + +HasComponent, + +SRC5Component::HasComponent, + +Drop + > of interface::IEthPublicKeyCamel> { + fn getPublicKey(self: @ComponentState) -> EthPublicKey { + self.EthAccount_public_key.read() + } + + fn setPublicKey(ref self: ComponentState, newPublicKey: EthPublicKey) { + self.set_public_key(newPublicKey); + } + } + + #[generate_trait] + impl InternalImpl< + TContractState, + +HasComponent, + impl SRC5: SRC5Component::HasComponent, + +Drop + > of InternalTrait { + /// Initializes the account by setting the initial public key + /// and registering the ISRC6 interface Id. + fn initializer(ref self: ComponentState, public_key: EthPublicKey) { + let mut src5_component = get_dep_component_mut!(ref self, SRC5); + src5_component.register_interface(interface::ISRC6_ID); + self._set_public_key(public_key); + } + + /// Validates that the caller is the account itself. Otherwise it reverts. + fn assert_only_self(self: @ComponentState) { + let caller = get_caller_address(); + let self = get_contract_address(); + assert(self == caller, Errors::UNAUTHORIZED); + } + + /// Validates the signature for the current transaction. + /// Returns the short string `VALID` if valid, otherwise it reverts. + fn validate_transaction(self: @ComponentState) -> felt252 { + let tx_info = get_tx_info().unbox(); + let tx_hash = tx_info.transaction_hash; + let signature = tx_info.signature; + assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); + starknet::VALIDATED + } + + /// Sets the public key without validating the caller. + /// The usage of this method outside the `set_public_key` function is discouraged. + /// + /// Emits an `OwnerAdded` event. + fn _set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { + self.EthAccount_public_key.write(new_public_key); + let new_owner_guid = _get_guid_from_public_key(new_public_key); + self.emit(OwnerAdded { new_owner_guid }); + } + + /// Returns whether the given signature is valid for the given hash + /// using the account's current public key. + fn _is_valid_signature( + self: @ComponentState, hash: felt252, signature: Span + ) -> bool { + let public_key: EthPublicKey = self.EthAccount_public_key.read(); + is_valid_eth_signature(hash, public_key, signature) + } + } + + fn _get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { + let (x, y) = public_key.get_coordinates().unwrap(); + poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) + } +} diff --git a/src/account/eth_account/eth_account.cairo b/src/account/eth_account/eth_account.cairo deleted file mode 100644 index 150283e60..000000000 --- a/src/account/eth_account/eth_account.cairo +++ /dev/null @@ -1,262 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/eth_account.cairo) - -/// # EthAccount Component -/// -/// The EthAccount component enables contracts to behave as accounts signing with Ethereum keys. -#[starknet::component] -mod EthAccountComponent { - use core::starknet::secp256_trait::Secp256PointTrait; - use openzeppelin::account::eth_account::interface::EthPublicKey; - use openzeppelin::account::eth_account::interface; - use openzeppelin::account::utils::secp256k1::{Secp256k1PointSerde, Secp256k1PointStorePacking}; - use openzeppelin::account::utils::{MIN_TRANSACTION_VERSION, QUERY_VERSION, QUERY_OFFSET}; - use openzeppelin::account::utils::{execute_calls, is_valid_eth_signature}; - use openzeppelin::introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; - use openzeppelin::introspection::src5::SRC5Component; - use poseidon::poseidon_hash_span; - use starknet::account::Call; - use starknet::get_caller_address; - use starknet::get_contract_address; - use starknet::get_tx_info; - - #[storage] - struct Storage { - EthAccount_public_key: EthPublicKey - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - OwnerAdded: OwnerAdded, - OwnerRemoved: OwnerRemoved - } - - #[derive(Drop, starknet::Event)] - struct OwnerAdded { - #[key] - new_owner_guid: felt252 - } - - #[derive(Drop, starknet::Event)] - struct OwnerRemoved { - #[key] - removed_owner_guid: felt252 - } - - mod Errors { - const INVALID_CALLER: felt252 = 'EthAccount: invalid caller'; - const INVALID_SIGNATURE: felt252 = 'EthAccount: invalid signature'; - const INVALID_TX_VERSION: felt252 = 'EthAccount: invalid tx version'; - const UNAUTHORIZED: felt252 = 'EthAccount: unauthorized'; - } - - #[embeddable_as(SRC6Impl)] - impl SRC6< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6> { - /// Executes a list of calls from the account. - /// - /// Requirements: - /// - /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. - /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be - /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. - fn __execute__( - self: @ComponentState, mut calls: Array - ) -> Array> { - // Avoid calls from other contracts - // https://github.com/OpenZeppelin/cairo-contracts/issues/344 - let sender = get_caller_address(); - assert(sender.is_zero(), Errors::INVALID_CALLER); - - // Check tx version - let tx_info = get_tx_info().unbox(); - let tx_version: u256 = tx_info.version.into(); - // Check if tx is a query - if (tx_version >= QUERY_OFFSET) { - assert( - QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION - ); - } else { - assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); - } - - execute_calls(calls) - } - - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `invoke` transactions. - fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { - self.validate_transaction() - } - - /// Verifies that the given signature is valid for the given hash. - fn is_valid_signature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - if self._is_valid_signature(hash, signature.span()) { - starknet::VALIDATED - } else { - 0 - } - } - } - - #[embeddable_as(DeclarerImpl)] - impl Declarer< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeclarer> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `declare` transactions. - fn __validate_declare__( - self: @ComponentState, class_hash: felt252 - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(DeployableImpl)] - impl Deployable< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IDeployable> { - /// Verifies the validity of the signature for the current transaction. - /// This function is used by the protocol to verify `deploy_account` transactions. - fn __validate_deploy__( - self: @ComponentState, - class_hash: felt252, - contract_address_salt: felt252, - public_key: EthPublicKey - ) -> felt252 { - self.validate_transaction() - } - } - - #[embeddable_as(PublicKeyImpl)] - impl PublicKey< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKey> { - /// Returns the current public key of the account. - fn get_public_key(self: @ComponentState) -> EthPublicKey { - self.EthAccount_public_key.read() - } - - /// Sets the public key of the account to `new_public_key`. - /// - /// Requirements: - /// - /// - The caller must be the contract itself. - /// - /// Emits an `OwnerRemoved` event. - fn set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { - self.assert_only_self(); - - let current_public_key: EthPublicKey = self.EthAccount_public_key.read(); - let removed_owner_guid = _get_guid_from_public_key(current_public_key); - - self.emit(OwnerRemoved { removed_owner_guid }); - self._set_public_key(new_public_key); - } - } - - /// Adds camelCase support for `ISRC6`. - #[embeddable_as(SRC6CamelOnlyImpl)] - impl SRC6CamelOnly< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::ISRC6CamelOnly> { - fn isValidSignature( - self: @ComponentState, hash: felt252, signature: Array - ) -> felt252 { - self.is_valid_signature(hash, signature) - } - } - - /// Adds camelCase support for `PublicKeyTrait`. - #[embeddable_as(PublicKeyCamelImpl)] - impl PublicKeyCamel< - TContractState, - +HasComponent, - +SRC5Component::HasComponent, - +Drop - > of interface::IPublicKeyCamel> { - fn getPublicKey(self: @ComponentState) -> EthPublicKey { - self.EthAccount_public_key.read() - } - - fn setPublicKey(ref self: ComponentState, newPublicKey: EthPublicKey) { - self.set_public_key(newPublicKey); - } - } - - #[generate_trait] - impl InternalImpl< - TContractState, - +HasComponent, - impl SRC5: SRC5Component::HasComponent, - +Drop - > of InternalTrait { - /// Initializes the account by setting the initial public key - /// and registering the ISRC6 interface Id. - fn initializer(ref self: ComponentState, public_key: EthPublicKey) { - let mut src5_component = get_dep_component_mut!(ref self, SRC5); - src5_component.register_interface(interface::ISRC6_ID); - self._set_public_key(public_key); - } - - /// Validates that the caller is the account itself. Otherwise it reverts. - fn assert_only_self(self: @ComponentState) { - let caller = get_caller_address(); - let self = get_contract_address(); - assert(self == caller, Errors::UNAUTHORIZED); - } - - /// Validates the signature for the current transaction. - /// Returns the short string `VALID` if valid, otherwise it reverts. - fn validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); - let tx_hash = tx_info.transaction_hash; - let signature = tx_info.signature; - assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); - starknet::VALIDATED - } - - /// Sets the public key without validating the caller. - /// The usage of this method outside the `set_public_key` function is discouraged. - /// - /// Emits an `OwnerAdded` event. - fn _set_public_key(ref self: ComponentState, new_public_key: EthPublicKey) { - self.EthAccount_public_key.write(new_public_key); - let new_owner_guid = _get_guid_from_public_key(new_public_key); - self.emit(OwnerAdded { new_owner_guid }); - } - - /// Returns whether the given signature is valid for the given hash - /// using the account's current public key. - fn _is_valid_signature( - self: @ComponentState, hash: felt252, signature: Span - ) -> bool { - let public_key: EthPublicKey = self.EthAccount_public_key.read(); - is_valid_eth_signature(hash, public_key, signature) - } - } - - fn _get_guid_from_public_key(public_key: EthPublicKey) -> felt252 { - let (x, y) = public_key.get_coordinates().unwrap(); - poseidon_hash_span(array![x.low.into(), x.high.into(), y.low.into(), y.high.into()].span()) - } -} diff --git a/src/account/eth_account/interface.cairo b/src/account/eth_account/interface.cairo deleted file mode 100644 index 05e9b6733..000000000 --- a/src/account/eth_account/interface.cairo +++ /dev/null @@ -1,66 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/eth_account/interface.cairo) - -use openzeppelin::account::interface::{ISRC6, ISRC6CamelOnly, IDeclarer, ISRC6_ID}; -use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; -use starknet::ContractAddress; -use starknet::account::Call; - -type EthPublicKey = starknet::secp256k1::Secp256k1Point; - -#[starknet::interface] -trait IDeployable { - fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey - ) -> felt252; -} - -#[starknet::interface] -trait IPublicKey { - fn get_public_key(self: @TState) -> EthPublicKey; - fn set_public_key(ref self: TState, new_public_key: EthPublicKey); -} - - -#[starknet::interface] -trait IPublicKeyCamel { - fn getPublicKey(self: @TState) -> EthPublicKey; - fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); -} - -// -// EthAccount ABI -// - -#[starknet::interface] -trait EthAccountABI { - // ISRC6 - fn __execute__(self: @TState, calls: Array) -> Array>; - fn __validate__(self: @TState, calls: Array) -> felt252; - fn is_valid_signature(self: @TState, hash: felt252, signature: Array) -> felt252; - - // ISRC5 - fn supports_interface(self: @TState, interface_id: felt252) -> bool; - - // IDeclarer - fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; - - // IDeployable - fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey - ) -> felt252; - - // IPublicKey - fn get_public_key(self: @TState) -> EthPublicKey; - fn set_public_key(ref self: TState, new_public_key: EthPublicKey); - - // ISRC6CamelOnly - fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; - - // ISRC5Camel - fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; - - // IPublicKeyCamel - fn getPublicKey(self: @TState) -> EthPublicKey; - fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); -} diff --git a/src/account/account/interface.cairo b/src/account/interface.cairo similarity index 55% rename from src/account/account/interface.cairo rename to src/account/interface.cairo index 7b7ea7c47..6d2100148 100644 --- a/src/account/account/interface.cairo +++ b/src/account/interface.cairo @@ -1,11 +1,18 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.1 (account/account/interface.cairo) +// OpenZeppelin Contracts for Cairo v0.8.1 (account/interface.cairo) +use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use starknet::ContractAddress; use starknet::account::Call; +type EthPublicKey = starknet::secp256k1::Secp256k1Point; + const ISRC6_ID: felt252 = 0x2ceccef7f994940b3962a6c67e0ba4fcd37df7d131417c604f91e03caecc1cd; +// +// Account +// + #[starknet::interface] trait ISRC6 { fn __execute__(self: @TState, calls: Array) -> Array>; @@ -78,3 +85,64 @@ trait AccountABI { fn getPublicKey(self: @TState) -> felt252; fn setPublicKey(ref self: TState, newPublicKey: felt252); } + +// +// EthAccount +// + +#[starknet::interface] +trait IEthDeployable { + fn __validate_deploy__( + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey + ) -> felt252; +} + +#[starknet::interface] +trait IEthPublicKey { + fn get_public_key(self: @TState) -> EthPublicKey; + fn set_public_key(ref self: TState, new_public_key: EthPublicKey); +} + + +#[starknet::interface] +trait IEthPublicKeyCamel { + fn getPublicKey(self: @TState) -> EthPublicKey; + fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); +} + +// +// EthAccount ABI +// + +#[starknet::interface] +trait EthAccountABI { + // ISRC6 + fn __execute__(self: @TState, calls: Array) -> Array>; + fn __validate__(self: @TState, calls: Array) -> felt252; + fn is_valid_signature(self: @TState, hash: felt252, signature: Array) -> felt252; + + // ISRC5 + fn supports_interface(self: @TState, interface_id: felt252) -> bool; + + // IDeclarer + fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; + + // IDeployable + fn __validate_deploy__( + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey + ) -> felt252; + + // IPublicKey + fn get_public_key(self: @TState) -> EthPublicKey; + fn set_public_key(ref self: TState, new_public_key: EthPublicKey); + + // ISRC6CamelOnly + fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; + + // ISRC5Camel + fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; + + // IPublicKeyCamel + fn getPublicKey(self: @TState) -> EthPublicKey; + fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); +} diff --git a/src/account/utils/signature.cairo b/src/account/utils/signature.cairo index 1fb5ae354..1164d0e56 100644 --- a/src/account/utils/signature.cairo +++ b/src/account/utils/signature.cairo @@ -2,7 +2,7 @@ // OpenZeppelin Contracts for Cairo v0.8.1 (account/utils/signature.cairo) use ecdsa::check_ecdsa_signature; -use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointPartialEq; use starknet::eth_signature::Signature; use starknet::secp256_trait::{is_signature_entry_valid, recover_public_key}; diff --git a/src/presets/eth_account.cairo b/src/presets/eth_account.cairo index 4d8952876..71bc87c4d 100644 --- a/src/presets/eth_account.cairo +++ b/src/presets/eth_account.cairo @@ -7,8 +7,8 @@ /// deploy, or call contracts, using Ethereum signing keys. #[starknet::contract] mod EthAccountUpgradeable { - use openzeppelin::account::eth_account::EthAccountComponent; - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::EthAccountComponent; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; use openzeppelin::upgrades::UpgradeableComponent; diff --git a/src/tests/account/test_dual_eth_account.cairo b/src/tests/account/test_dual_eth_account.cairo index 46af544b9..cabb8d32f 100644 --- a/src/tests/account/test_dual_eth_account.cairo +++ b/src/tests/account/test_dual_eth_account.cairo @@ -1,9 +1,5 @@ -use openzeppelin::account::eth_account::dual_eth_account::{ - DualCaseEthAccountABI, DualCaseEthAccount -}; -use openzeppelin::account::eth_account::interface::{ - EthAccountABIDispatcherTrait, EthAccountABIDispatcher -}; +use openzeppelin::account::dual_eth_account::{DualCaseEthAccountABI, DualCaseEthAccount}; +use openzeppelin::account::interface::{EthAccountABIDispatcherTrait, EthAccountABIDispatcher}; use openzeppelin::account::utils::secp256k1::{ DebugSecp256k1Point, Secp256k1PointPartialEq, Secp256k1PointSerde }; diff --git a/src/tests/account/test_eth_account.cairo b/src/tests/account/test_eth_account.cairo index 35bf11966..308ddaac3 100644 --- a/src/tests/account/test_eth_account.cairo +++ b/src/tests/account/test_eth_account.cairo @@ -1,12 +1,10 @@ use core::starknet::secp256_trait::Secp256PointTrait; -use openzeppelin::account::eth_account::EthAccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; -use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; -use openzeppelin::account::eth_account::EthAccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; -use openzeppelin::account::eth_account::EthAccountComponent; -use openzeppelin::account::eth_account::interface::EthPublicKey; -use openzeppelin::account::eth_account::interface::{ - EthAccountABIDispatcherTrait, EthAccountABIDispatcher -}; +use openzeppelin::account::EthAccountComponent::{InternalTrait, SRC6CamelOnlyImpl}; +use openzeppelin::account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::EthAccountComponent::{PublicKeyCamelImpl, PublicKeyImpl}; +use openzeppelin::account::EthAccountComponent; +use openzeppelin::account::interface::EthPublicKey; +use openzeppelin::account::interface::{EthAccountABIDispatcherTrait, EthAccountABIDispatcher}; use openzeppelin::account::interface::{ISRC6, ISRC6_ID}; use openzeppelin::account::utils::secp256k1::{ DebugSecp256k1Point, Secp256k1PointPartialEq, Secp256k1PointSerde diff --git a/src/tests/mocks/eth_account_mocks.cairo b/src/tests/mocks/eth_account_mocks.cairo index f312e75a5..839706fc9 100644 --- a/src/tests/mocks/eth_account_mocks.cairo +++ b/src/tests/mocks/eth_account_mocks.cairo @@ -1,7 +1,7 @@ #[starknet::contract] mod DualCaseEthAccountMock { - use openzeppelin::account::eth_account::EthAccountComponent; - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::EthAccountComponent; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; @@ -45,8 +45,8 @@ mod DualCaseEthAccountMock { #[starknet::contract] mod SnakeEthAccountMock { - use openzeppelin::account::eth_account::EthAccountComponent; - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::EthAccountComponent; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; @@ -86,8 +86,8 @@ mod SnakeEthAccountMock { #[starknet::contract] mod CamelEthAccountMock { - use openzeppelin::account::eth_account::EthAccountComponent; - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::EthAccountComponent; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use openzeppelin::introspection::src5::SRC5Component; use starknet::account::Call; @@ -148,7 +148,7 @@ mod CamelEthAccountMock { #[starknet::contract] mod SnakeEthAccountPanicMock { - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use starknet::secp256k1::secp256k1_new_syscall; @@ -183,7 +183,7 @@ mod SnakeEthAccountPanicMock { #[starknet::contract] mod CamelEthAccountPanicMock { - use openzeppelin::account::eth_account::interface::EthPublicKey; + use openzeppelin::account::interface::EthPublicKey; use openzeppelin::account::utils::secp256k1::Secp256k1PointSerde; use starknet::secp256k1::secp256k1_new_syscall; diff --git a/src/tests/presets/test_eth_account.cairo b/src/tests/presets/test_eth_account.cairo index c5b2e69f7..9154c4433 100644 --- a/src/tests/presets/test_eth_account.cairo +++ b/src/tests/presets/test_eth_account.cairo @@ -1,10 +1,8 @@ use core::serde::Serde; use core::traits::TryInto; -use openzeppelin::account::eth_account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; -use openzeppelin::account::eth_account::interface::ISRC6_ID; -use openzeppelin::account::eth_account::interface::{ - EthAccountABIDispatcherTrait, EthAccountABIDispatcher -}; +use openzeppelin::account::EthAccountComponent::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::interface::ISRC6_ID; +use openzeppelin::account::interface::{EthAccountABIDispatcherTrait, EthAccountABIDispatcher}; use openzeppelin::account::utils::secp256k1::{ DebugSecp256k1Point, Secp256k1PointSerde, Secp256k1PointPartialEq }; diff --git a/src/tests/utils/constants.cairo b/src/tests/utils/constants.cairo index 417d84b11..7dd146ada 100644 --- a/src/tests/utils/constants.cairo +++ b/src/tests/utils/constants.cairo @@ -1,4 +1,4 @@ -use openzeppelin::account::eth_account::interface::EthPublicKey; +use openzeppelin::account::interface::EthPublicKey; use starknet::ClassHash; use starknet::ContractAddress; use starknet::class_hash_const; From 8a3939b12209820e4a5ca0f4f7ba353059e559ae Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 31 Jan 2024 11:54:13 +0100 Subject: [PATCH 23/24] fix: comments --- src/account/interface.cairo | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/account/interface.cairo b/src/account/interface.cairo index 6d2100148..a04af7b40 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -127,12 +127,12 @@ trait EthAccountABI { // IDeclarer fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; - // IDeployable + // IEthDeployable fn __validate_deploy__( self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: EthPublicKey ) -> felt252; - // IPublicKey + // IEthPublicKey fn get_public_key(self: @TState) -> EthPublicKey; fn set_public_key(ref self: TState, new_public_key: EthPublicKey); @@ -142,7 +142,7 @@ trait EthAccountABI { // ISRC5Camel fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; - // IPublicKeyCamel + // IEthPublicKeyCamel fn getPublicKey(self: @TState) -> EthPublicKey; fn setPublicKey(ref self: TState, newPublicKey: EthPublicKey); } From 0f9b41c84a930a975a1e884eaf41add9e0119f14 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 31 Jan 2024 12:01:32 +0100 Subject: [PATCH 24/24] fix: class hashes --- docs/modules/ROOT/pages/utils/_class_hashes.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/modules/ROOT/pages/utils/_class_hashes.adoc b/docs/modules/ROOT/pages/utils/_class_hashes.adoc index 4adf797ff..90290e7c0 100644 --- a/docs/modules/ROOT/pages/utils/_class_hashes.adoc +++ b/docs/modules/ROOT/pages/utils/_class_hashes.adoc @@ -2,8 +2,8 @@ :class-hash-cairo-version: https://crates.io/crates/cairo-lang-compiler/2.4.4[cairo 2.4.4] // Class Hashes -:account-class-hash: 0x05e41b67aff0a188509f36697ddc7858ae45431ae7830cba38f5d29e98acf038 -:eth-account-upgradeable-class-hash: 0x0402765bcede84b1267a9d4658a7737c3c41a8caf6201984c3df95577c2298a3 +:account-class-hash: 0x0402765bcede84b1267a9d4658a7737c3c41a8caf6201984c3df95577c2298a3 +:eth-account-upgradeable-class-hash: 0x03dda9bcfa854795d91d586b1a4275a68ab1ab185b33a5c00ce647c75875b0ff :erc20-class-hash: 0x03af5816946625d3d2c94ea451225715784762050eba736f0b0ad9186685bced :erc721-class-hash: 0x045c96d1b24c3dc060680e4bfd4bdc32161aefe8f8939cd4be3954c5d8688d75