From 331844dcf278ccdf96ce3b63fb3e5f2c78970561 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Sat, 21 Jan 2023 00:19:38 +0100 Subject: [PATCH 01/25] fix: link (#545) --- docs/modules/ROOT/pages/udc.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/udc.adoc b/docs/modules/ROOT/pages/udc.adoc index 70a95bf70..61783690b 100644 --- a/docs/modules/ROOT/pages/udc.adoc +++ b/docs/modules/ROOT/pages/udc.adoc @@ -2,7 +2,7 @@ The Universal Deployer Contract (UDC) is a singleton smart contract that wraps the https://www.cairo-lang.org/docs/hello_starknet/deploying_from_contracts.html#the-deploy-system-call[`deploy syscall`] to expose it to any contract that doesn't implement it, such as account contracts. You can think of it as a **standardized generic factory for StarkNet contracts**. -And since StarkNet has no deployment transaction type, it offers a canonical way to deploy smart contracts by following the [standard deployer interface](https://community.starknet.io/t/snip-deployer-contract-interface/2772) and emitting a `ContractDeployed` event. +And since StarkNet has no deployment transaction type, it offers a canonical way to deploy smart contracts by following the https://community.starknet.io/t/snip-deployer-contract-interface/2772[standard deployer interface] and emitting a `ContractDeployed` event. TIP: For information on design decisions, see the https://community.starknet.io/t/universal-deployer-contract-proposal/1864[Universal Deployer Contract Proposal]. From 4fc9be57b27d456d6ba8fb2f275bbc9622210651 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 10 Oct 2023 14:37:52 -0400 Subject: [PATCH 02/25] feat: update logic --- src/account/account.cairo | 165 +++++++++++++--------- src/account/interface.cairo | 7 + src/tests/account/test_account.cairo | 153 ++++++++------------ src/tests/account/test_dual_account.cairo | 46 +++--- src/tests/mocks.cairo | 4 +- src/tests/mocks/account_panic_mock.cairo | 65 --------- src/tests/mocks/camel_account_mock.cairo | 37 ----- src/tests/mocks/snake_account_mock.cairo | 39 ----- src/tests/token/test_erc721.cairo | 7 +- src/tests/utils/constants.cairo | 2 + 10 files changed, 186 insertions(+), 339 deletions(-) delete mode 100644 src/tests/mocks/account_panic_mock.cairo delete mode 100644 src/tests/mocks/camel_account_mock.cairo delete mode 100644 src/tests/mocks/snake_account_mock.cairo diff --git a/src/account/account.cairo b/src/account/account.cairo index 30160dfbe..2adf1c079 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -11,11 +11,15 @@ trait PublicKeyCamelTrait { fn getPublicKey(self: @TState) -> felt252; } -#[starknet::contract] +/// # Account Component +/// +/// The Account component enables contracts for acting as accounts. +#[starknet::component] mod Account { use ecdsa::check_ecdsa_signature; use openzeppelin::account::interface; - use openzeppelin::introspection::src5::SRC5 as src5_component; + use openzeppelin::introspection::src5::SRC5::InternalTrait as SRC5InternalTrait; + use openzeppelin::introspection::src5::SRC5; use starknet::account::Call; use starknet::get_caller_address; use starknet::get_contract_address; @@ -25,27 +29,16 @@ mod Account { // 2**128 + TRANSACTION_VERSION const QUERY_VERSION: felt252 = 0x100000000000000000000000000000001; - component!(path: src5_component, storage: src5, event: SRC5Event); - - #[abi(embed_v0)] - impl SRC5Impl = src5_component::SRC5Impl; - #[abi(embed_v0)] - impl SRC5CamelImpl = src5_component::SRC5CamelImpl; - impl SRC5InternalImpl = src5_component::InternalImpl; - #[storage] struct Storage { - Account_public_key: felt252, - #[substorage(v0)] - src5: src5_component::Storage + Account_public_key: felt252 } #[event] #[derive(Drop, starknet::Event)] enum Event { OwnerAdded: OwnerAdded, - OwnerRemoved: OwnerRemoved, - SRC5Event: src5_component::Event + OwnerRemoved: OwnerRemoved } #[derive(Drop, starknet::Event)] @@ -65,18 +58,16 @@ mod Account { const UNAUTHORIZED: felt252 = 'Account: unauthorized'; } - #[constructor] - fn constructor(ref self: ContractState, _public_key: felt252) { - self.initializer(_public_key); - } - - // - // External - // - - #[external(v0)] - impl SRC6Impl of interface::ISRC6 { - fn __execute__(self: @ContractState, mut calls: Array) -> Array> { + #[embeddable_as(SRC6Impl)] + impl SRC6< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of interface::ISRC6> { + 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(); @@ -92,12 +83,12 @@ mod Account { _execute_calls(calls) } - fn __validate__(self: @ContractState, mut calls: Array) -> felt252 { + fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { self.validate_transaction() } fn is_valid_signature( - self: @ContractState, hash: felt252, signature: Array + self: @ComponentState, hash: felt252, signature: Array ) -> felt252 { if self._is_valid_signature(hash, signature.span()) { starknet::VALIDATED @@ -107,68 +98,108 @@ mod Account { } } - #[external(v0)] - impl SRC6CamelOnlyImpl of interface::ISRC6CamelOnly { + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of interface::ISRC6CamelOnly> { fn isValidSignature( - self: @ContractState, hash: felt252, signature: Array + self: @ComponentState, hash: felt252, signature: Array ) -> felt252 { - SRC6Impl::is_valid_signature(self, hash, signature) + self.is_valid_signature(hash, signature) } } - #[external(v0)] - impl DeclarerImpl of interface::IDeclarer { - fn __validate_declare__(self: @ContractState, class_hash: felt252) -> felt252 { + #[embeddable_as(DeclarerImpl)] + impl Declarer< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of interface::IDeclarer> { + fn __validate_declare__( + self: @ComponentState, class_hash: felt252 + ) -> felt252 { self.validate_transaction() } } - #[external(v0)] - impl PublicKeyImpl of super::PublicKeyTrait { - fn get_public_key(self: @ContractState) -> felt252 { + #[embeddable_as(PublicKeyImpl)] + impl PublicKey< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of super::PublicKeyTrait> { + fn get_public_key(self: @ComponentState) -> felt252 { self.Account_public_key.read() } - fn set_public_key(ref self: ContractState, new_public_key: felt252) { - assert_only_self(); + 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); } } - #[external(v0)] - impl PublicKeyCamelImpl of super::PublicKeyCamelTrait { - fn getPublicKey(self: @ContractState) -> felt252 { + #[embeddable_as(PublicKeyCamelImpl)] + impl PublicKeyCamel< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of super::PublicKeyCamelTrait> { + fn getPublicKey(self: @ComponentState) -> felt252 { self.Account_public_key.read() } - fn setPublicKey(ref self: ContractState, newPublicKey: felt252) { - PublicKeyImpl::set_public_key(ref self, newPublicKey); + fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { + self.set_public_key(newPublicKey); } } - #[external(v0)] - fn __validate_deploy__( - self: @ContractState, - class_hash: felt252, - contract_address_salt: felt252, - _public_key: felt252 - ) -> felt252 { - self.validate_transaction() + #[embeddable_as(DeployableImpl)] + impl Deployable< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of interface::IDeployable> { + fn __validate_deploy__( + self: @ComponentState, + class_hash: felt252, + contract_address_salt: felt252, + _public_key: felt252 + ) -> felt252 { + self.validate_transaction() + } } - // - // Internal - // - #[generate_trait] - impl InternalImpl of InternalTrait { - fn initializer(ref self: ContractState, _public_key: felt252) { - self.src5.register_interface(interface::ISRC6_ID); + impl InternalImpl< + TContractState, + +HasComponent, + +SRC5::HasComponent, + +Drop + > of InternalTrait { + fn initializer(ref self: ComponentState, _public_key: felt252) { + let mut contract = self.get_contract_mut(); + let mut src5_component = SRC5::HasComponent::< + TContractState + >::get_component_mut(ref contract); + src5_component.register_interface(interface::ISRC6_ID); self._set_public_key(_public_key); } - fn validate_transaction(self: @ContractState) -> felt252 { + fn assert_only_self(self: @ComponentState) { + let caller = get_caller_address(); + let self = get_contract_address(); + assert(self == caller, Errors::UNAUTHORIZED); + } + + 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; @@ -176,13 +207,13 @@ mod Account { starknet::VALIDATED } - fn _set_public_key(ref self: ContractState, new_public_key: felt252) { + 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 }); } fn _is_valid_signature( - self: @ContractState, hash: felt252, signature: Span + self: @ComponentState, hash: felt252, signature: Span ) -> bool { let valid_length = signature.len() == 2_u32; @@ -196,12 +227,6 @@ mod Account { } } - fn assert_only_self() { - let caller = get_caller_address(); - let self = get_contract_address(); - assert(self == caller, Errors::UNAUTHORIZED); - } - fn _execute_calls(mut calls: Array) -> Array> { let mut res = ArrayTrait::new(); loop { diff --git a/src/account/interface.cairo b/src/account/interface.cairo index e58c5bb30..3551cdc7f 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -23,6 +23,13 @@ trait IDeclarer { fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; } +#[starknet::interface] +trait IDeployable { + fn __validate_deploy__( + self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 + ) -> felt252; +} + #[starknet::interface] trait AccountABI { fn __execute__(self: @TState, calls: Array) -> Array>; diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 012277f4b..70a339469 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -1,18 +1,15 @@ -use openzeppelin::account::Account::OwnerAdded; -use openzeppelin::account::Account::OwnerRemoved; -use openzeppelin::account::Account::PublicKeyCamelImpl; -use openzeppelin::account::Account::PublicKeyImpl; +use openzeppelin::account::Account::{InternalTrait, OwnerAdded, OwnerRemoved}; +use openzeppelin::account::Account::{PublicKeyCamelImpl, PublicKeyImpl}; use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::Account; -use openzeppelin::account::AccountABIDispatcher; -use openzeppelin::account::AccountABIDispatcherTrait; -use openzeppelin::account::interface::ISRC6_ID; -use openzeppelin::introspection::interface::ISRC5_ID; -use openzeppelin::tests::utils::constants::ZERO; +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::utils::constants::{PUBKEY, NEW_PUBKEY, SALT, ZERO}; use openzeppelin::tests::utils; use openzeppelin::token::erc20::ERC20; -use openzeppelin::token::erc20::interface::IERC20Dispatcher; -use openzeppelin::token::erc20::interface::IERC20DispatcherTrait; +use openzeppelin::token::erc20::interface::{IERC20DispatcherTrait, IERC20Dispatcher}; use openzeppelin::utils::selectors; use openzeppelin::utils::serde::SerializedAppend; use starknet::ContractAddress; @@ -20,14 +17,6 @@ use starknet::account::Call; use starknet::contract_address_const; use starknet::testing; -// -// Constants -// - -const PUBLIC_KEY: felt252 = 0x333333; -const NEW_PUBKEY: felt252 = 0x789789; -const SALT: felt252 = 123; - #[derive(Drop)] struct SignedTransactionData { private_key: felt252, @@ -37,15 +26,25 @@ struct SignedTransactionData { s: felt252 } -fn STATE() -> Account::ContractState { - Account::contract_state_for_testing() +fn STATE() -> DualCaseAccountMock::ContractState { + DualCaseAccountMock::contract_state_for_testing() +} + +fn setup() -> DualCaseAccountMock::ContractState { + let mut state = STATE(); + state.account.initializer(PUBKEY); + utils::drop_event(ZERO()); + state } + fn CLASS_HASH() -> felt252 { - Account::TEST_CLASS_HASH + DualCaseAccountMock::TEST_CLASS_HASH } + fn ACCOUNT_ADDRESS() -> ContractAddress { contract_address_const::<0x111111>() } + fn SIGNED_TX_DATA() -> SignedTransactionData { SignedTransactionData { private_key: 1234, @@ -56,10 +55,6 @@ fn SIGNED_TX_DATA() -> SignedTransactionData { } } -// -// Setup -// - fn setup_dispatcher(data: Option<@SignedTransactionData>) -> AccountABIDispatcher { testing::set_version(TRANSACTION_VERSION); @@ -71,7 +66,7 @@ fn setup_dispatcher(data: Option<@SignedTransactionData>) -> AccountABIDispatche calldata.append(*data.public_key); } else { - calldata.append(PUBLIC_KEY); + calldata.append(PUBKEY); } let address = utils::deploy(CLASS_HASH(), calldata); AccountABIDispatcher { contract_address: address } @@ -91,23 +86,6 @@ fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> IERC20Dispa IERC20Dispatcher { contract_address: address } } -// -// constructor -// - -#[test] -#[available_gas(2000000)] -fn test_constructor() { - let mut state = STATE(); - Account::constructor(ref state, PUBLIC_KEY); - - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); - utils::assert_no_events_left(ZERO()); - - assert(PublicKeyImpl::get_public_key(@state) == PUBLIC_KEY, 'Should return public key'); -} - // // supports_interface & supportsInterface // @@ -115,26 +93,12 @@ fn test_constructor() { #[test] #[available_gas(2000000)] fn test_supports_interface() { - let mut state = STATE(); - Account::constructor(ref state, PUBLIC_KEY); - - let supports_default_interface = Account::SRC5Impl::supports_interface(@state, ISRC5_ID); - assert(supports_default_interface, 'Should support base interface'); - - let supports_account_interface = Account::SRC5Impl::supports_interface(@state, ISRC6_ID); - assert(supports_account_interface, 'Should support account id'); -} - -#[test] -#[available_gas(2000000)] -fn test_supportsInterface() { - let mut state = STATE(); - Account::constructor(ref state, PUBLIC_KEY); + let state = setup(); - let supports_default_interface = Account::SRC5CamelImpl::supportsInterface(@state, ISRC5_ID); + let supports_default_interface = state.src5.supports_interface(ISRC5_ID); assert(supports_default_interface, 'Should support base interface'); - let supports_account_interface = Account::SRC5CamelImpl::supportsInterface(@state, ISRC6_ID); + let supports_account_interface = state.src5.supports_interface(ISRC6_ID); assert(supports_account_interface, 'Should support account id'); } @@ -152,12 +116,12 @@ fn test_is_valid_signature() { let mut good_signature = array![data.r, data.s]; let mut bad_signature = array![0x987, 0x564]; - PublicKeyImpl::set_public_key(ref state, data.public_key); + state.set_public_key(data.public_key); - let is_valid = Account::SRC6Impl::is_valid_signature(@state, hash, good_signature); + let is_valid = state.account.is_valid_signature(hash, good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); - let is_valid = Account::SRC6Impl::is_valid_signature(@state, hash, bad_signature); + let is_valid = state.account.is_valid_signature(hash, bad_signature); assert(is_valid == 0, 'Should reject invalid signature'); } @@ -171,7 +135,7 @@ fn test_isValidSignature() { let mut good_signature = array![data.r, data.s]; let mut bad_signature = array![0x987, 0x564]; - PublicKeyImpl::set_public_key(ref state, data.public_key); + state.account.set_public_key(data.public_key); let is_valid = Account::SRC6CamelOnlyImpl::isValidSignature(@state, hash, good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); @@ -193,7 +157,7 @@ fn test_validate_deploy() { // 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, PUBLIC_KEY) == starknet::VALIDATED, + account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY) == starknet::VALIDATED, 'Should validate correctly' ); } @@ -206,7 +170,7 @@ fn test_validate_deploy_invalid_signature_data() { data.transaction_hash += 1; let account = setup_dispatcher(Option::Some(@data)); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBLIC_KEY); + account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); } #[test] @@ -219,7 +183,7 @@ fn test_validate_deploy_invalid_signature_length() { signature.append(0x1); testing::set_signature(signature.span()); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBLIC_KEY); + account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); } #[test] @@ -230,7 +194,7 @@ fn test_validate_deploy_empty_signature() { let empty_sig = array![]; testing::set_signature(empty_sig.span()); - account.__validate_deploy__(CLASS_HASH(), SALT, PUBLIC_KEY); + account.__validate_deploy__(CLASS_HASH(), SALT, PUBKEY); } #[test] @@ -420,13 +384,14 @@ fn test_multicall_zero_calls() { #[available_gas(2000000)] #[should_panic(expected: ('Account: 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); - Account::SRC6Impl::__execute__(@STATE(), calls); + state.account.__execute__(calls); } // @@ -441,11 +406,11 @@ fn test_public_key_setter_and_getter() { testing::set_caller_address(ACCOUNT_ADDRESS()); // Check default - let public_key = PublicKeyImpl::get_public_key(@state); + let public_key = state.account.get_public_key(); assert(public_key == 0, 'Should be zero'); // Set key - PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); + state.account.set_public_key(NEW_PUBKEY); let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.removed_owner_guid == 0, 'Invalid old owner key'); @@ -454,7 +419,7 @@ fn test_public_key_setter_and_getter() { assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); utils::assert_no_events_left(ACCOUNT_ADDRESS()); - let public_key = PublicKeyImpl::get_public_key(@state); + let public_key = state.account.get_public_key(); assert(public_key == NEW_PUBKEY, 'Should update key'); } @@ -467,7 +432,7 @@ fn test_public_key_setter_different_account() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(caller); - PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); + state.account.set_public_key(NEW_PUBKEY); } // @@ -482,11 +447,11 @@ fn test_public_key_setter_and_getter_camel() { testing::set_caller_address(ACCOUNT_ADDRESS()); // Check default - let public_key = PublicKeyCamelImpl::getPublicKey(@state); + let public_key = state.getPublicKey(); assert(public_key == 0, 'Should be zero'); // Set key - PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); + state.setPublicKey(NEW_PUBKEY); let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.removed_owner_guid == 0, 'Invalid old owner key'); @@ -495,7 +460,7 @@ fn test_public_key_setter_and_getter_camel() { assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); utils::assert_no_events_left(ACCOUNT_ADDRESS()); - let public_key = PublicKeyCamelImpl::getPublicKey(@state); + let public_key = state.getPublicKey(); assert(public_key == NEW_PUBKEY, 'Should update key'); } @@ -508,7 +473,7 @@ fn test_public_key_setter_different_account_camel() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(caller); - PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); + state.setPublicKey(NEW_PUBKEY); } // @@ -519,31 +484,35 @@ fn test_public_key_setter_different_account_camel() { #[available_gas(2000000)] fn test_initializer() { let mut state = STATE(); - Account::InternalImpl::initializer(ref state, PUBLIC_KEY); + state.account.initializer(PUBKEY); let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); + assert(event.new_owner_guid == PUBKEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); - assert(PublicKeyImpl::get_public_key(@state) == PUBLIC_KEY, 'Should return public key'); + assert(state.account.get_public_key() == PUBKEY, 'Should return public key'); } #[test] #[available_gas(2000000)] fn test_assert_only_self_true() { + let mut state = STATE(); + testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(ACCOUNT_ADDRESS()); - Account::assert_only_self(); + state.account.assert_only_self(); } #[test] #[available_gas(2000000)] #[should_panic(expected: ('Account: unauthorized',))] fn test_assert_only_self_false() { + let mut state = STATE(); + testing::set_contract_address(ACCOUNT_ADDRESS()); let other = contract_address_const::<0x4567>(); testing::set_caller_address(other); - Account::assert_only_self(); + state.account.assert_only_self(); } #[test] @@ -557,17 +526,15 @@ fn test__is_valid_signature() { let mut bad_signature = array![0x987, 0x564]; let mut invalid_length_signature = array![0x987]; - PublicKeyImpl::set_public_key(ref state, data.public_key); + state.account.set_public_key(data.public_key); - let is_valid = Account::InternalImpl::_is_valid_signature(@state, hash, good_signature.span()); + let is_valid = state.account._is_valid_signature(hash, good_signature.span()); assert(is_valid, 'Should accept valid signature'); - let is_valid = Account::InternalImpl::_is_valid_signature(@state, hash, bad_signature.span()); + let is_valid = state.account._is_valid_signature(hash, bad_signature.span()); assert(!is_valid, 'Should reject invalid signature'); - let is_valid = Account::InternalImpl::_is_valid_signature( - @state, hash, invalid_length_signature.span() - ); + let is_valid = state.account._is_valid_signature(hash, invalid_length_signature.span()); assert(!is_valid, 'Should reject invalid length'); } @@ -575,12 +542,12 @@ fn test__is_valid_signature() { #[available_gas(2000000)] fn test__set_public_key() { let mut state = STATE(); - Account::InternalImpl::_set_public_key(ref state, PUBLIC_KEY); + state.account._set_public_key(PUBKEY); let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); + assert(event.new_owner_guid == PUBKEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); - let public_key = PublicKeyImpl::get_public_key(@state); - assert(public_key == PUBLIC_KEY, 'Should update key'); + let public_key = state.account.get_public_key(); + assert(public_key == PUBKEY, 'Should update key'); } diff --git a/src/tests/account/test_dual_account.cairo b/src/tests/account/test_dual_account.cairo index 2c2156ed6..e41f7e47c 100644 --- a/src/tests/account/test_dual_account.cairo +++ b/src/tests/account/test_dual_account.cairo @@ -1,32 +1,22 @@ -use openzeppelin::account::AccountABIDispatcher; -use openzeppelin::account::AccountABIDispatcherTrait; -use openzeppelin::account::AccountCamelABIDispatcher; -use openzeppelin::account::AccountCamelABIDispatcherTrait; -use openzeppelin::account::dual_account::DualCaseAccount; -use openzeppelin::account::dual_account::DualCaseAccountABI; +use openzeppelin::account::dual_account::{DualCaseAccountABI, DualCaseAccount}; +use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; +use openzeppelin::account::{AccountCamelABIDispatcherTrait, AccountCamelABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::test_account::SIGNED_TX_DATA; -use openzeppelin::tests::mocks::account_panic_mock::CamelAccountPanicMock; -use openzeppelin::tests::mocks::account_panic_mock::SnakeAccountPanicMock; -use openzeppelin::tests::mocks::camel_account_mock::CamelAccountMock; +use openzeppelin::tests::mocks::account_mocks::{ + CamelAccountPanicMock, CamelAccountMock, SnakeAccountMock, SnakeAccountPanicMock +}; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; -use openzeppelin::tests::mocks::snake_account_mock::SnakeAccountMock; +use openzeppelin::tests::utils::constants::{PUBKEY, NEW_PUBKEY}; use openzeppelin::tests::utils; use starknet::testing; -// -// Constants -// - -const PUBLIC_KEY: felt252 = 0x333333; -const NEW_PUBLIC_KEY: felt252 = 0x444444; - // // Setup // fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { - let mut calldata = array![PUBLIC_KEY]; + let mut calldata = array![PUBKEY]; let target = utils::deploy(SnakeAccountMock::TEST_CLASS_HASH, calldata); ( DualCaseAccount { contract_address: target }, @@ -35,7 +25,7 @@ fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { } fn setup_camel() -> (DualCaseAccount, AccountCamelABIDispatcher) { - let mut calldata = array![PUBLIC_KEY]; + let mut calldata = array![PUBKEY]; let target = utils::deploy(CamelAccountMock::TEST_CLASS_HASH, calldata); ( DualCaseAccount { contract_address: target }, @@ -69,8 +59,8 @@ fn test_dual_set_public_key() { testing::set_contract_address(snake_dispatcher.contract_address); - snake_dispatcher.set_public_key(NEW_PUBLIC_KEY); - assert(target.get_public_key() == NEW_PUBLIC_KEY, 'Should return NEW_PUBLIC_KEY'); + snake_dispatcher.set_public_key(NEW_PUBKEY); + assert(target.get_public_key() == NEW_PUBKEY, 'Should return NEW_PUBKEY'); } #[test] @@ -78,7 +68,7 @@ fn test_dual_set_public_key() { #[should_panic(expected: ('ENTRYPOINT_NOT_FOUND',))] fn test_dual_no_set_public_key() { let dispatcher = setup_non_account(); - dispatcher.set_public_key(NEW_PUBLIC_KEY); + dispatcher.set_public_key(NEW_PUBKEY); } #[test] @@ -86,14 +76,14 @@ fn test_dual_no_set_public_key() { #[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_PUBLIC_KEY); + dispatcher.set_public_key(NEW_PUBKEY); } #[test] #[available_gas(2000000)] fn test_dual_get_public_key() { let (snake_dispatcher, _) = setup_snake(); - assert(snake_dispatcher.get_public_key() == PUBLIC_KEY, 'Should return PUBLIC_KEY'); + assert(snake_dispatcher.get_public_key() == PUBKEY, 'Should return PUBKEY'); } #[test] @@ -184,8 +174,8 @@ 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_PUBLIC_KEY'); + camel_dispatcher.set_public_key(NEW_PUBKEY); + assert(target.getPublicKey() == NEW_PUBKEY, 'Should return NEW_PUBKEY'); } #[test] @@ -193,14 +183,14 @@ fn test_dual_setPublicKey() { #[should_panic(expected: ('Some error', 'ENTRYPOINT_FAILED',))] fn test_dual_setPublicKey_exists_and_panics() { let (_, dispatcher) = setup_account_panic(); - dispatcher.set_public_key(NEW_PUBLIC_KEY); + dispatcher.set_public_key(NEW_PUBKEY); } #[test] #[available_gas(2000000)] fn test_dual_getPublicKey() { let (camel_dispatcher, _) = setup_camel(); - assert(camel_dispatcher.get_public_key() == PUBLIC_KEY, 'Should return PUBLIC_KEY'); + assert(camel_dispatcher.get_public_key() == PUBKEY, 'Should return PUBKEY'); } #[test] diff --git a/src/tests/mocks.cairo b/src/tests/mocks.cairo index 11a30de47..09a062496 100644 --- a/src/tests/mocks.cairo +++ b/src/tests/mocks.cairo @@ -1,9 +1,8 @@ mod accesscontrol_panic_mock; -mod account_panic_mock; +mod account_mocks; mod camel20_mock; mod camel721_mock; mod camel_accesscontrol_mock; -mod camel_account_mock; mod dual721_receiver_mocks; mod dual_ownable_mocks; mod erc20_panic; @@ -15,7 +14,6 @@ mod reentrancy_mock; mod snake20_mock; mod snake721_mock; mod snake_accesscontrol_mock; -mod snake_account_mock; mod src5_mocks; mod upgrades_v1; mod upgrades_v2; diff --git a/src/tests/mocks/account_panic_mock.cairo b/src/tests/mocks/account_panic_mock.cairo deleted file mode 100644 index 0a85c890c..000000000 --- a/src/tests/mocks/account_panic_mock.cairo +++ /dev/null @@ -1,65 +0,0 @@ -// 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 SnakeAccountPanicMock { - #[storage] - struct Storage {} - - #[external(v0)] - fn set_public_key(ref self: ContractState, new_public_key: felt252) { - panic_with_felt252('Some error'); - } - - #[external(v0)] - fn get_public_key(self: @ContractState) -> felt252 { - panic_with_felt252('Some error'); - 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 CamelAccountPanicMock { - #[storage] - struct Storage {} - - #[external(v0)] - fn setPublicKey(ref self: ContractState, newPublicKey: felt252) { - panic_with_felt252('Some error'); - } - - #[external(v0)] - fn getPublicKey(self: @ContractState) -> felt252 { - panic_with_felt252('Some error'); - 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/mocks/camel_account_mock.cairo b/src/tests/mocks/camel_account_mock.cairo deleted file mode 100644 index 19cf1725c..000000000 --- a/src/tests/mocks/camel_account_mock.cairo +++ /dev/null @@ -1,37 +0,0 @@ -#[starknet::contract] -mod CamelAccountMock { - use openzeppelin::account::Account; - - #[storage] - struct Storage {} - - #[constructor] - fn constructor(ref self: ContractState, _publicKey: felt252) { - let mut unsafe_state = Account::unsafe_new_contract_state(); - Account::InternalImpl::initializer(ref unsafe_state, _publicKey); - } - - #[external(v0)] - fn setPublicKey(ref self: ContractState, newPublicKey: felt252) { - let mut unsafe_state = Account::unsafe_new_contract_state(); - Account::PublicKeyCamelImpl::setPublicKey(ref unsafe_state, newPublicKey); - } - - #[external(v0)] - fn getPublicKey(self: @ContractState) -> felt252 { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::PublicKeyCamelImpl::getPublicKey(@unsafe_state) - } - - #[external(v0)] - fn isValidSignature(self: @ContractState, hash: felt252, signature: Array) -> felt252 { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::SRC6CamelOnlyImpl::isValidSignature(@unsafe_state, hash, signature) - } - - #[external(v0)] - fn supportsInterface(self: @ContractState, interfaceId: felt252) -> bool { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::SRC5CamelImpl::supportsInterface(@unsafe_state, interfaceId) - } -} diff --git a/src/tests/mocks/snake_account_mock.cairo b/src/tests/mocks/snake_account_mock.cairo deleted file mode 100644 index 80dc1f715..000000000 --- a/src/tests/mocks/snake_account_mock.cairo +++ /dev/null @@ -1,39 +0,0 @@ -#[starknet::contract] -mod SnakeAccountMock { - use openzeppelin::account::Account; - - #[storage] - struct Storage {} - - #[constructor] - fn constructor(ref self: ContractState, _public_key: felt252) { - let mut unsafe_state = Account::unsafe_new_contract_state(); - Account::InternalImpl::initializer(ref unsafe_state, _public_key); - } - - #[external(v0)] - fn set_public_key(ref self: ContractState, new_public_key: felt252) { - let mut unsafe_state = Account::unsafe_new_contract_state(); - Account::PublicKeyImpl::set_public_key(ref unsafe_state, new_public_key); - } - - #[external(v0)] - fn get_public_key(self: @ContractState) -> felt252 { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::PublicKeyImpl::get_public_key(@unsafe_state) - } - - #[external(v0)] - fn is_valid_signature( - self: @ContractState, hash: felt252, signature: Array - ) -> felt252 { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::SRC6Impl::is_valid_signature(@unsafe_state, hash, signature) - } - - #[external(v0)] - fn supports_interface(self: @ContractState, interface_id: felt252) -> bool { - let unsafe_state = Account::unsafe_new_contract_state(); - Account::SRC5Impl::supports_interface(@unsafe_state, interface_id) - } -} diff --git a/src/tests/token/test_erc721.cairo b/src/tests/token/test_erc721.cairo index 9b9016e95..3bf966268 100644 --- a/src/tests/token/test_erc721.cairo +++ b/src/tests/token/test_erc721.cairo @@ -1,12 +1,11 @@ use ERC721::ERC721_owners::InternalContractMemberStateTrait as OwnersTrait; use ERC721::ERC721_token_approvals::InternalContractMemberStateTrait as TokenApprovalsTrait; -use integer::u256; -use integer::u256_from_felt252; +use integer::{u256, u256_from_felt252}; use openzeppelin::account::Account; use openzeppelin::introspection::src5; use openzeppelin::introspection; -use openzeppelin::tests::mocks::camel_account_mock::CamelAccountMock; +use openzeppelin::tests::mocks::account_mocks::{DualCaseAccountMock, CamelAccountMock}; use openzeppelin::tests::mocks::dual721_receiver_mocks::CamelERC721ReceiverMock; use openzeppelin::tests::mocks::erc721_receiver::ERC721Receiver; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; @@ -51,7 +50,7 @@ fn setup_camel_receiver() -> ContractAddress { fn setup_account() -> ContractAddress { let mut calldata = array![PUBKEY]; - utils::deploy(Account::TEST_CLASS_HASH, calldata) + utils::deploy(DualCaseAccountMock::TEST_CLASS_HASH, calldata) } fn setup_camel_account() -> ContractAddress { diff --git a/src/tests/utils/constants.cairo b/src/tests/utils/constants.cairo index 73d22eb41..846a899fc 100644 --- a/src/tests/utils/constants.cairo +++ b/src/tests/utils/constants.cairo @@ -13,6 +13,8 @@ const OTHER_ROLE: felt252 = 'OTHER_ROLE'; const URI: felt252 = 'URI'; const TOKEN_ID: u256 = 21; const PUBKEY: felt252 = 'PUBKEY'; +const NEW_PUBKEY: felt252 = 'NEW_PUBKEY'; +const SALT: felt252 = 'SALT'; const SUCCESS: felt252 = 123123; const FAILURE: felt252 = 456456; From ce91ed03d6df1986ffa3948bde45ee36c2aa1d44 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 10 Oct 2023 14:38:41 -0400 Subject: [PATCH 03/25] feat: add mocks --- src/tests/mocks/account_mocks.cairo | 183 ++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 src/tests/mocks/account_mocks.cairo diff --git a/src/tests/mocks/account_mocks.cairo b/src/tests/mocks/account_mocks.cairo new file mode 100644 index 000000000..88cfa4b76 --- /dev/null +++ b/src/tests/mocks/account_mocks.cairo @@ -0,0 +1,183 @@ +#[starknet::contract] +mod DualCaseAccountMock { + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; + + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6Impl = account_component::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl DeclarerImpl = account_component::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = account_component::DeployableImpl; + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; + impl AccountInternalImpl = account_component::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + AccountEvent: account_component::Event, + SRC5Event: src5_component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} + +#[starknet::contract] +mod SnakeAccountMock { + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; + + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6Impl = account_component::SRC6Impl; + #[abi(embed_v0)] + impl PublicKeyImpl = account_component::PublicKeyImpl; + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; + impl AccountInternalImpl = account_component::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + AccountEvent: account_component::Event, + SRC5Event: src5_component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} + +#[starknet::contract] +mod CamelAccountMock { + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; + + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); + + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = account_component::PublicKeyCamelImpl; + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; + impl AccountInternalImpl = account_component::InternalImpl; + + + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + AccountEvent: account_component::Event, + SRC5Event: src5_component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, publicKey: felt252) { + self.account.initializer(publicKey); + } +} + +// 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 SnakeAccountPanicMock { + #[storage] + struct Storage {} + + #[external(v0)] + fn set_public_key(ref self: ContractState, new_public_key: felt252) { + panic_with_felt252('Some error'); + } + + #[external(v0)] + fn get_public_key(self: @ContractState) -> felt252 { + panic_with_felt252('Some error'); + 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 CamelAccountPanicMock { + #[storage] + struct Storage {} + + #[external(v0)] + fn setPublicKey(ref self: ContractState, newPublicKey: felt252) { + panic_with_felt252('Some error'); + } + + #[external(v0)] + fn getPublicKey(self: @ContractState) -> felt252 { + panic_with_felt252('Some error'); + 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 + } +} From 4a903587f3400854a24454d283a84142a075b959 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 11 Oct 2023 12:22:41 -0400 Subject: [PATCH 04/25] docs: update --- docs/modules/ROOT/pages/api/account.adoc | 103 +++++++++++++---------- src/account/account.cairo | 28 +++++- src/account/interface.cairo | 6 +- 3 files changed, 87 insertions(+), 50 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 520d1f21f..bb4f8bbaf 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -71,19 +71,11 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. ```javascript use openzeppelin::account::Account; ``` -Account contract implementation extending xref:ISRC6[`ISRC6`]. +Account component extending xref:ISRC6[`ISRC6`]. [.contract-index] -.Constructor +.Embeddable Functions -- -* xref:#Account-constructor[`++constructor(self, _public_key)++`] --- - -[.contract-index] -.External Functions --- -* xref:#Account-\\__validate_deploy__[`++__validate_deploy__(self, hash, signature)++`] - .SRC6Impl * xref:#Account-\\__execute__[`++__execute__(self, calls)++`] @@ -98,10 +90,23 @@ Account contract implementation extending xref:ISRC6[`ISRC6`]. * xref:#Account-\\__validate_declare__[`++__validate_declare__(self, class_hash)++`] +.DeployableImpl + +* xref:#Account-\\__validate_deploy__[`++__validate_deploy__(self, hash, signature)++`] + .PublicKeyImpl * xref:#Account-set_public_key[`++set_public_key(self, new_public_key)++`] * xref:#Account-get_public_key[`++get_public_key(self)++`] + +.SRC6CamelOnlyImpl + +* xref:#Account-isValidSignature[`++isValidSignature(self, hash, signature)++`] + +.PublicKeyCamelImpl + +* xref:#Account-setPublicKey[`++setPublicKey(self, newPublicKey)++`] +* xref:#Account-getPublicKey[`++getPublicKey(self)++`] -- [.contract-index] @@ -110,10 +115,10 @@ Account contract implementation extending xref:ISRC6[`ISRC6`]. .InternalImpl * xref:#Account-initializer[`++initializer(self, _public_key)++`] +* xref:#Account-assert_only_self[`++assert_only_self(self)++`] * xref:#Account-validate_transaction[`++validate_transaction(self)++`] * xref:#Account-_set_public_key[`++_set_public_key(self, new_public_key)++`] * xref:#Account-_is_valid_signature[`++_is_valid_signature(self, hash, signature)++`] -* xref:#Account-assert_only_self[`++assert_only_self(self)++`] -- [.contract-index] @@ -123,28 +128,8 @@ Account contract implementation extending xref:ISRC6[`ISRC6`]. * xref:#Account-OwnerRemoved[`++OwnerRemoved(removed_owner_guid)++`] -- -[#Account-Constructor] -==== Constructor - -[.contract-item] -[[Account-constructor]] -==== `[.contract-item-name]#++constructor++#++(ref self: ContractState, _public_key: felt252)++` [.item-kind]#constructor# - -Initializes the account with the given public key, and registers the ISRC6 interface ID. - -Emits an {OwnerAdded} event. - -[#Account-External-Functions] -==== External Functions - -[.contract-item] -[[Account-__validate_deploy__]] -==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252) → 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. +[#Account-Embeddable-Functions] +==== Embeddable Functions [.contract-item] [[Account-__execute__]] @@ -178,6 +163,15 @@ Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Net Returns the short string `'VALID'` if valid, otherwise it reverts. +[.contract-item] +[[Account-__validate_deploy__]] +==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252) → 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] [[Account-set_public_key]] ==== `[.contract-item-name]#++set_public_key++#++(ref self: ContractState, new_public_key: felt252)++` [.item-kind]#external# @@ -192,20 +186,47 @@ Emits both an {OwnerRemoved} and an {OwnerAdded} event. Returns the current public key of the account. +[#Account-camelCase-Support] +==== camelCase Support + +[.contract-item] +[[Account-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] +[[Account-setPublicKey]] +==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: felt252)++` [.item-kind]#external# + +See xref:Account-set_public_key[set_public_key]. + +[.contract-item] +[[Account-getPublicKey]] +==== `[.contract-item-name]#++getPublicKey++#++(self: @ContractState)++ → felt252` [.item-kind]#external# + +See xref:Account-get_public_key[get_public_key]. + [#Account-Internal-Functions] ==== Internal Functions [.contract-item] [[Account-initializer]] -==== `[.contract-item-name]#++initializer++#++(ref self: ContractState, _public_key: felt252)++` [.item-kind]#internal# +==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, _public_key: felt252)++` [.item-kind]#internal# Initializes the account with the given public key, and registers the ISRC6 interface ID. Emits an {OwnerAdded} event. +[.contract-item] +[[Account-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] [[Account-validate_transaction]] -==== `[.contract-item-name]#++validate_transaction++#++(self: @ContractState)++ → felt252` [.item-kind]#internal# +==== `[.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]. @@ -214,7 +235,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] [[Account-_set_public_key]] -==== `[.contract-item-name]#++_set_public_key++#++(ref self: ContractState, new_public_key: felt252)++` [.item-kind]#internal# +==== `[.contract-item-name]#++_set_public_key++#++(ref self: ComponentState, new_public_key: felt252)++` [.item-kind]#internal# Set the public key without validating the caller. @@ -224,15 +245,9 @@ CAUTION: The usage of this method outside the `set_public_key` function is disco [.contract-item] [[Account-_is_valid_signature]] -==== `[.contract-item-name]#++_is_valid_signature++#++(self: @ContractState, hash: felt252, signature: Span)++ → bool` [.item-kind]#internal# +==== `[.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 current public key. - -[.contract-item] -[[Account-assert_only_self]] -==== `[.contract-item-name]#++assert_only_self++#++(self: @ContractState)++` [.item-kind]#internal# - -Validates that the caller is the account itself. Otherwise it reverts. +Validates the provided `signature` for the `hash`, using the account's current public key. [#Account-Events] ==== Events diff --git a/src/account/account.cairo b/src/account/account.cairo index 2adf1c079..0cad56c67 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -65,6 +65,7 @@ mod Account { +SRC5::HasComponent, +Drop > of interface::ISRC6> { + /// Executes a list of calls from the account. fn __execute__( self: @ComponentState, mut calls: Array ) -> Array> { @@ -83,10 +84,13 @@ mod Account { _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 { @@ -105,6 +109,7 @@ mod Account { +SRC5::HasComponent, +Drop > of interface::ISRC6CamelOnly> { + /// Adds camelCase support for `is_valid_signature`. fn isValidSignature( self: @ComponentState, hash: felt252, signature: Array ) -> felt252 { @@ -119,6 +124,8 @@ mod Account { +SRC5::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 { @@ -133,10 +140,12 @@ mod Account { +SRC5::HasComponent, +Drop > of super::PublicKeyTrait> { + /// 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() }); @@ -151,10 +160,12 @@ mod Account { +SRC5::HasComponent, +Drop > of super::PublicKeyCamelTrait> { + /// Adds camelCase support for `get_public_key`. fn getPublicKey(self: @ComponentState) -> felt252 { self.Account_public_key.read() } + /// Adds camelCase support for `set_public_key`. fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { self.set_public_key(newPublicKey); } @@ -167,11 +178,13 @@ mod Account { +SRC5::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 + public_key: felt252 ) -> felt252 { self.validate_transaction() } @@ -184,21 +197,26 @@ mod Account { +SRC5::HasComponent, +Drop > of InternalTrait { - fn initializer(ref self: ComponentState, _public_key: felt252) { + /// 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 contract = self.get_contract_mut(); let mut src5_component = SRC5::HasComponent::< TContractState >::get_component_mut(ref contract); src5_component.register_interface(interface::ISRC6_ID); - self._set_public_key(_public_key); + 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; @@ -207,11 +225,15 @@ mod Account { 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 { diff --git a/src/account/interface.cairo b/src/account/interface.cairo index 3551cdc7f..5955dc638 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -26,7 +26,7 @@ trait IDeclarer { #[starknet::interface] trait IDeployable { fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252 ) -> felt252; } @@ -36,7 +36,7 @@ trait AccountABI { fn __validate__(self: @TState, calls: Array) -> felt252; fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252 ) -> felt252; fn set_public_key(ref self: TState, new_public_key: felt252); fn get_public_key(self: @TState) -> felt252; @@ -51,7 +51,7 @@ trait AccountCamelABI { fn __validate__(self: @TState, calls: Array) -> felt252; fn __validate_declare__(self: @TState, classHash: felt252) -> felt252; fn __validate_deploy__( - self: @TState, classHash: felt252, contractAddressSalt: felt252, _publicKey: felt252 + self: @TState, classHash: felt252, contractAddressSalt: felt252, publicKey: felt252 ) -> felt252; fn setPublicKey(ref self: TState, newPublicKey: felt252); fn getPublicKey(self: @TState) -> felt252; From 7f741505f0f6c734de74638006138eaa711a44f5 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 11 Oct 2023 12:38:35 -0400 Subject: [PATCH 05/25] feat: update docs --- docs/modules/ROOT/pages/api/account.adoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index bb4f8bbaf..aa76a9e74 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -98,7 +98,11 @@ Account component extending xref:ISRC6[`ISRC6`]. * xref:#Account-set_public_key[`++set_public_key(self, new_public_key)++`] * xref:#Account-get_public_key[`++get_public_key(self)++`] +-- +[.contract-index] +.camelCase Support +-- .SRC6CamelOnlyImpl * xref:#Account-isValidSignature[`++isValidSignature(self, hash, signature)++`] From bff91794e55c9b55e63a842058f0502826983fbb Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 18 Oct 2023 12:38:48 +0200 Subject: [PATCH 06/25] Update src/account/account.cairo Co-authored-by: Andrew Fleming --- src/account/account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/account/account.cairo b/src/account/account.cairo index 0cad56c67..c7cfece8f 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -13,7 +13,7 @@ trait PublicKeyCamelTrait { /// # Account Component /// -/// The Account component enables contracts for acting as accounts. +/// The Account component enables contracts to behave as accounts. #[starknet::component] mod Account { use ecdsa::check_ecdsa_signature; From 100c483051e1569318a03ca51923503198317e6d Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 18 Oct 2023 12:58:50 +0200 Subject: [PATCH 07/25] feat: apply review updates --- docs/modules/ROOT/pages/api/account.adoc | 30 ++++++++++++------------ src/account/account.cairo | 5 ++-- src/tests/account/test_account.cairo | 2 +- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index aa76a9e74..4a6863b8c 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -96,8 +96,8 @@ Account component extending xref:ISRC6[`ISRC6`]. .PublicKeyImpl -* xref:#Account-set_public_key[`++set_public_key(self, new_public_key)++`] * xref:#Account-get_public_key[`++get_public_key(self)++`] +* xref:#Account-set_public_key[`++set_public_key(self, new_public_key)++`] -- [.contract-index] @@ -109,8 +109,8 @@ Account component extending xref:ISRC6[`ISRC6`]. .PublicKeyCamelImpl -* xref:#Account-setPublicKey[`++setPublicKey(self, newPublicKey)++`] * xref:#Account-getPublicKey[`++getPublicKey(self)++`] +* xref:#Account-setPublicKey[`++setPublicKey(self, newPublicKey)++`] -- [.contract-index] @@ -137,7 +137,7 @@ Account component extending xref:ISRC6[`ISRC6`]. [.contract-item] [[Account-__execute__]] -==== `[.contract-item-name]#++__execute__++#++(ref self: ContractState, calls: Array) → Array>++` [.item-kind]#external# +==== `[.contract-item-name]#++__execute__++#++(self: @ContractState, calls: Array) → Array>++` [.item-kind]#external# See xref:ISRC6-\\__execute__[ISRC6::\\__execute__]. @@ -176,6 +176,12 @@ See xref:/guides/deployment.adoc[Counterfactual Deployments]. Returns the short string `'VALID'` if valid, otherwise it reverts. +[.contract-item] +[[Account-get_public_key]] +==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → felt252` [.item-kind]#external# + +Returns the current public key of the account. + [.contract-item] [[Account-set_public_key]] ==== `[.contract-item-name]#++set_public_key++#++(ref self: ContractState, new_public_key: felt252)++` [.item-kind]#external# @@ -184,12 +190,6 @@ Sets a new public key for the account. Only accesible by the account calling its Emits both an {OwnerRemoved} and an {OwnerAdded} event. -[.contract-item] -[[Account-get_public_key]] -==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → felt252` [.item-kind]#external# - -Returns the current public key of the account. - [#Account-camelCase-Support] ==== camelCase Support @@ -199,18 +199,18 @@ Returns the current public key of the account. See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. -[.contract-item] -[[Account-setPublicKey]] -==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: felt252)++` [.item-kind]#external# - -See xref:Account-set_public_key[set_public_key]. - [.contract-item] [[Account-getPublicKey]] ==== `[.contract-item-name]#++getPublicKey++#++(self: @ContractState)++ → felt252` [.item-kind]#external# See xref:Account-get_public_key[get_public_key]. +[.contract-item] +[[Account-setPublicKey]] +==== `[.contract-item-name]#++setPublicKey++#++(ref self: ContractState, newPublicKey: felt252)++` [.item-kind]#external# + +See xref:Account-set_public_key[set_public_key]. + [#Account-Internal-Functions] ==== Internal Functions diff --git a/src/account/account.cairo b/src/account/account.cairo index 0cad56c67..c1467ecc2 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -102,6 +102,7 @@ mod Account { } } + /// Adds camelCase support for `ISRC6`. #[embeddable_as(SRC6CamelOnlyImpl)] impl SRC6CamelOnly< TContractState, @@ -109,7 +110,6 @@ mod Account { +SRC5::HasComponent, +Drop > of interface::ISRC6CamelOnly> { - /// Adds camelCase support for `is_valid_signature`. fn isValidSignature( self: @ComponentState, hash: felt252, signature: Array ) -> felt252 { @@ -153,6 +153,7 @@ mod Account { } } + /// Adds camelCase support for `PublicKeyTrait`. #[embeddable_as(PublicKeyCamelImpl)] impl PublicKeyCamel< TContractState, @@ -160,12 +161,10 @@ mod Account { +SRC5::HasComponent, +Drop > of super::PublicKeyCamelTrait> { - /// Adds camelCase support for `get_public_key`. fn getPublicKey(self: @ComponentState) -> felt252 { self.Account_public_key.read() } - /// Adds camelCase support for `set_public_key`. fn setPublicKey(ref self: ComponentState, newPublicKey: felt252) { self.set_public_key(newPublicKey); } diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 70a339469..f32f6eee5 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -87,7 +87,7 @@ fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> IERC20Dispa } // -// supports_interface & supportsInterface +// supports_interface // #[test] From 52df8b158f65b03bc79d27b0a1346818d9e8f3fa Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 18 Oct 2023 14:25:13 +0200 Subject: [PATCH 08/25] feat: update overview --- docs/modules/ROOT/pages/index.adoc | 93 ++++++++++-------------------- 1 file changed, 32 insertions(+), 61 deletions(-) diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc index aa01b689a..312ca9066 100644 --- a/docs/modules/ROOT/pages/index.adoc +++ b/docs/modules/ROOT/pages/index.adoc @@ -61,82 +61,53 @@ WARNING: Make sure the tag matches the target release. == Basic usage -This is how it looks to build an account contract using the xref:accounts.adoc[account module]. +This is how it looks to build an account contract using the xref:accounts.adoc[account] component. Copy the code into `src/lib.cairo`. [,javascript] ---- #[starknet::contract] mod MyAccount { - use openzeppelin::account::Account; - use openzeppelin::account::account::PublicKeyTrait; - use openzeppelin::account::interface; - use openzeppelin::introspection::interface::ISRC5; - use starknet::account::Call; - - // Storage members used by this contract are defined in each imported - // module whose `unsafe_state` is used. This design will be improved - // with the addition of components in the future. - #[storage] - struct Storage {} + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; - #[constructor] - fn constructor(ref self: ContractState, public_key: felt252) { - let mut unsafe_state = _unsafe_state(); - Account::InternalImpl::initializer(ref unsafe_state, public_key); - } + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); - #[external(v0)] - impl SRC6Impl of interface::ISRC6 { - fn __execute__(self: @ContractState, mut calls: Array) -> Array> { - Account::SRC6Impl::__execute__(@_unsafe_state(), calls) - } - - fn __validate__(self: @ContractState, mut calls: Array) -> felt252 { - Account::SRC6Impl::__validate__(@_unsafe_state(), calls) - } - - fn is_valid_signature( - self: @ContractState, hash: felt252, signature: Array - ) -> felt252 { - Account::SRC6Impl::is_valid_signature(@_unsafe_state(), hash, signature) - } - } + /// Account + #[abi(embed_v0)] + impl SRC6Impl = account_component::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl DeclarerImpl = account_component::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = account_component::DeployableImpl; + impl AccountInternalImpl = account_component::InternalImpl; - #[external(v0)] - impl SRC5Impl of ISRC5 { - fn supports_interface(self: @ContractState, interface_id: felt252) -> bool { - Account::SRC5Impl::supports_interface(@_unsafe_state(), interface_id) - } - } + /// SRC5 + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; - #[external(v0)] - impl PublicKeyImpl of PublicKeyTrait { - fn get_public_key(self: @ContractState) -> felt252 { - Account::PublicKeyImpl::get_public_key(@_unsafe_state()) - } - fn set_public_key(ref self: ContractState, new_public_key: felt252) { - let mut unsafe_state = _unsafe_state(); - Account::PublicKeyImpl::set_public_key(ref unsafe_state, new_public_key); - } + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage } - #[external(v0)] - fn __validate_deploy__( - self: @ContractState, - class_hash: felt252, - contract_address_salt: felt252, - _public_key: felt252 - ) -> felt252 { - Account::__validate_deploy__( - @_unsafe_state(), class_hash, contract_address_salt, _public_key - ) + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + AccountEvent: account_component::Event, + SRC5Event: src5_component::Event } - #[inline(always)] - fn _unsafe_state() -> Account::ContractState { - Account::unsafe_new_contract_state() + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); } } ---- From 9705ce7363e56df890959036fc49c87ba4821839 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 23 Oct 2023 12:10:15 +0200 Subject: [PATCH 09/25] 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 4a6863b8c..b5f00247a 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -71,7 +71,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. ```javascript use openzeppelin::account::Account; ``` -Account component extending xref:ISRC6[`ISRC6`]. +Account component implementing xref:ISRC6[`ISRC6`]. [.contract-index] .Embeddable Functions From feeac44c53ebaa0a777b9483ec10ef8a8697d97f Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 23 Oct 2023 12:20:06 +0200 Subject: [PATCH 10/25] feat: apply review updates --- src/account/account.cairo | 64 ++++++++++++++--------------- src/tests/mocks/account_mocks.cairo | 14 +++++++ 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/account/account.cairo b/src/account/account.cairo index 3292b63d5..6a60a5a0f 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -102,32 +102,36 @@ mod Account { } } - /// Adds camelCase support for `ISRC6`. - #[embeddable_as(SRC6CamelOnlyImpl)] - impl SRC6CamelOnly< + #[embeddable_as(DeclarerImpl)] + impl Declarer< TContractState, +HasComponent, +SRC5::HasComponent, +Drop - > of interface::ISRC6CamelOnly> { - fn isValidSignature( - self: @ComponentState, hash: felt252, signature: Array + > 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.is_valid_signature(hash, signature) + self.validate_transaction() } } - #[embeddable_as(DeclarerImpl)] - impl Declarer< + #[embeddable_as(DeployableImpl)] + impl Deployable< TContractState, +HasComponent, +SRC5::HasComponent, +Drop - > of interface::IDeclarer> { + > of interface::IDeployable> { /// 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 + /// 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() } @@ -153,6 +157,21 @@ mod Account { } } + /// Adds camelCase support for `ISRC6`. + #[embeddable_as(SRC6CamelOnlyImpl)] + impl SRC6CamelOnly< + TContractState, + +HasComponent, + +SRC5::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< @@ -170,25 +189,6 @@ mod Account { } } - #[embeddable_as(DeployableImpl)] - impl Deployable< - TContractState, - +HasComponent, - +SRC5::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() - } - } - #[generate_trait] impl InternalImpl< TContractState, diff --git a/src/tests/mocks/account_mocks.cairo b/src/tests/mocks/account_mocks.cairo index 88cfa4b76..067da52e5 100644 --- a/src/tests/mocks/account_mocks.cairo +++ b/src/tests/mocks/account_mocks.cairo @@ -82,6 +82,7 @@ mod SnakeAccountMock { mod CamelAccountMock { use openzeppelin::account::Account as account_component; use openzeppelin::introspection::src5::SRC5 as src5_component; + use starknet::account::Call; component!(path: account_component, storage: account, event: AccountEvent); component!(path: src5_component, storage: src5, event: SRC5Event); @@ -92,6 +93,7 @@ mod CamelAccountMock { impl PublicKeyCamelImpl = account_component::PublicKeyCamelImpl; #[abi(embed_v0)] impl SRC5Impl = src5_component::SRC5Impl; + impl SRC6Impl = account_component::SRC6Impl; impl AccountInternalImpl = account_component::InternalImpl; @@ -114,6 +116,18 @@ mod CamelAccountMock { fn constructor(ref self: ContractState, publicKey: felt252) { self.account.initializer(publicKey); } + + #[external(v0)] + #[generate_trait] + impl ExternalImpl of ExternalTrait { + fn __execute__(self: @ContractState, mut calls: Array) -> Array> { + self.account.__execute__(calls) + } + + fn __validate__(self: @ContractState, mut calls: Array) -> felt252 { + self.account.__validate__(calls) + } + } } // Although these modules are designed to panic, functions From fcdcdf76e2bf5b2c3e55ffd890d8e6d7234454ac Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 24 Oct 2023 13:51:46 +0200 Subject: [PATCH 11/25] feat: flatten events --- docs/modules/ROOT/pages/index.adoc | 2 ++ src/tests/mocks/account_mocks.cairo | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc index a43b4453e..60eaf1ca7 100644 --- a/docs/modules/ROOT/pages/index.adoc +++ b/docs/modules/ROOT/pages/index.adoc @@ -103,7 +103,9 @@ mod MyAccount { #[event] #[derive(Drop, starknet::Event)] enum Event { + #[flat] AccountEvent: account_component::Event, + #[flat] SRC5Event: src5_component::Event } diff --git a/src/tests/mocks/account_mocks.cairo b/src/tests/mocks/account_mocks.cairo index 067da52e5..b407f227b 100644 --- a/src/tests/mocks/account_mocks.cairo +++ b/src/tests/mocks/account_mocks.cairo @@ -30,7 +30,9 @@ mod DualCaseAccountMock { #[event] #[derive(Drop, starknet::Event)] enum Event { + #[flat] AccountEvent: account_component::Event, + #[flat] SRC5Event: src5_component::Event } @@ -68,7 +70,9 @@ mod SnakeAccountMock { #[event] #[derive(Drop, starknet::Event)] enum Event { + #[flat] AccountEvent: account_component::Event, + #[flat] SRC5Event: src5_component::Event } @@ -108,7 +112,9 @@ mod CamelAccountMock { #[event] #[derive(Drop, starknet::Event)] enum Event { + #[flat] AccountEvent: account_component::Event, + #[flat] SRC5Event: src5_component::Event } From d12a6101c6c4e9df3b52310325d4a65e39ed0a0f Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Wed, 25 Oct 2023 13:18:41 +0200 Subject: [PATCH 12/25] feat: add account preset --- .github/workflows/test.yml | 2 +- Scarb.lock | 6 + Scarb.toml | 4 +- src/account.cairo | 3 +- src/account/interface.cairo | 38 +- src/account/presets.cairo | 3 + src/account/presets/account.cairo | 55 +++ src/tests.cairo | 1 + src/tests/account/test_account.cairo | 14 +- src/tests/account/test_dual_account.cairo | 6 +- src/tests/presets.cairo | 1 + src/tests/presets/test_account.cairo | 444 ++++++++++++++++++++++ 12 files changed, 546 insertions(+), 31 deletions(-) create mode 100644 Scarb.lock create mode 100644 src/account/presets.cairo create mode 100644 src/account/presets/account.cairo create mode 100644 src/tests/presets.cairo create mode 100644 src/tests/presets/test_account.cairo diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c4b10d16c..9b25e869d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: - uses: actions/checkout@v3 - uses: software-mansion/setup-scarb@v1 with: - scarb-version: "2.3.0-rc1" + scarb-version: "2.3.0" - name: Markdown lint uses: DavidAnson/markdownlint-cli2-action@5b7c9f74fec47e6b15667b2cc23c63dff11e449e # v9 with: diff --git a/Scarb.lock b/Scarb.lock new file mode 100644 index 000000000..a34bfaf41 --- /dev/null +++ b/Scarb.lock @@ -0,0 +1,6 @@ +# Code generated by scarb DO NOT EDIT. +version = 1 + +[[package]] +name = "openzeppelin" +version = "0.7.0" diff --git a/Scarb.toml b/Scarb.toml index 3c0ae199b..48dc53cac 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -1,7 +1,7 @@ [package] name = "openzeppelin" version = "0.7.0" -cairo-version = "2.3.0-rc0" +cairo-version = "2.3.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 +11,7 @@ license-file = "LICENSE" keywords = ["openzeppelin", "starknet", "cairo", "contracts", "security", "standards"] [dependencies] -starknet = "=2.3.0-rc0" +starknet = "2.3.0" [lib] diff --git a/src/account.cairo b/src/account.cairo index 501227b0b..524987bc4 100644 --- a/src/account.cairo +++ b/src/account.cairo @@ -1,9 +1,8 @@ mod account; mod dual_account; mod interface; +mod presets; use account::Account; use interface::AccountABIDispatcher; use interface::AccountABIDispatcherTrait; -use interface::AccountCamelABIDispatcher; -use interface::AccountCamelABIDispatcherTrait; diff --git a/src/account/interface.cairo b/src/account/interface.cairo index 5955dc638..afd8a8829 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -30,31 +30,39 @@ trait IDeployable { ) -> felt252; } +// +// Account ABI +// + #[starknet::interface] trait AccountABI { + // 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; + + // DeployerTrait fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252 + self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 ) -> felt252; + + // PublicKeyTrait fn set_public_key(ref self: TState, new_public_key: felt252); fn get_public_key(self: @TState) -> felt252; - fn is_valid_signature(self: @TState, hash: felt252, signature: Array) -> felt252; - fn supports_interface(self: @TState, interface_id: felt252) -> bool; -} -// Entry points case-convention is enforced by the protocol -#[starknet::interface] -trait AccountCamelABI { - fn __execute__(self: @TState, calls: Array) -> Array>; - fn __validate__(self: @TState, calls: Array) -> felt252; - fn __validate_declare__(self: @TState, classHash: felt252) -> felt252; - fn __validate_deploy__( - self: @TState, classHash: felt252, contractAddressSalt: felt252, publicKey: felt252 - ) -> felt252; - fn setPublicKey(ref self: TState, newPublicKey: felt252); - fn getPublicKey(self: @TState) -> felt252; + // ISRC6CamelOnly fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; + + // ISRC5Camel fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; + + // PublicKeyCamelTrait + fn setPublicKey(ref self: TState, newPublicKey: felt252); + fn getPublicKey(self: @TState) -> felt252; } diff --git a/src/account/presets.cairo b/src/account/presets.cairo new file mode 100644 index 000000000..ba3526921 --- /dev/null +++ b/src/account/presets.cairo @@ -0,0 +1,3 @@ +mod account; + +use account::Account; diff --git a/src/account/presets/account.cairo b/src/account/presets/account.cairo new file mode 100644 index 000000000..941ba6cdd --- /dev/null +++ b/src/account/presets/account.cairo @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.7.0 (account/presets/account.cairo) + +/// # Account Preset +/// +/// Openzeppelin's account contract. +#[starknet::contract] +mod Account { + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; + + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6Impl = account_component::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyImpl = account_component::PublicKeyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = account_component::PublicKeyCamelImpl; + #[abi(embed_v0)] + impl DeclarerImpl = account_component::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = account_component::DeployableImpl; + impl AccountInternalImpl = account_component::InternalImpl; + + // SRC5 + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; + + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + AccountEvent: account_component::Event, + #[flat] + SRC5Event: src5_component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} diff --git a/src/tests.cairo b/src/tests.cairo index 7145f5650..d8b09324d 100644 --- a/src/tests.cairo +++ b/src/tests.cairo @@ -2,6 +2,7 @@ mod access; mod account; mod introspection; mod mocks; +mod presets; mod security; mod token; mod upgrades; diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index f32f6eee5..58c1bf1e2 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -1,4 +1,4 @@ -use openzeppelin::account::Account::{InternalTrait, OwnerAdded, OwnerRemoved}; +use openzeppelin::account::Account::{InternalTrait, OwnerAdded, OwnerRemoved, SRC6CamelOnlyImpl}; use openzeppelin::account::Account::{PublicKeyCamelImpl, PublicKeyImpl}; use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::Account; @@ -48,10 +48,10 @@ fn ACCOUNT_ADDRESS() -> ContractAddress { fn SIGNED_TX_DATA() -> SignedTransactionData { SignedTransactionData { private_key: 1234, - public_key: 883045738439352841478194533192765345509759306772397516907181243450667673002, - transaction_hash: 2717105892474786771566982177444710571376803476229898722748888396642649184538, - r: 3068558690657879390136740086327753007413919701043650133111397282816679110801, - s: 3355728545224320878895493649495491771252432631648740019139167265522817576501 + public_key: 0x1f3c942d7f492a37608cde0d77b884a5aa9e11d2919225968557370ddb5a5aa, + transaction_hash: 0x601d3d2e265c10ff645e1554c435e72ce6721f0ba5fc96f0c650bfc6231191a, + r: 0x6c8be1fb0fb5c730fbd7abaecbed9d980376ff2e660dfcd157e158d2b026891, + s: 0x76b4669998eb933f44a59eace12b41328ab975ceafddf92602b21eb23e22e35 } } @@ -137,10 +137,10 @@ fn test_isValidSignature() { state.account.set_public_key(data.public_key); - let is_valid = Account::SRC6CamelOnlyImpl::isValidSignature(@state, hash, good_signature); + let is_valid = state.account.isValidSignature(hash, good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); - let is_valid = Account::SRC6CamelOnlyImpl::isValidSignature(@state, hash, bad_signature); + let is_valid = state.account.isValidSignature(hash, bad_signature); assert(is_valid == 0, 'Should reject invalid signature'); } diff --git a/src/tests/account/test_dual_account.cairo b/src/tests/account/test_dual_account.cairo index e41f7e47c..ef7ee88c5 100644 --- a/src/tests/account/test_dual_account.cairo +++ b/src/tests/account/test_dual_account.cairo @@ -1,6 +1,5 @@ use openzeppelin::account::dual_account::{DualCaseAccountABI, DualCaseAccount}; use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; -use openzeppelin::account::{AccountCamelABIDispatcherTrait, AccountCamelABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::account::test_account::SIGNED_TX_DATA; use openzeppelin::tests::mocks::account_mocks::{ @@ -24,12 +23,12 @@ fn setup_snake() -> (DualCaseAccount, AccountABIDispatcher) { ) } -fn setup_camel() -> (DualCaseAccount, AccountCamelABIDispatcher) { +fn setup_camel() -> (DualCaseAccount, AccountABIDispatcher) { let mut calldata = array![PUBKEY]; let target = utils::deploy(CamelAccountMock::TEST_CLASS_HASH, calldata); ( DualCaseAccount { contract_address: target }, - AccountCamelABIDispatcher { contract_address: target } + AccountABIDispatcher { contract_address: target } ) } @@ -242,4 +241,3 @@ fn test_dual_supportsInterface_exists_and_panics() { let (_, dispatcher) = setup_account_panic(); dispatcher.supports_interface(ISRC5_ID); } - diff --git a/src/tests/presets.cairo b/src/tests/presets.cairo new file mode 100644 index 000000000..2c61e9dca --- /dev/null +++ b/src/tests/presets.cairo @@ -0,0 +1 @@ +mod test_account; diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo new file mode 100644 index 000000000..d5447be2b --- /dev/null +++ b/src/tests/presets/test_account.cairo @@ -0,0 +1,444 @@ +use openzeppelin::account::Account::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::interface::ISRC6_ID; +use openzeppelin::account::presets::Account; +use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; +use openzeppelin::introspection::interface::ISRC5_ID; +use openzeppelin::tests::account::test_account::{ + deploy_erc20, SIGNED_TX_DATA, SignedTransactionData, TRANSACTION_VERSION, QUERY_VERSION +}; +use openzeppelin::tests::utils::constants::{PUBKEY, NEW_PUBKEY, SALT, ZERO, RECIPIENT}; +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 { + Account::TEST_CLASS_HASH +} + +// +// Setup +// + +fn setup_dispatcher() -> AccountABIDispatcher { + let mut calldata = array![PUBKEY]; + let target = utils::deploy(CLASS_HASH(), calldata); + utils::drop_event(target); + + AccountABIDispatcher { contract_address: target } +} + +fn setup_dispatcher_with_data(data: Option<@SignedTransactionData>) -> AccountABIDispatcher { + testing::set_version(TRANSACTION_VERSION); + + let mut calldata = array![]; + if data.is_some() { + let data = data.unwrap(); + testing::set_signature(array![*data.r, *data.s].span()); + testing::set_transaction_hash(*data.transaction_hash); + + calldata.append(*data.public_key); + } else { + calldata.append(PUBKEY); + } + let address = utils::deploy(CLASS_HASH(), calldata); + AccountABIDispatcher { contract_address: address } +} + +// +// constructor +// + +#[test] +#[available_gas(2000000)] +fn test_constructor() { + let mut state = Account::contract_state_for_testing(); + Account::constructor(ref state, PUBKEY); + + assert_only_event_owner_added(PUBKEY, ZERO()); + + assert(Account::PublicKeyImpl::get_public_key(@state) == PUBKEY, 'Should return PUBKEY'); + assert(Account::SRC5Impl::supports_interface(@state, ISRC5_ID), 'Should implement ISRC5'); + assert(Account::SRC5Impl::supports_interface(@state, ISRC6_ID), 'Should implement ISRC6'); +} + +// +// set_public_key & setPublicKey +// + +#[test] +#[available_gas(2000000)] +fn test_public_key_setter_and_getter() { + let dispatcher = setup_dispatcher(); + + testing::set_contract_address(dispatcher.contract_address); + + 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); +} + +#[test] +#[available_gas(2000000)] +fn test_public_key_setter_and_getter_camel() { + let dispatcher = setup_dispatcher(); + + testing::set_contract_address(dispatcher.contract_address); + + 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); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_set_public_key_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.set_public_key(NEW_PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_setPublicKey_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.setPublicKey(NEW_PUBKEY); +} + +// +// is_valid_signature & isValidSignature +// + +fn is_valid_sig_dispatcher() -> (AccountABIDispatcher, felt252, Array) { + let dispatcher = setup_dispatcher(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut signature = array![data.r, data.s]; + + testing::set_contract_address(dispatcher.contract_address); + dispatcher.set_public_key(data.public_key); + + (dispatcher, hash, signature) +} + +#[test] +#[available_gas(2000000)] +fn test_is_valid_signature_bad_sig() { + let (dispatcher, hash, _) = is_valid_sig_dispatcher(); + + let bad_signature = array![0x987, 0x564]; + + let is_valid = dispatcher.is_valid_signature(hash, bad_signature.clone()); + assert(is_valid == 0, 'Should reject invalid signature'); + + let is_valid = dispatcher.isValidSignature(hash, bad_signature); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +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'); +} + +// +// supports_interface +// + +#[test] +#[available_gas(2000000)] +fn test_supports_interface() { + let dispatcher = setup_dispatcher(); + assert(dispatcher.supports_interface(ISRC5_ID), 'Should implement ISRC5'); + assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); +} + +// +// Entry points +// + +#[test] +#[available_gas(2000000)] +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, PUBKEY) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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); + + // Craft call and add to calls array + 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); + + // 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] +#[available_gas(2000000)] +fn test_execute() { + test_execute_with_version(Option::None(())); +} + +#[test] +#[available_gas(2000000)] +fn test_execute_query_version() { + test_execute_with_version(Option::Some(QUERY_VERSION)); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid tx version', 'ENTRYPOINT_FAILED'))] +fn test_execute_invalid_version() { + test_execute_with_version(Option::Some(TRANSACTION_VERSION - 1)); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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] +#[available_gas(20000000)] +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![]; + + // 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] +#[available_gas(2000000)] +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); + + // Test return value + assert(ret.len() == 0, 'Should have an empty response'); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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); +} + +// +// Helpers +// + +fn assert_event_owner_removed(removed_owner_guid: felt252, contract: ContractAddress) { + 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) { + 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); + utils::assert_no_events_left(contract); +} From 66c3fd7417171a3df1adeaa5a4c70c4ee012c0d7 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 30 Oct 2023 14:17:40 +0100 Subject: [PATCH 13/25] feat: apply review updates --- src/account.cairo | 1 - src/account/account.cairo | 4 ++-- src/account/interface.cairo | 4 ++-- src/lib.cairo | 1 + src/{account => }/presets.cairo | 0 src/{account => }/presets/account.cairo | 0 src/tests/presets/test_account.cairo | 32 ++++++++++++++++--------- 7 files changed, 26 insertions(+), 16 deletions(-) rename src/{account => }/presets.cairo (100%) rename src/{account => }/presets/account.cairo (100%) diff --git a/src/account.cairo b/src/account.cairo index 524987bc4..74a58229d 100644 --- a/src/account.cairo +++ b/src/account.cairo @@ -1,7 +1,6 @@ mod account; mod dual_account; mod interface; -mod presets; use account::Account; use interface::AccountABIDispatcher; diff --git a/src/account/account.cairo b/src/account/account.cairo index 6a60a5a0f..03bda2a5b 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -2,13 +2,13 @@ // OpenZeppelin Contracts for Cairo v0.7.0 (account/account.cairo) trait PublicKeyTrait { - fn set_public_key(ref self: TState, new_public_key: felt252); fn get_public_key(self: @TState) -> felt252; + fn set_public_key(ref self: TState, new_public_key: felt252); } trait PublicKeyCamelTrait { - fn setPublicKey(ref self: TState, newPublicKey: felt252); fn getPublicKey(self: @TState) -> felt252; + fn setPublicKey(ref self: TState, newPublicKey: felt252); } /// # Account Component diff --git a/src/account/interface.cairo b/src/account/interface.cairo index afd8a8829..789b02646 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -53,8 +53,8 @@ trait AccountABI { ) -> felt252; // PublicKeyTrait - fn set_public_key(ref self: TState, new_public_key: felt252); fn get_public_key(self: @TState) -> felt252; + fn set_public_key(ref self: TState, new_public_key: felt252); // ISRC6CamelOnly fn isValidSignature(self: @TState, hash: felt252, signature: Array) -> felt252; @@ -63,6 +63,6 @@ trait AccountABI { fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; // PublicKeyCamelTrait - fn setPublicKey(ref self: TState, newPublicKey: felt252); fn getPublicKey(self: @TState) -> felt252; + fn setPublicKey(ref self: TState, newPublicKey: felt252); } diff --git a/src/lib.cairo b/src/lib.cairo index 71de24d54..a87b719b8 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -1,6 +1,7 @@ mod access; mod account; mod introspection; +mod presets; mod security; #[cfg(test)] mod tests; diff --git a/src/account/presets.cairo b/src/presets.cairo similarity index 100% rename from src/account/presets.cairo rename to src/presets.cairo diff --git a/src/account/presets/account.cairo b/src/presets/account.cairo similarity index 100% rename from src/account/presets/account.cairo rename to src/presets/account.cairo diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index d5447be2b..c5b8ba8e5 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -1,10 +1,11 @@ use openzeppelin::account::Account::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::interface::ISRC6_ID; -use openzeppelin::account::presets::Account; use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; use openzeppelin::introspection::interface::ISRC5_ID; +use openzeppelin::presets::Account; use openzeppelin::tests::account::test_account::{ - deploy_erc20, SIGNED_TX_DATA, SignedTransactionData, TRANSACTION_VERSION, QUERY_VERSION + deploy_erc20, SIGNED_TX_DATA, SignedTransactionData }; use openzeppelin::tests::utils::constants::{PUBKEY, NEW_PUBKEY, SALT, ZERO, RECIPIENT}; use openzeppelin::tests::utils; @@ -131,6 +132,15 @@ fn is_valid_sig_dispatcher() -> (AccountABIDispatcher, felt252, Array) (dispatcher, hash, signature) } +#[test] +#[available_gas(2000000)] +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] #[available_gas(2000000)] fn test_is_valid_signature_bad_sig() { @@ -140,27 +150,26 @@ fn test_is_valid_signature_bad_sig() { let is_valid = dispatcher.is_valid_signature(hash, bad_signature.clone()); assert(is_valid == 0, 'Should reject invalid signature'); - - let is_valid = dispatcher.isValidSignature(hash, bad_signature); - assert(is_valid == 0, 'Should reject invalid signature'); } #[test] #[available_gas(2000000)] -fn test_is_valid_signature() { +fn test_isValidSignature() { let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); - let is_valid = dispatcher.is_valid_signature(hash, signature); + let is_valid = dispatcher.isValidSignature(hash, signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); } #[test] #[available_gas(2000000)] -fn test_isValidSignature() { - let (dispatcher, hash, signature) = is_valid_sig_dispatcher(); +fn test_isValidSignature_bad_sig() { + let (dispatcher, hash, _) = is_valid_sig_dispatcher(); - let is_valid = dispatcher.isValidSignature(hash, signature); - assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); + let bad_signature = array![0x987, 0x564]; + + let is_valid = dispatcher.isValidSignature(hash, bad_signature); + assert(is_valid == 0, 'Should reject invalid signature'); } // @@ -172,6 +181,7 @@ fn test_isValidSignature() { 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 ISRC5'); assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); } From 495ff94df2194fa729ca5aa00cbc1daca78d24e6 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 30 Oct 2023 14:35:02 +0100 Subject: [PATCH 14/25] feat: apply review updates --- docs/modules/ROOT/pages/api/account.adoc | 18 +++++------------- docs/modules/ROOT/pages/guides/deployment.adoc | 2 +- src/account/interface.cairo | 8 ++++---- src/tests/account/test_account.cairo | 3 ++- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index e4dae14a3..155161618 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -73,6 +73,8 @@ use openzeppelin::account::Account; ``` Account component implementing xref:ISRC6[`ISRC6`]. +NOTE: Implementing xref:api/introspection.adoc#SRC5[SRC5] is a requirement for this component to be implemented. + [.contract-index] .Embeddable Functions -- @@ -82,10 +84,6 @@ Account component implementing xref:ISRC6[`ISRC6`]. * xref:#Account-\\__validate__[`++__validate__(self, calls)++`] * xref:#Account-is_valid_signature[`++is_valid_signature(self, hash, signature)++`] -.SRC5Impl - -* xref:#Account-supports_interface[`++supports_interface(self, interface_id)++`] - .DeclarerImpl * xref:#Account-\\__validate_declare__[`++__validate_declare__(self, class_hash)++`] @@ -118,7 +116,7 @@ Account component implementing xref:ISRC6[`ISRC6`]. -- .InternalImpl -* xref:#Account-initializer[`++initializer(self, _public_key)++`] +* xref:#Account-initializer[`++initializer(self, public_key)++`] * xref:#Account-assert_only_self[`++assert_only_self(self)++`] * xref:#Account-validate_transaction[`++validate_transaction(self)++`] * xref:#Account-_set_public_key[`++_set_public_key(self, new_public_key)++`] @@ -153,12 +151,6 @@ See xref:ISRC6-\\__validate__[ISRC6::\\__validate__]. See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. -[.contract-item] -[[Account-supports_interface]] -==== `[.contract-item-name]#++supports_interface++#++(self: @ContractState, interface_id: felt252) → bool++` [.item-kind]#external# - -See xref:api/introspection.adoc#ISRC5-supports_interface[ISRC5::supports_interface]. - [.contract-item] [[Account-__validate_declare__]] ==== `[.contract-item-name]#++__validate_declare__++#++(self: @ContractState, class_hash: felt252) → felt252++` [.item-kind]#external# @@ -169,7 +161,7 @@ Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] [[Account-__validate_deploy__]] -==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252) → felt252++` [.item-kind]#external# +==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252) → 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]. @@ -216,7 +208,7 @@ See xref:Account-set_public_key[set_public_key]. [.contract-item] [[Account-initializer]] -==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, _public_key: felt252)++` [.item-kind]#internal# +==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: felt252)++` [.item-kind]#internal# Initializes the account with the given public key, and registers the ISRC6 interface ID. diff --git a/docs/modules/ROOT/pages/guides/deployment.adoc b/docs/modules/ROOT/pages/guides/deployment.adoc index 92f6a1bd2..68a37daf2 100644 --- a/docs/modules/ROOT/pages/guides/deployment.adoc +++ b/docs/modules/ROOT/pages/guides/deployment.adoc @@ -36,7 +36,7 @@ called by the protocol when a `DeployAccount` transaction is sent to the network trait IDeployable { /// Must return 'VALID' when the validation is successful. fn __validate_deploy__( - class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 + class_hash: felt252, contract_address_salt: felt252, public_key: felt252 ) -> felt252; } ---- diff --git a/src/account/interface.cairo b/src/account/interface.cairo index db7d3a475..8f7c4b992 100644 --- a/src/account/interface.cairo +++ b/src/account/interface.cairo @@ -47,12 +47,12 @@ trait AccountABI { // IDeclarer fn __validate_declare__(self: @TState, class_hash: felt252) -> felt252; - // DeployerTrait + // IDeployable fn __validate_deploy__( - self: @TState, class_hash: felt252, contract_address_salt: felt252, _public_key: felt252 + self: @TState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252 ) -> felt252; - // PublicKeyTrait + // IPublicKey fn get_public_key(self: @TState) -> felt252; fn set_public_key(ref self: TState, new_public_key: felt252); @@ -62,7 +62,7 @@ trait AccountABI { // ISRC5Camel fn supportsInterface(self: @TState, interfaceId: felt252) -> bool; - // PublicKeyCamelTrait + // IPublicKeyCamel fn getPublicKey(self: @TState) -> felt252; fn setPublicKey(ref self: TState, newPublicKey: felt252); } diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 58c1bf1e2..c5599671f 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -1,4 +1,5 @@ -use openzeppelin::account::Account::{InternalTrait, OwnerAdded, OwnerRemoved, SRC6CamelOnlyImpl}; +use openzeppelin::account::Account::{InternalTrait, SRC6CamelOnlyImpl}; +use openzeppelin::account::Account::{OwnerAdded, OwnerRemoved}; use openzeppelin::account::Account::{PublicKeyCamelImpl, PublicKeyImpl}; use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; use openzeppelin::account::Account; From 472b04c197b3d7ec36cf42fd4fa086ad22ae0d10 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 30 Oct 2023 14:39:57 +0100 Subject: [PATCH 15/25] feat: remove preset --- src/lib.cairo | 1 - src/presets.cairo | 3 - src/presets/account.cairo | 55 ---- src/tests.cairo | 1 - src/tests/presets.cairo | 1 - src/tests/presets/test_account.cairo | 454 --------------------------- 6 files changed, 515 deletions(-) delete mode 100644 src/presets.cairo delete mode 100644 src/presets/account.cairo delete mode 100644 src/tests/presets.cairo delete mode 100644 src/tests/presets/test_account.cairo diff --git a/src/lib.cairo b/src/lib.cairo index a87b719b8..71de24d54 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -1,7 +1,6 @@ mod access; mod account; mod introspection; -mod presets; mod security; #[cfg(test)] mod tests; diff --git a/src/presets.cairo b/src/presets.cairo deleted file mode 100644 index ba3526921..000000000 --- a/src/presets.cairo +++ /dev/null @@ -1,3 +0,0 @@ -mod account; - -use account::Account; diff --git a/src/presets/account.cairo b/src/presets/account.cairo deleted file mode 100644 index 941ba6cdd..000000000 --- a/src/presets/account.cairo +++ /dev/null @@ -1,55 +0,0 @@ -// SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.7.0 (account/presets/account.cairo) - -/// # Account Preset -/// -/// Openzeppelin's account contract. -#[starknet::contract] -mod Account { - use openzeppelin::account::Account as account_component; - use openzeppelin::introspection::src5::SRC5 as src5_component; - - component!(path: account_component, storage: account, event: AccountEvent); - component!(path: src5_component, storage: src5, event: SRC5Event); - - // Account - #[abi(embed_v0)] - impl SRC6Impl = account_component::SRC6Impl; - #[abi(embed_v0)] - impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; - #[abi(embed_v0)] - impl PublicKeyImpl = account_component::PublicKeyImpl; - #[abi(embed_v0)] - impl PublicKeyCamelImpl = account_component::PublicKeyCamelImpl; - #[abi(embed_v0)] - impl DeclarerImpl = account_component::DeclarerImpl; - #[abi(embed_v0)] - impl DeployableImpl = account_component::DeployableImpl; - impl AccountInternalImpl = account_component::InternalImpl; - - // SRC5 - #[abi(embed_v0)] - impl SRC5Impl = src5_component::SRC5Impl; - - #[storage] - struct Storage { - #[substorage(v0)] - account: account_component::Storage, - #[substorage(v0)] - src5: src5_component::Storage - } - - #[event] - #[derive(Drop, starknet::Event)] - enum Event { - #[flat] - AccountEvent: account_component::Event, - #[flat] - SRC5Event: src5_component::Event - } - - #[constructor] - fn constructor(ref self: ContractState, public_key: felt252) { - self.account.initializer(public_key); - } -} diff --git a/src/tests.cairo b/src/tests.cairo index d8b09324d..7145f5650 100644 --- a/src/tests.cairo +++ b/src/tests.cairo @@ -2,7 +2,6 @@ mod access; mod account; mod introspection; mod mocks; -mod presets; mod security; mod token; mod upgrades; diff --git a/src/tests/presets.cairo b/src/tests/presets.cairo deleted file mode 100644 index 2c61e9dca..000000000 --- a/src/tests/presets.cairo +++ /dev/null @@ -1 +0,0 @@ -mod test_account; diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo deleted file mode 100644 index c5b8ba8e5..000000000 --- a/src/tests/presets/test_account.cairo +++ /dev/null @@ -1,454 +0,0 @@ -use openzeppelin::account::Account::{OwnerAdded, OwnerRemoved}; -use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; -use openzeppelin::account::interface::ISRC6_ID; -use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; -use openzeppelin::introspection::interface::ISRC5_ID; -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; -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 { - Account::TEST_CLASS_HASH -} - -// -// Setup -// - -fn setup_dispatcher() -> AccountABIDispatcher { - let mut calldata = array![PUBKEY]; - let target = utils::deploy(CLASS_HASH(), calldata); - utils::drop_event(target); - - AccountABIDispatcher { contract_address: target } -} - -fn setup_dispatcher_with_data(data: Option<@SignedTransactionData>) -> AccountABIDispatcher { - testing::set_version(TRANSACTION_VERSION); - - let mut calldata = array![]; - if data.is_some() { - let data = data.unwrap(); - testing::set_signature(array![*data.r, *data.s].span()); - testing::set_transaction_hash(*data.transaction_hash); - - calldata.append(*data.public_key); - } else { - calldata.append(PUBKEY); - } - let address = utils::deploy(CLASS_HASH(), calldata); - AccountABIDispatcher { contract_address: address } -} - -// -// constructor -// - -#[test] -#[available_gas(2000000)] -fn test_constructor() { - let mut state = Account::contract_state_for_testing(); - Account::constructor(ref state, PUBKEY); - - assert_only_event_owner_added(PUBKEY, ZERO()); - - assert(Account::PublicKeyImpl::get_public_key(@state) == PUBKEY, 'Should return PUBKEY'); - assert(Account::SRC5Impl::supports_interface(@state, ISRC5_ID), 'Should implement ISRC5'); - assert(Account::SRC5Impl::supports_interface(@state, ISRC6_ID), 'Should implement ISRC6'); -} - -// -// set_public_key & setPublicKey -// - -#[test] -#[available_gas(2000000)] -fn test_public_key_setter_and_getter() { - let dispatcher = setup_dispatcher(); - - testing::set_contract_address(dispatcher.contract_address); - - 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); -} - -#[test] -#[available_gas(2000000)] -fn test_public_key_setter_and_getter_camel() { - let dispatcher = setup_dispatcher(); - - testing::set_contract_address(dispatcher.contract_address); - - 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); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] -fn test_set_public_key_different_account() { - let dispatcher = setup_dispatcher(); - dispatcher.set_public_key(NEW_PUBKEY); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] -fn test_setPublicKey_different_account() { - let dispatcher = setup_dispatcher(); - dispatcher.setPublicKey(NEW_PUBKEY); -} - -// -// is_valid_signature & isValidSignature -// - -fn is_valid_sig_dispatcher() -> (AccountABIDispatcher, felt252, Array) { - let dispatcher = setup_dispatcher(); - - let data = SIGNED_TX_DATA(); - let hash = data.transaction_hash; - let mut signature = array![data.r, data.s]; - - testing::set_contract_address(dispatcher.contract_address); - dispatcher.set_public_key(data.public_key); - - (dispatcher, hash, signature) -} - -#[test] -#[available_gas(2000000)] -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] -#[available_gas(2000000)] -fn test_is_valid_signature_bad_sig() { - let (dispatcher, hash, _) = is_valid_sig_dispatcher(); - - let bad_signature = array![0x987, 0x564]; - - let is_valid = dispatcher.is_valid_signature(hash, bad_signature.clone()); - assert(is_valid == 0, 'Should reject invalid signature'); -} - -#[test] -#[available_gas(2000000)] -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] -#[available_gas(2000000)] -fn test_isValidSignature_bad_sig() { - let (dispatcher, hash, _) = is_valid_sig_dispatcher(); - - let bad_signature = array![0x987, 0x564]; - - let is_valid = dispatcher.isValidSignature(hash, bad_signature); - assert(is_valid == 0, 'Should reject invalid signature'); -} - -// -// supports_interface -// - -#[test] -#[available_gas(2000000)] -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 ISRC5'); - assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); -} - -// -// Entry points -// - -#[test] -#[available_gas(2000000)] -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, PUBKEY) == starknet::VALIDATED, - 'Should validate correctly' - ); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: 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, PUBKEY); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); -} - -#[test] -#[available_gas(2000000)] -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] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: 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] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: invalid signature', '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] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: invalid signature', '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); - - // Craft call and add to calls array - 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); - - // 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] -#[available_gas(2000000)] -fn test_execute() { - test_execute_with_version(Option::None(())); -} - -#[test] -#[available_gas(2000000)] -fn test_execute_query_version() { - test_execute_with_version(Option::Some(QUERY_VERSION)); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: invalid tx version', 'ENTRYPOINT_FAILED'))] -fn test_execute_invalid_version() { - test_execute_with_version(Option::Some(TRANSACTION_VERSION - 1)); -} - -#[test] -#[available_gas(2000000)] -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] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: 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] -#[available_gas(20000000)] -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![]; - - // 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] -#[available_gas(2000000)] -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); - - // Test return value - assert(ret.len() == 0, 'Should have an empty response'); -} - -#[test] -#[available_gas(2000000)] -#[should_panic(expected: ('Account: 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); -} - -// -// Helpers -// - -fn assert_event_owner_removed(removed_owner_guid: felt252, contract: ContractAddress) { - 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) { - 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); - utils::assert_no_events_left(contract); -} From ce3ef470785ed4b3ba2080b41345c1a47fc03fa2 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Mon, 30 Oct 2023 14:41:39 +0100 Subject: [PATCH 16/25] Revert "feat: remove preset" This reverts commit 472b04c197b3d7ec36cf42fd4fa086ad22ae0d10. --- src/lib.cairo | 1 + src/presets.cairo | 3 + src/presets/account.cairo | 55 ++++ src/tests.cairo | 1 + src/tests/presets.cairo | 1 + src/tests/presets/test_account.cairo | 454 +++++++++++++++++++++++++++ 6 files changed, 515 insertions(+) create mode 100644 src/presets.cairo create mode 100644 src/presets/account.cairo create mode 100644 src/tests/presets.cairo create mode 100644 src/tests/presets/test_account.cairo diff --git a/src/lib.cairo b/src/lib.cairo index 71de24d54..a87b719b8 100644 --- a/src/lib.cairo +++ b/src/lib.cairo @@ -1,6 +1,7 @@ mod access; mod account; mod introspection; +mod presets; mod security; #[cfg(test)] mod tests; diff --git a/src/presets.cairo b/src/presets.cairo new file mode 100644 index 000000000..ba3526921 --- /dev/null +++ b/src/presets.cairo @@ -0,0 +1,3 @@ +mod account; + +use account::Account; diff --git a/src/presets/account.cairo b/src/presets/account.cairo new file mode 100644 index 000000000..941ba6cdd --- /dev/null +++ b/src/presets/account.cairo @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts for Cairo v0.7.0 (account/presets/account.cairo) + +/// # Account Preset +/// +/// Openzeppelin's account contract. +#[starknet::contract] +mod Account { + use openzeppelin::account::Account as account_component; + use openzeppelin::introspection::src5::SRC5 as src5_component; + + component!(path: account_component, storage: account, event: AccountEvent); + component!(path: src5_component, storage: src5, event: SRC5Event); + + // Account + #[abi(embed_v0)] + impl SRC6Impl = account_component::SRC6Impl; + #[abi(embed_v0)] + impl SRC6CamelOnlyImpl = account_component::SRC6CamelOnlyImpl; + #[abi(embed_v0)] + impl PublicKeyImpl = account_component::PublicKeyImpl; + #[abi(embed_v0)] + impl PublicKeyCamelImpl = account_component::PublicKeyCamelImpl; + #[abi(embed_v0)] + impl DeclarerImpl = account_component::DeclarerImpl; + #[abi(embed_v0)] + impl DeployableImpl = account_component::DeployableImpl; + impl AccountInternalImpl = account_component::InternalImpl; + + // SRC5 + #[abi(embed_v0)] + impl SRC5Impl = src5_component::SRC5Impl; + + #[storage] + struct Storage { + #[substorage(v0)] + account: account_component::Storage, + #[substorage(v0)] + src5: src5_component::Storage + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + AccountEvent: account_component::Event, + #[flat] + SRC5Event: src5_component::Event + } + + #[constructor] + fn constructor(ref self: ContractState, public_key: felt252) { + self.account.initializer(public_key); + } +} diff --git a/src/tests.cairo b/src/tests.cairo index 7145f5650..d8b09324d 100644 --- a/src/tests.cairo +++ b/src/tests.cairo @@ -2,6 +2,7 @@ mod access; mod account; mod introspection; mod mocks; +mod presets; mod security; mod token; mod upgrades; diff --git a/src/tests/presets.cairo b/src/tests/presets.cairo new file mode 100644 index 000000000..2c61e9dca --- /dev/null +++ b/src/tests/presets.cairo @@ -0,0 +1 @@ +mod test_account; diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo new file mode 100644 index 000000000..c5b8ba8e5 --- /dev/null +++ b/src/tests/presets/test_account.cairo @@ -0,0 +1,454 @@ +use openzeppelin::account::Account::{OwnerAdded, OwnerRemoved}; +use openzeppelin::account::Account::{TRANSACTION_VERSION, QUERY_VERSION}; +use openzeppelin::account::interface::ISRC6_ID; +use openzeppelin::account::{AccountABIDispatcherTrait, AccountABIDispatcher}; +use openzeppelin::introspection::interface::ISRC5_ID; +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; +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 { + Account::TEST_CLASS_HASH +} + +// +// Setup +// + +fn setup_dispatcher() -> AccountABIDispatcher { + let mut calldata = array![PUBKEY]; + let target = utils::deploy(CLASS_HASH(), calldata); + utils::drop_event(target); + + AccountABIDispatcher { contract_address: target } +} + +fn setup_dispatcher_with_data(data: Option<@SignedTransactionData>) -> AccountABIDispatcher { + testing::set_version(TRANSACTION_VERSION); + + let mut calldata = array![]; + if data.is_some() { + let data = data.unwrap(); + testing::set_signature(array![*data.r, *data.s].span()); + testing::set_transaction_hash(*data.transaction_hash); + + calldata.append(*data.public_key); + } else { + calldata.append(PUBKEY); + } + let address = utils::deploy(CLASS_HASH(), calldata); + AccountABIDispatcher { contract_address: address } +} + +// +// constructor +// + +#[test] +#[available_gas(2000000)] +fn test_constructor() { + let mut state = Account::contract_state_for_testing(); + Account::constructor(ref state, PUBKEY); + + assert_only_event_owner_added(PUBKEY, ZERO()); + + assert(Account::PublicKeyImpl::get_public_key(@state) == PUBKEY, 'Should return PUBKEY'); + assert(Account::SRC5Impl::supports_interface(@state, ISRC5_ID), 'Should implement ISRC5'); + assert(Account::SRC5Impl::supports_interface(@state, ISRC6_ID), 'Should implement ISRC6'); +} + +// +// set_public_key & setPublicKey +// + +#[test] +#[available_gas(2000000)] +fn test_public_key_setter_and_getter() { + let dispatcher = setup_dispatcher(); + + testing::set_contract_address(dispatcher.contract_address); + + 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); +} + +#[test] +#[available_gas(2000000)] +fn test_public_key_setter_and_getter_camel() { + let dispatcher = setup_dispatcher(); + + testing::set_contract_address(dispatcher.contract_address); + + 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); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_set_public_key_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.set_public_key(NEW_PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: unauthorized', 'ENTRYPOINT_FAILED'))] +fn test_setPublicKey_different_account() { + let dispatcher = setup_dispatcher(); + dispatcher.setPublicKey(NEW_PUBKEY); +} + +// +// is_valid_signature & isValidSignature +// + +fn is_valid_sig_dispatcher() -> (AccountABIDispatcher, felt252, Array) { + let dispatcher = setup_dispatcher(); + + let data = SIGNED_TX_DATA(); + let hash = data.transaction_hash; + let mut signature = array![data.r, data.s]; + + testing::set_contract_address(dispatcher.contract_address); + dispatcher.set_public_key(data.public_key); + + (dispatcher, hash, signature) +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +fn test_is_valid_signature_bad_sig() { + let (dispatcher, hash, _) = is_valid_sig_dispatcher(); + + let bad_signature = array![0x987, 0x564]; + + let is_valid = dispatcher.is_valid_signature(hash, bad_signature.clone()); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +fn test_isValidSignature_bad_sig() { + let (dispatcher, hash, _) = is_valid_sig_dispatcher(); + + let bad_signature = array![0x987, 0x564]; + + let is_valid = dispatcher.isValidSignature(hash, bad_signature); + assert(is_valid == 0, 'Should reject invalid signature'); +} + +// +// supports_interface +// + +#[test] +#[available_gas(2000000)] +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 ISRC5'); + assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); +} + +// +// Entry points +// + +#[test] +#[available_gas(2000000)] +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, PUBKEY) == starknet::VALIDATED, + 'Should validate correctly' + ); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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, PUBKEY); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid signature', '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); + + // Craft call and add to calls array + 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); + + // 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] +#[available_gas(2000000)] +fn test_execute() { + test_execute_with_version(Option::None(())); +} + +#[test] +#[available_gas(2000000)] +fn test_execute_query_version() { + test_execute_with_version(Option::Some(QUERY_VERSION)); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: invalid tx version', 'ENTRYPOINT_FAILED'))] +fn test_execute_invalid_version() { + test_execute_with_version(Option::Some(TRANSACTION_VERSION - 1)); +} + +#[test] +#[available_gas(2000000)] +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] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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] +#[available_gas(20000000)] +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![]; + + // 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] +#[available_gas(2000000)] +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); + + // Test return value + assert(ret.len() == 0, 'Should have an empty response'); +} + +#[test] +#[available_gas(2000000)] +#[should_panic(expected: ('Account: 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); +} + +// +// Helpers +// + +fn assert_event_owner_removed(removed_owner_guid: felt252, contract: ContractAddress) { + 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) { + 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); + utils::assert_no_events_left(contract); +} From 5e03f4c46dc0091f1b7fe4d474c22f4e5c1bf95d Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 3 Nov 2023 14:41:51 +0100 Subject: [PATCH 17/25] fix: merge unresolved conflicts --- docs/modules/ROOT/pages/api/account.adoc | 59 ------------------------ 1 file changed, 59 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 9f063c93b..bff661629 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -73,18 +73,6 @@ use openzeppelin::account::AccountComponent; ``` Account component implementing xref:ISRC6[`ISRC6`]. -<<<<<<< HEAD -NOTE: Implementing xref:api/introspection.adoc#SRC5[SRC5] is a requirement for this component to be implemented. - -[.contract-index] -.Embeddable Functions --- -.SRC6Impl - -* xref:#Account-\\__execute__[`++__execute__(self, calls)++`] -* xref:#Account-\\__validate__[`++__validate__(self, calls)++`] -* xref:#Account-is_valid_signature[`++is_valid_signature(self, hash, signature)++`] -======= NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. [.contract-index] @@ -95,7 +83,6 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a * xref:#AccountComponent-\\__execute__[`++__execute__(self, calls)++`] * xref:#AccountComponent-\\__validate__[`++__validate__(self, calls)++`] * xref:#AccountComponent-is_valid_signature[`++is_valid_signature(self, hash, signature)++`] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 .DeclarerImpl @@ -111,26 +98,8 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a .PublicKeyImpl -<<<<<<< HEAD -* xref:#Account-get_public_key[`++get_public_key(self)++`] -* xref:#Account-set_public_key[`++set_public_key(self, new_public_key)++`] --- - -[.contract-index] -.camelCase Support --- -.SRC6CamelOnlyImpl - -* xref:#Account-isValidSignature[`++isValidSignature(self, hash, signature)++`] - -.PublicKeyCamelImpl - -* xref:#Account-getPublicKey[`++getPublicKey(self)++`] -* xref:#Account-setPublicKey[`++setPublicKey(self, newPublicKey)++`] -======= * xref:#AccountComponent-get_public_key[`++get_public_key(self)++`] * xref:#AccountComponent-set_public_key[`++set_public_key(self, new_public_key)++`] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 -- [.contract-index] @@ -151,19 +120,11 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a -- .InternalImpl -<<<<<<< HEAD -* xref:#Account-initializer[`++initializer(self, public_key)++`] -* xref:#Account-assert_only_self[`++assert_only_self(self)++`] -* xref:#Account-validate_transaction[`++validate_transaction(self)++`] -* xref:#Account-_set_public_key[`++_set_public_key(self, new_public_key)++`] -* xref:#Account-_is_valid_signature[`++_is_valid_signature(self, hash, signature)++`] -======= * xref:#AccountComponent-initializer[`++initializer(self, public_key)++`] * xref:#AccountComponent-assert_only_self[`++assert_only_self(self)++`] * xref:#AccountComponent-validate_transaction[`++validate_transaction(self)++`] * xref:#AccountComponent-_set_public_key[`++_set_public_key(self, new_public_key)++`] * xref:#AccountComponent-_is_valid_signature[`++_is_valid_signature(self, hash, signature)++`] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 -- [.contract-index] @@ -173,19 +134,11 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a * xref:#AccountComponent-OwnerRemoved[`++OwnerRemoved(removed_owner_guid)++`] -- -<<<<<<< HEAD -[#Account-Embeddable-Functions] -==== Embeddable Functions - -[.contract-item] -[[Account-__execute__]] -======= [#AccountComponent-Embeddable-Functions] ==== Embeddable Functions [.contract-item] [[AccountComponent-__execute__]] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 ==== `[.contract-item-name]#++__execute__++#++(self: @ContractState, calls: Array) → Array>++` [.item-kind]#external# See xref:ISRC6-\\__execute__[ISRC6::\\__execute__]. @@ -203,11 +156,7 @@ See xref:ISRC6-\\__validate__[ISRC6::\\__validate__]. See xref:ISRC6-is_valid_signature[ISRC6::is_valid_signature]. [.contract-item] -<<<<<<< HEAD -[[Account-__validate_declare__]] -======= [[AccountComponent-__validate_declare__]] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 ==== `[.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]. @@ -215,11 +164,7 @@ Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Net Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] -<<<<<<< HEAD -[[Account-__validate_deploy__]] -======= [[AccountComponent-__validate_deploy__]] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 ==== `[.contract-item-name]#++__validate_deploy__++#++(self: @ContractState, class_hash: felt252, contract_address_salt: felt252, public_key: felt252) → felt252++` [.item-kind]#external# Validates a https://docs.starknet.io/documentation/architecture_and_concepts/Network_Architecture/Blocks/transactions/#deploy_account_transaction[`DeployAccount` transaction]. @@ -228,11 +173,7 @@ See xref:/guides/deployment.adoc[Counterfactual Deployments]. Returns the short string `'VALID'` if valid, otherwise it reverts. [.contract-item] -<<<<<<< HEAD -[[Account-get_public_key]] -======= [[AccountComponent-get_public_key]] ->>>>>>> ad1ae70439f8a0791adedbdf3b7ac05b1ab60db6 ==== `[.contract-item-name]#++get_public_key++#++(self: @ContractState)++ → felt252` [.item-kind]#external# Returns the current public key of the account. From e8d6d89cfbb74e4d7b89226da6de154ad058660d Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 3 Nov 2023 14:43:06 +0100 Subject: [PATCH 18/25] docs: update --- docs/modules/ROOT/pages/api/account.adoc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index bff661629..595dacbe6 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -92,10 +92,6 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a * xref:#AccountComponent-\\__validate_deploy__[`++__validate_deploy__(self, hash, signature)++`] -.DeployableImpl - -* xref:#Account-\\__validate_deploy__[`++__validate_deploy__(self, hash, signature)++`] - .PublicKeyImpl * xref:#AccountComponent-get_public_key[`++get_public_key(self)++`] From 95dd1a6234949150f7bb2ee4bf4dbd508ff3bc4b Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 7 Nov 2023 13:30:43 +0100 Subject: [PATCH 19/25] Update src/presets/account.cairo Co-authored-by: Andrew Fleming --- src/presets/account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/presets/account.cairo b/src/presets/account.cairo index 4127e0397..bdcde87dc 100644 --- a/src/presets/account.cairo +++ b/src/presets/account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.7.0 (account/presets/account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0-beta.0 (account/presets/account.cairo) /// # Account Preset /// From b6628ce827db56cdcce0d45363d30299645648dd Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 7 Nov 2023 13:36:49 +0100 Subject: [PATCH 20/25] feat: apply review updates --- docs/modules/ROOT/pages/api/account.adoc | 2 +- src/presets/account.cairo | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index 595dacbe6..b3aee6be3 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -10,7 +10,7 @@ Reference of interfaces, presets, and utilities related to account contracts. [.contract] [[ISRC6]] -=== `++ISRC6++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0-beta.0/src/account/interface.cairo#L12[{github-icon},role=heading-link] +=== `++ISRC6++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0-beta.0/src/account/interface.cairo[{github-icon},role=heading-link] ```javascript use openzeppelin::account::interface::ISRC6; diff --git a/src/presets/account.cairo b/src/presets/account.cairo index 4127e0397..a9aeb14f1 100644 --- a/src/presets/account.cairo +++ b/src/presets/account.cairo @@ -3,7 +3,7 @@ /// # Account Preset /// -/// Openzeppelin's account contract. +/// OpenZeppelin's basic account which can declare or invoke other contracts. #[starknet::contract] mod Account { use openzeppelin::account::AccountComponent; From 26b7988e991370194bb1c9fbc6a6486409dc0b45 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Tue, 7 Nov 2023 13:58:46 +0100 Subject: [PATCH 21/25] feat: add account preset section --- docs/modules/ROOT/pages/api/account.adoc | 48 ++++++++++++++++++- .../modules/ROOT/pages/api/introspection.adoc | 2 +- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index b3aee6be3..f692697fa 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -75,7 +75,7 @@ Account component implementing xref:ISRC6[`ISRC6`]. NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a requirement for this component to be implemented. -[.contract-index] +[.contract-index#AccountComponent-Embeddable-Impls] .Embeddable Implementations -- .SRC6Impl @@ -98,7 +98,7 @@ NOTE: Implementing xref:api/introspection.adoc#SRC5Component[SRC5Component] is a * xref:#AccountComponent-set_public_key[`++set_public_key(self, new_public_key)++`] -- -[.contract-index] +[.contract-index#AccountComponent-Embeddable-Impls-camelCase] .Embeddable Implementations (camelCase) -- .SRC6CamelOnlyImpl @@ -259,3 +259,47 @@ Emitted when a `public_key` is added. ==== `[.contract-item-name]#++OwnerRemoved++#++(removed_owner_guid: felt252)++` [.item-kind]#event# Emitted when a `public_key` is removed. + +== Presets + +[.contract] +[[Account]] +=== `++Account++` link:https://github.com/OpenZeppelin/cairo-contracts/blob/release-v0.8.0-beta.0/src/presets/account.cairo[{github-icon},role=heading-link] + +```javascript +use openzeppelin::presets::Account; +``` + +Basic Account preset leveraging xref:#AccountComponent[AccountComponent]. + +[.contract-index] +.Constructor +-- +* xref:#Account-constructor[`++constructor(self, public_key)++`] +-- + +[.contract-index] +.Embedded Implementations +-- +.AccountComponent + +* xref:#AccountComponent-Embeddable-Impls[`++SRC6Impl++`] +* xref:#AccountComponent-Embeddable-Impls[`++PublicKeyImpl++`] +* xref:#AccountComponent-Embeddable-Impls[`++DeclarerImpl++`] +* xref:#AccountComponent-Embeddable-Impls[`++DeployableImpl++`] +* xref:#AccountComponent-Embeddable-Impls-camelCase[`++SRC6CamelOnlyImpl++`] +* xref:#AccountComponent-Embeddable-Impls-camelCase[`++PublicKeyCamelImpl++`] + +.SRC5Component + +* xref:api/introspection.adoc#SRC5Component-Embeddable-Impls[`++SRC5Impl++`] +-- + +[#Account-constructor-section] +==== Constructor + +[.contract-item] +[[Account-constructor]] +==== `[.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. diff --git a/docs/modules/ROOT/pages/api/introspection.adoc b/docs/modules/ROOT/pages/api/introspection.adoc index 1ae2011a2..91def5579 100644 --- a/docs/modules/ROOT/pages/api/introspection.adoc +++ b/docs/modules/ROOT/pages/api/introspection.adoc @@ -52,7 +52,7 @@ use openzeppelin::introspection::src5::SRC5Component; SRC5 component extending xref:ISRC5[`ISRC5`]. -[.contract-index] +[.contract-index#SRC5Component-Embeddable-Impls] .Embeddable Implementations -- .SRC5Impl From 4dc2a22ae8c4f83282826066b39422eb5a6a0600 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 10 Nov 2023 13:53:48 +0100 Subject: [PATCH 22/25] 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 f692697fa..188846a12 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -270,7 +270,7 @@ Emitted when a `public_key` is removed. use openzeppelin::presets::Account; ``` -Basic Account preset leveraging xref:#AccountComponent[AccountComponent]. +Basic account contract leveraging xref:#AccountComponent[AccountComponent]. [.contract-index] .Constructor From af26be2f7781dd75da7717a660dc459694a327fa Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 10 Nov 2023 13:54:04 +0100 Subject: [PATCH 23/25] Update src/presets/account.cairo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martín Triay --- src/presets/account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/presets/account.cairo b/src/presets/account.cairo index f71b54936..f8438fd29 100644 --- a/src/presets/account.cairo +++ b/src/presets/account.cairo @@ -3,7 +3,7 @@ /// # Account Preset /// -/// OpenZeppelin's basic account which can declare or invoke other contracts. +/// OpenZeppelin's basic account which can change its public key and declare, deploy, or call contracts. #[starknet::contract] mod Account { use openzeppelin::account::AccountComponent; From ba948714455b16868a5c7a9478b82318d20b1e71 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 10 Nov 2023 13:54:19 +0100 Subject: [PATCH 24/25] Update src/presets/account.cairo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martín Triay --- src/presets/account.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/presets/account.cairo b/src/presets/account.cairo index f8438fd29..a8c26a52c 100644 --- a/src/presets/account.cairo +++ b/src/presets/account.cairo @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts for Cairo v0.8.0-beta.0 (account/presets/account.cairo) +// OpenZeppelin Contracts for Cairo v0.8.0-beta.0 (presets/account.cairo) /// # Account Preset /// From b36755f6a49b1eb402ee60ddd9094477b0b37a5a Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Fri, 10 Nov 2023 13:57:55 +0100 Subject: [PATCH 25/25] feat: apply review updates --- src/tests/account/test_account.cairo | 2 +- src/tests/presets/test_account.cairo | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 67baf8e20..a5b1f0a40 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -490,7 +490,7 @@ fn test_initializer() { assert(event.new_owner_guid == PUBKEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); - assert(state.account.get_public_key() == PUBKEY, 'Should return public key'); + assert(state.account.get_public_key() == PUBKEY, 'Should return PUBKEY'); } #[test] diff --git a/src/tests/presets/test_account.cairo b/src/tests/presets/test_account.cairo index b905a73fa..080b48d41 100644 --- a/src/tests/presets/test_account.cairo +++ b/src/tests/presets/test_account.cairo @@ -181,7 +181,7 @@ fn test_isValidSignature_bad_sig() { 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 ISRC5'); + assert(dispatcher.supports_interface(ISRC6_ID), 'Should implement ISRC6'); assert(!dispatcher.supports_interface(0x123), 'Should not implement 0x123'); }