diff --git a/src/account/account.cairo b/src/account/account.cairo index 8d8105521..8fffb55ea 100644 --- a/src/account/account.cairo +++ b/src/account/account.cairo @@ -49,6 +49,23 @@ mod Account { public_key: felt252 } + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + OwnerAdded: OwnerAdded, + OwnerRemoved: OwnerRemoved, + } + + #[derive(Drop, starknet::Event)] + struct OwnerAdded { + new_owner_guid: felt252 + } + + #[derive(Drop, starknet::Event)] + struct OwnerRemoved { + removed_owner_guid: felt252 + } + #[constructor] fn constructor(ref self: ContractState, _public_key: felt252) { self.initializer(_public_key); @@ -131,7 +148,8 @@ mod Account { fn set_public_key(ref self: ContractState, new_public_key: felt252) { assert_only_self(); - self.public_key.write(new_public_key); + self.emit(OwnerRemoved { removed_owner_guid: self.public_key.read() }); + self._set_public_key(new_public_key); } } @@ -142,8 +160,7 @@ mod Account { } fn setPublicKey(ref self: ContractState, newPublicKey: felt252) { - assert_only_self(); - self.public_key.write(newPublicKey); + PublicKeyImpl::set_public_key(ref self, newPublicKey); } } @@ -166,7 +183,7 @@ mod Account { fn initializer(ref self: ContractState, _public_key: felt252) { let mut unsafe_state = SRC5::unsafe_new_contract_state(); SRC5::InternalImpl::register_interface(ref unsafe_state, interface::ISRC6_ID); - self.public_key.write(_public_key); + self._set_public_key(_public_key); } fn validate_transaction(self: @ContractState) -> felt252 { @@ -177,6 +194,11 @@ mod Account { starknet::VALIDATED } + fn _set_public_key(ref self: ContractState, new_public_key: felt252) { + self.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 ) -> bool { diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index b2fe45ceb..d66e9fbfe 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -1,5 +1,9 @@ use array::ArrayTrait; use core::traits::Into; +use openzeppelin::account::Account::OwnerAdded; +use openzeppelin::account::Account::OwnerRemoved; +use openzeppelin::account::Account::PublicKeyCamelImpl; +use openzeppelin::account::Account::PublicKeyImpl; use openzeppelin::account::Account; use openzeppelin::account::AccountABIDispatcher; use openzeppelin::account::AccountABIDispatcherTrait; @@ -8,6 +12,7 @@ use openzeppelin::account::TRANSACTION_VERSION; use openzeppelin::account::interface::ISRC6_ID; use openzeppelin::introspection::interface::ISRC5_ID; use openzeppelin::tests::utils; +use openzeppelin::tests::utils::constants::ZERO; use openzeppelin::token::erc20::ERC20; use openzeppelin::token::erc20::interface::IERC20Dispatcher; use openzeppelin::token::erc20::interface::IERC20DispatcherTrait; @@ -100,9 +105,12 @@ fn deploy_erc20(recipient: ContractAddress, initial_supply: u256) -> IERC20Dispa fn test_constructor() { let mut state = STATE(); Account::constructor(ref state, PUBLIC_KEY); - assert( - Account::PublicKeyImpl::get_public_key(@state) == PUBLIC_KEY, 'Should return public key' - ); + + let event = testing::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'); } // @@ -149,7 +157,7 @@ fn test_is_valid_signature() { let mut good_signature = array![data.r, data.s]; let mut bad_signature = array![0x987, 0x564]; - Account::PublicKeyImpl::set_public_key(ref state, data.public_key); + PublicKeyImpl::set_public_key(ref state, data.public_key); let is_valid = Account::SRC6Impl::is_valid_signature(@state, hash, good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); @@ -168,7 +176,7 @@ fn test_isValidSignature() { let mut good_signature = array![data.r, data.s]; let mut bad_signature = array![0x987, 0x564]; - Account::PublicKeyImpl::set_public_key(ref state, data.public_key); + PublicKeyImpl::set_public_key(ref state, data.public_key); let is_valid = Account::SRC6CamelOnlyImpl::isValidSignature(@state, hash, good_signature); assert(is_valid == starknet::VALIDATED, 'Should accept valid signature'); @@ -437,9 +445,21 @@ fn test_public_key_setter_and_getter() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(ACCOUNT_ADDRESS()); - Account::PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); + // Check default + let public_key = PublicKeyImpl::get_public_key(@state); + assert(public_key == 0, 'Should be zero'); + + // Set key + PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); + + let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - let public_key = Account::PublicKeyImpl::get_public_key(@state); + let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + 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); assert(public_key == NEW_PUBKEY, 'Should update key'); } @@ -452,7 +472,7 @@ fn test_public_key_setter_different_account() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(caller); - Account::PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); + PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); } // @@ -466,9 +486,21 @@ fn test_public_key_setter_and_getter_camel() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(ACCOUNT_ADDRESS()); - Account::PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); + // Check default + let public_key = PublicKeyCamelImpl::getPublicKey(@state); + assert(public_key == 0, 'Should be zero'); + + // Set key + PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); + + let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - let public_key = Account::PublicKeyCamelImpl::getPublicKey(@state); + let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); + utils::assert_no_events_left(ACCOUNT_ADDRESS()); + + let public_key = PublicKeyCamelImpl::getPublicKey(@state); assert(public_key == NEW_PUBKEY, 'Should update key'); } @@ -481,7 +513,7 @@ fn test_public_key_setter_different_account_camel() { testing::set_contract_address(ACCOUNT_ADDRESS()); testing::set_caller_address(caller); - Account::PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); + PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); } // @@ -493,9 +525,12 @@ fn test_public_key_setter_different_account_camel() { fn test_initializer() { let mut state = STATE(); Account::InternalImpl::initializer(ref state, PUBLIC_KEY); - assert( - Account::PublicKeyImpl::get_public_key(@state) == PUBLIC_KEY, 'Should return public key' - ); + + let event = testing::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'); } #[test] @@ -527,7 +562,7 @@ fn test__is_valid_signature() { let mut bad_signature = array![0x987, 0x564]; let mut invalid_length_signature = array![0x987]; - Account::PublicKeyImpl::set_public_key(ref state, data.public_key); + PublicKeyImpl::set_public_key(ref state, data.public_key); let is_valid = Account::InternalImpl::_is_valid_signature(@state, hash, good_signature.span()); assert(is_valid, 'Should accept valid signature'); @@ -540,3 +575,17 @@ fn test__is_valid_signature() { ); assert(!is_valid, 'Should reject invalid length'); } + +#[test] +#[available_gas(2000000)] +fn test__set_public_key() { + let mut state = STATE(); + Account::InternalImpl::_set_public_key(ref state, PUBLIC_KEY); + + let event = testing::pop_log::(ZERO()).unwrap(); + assert(event.new_owner_guid == PUBLIC_KEY, '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'); +}