From 6a4441208026aacd59f7875a1f120f857333ec9e Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 21 Sep 2023 23:38:56 -0400 Subject: [PATCH 01/16] add indexed keys to events --- src/token/erc20/erc20.cairo | 10 +++++++--- src/token/erc721/erc721.cairo | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/token/erc20/erc20.cairo b/src/token/erc20/erc20.cairo index 9a97cee58..dc5e8ff84 100644 --- a/src/token/erc20/erc20.cairo +++ b/src/token/erc20/erc20.cairo @@ -20,22 +20,26 @@ mod ERC20 { } #[event] - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] enum Event { Transfer: Transfer, Approval: Approval, } - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] struct Transfer { + #[key] from: ContractAddress, + #[key] to: ContractAddress, value: u256 } - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] struct Approval { + #[key] owner: ContractAddress, + #[key] spender: ContractAddress, value: u256 } diff --git a/src/token/erc721/erc721.cairo b/src/token/erc721/erc721.cairo index 0373dad36..c6bdf429e 100644 --- a/src/token/erc721/erc721.cairo +++ b/src/token/erc721/erc721.cairo @@ -32,30 +32,38 @@ mod ERC721 { } #[event] - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] enum Event { Transfer: Transfer, Approval: Approval, ApprovalForAll: ApprovalForAll } - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] struct Transfer { + #[key] from: ContractAddress, + #[key] to: ContractAddress, + #[key] token_id: u256 } - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] struct Approval { + #[key] owner: ContractAddress, + #[key] approved: ContractAddress, + #[key] token_id: u256 } - #[derive(Drop, starknet::Event)] + #[derive(Drop, starknet::Event, PartialEq)] struct ApprovalForAll { + #[key] owner: ContractAddress, + #[key] operator: ContractAddress, approved: bool } From 922cfb36712da7edf15c853bcc055cef95f87ede Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 21 Sep 2023 23:39:35 -0400 Subject: [PATCH 02/16] fix event assertions --- src/tests/token/test_erc20.cairo | 22 ++++++++++--------- src/tests/token/test_erc721.cairo | 36 +++++++++++++++++-------------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/tests/token/test_erc20.cairo b/src/tests/token/test_erc20.cairo index 4109e909a..98a08ac50 100644 --- a/src/tests/token/test_erc20.cairo +++ b/src/tests/token/test_erc20.cairo @@ -5,11 +5,9 @@ use openzeppelin::tests::utils::constants::{ ZERO, OWNER, SPENDER, RECIPIENT, NAME, SYMBOL, DECIMALS, SUPPLY, VALUE }; use openzeppelin::tests::utils; -use openzeppelin::token::erc20::ERC20::Approval; use openzeppelin::token::erc20::ERC20::ERC20CamelOnlyImpl; use openzeppelin::token::erc20::ERC20::ERC20Impl; use openzeppelin::token::erc20::ERC20::InternalImpl; -use openzeppelin::token::erc20::ERC20::Transfer; use openzeppelin::token::erc20::ERC20; use option::OptionTrait; use starknet::ContractAddress; @@ -598,10 +596,12 @@ fn test__burn_from_zero() { // fn assert_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.owner == owner, 'Invalid `owner`'); - assert(event.spender == spender, 'Invalid `spender`'); - assert(event.value == value, 'Invalid `value`'); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event == ERC20::Event::Approval(ERC20::Approval { + owner: owner, + spender: spender, + value: value + }), 'Invalid `Approval` event'); } fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { @@ -610,10 +610,12 @@ fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.from == from, 'Invalid `from`'); - assert(event.to == to, 'Invalid `to`'); - assert(event.value == value, 'Invalid `value`'); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event == ERC20::Event::Transfer(ERC20::Transfer { + from: from, + to: to, + value: value + }), 'Invalid `Transfer` event'); } fn assert_only_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { diff --git a/src/tests/token/test_erc721.cairo b/src/tests/token/test_erc721.cairo index 1351e0385..811206d14 100644 --- a/src/tests/token/test_erc721.cairo +++ b/src/tests/token/test_erc721.cairo @@ -18,8 +18,8 @@ use openzeppelin::tests::utils::constants::{ }; use openzeppelin::tests::utils; use openzeppelin::token::erc721::ERC721::{ - Approval, ApprovalForAll, ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, - ERC721MetadataImpl, InternalImpl, SRC5CamelImpl, SRC5Impl, Transfer, + ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, ERC721MetadataImpl, + InternalImpl, SRC5CamelImpl, SRC5Impl, }; use openzeppelin::token::erc721::ERC721; use openzeppelin::token::erc721; @@ -1400,25 +1400,29 @@ fn assert_state_after_mint(recipient: ContractAddress, token_id: u256) { fn assert_event_approval_for_all( owner: ContractAddress, operator: ContractAddress, approved: bool ) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.owner == owner, 'Invalid `owner`'); - assert(event.operator == operator, 'Invalid `operator`'); - assert(event.approved == approved, 'Invalid `approved`'); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event == ERC721::Event::ApprovalForAll(ERC721::ApprovalForAll { + owner: owner, + operator: operator, + approved: approved + }), 'Invalid `ApprovalForAll` event'); utils::assert_no_events_left(ZERO()); } fn assert_event_approval(owner: ContractAddress, approved: ContractAddress, token_id: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.owner == owner, 'Invalid `owner`'); - assert(event.approved == approved, 'Invalid `approved`'); - assert(event.token_id == token_id, 'Invalid `token_id`'); - utils::assert_no_events_left(ZERO()); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event == ERC721::Event::Approval(ERC721::Approval { + owner: owner, + approved: approved, + token_id: token_id + }), 'Invalid `Approval` event'); } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, token_id: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert(event.from == from, 'Invalid `from`'); - assert(event.to == to, 'Invalid `to`'); - assert(event.token_id == token_id, 'Invalid `token_id`'); - utils::assert_no_events_left(ZERO()); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event == ERC721::Event::Transfer(ERC721::Transfer { + from: from, + to: to, + token_id: token_id + }), 'Invalid `Transfer` event'); } From 3c202dee5480397ad4cc9f4efb9a4449801f3afe Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 21 Sep 2023 23:40:11 -0400 Subject: [PATCH 03/16] use pop_log from utils --- src/tests/account/test_account.cairo | 14 +++++++------- src/tests/upgrades/test_upgradeable.cairo | 3 +-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/tests/account/test_account.cairo b/src/tests/account/test_account.cairo index 83bea533b..c10c731b1 100644 --- a/src/tests/account/test_account.cairo +++ b/src/tests/account/test_account.cairo @@ -106,7 +106,7 @@ fn test_constructor() { let mut state = STATE(); Account::constructor(ref state, PUBLIC_KEY); - let event = testing::pop_log::(ZERO()).unwrap(); + let event = utils::pop_log::(ZERO()).unwrap(); assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); @@ -452,10 +452,10 @@ fn test_public_key_setter_and_getter() { // Set key PublicKeyImpl::set_public_key(ref state, NEW_PUBKEY); - let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); utils::assert_no_events_left(ACCOUNT_ADDRESS()); @@ -493,10 +493,10 @@ fn test_public_key_setter_and_getter_camel() { // Set key PublicKeyCamelImpl::setPublicKey(ref state, NEW_PUBKEY); - let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.removed_owner_guid == 0, 'Invalid old owner key'); - let event = testing::pop_log::(ACCOUNT_ADDRESS()).unwrap(); + let event = utils::pop_log::(ACCOUNT_ADDRESS()).unwrap(); assert(event.new_owner_guid == NEW_PUBKEY, 'Invalid new owner key'); utils::assert_no_events_left(ACCOUNT_ADDRESS()); @@ -526,7 +526,7 @@ fn test_initializer() { let mut state = STATE(); Account::InternalImpl::initializer(ref state, PUBLIC_KEY); - let event = testing::pop_log::(ZERO()).unwrap(); + let event = utils::pop_log::(ZERO()).unwrap(); assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); @@ -582,7 +582,7 @@ 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(); + let event = utils::pop_log::(ZERO()).unwrap(); assert(event.new_owner_guid == PUBLIC_KEY, 'Invalid owner key'); utils::assert_no_events_left(ZERO()); diff --git a/src/tests/upgrades/test_upgradeable.cairo b/src/tests/upgrades/test_upgradeable.cairo index 20b9c48c3..9f42fb928 100644 --- a/src/tests/upgrades/test_upgradeable.cairo +++ b/src/tests/upgrades/test_upgradeable.cairo @@ -13,7 +13,6 @@ use starknet::ContractAddress; use starknet::Felt252TryIntoClassHash; use starknet::class_hash_const; use starknet::contract_address_const; -use starknet::testing; use traits::TryInto; const VALUE: felt252 = 123; @@ -58,7 +57,7 @@ fn test_upgraded_event() { let v1 = deploy_v1(); v1.upgrade(V2_CLASS_HASH()); - let event = testing::pop_log::(v1.contract_address).unwrap(); + let event = utils::pop_log::(v1.contract_address).unwrap(); assert(event.class_hash == V2_CLASS_HASH(), 'Invalid class hash'); } From f568621dfdebfe3041236e0922e7da307d498a9c Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 21 Sep 2023 23:42:36 -0400 Subject: [PATCH 04/16] fix formatting --- src/tests/token/test_erc20.cairo | 20 ++++++++--------- src/tests/token/test_erc721.cairo | 37 +++++++++++++++++-------------- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/tests/token/test_erc20.cairo b/src/tests/token/test_erc20.cairo index 98a08ac50..9344ccec8 100644 --- a/src/tests/token/test_erc20.cairo +++ b/src/tests/token/test_erc20.cairo @@ -597,11 +597,12 @@ fn test__burn_from_zero() { fn assert_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { let event = utils::pop_log::(ZERO()).unwrap(); - assert(event == ERC20::Event::Approval(ERC20::Approval { - owner: owner, - spender: spender, - value: value - }), 'Invalid `Approval` event'); + assert( + event == ERC20::Event::Approval( + ERC20::Approval { owner: owner, spender: spender, value: value } + ), + 'Invalid `Approval` event' + ); } fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { @@ -611,11 +612,10 @@ fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, fn assert_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { let event = utils::pop_log::(ZERO()).unwrap(); - assert(event == ERC20::Event::Transfer(ERC20::Transfer { - from: from, - to: to, - value: value - }), 'Invalid `Transfer` event'); + assert( + event == ERC20::Event::Transfer(ERC20::Transfer { from: from, to: to, value: value }), + 'Invalid `Transfer` event' + ); } fn assert_only_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { diff --git a/src/tests/token/test_erc721.cairo b/src/tests/token/test_erc721.cairo index 811206d14..cdcbf6eb4 100644 --- a/src/tests/token/test_erc721.cairo +++ b/src/tests/token/test_erc721.cairo @@ -18,8 +18,8 @@ use openzeppelin::tests::utils::constants::{ }; use openzeppelin::tests::utils; use openzeppelin::token::erc721::ERC721::{ - ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, ERC721MetadataImpl, - InternalImpl, SRC5CamelImpl, SRC5Impl, + ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, ERC721MetadataImpl, InternalImpl, + SRC5CamelImpl, SRC5Impl, }; use openzeppelin::token::erc721::ERC721; use openzeppelin::token::erc721; @@ -1401,28 +1401,31 @@ fn assert_event_approval_for_all( owner: ContractAddress, operator: ContractAddress, approved: bool ) { let event = utils::pop_log::(ZERO()).unwrap(); - assert(event == ERC721::Event::ApprovalForAll(ERC721::ApprovalForAll { - owner: owner, - operator: operator, - approved: approved - }), 'Invalid `ApprovalForAll` event'); + assert( + event == ERC721::Event::ApprovalForAll( + ERC721::ApprovalForAll { owner: owner, operator: operator, approved: approved } + ), + 'Invalid `ApprovalForAll` event' + ); utils::assert_no_events_left(ZERO()); } fn assert_event_approval(owner: ContractAddress, approved: ContractAddress, token_id: u256) { let event = utils::pop_log::(ZERO()).unwrap(); - assert(event == ERC721::Event::Approval(ERC721::Approval { - owner: owner, - approved: approved, - token_id: token_id - }), 'Invalid `Approval` event'); + assert( + event == ERC721::Event::Approval( + ERC721::Approval { owner: owner, approved: approved, token_id: token_id } + ), + 'Invalid `Approval` event' + ); } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, token_id: u256) { let event = utils::pop_log::(ZERO()).unwrap(); - assert(event == ERC721::Event::Transfer(ERC721::Transfer { - from: from, - to: to, - token_id: token_id - }), 'Invalid `Transfer` event'); + assert( + event == ERC721::Event::Transfer( + ERC721::Transfer { from: from, to: to, token_id: token_id } + ), + 'Invalid `Transfer` event' + ); } From e194bd92bf67388dad87312baca837228b1fec4d Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 22 Sep 2023 11:52:08 -0400 Subject: [PATCH 05/16] remove event id from keys --- src/tests/utils.cairo | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index 77d711c53..657d00c3b 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -19,16 +19,22 @@ fn deploy(contract_class_hash: felt252, calldata: Array) -> ContractAdd /// Pop the earliest unpopped logged event for the contract as the requested type /// and checks there's no more data left on the event, preventing unaccounted params. -/// Indexed event members are currently not supported, so they are ignored. +/// This function also removes the hashed event name key (the first key) +/// to support indexed event members. fn pop_log, impl TEvent: starknet::Event>( address: ContractAddress ) -> Option { let (mut keys, mut data) = testing::pop_log_raw(address)?; + + // Remove the event ID from the keys. + keys.pop_front(); + let ret = starknet::Event::deserialize(ref keys, ref data); assert(data.is_empty(), 'Event has extra data'); ret } + fn assert_no_events_left(address: ContractAddress) { assert(testing::pop_log_raw(address).is_none(), 'Events remaining on queue'); } From 4109355765f2eacdcd6a2c7b47d1fbc796562407 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 22 Sep 2023 11:53:02 -0400 Subject: [PATCH 06/16] remove PartialEq from events --- src/token/erc20/erc20.cairo | 6 +++--- src/token/erc721/erc721.cairo | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/token/erc20/erc20.cairo b/src/token/erc20/erc20.cairo index dc5e8ff84..90217c1e5 100644 --- a/src/token/erc20/erc20.cairo +++ b/src/token/erc20/erc20.cairo @@ -20,13 +20,13 @@ mod ERC20 { } #[event] - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] enum Event { Transfer: Transfer, Approval: Approval, } - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] struct Transfer { #[key] from: ContractAddress, @@ -35,7 +35,7 @@ mod ERC20 { value: u256 } - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] struct Approval { #[key] owner: ContractAddress, diff --git a/src/token/erc721/erc721.cairo b/src/token/erc721/erc721.cairo index c6bdf429e..abb819bdf 100644 --- a/src/token/erc721/erc721.cairo +++ b/src/token/erc721/erc721.cairo @@ -32,14 +32,14 @@ mod ERC721 { } #[event] - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] enum Event { Transfer: Transfer, Approval: Approval, ApprovalForAll: ApprovalForAll } - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] struct Transfer { #[key] from: ContractAddress, @@ -49,7 +49,7 @@ mod ERC721 { token_id: u256 } - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] struct Approval { #[key] owner: ContractAddress, @@ -59,7 +59,7 @@ mod ERC721 { token_id: u256 } - #[derive(Drop, starknet::Event, PartialEq)] + #[derive(Drop, starknet::Event)] struct ApprovalForAll { #[key] owner: ContractAddress, From 640e02d3a82a72764060764b3fcabf3a04c78401 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 22 Sep 2023 11:54:34 -0400 Subject: [PATCH 07/16] revert event assertion changes --- src/tests/token/test_erc20.cairo | 22 ++++++++--------- src/tests/token/test_erc721.cairo | 39 +++++++++++++------------------ 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/src/tests/token/test_erc20.cairo b/src/tests/token/test_erc20.cairo index 9344ccec8..4109e909a 100644 --- a/src/tests/token/test_erc20.cairo +++ b/src/tests/token/test_erc20.cairo @@ -5,9 +5,11 @@ use openzeppelin::tests::utils::constants::{ ZERO, OWNER, SPENDER, RECIPIENT, NAME, SYMBOL, DECIMALS, SUPPLY, VALUE }; use openzeppelin::tests::utils; +use openzeppelin::token::erc20::ERC20::Approval; use openzeppelin::token::erc20::ERC20::ERC20CamelOnlyImpl; use openzeppelin::token::erc20::ERC20::ERC20Impl; use openzeppelin::token::erc20::ERC20::InternalImpl; +use openzeppelin::token::erc20::ERC20::Transfer; use openzeppelin::token::erc20::ERC20; use option::OptionTrait; use starknet::ContractAddress; @@ -596,13 +598,10 @@ fn test__burn_from_zero() { // fn assert_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert( - event == ERC20::Event::Approval( - ERC20::Approval { owner: owner, spender: spender, value: value } - ), - 'Invalid `Approval` event' - ); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event.owner == owner, 'Invalid `owner`'); + assert(event.spender == spender, 'Invalid `spender`'); + assert(event.value == value, 'Invalid `value`'); } fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { @@ -611,11 +610,10 @@ fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert( - event == ERC20::Event::Transfer(ERC20::Transfer { from: from, to: to, value: value }), - 'Invalid `Transfer` event' - ); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event.from == from, 'Invalid `from`'); + assert(event.to == to, 'Invalid `to`'); + assert(event.value == value, 'Invalid `value`'); } fn assert_only_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { diff --git a/src/tests/token/test_erc721.cairo b/src/tests/token/test_erc721.cairo index cdcbf6eb4..1351e0385 100644 --- a/src/tests/token/test_erc721.cairo +++ b/src/tests/token/test_erc721.cairo @@ -18,8 +18,8 @@ use openzeppelin::tests::utils::constants::{ }; use openzeppelin::tests::utils; use openzeppelin::token::erc721::ERC721::{ - ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, ERC721MetadataImpl, InternalImpl, - SRC5CamelImpl, SRC5Impl, + Approval, ApprovalForAll, ERC721CamelOnlyImpl, ERC721Impl, ERC721MetadataCamelOnlyImpl, + ERC721MetadataImpl, InternalImpl, SRC5CamelImpl, SRC5Impl, Transfer, }; use openzeppelin::token::erc721::ERC721; use openzeppelin::token::erc721; @@ -1400,32 +1400,25 @@ fn assert_state_after_mint(recipient: ContractAddress, token_id: u256) { fn assert_event_approval_for_all( owner: ContractAddress, operator: ContractAddress, approved: bool ) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert( - event == ERC721::Event::ApprovalForAll( - ERC721::ApprovalForAll { owner: owner, operator: operator, approved: approved } - ), - 'Invalid `ApprovalForAll` event' - ); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event.owner == owner, 'Invalid `owner`'); + assert(event.operator == operator, 'Invalid `operator`'); + assert(event.approved == approved, 'Invalid `approved`'); utils::assert_no_events_left(ZERO()); } fn assert_event_approval(owner: ContractAddress, approved: ContractAddress, token_id: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert( - event == ERC721::Event::Approval( - ERC721::Approval { owner: owner, approved: approved, token_id: token_id } - ), - 'Invalid `Approval` event' - ); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event.owner == owner, 'Invalid `owner`'); + assert(event.approved == approved, 'Invalid `approved`'); + assert(event.token_id == token_id, 'Invalid `token_id`'); + utils::assert_no_events_left(ZERO()); } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, token_id: u256) { - let event = utils::pop_log::(ZERO()).unwrap(); - assert( - event == ERC721::Event::Transfer( - ERC721::Transfer { from: from, to: to, token_id: token_id } - ), - 'Invalid `Transfer` event' - ); + let event = utils::pop_log::(ZERO()).unwrap(); + assert(event.from == from, 'Invalid `from`'); + assert(event.to == to, 'Invalid `to`'); + assert(event.token_id == token_id, 'Invalid `token_id`'); + utils::assert_no_events_left(ZERO()); } From 7fdd9181591ba37027046b3ef573c148e44c0f70 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 22 Sep 2023 13:42:03 -0400 Subject: [PATCH 08/16] fix sentence --- src/tests/utils.cairo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index 657d00c3b..caea666c2 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -19,8 +19,8 @@ fn deploy(contract_class_hash: felt252, calldata: Array) -> ContractAdd /// Pop the earliest unpopped logged event for the contract as the requested type /// and checks there's no more data left on the event, preventing unaccounted params. -/// This function also removes the hashed event name key (the first key) -/// to support indexed event members. +/// This function also removes the hashed event-name key to support indexed event +/// members. fn pop_log, impl TEvent: starknet::Event>( address: ContractAddress ) -> Option { From 258daba0f4e85fcc8bc1f142ce1b2bdf328453b3 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 22 Sep 2023 13:43:59 -0400 Subject: [PATCH 09/16] remove extra line --- src/tests/utils.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index caea666c2..f3cf05e79 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -34,7 +34,6 @@ fn pop_log, impl TEvent: starknet::Event>( ret } - fn assert_no_events_left(address: ContractAddress) { assert(testing::pop_log_raw(address).is_none(), 'Events remaining on queue'); } From 91b0cf6c97ad09812de837a620f9680912cc2c60 Mon Sep 17 00:00:00 2001 From: Andrew Date: Tue, 26 Sep 2023 02:00:35 -0400 Subject: [PATCH 10/16] fix comment --- src/tests/utils.cairo | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index f3cf05e79..198c21616 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -19,14 +19,15 @@ fn deploy(contract_class_hash: felt252, calldata: Array) -> ContractAdd /// Pop the earliest unpopped logged event for the contract as the requested type /// and checks there's no more data left on the event, preventing unaccounted params. -/// This function also removes the hashed event-name key to support indexed event -/// members. +/// This functions also removes the first key from the event. This is because indexed +/// params are set as event keys, but the first event key is always set as the +/// event ID. fn pop_log, impl TEvent: starknet::Event>( address: ContractAddress ) -> Option { let (mut keys, mut data) = testing::pop_log_raw(address)?; - // Remove the event ID from the keys. + // Remove the event ID from the keys keys.pop_front(); let ret = starknet::Event::deserialize(ref keys, ref data); From 9ab96d3ca221ac56354c67386577e4c705d5f95c Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 27 Sep 2023 12:20:15 -0400 Subject: [PATCH 11/16] add assert_indexed_keys --- src/tests/utils.cairo | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index 198c21616..7d522c990 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -35,6 +35,17 @@ fn pop_log, impl TEvent: starknet::Event>( ret } +/// Asserts that `expected_keys` exactly matches the indexed keys from `event`. +/// `expected_keys` must include all indexed event keys for `event` in the order +/// that they're defined. +fn assert_indexed_keys, impl TEvent: starknet::Event>(event: T, expected_keys: Span) { + let mut keys = array![]; + let mut data = array![]; + + starknet::event::Event::append_keys_and_data(@event, ref keys, ref data); + assert(expected_keys == keys.span(), 'Invalid keys'); +} + fn assert_no_events_left(address: ContractAddress) { assert(testing::pop_log_raw(address).is_none(), 'Events remaining on queue'); } From cf195f92bf943b289e3702f7792a58a6c39b0a8e Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 27 Sep 2023 12:20:38 -0400 Subject: [PATCH 12/16] apply assert_indexed_keys to tests --- src/tests/token/test_erc20.cairo | 13 +++++++++++++ src/tests/token/test_erc721.cairo | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/tests/token/test_erc20.cairo b/src/tests/token/test_erc20.cairo index 4109e909a..483f43823 100644 --- a/src/tests/token/test_erc20.cairo +++ b/src/tests/token/test_erc20.cairo @@ -11,6 +11,7 @@ use openzeppelin::token::erc20::ERC20::ERC20Impl; use openzeppelin::token::erc20::ERC20::InternalImpl; use openzeppelin::token::erc20::ERC20::Transfer; use openzeppelin::token::erc20::ERC20; +use openzeppelin::utils::serde::SerializedAppend; use option::OptionTrait; use starknet::ContractAddress; use starknet::contract_address_const; @@ -602,6 +603,12 @@ fn assert_event_approval(owner: ContractAddress, spender: ContractAddress, value assert(event.owner == owner, 'Invalid `owner`'); assert(event.spender == spender, 'Invalid `spender`'); assert(event.value == value, 'Invalid `value`'); + + // Check indexed keys + let mut indexed_keys = array![]; + indexed_keys.append_serde(owner); + indexed_keys.append_serde(spender); + utils::assert_indexed_keys(event, indexed_keys.span()) } fn assert_only_event_approval(owner: ContractAddress, spender: ContractAddress, value: u256) { @@ -614,6 +621,12 @@ fn assert_event_transfer(from: ContractAddress, to: ContractAddress, value: u256 assert(event.from == from, 'Invalid `from`'); assert(event.to == to, 'Invalid `to`'); assert(event.value == value, 'Invalid `value`'); + + // Check indexed keys + let mut indexed_keys = array![]; + indexed_keys.append_serde(from); + indexed_keys.append_serde(to); + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_only_event_transfer(from: ContractAddress, to: ContractAddress, value: u256) { diff --git a/src/tests/token/test_erc721.cairo b/src/tests/token/test_erc721.cairo index 1351e0385..fab6eccdc 100644 --- a/src/tests/token/test_erc721.cairo +++ b/src/tests/token/test_erc721.cairo @@ -23,6 +23,7 @@ use openzeppelin::token::erc721::ERC721::{ }; use openzeppelin::token::erc721::ERC721; use openzeppelin::token::erc721; +use openzeppelin::utils::serde::SerializedAppend; use option::OptionTrait; use starknet::contract_address_const; use starknet::ContractAddress; @@ -1405,6 +1406,12 @@ fn assert_event_approval_for_all( assert(event.operator == operator, 'Invalid `operator`'); assert(event.approved == approved, 'Invalid `approved`'); utils::assert_no_events_left(ZERO()); + + // Check indexed keys + let mut indexed_keys = array![]; + indexed_keys.append_serde(owner); + indexed_keys.append_serde(operator); + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_event_approval(owner: ContractAddress, approved: ContractAddress, token_id: u256) { @@ -1413,6 +1420,13 @@ fn assert_event_approval(owner: ContractAddress, approved: ContractAddress, toke assert(event.approved == approved, 'Invalid `approved`'); assert(event.token_id == token_id, 'Invalid `token_id`'); utils::assert_no_events_left(ZERO()); + + // Check indexed keys + let mut indexed_keys = array![]; + indexed_keys.append_serde(owner); + indexed_keys.append_serde(approved); + indexed_keys.append_serde(token_id); + utils::assert_indexed_keys(event, indexed_keys.span()); } fn assert_event_transfer(from: ContractAddress, to: ContractAddress, token_id: u256) { @@ -1421,4 +1435,11 @@ fn assert_event_transfer(from: ContractAddress, to: ContractAddress, token_id: u assert(event.to == to, 'Invalid `to`'); assert(event.token_id == token_id, 'Invalid `token_id`'); utils::assert_no_events_left(ZERO()); + + // Check indexed keys + let mut indexed_keys = array![]; + indexed_keys.append_serde(from); + indexed_keys.append_serde(to); + indexed_keys.append_serde(token_id); + utils::assert_indexed_keys(event, indexed_keys.span()); } From 8f939d027b02845868267c7e129f0404ce24220f Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 27 Sep 2023 12:20:55 -0400 Subject: [PATCH 13/16] fix formatting --- src/tests/utils.cairo | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index 7d522c990..ea519cae6 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -38,7 +38,9 @@ fn pop_log, impl TEvent: starknet::Event>( /// Asserts that `expected_keys` exactly matches the indexed keys from `event`. /// `expected_keys` must include all indexed event keys for `event` in the order /// that they're defined. -fn assert_indexed_keys, impl TEvent: starknet::Event>(event: T, expected_keys: Span) { +fn assert_indexed_keys, impl TEvent: starknet::Event>( + event: T, expected_keys: Span +) { let mut keys = array![]; let mut data = array![]; From 3da16045c4b388e349d5fd70395d809b317245e5 Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 27 Sep 2023 12:30:23 -0400 Subject: [PATCH 14/16] tidy up code --- src/tests/utils.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index d73738e7f..8a055c907 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -43,7 +43,7 @@ fn assert_indexed_keys, impl TEvent: starknet::Event>( let mut keys = array![]; let mut data = array![]; - starknet::event::Event::append_keys_and_data(@event, ref keys, ref data); + event.append_keys_and_data(ref keys, ref data); assert(expected_keys == keys.span(), 'Invalid keys'); } From 53523df1619b44d3d06a8b419d5b41673cca5e4e Mon Sep 17 00:00:00 2001 From: Andrew Fleming Date: Fri, 29 Sep 2023 02:05:19 -0400 Subject: [PATCH 15/16] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martín Triay --- src/tests/utils.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/utils.cairo b/src/tests/utils.cairo index 8a055c907..74ce5893a 100644 --- a/src/tests/utils.cairo +++ b/src/tests/utils.cairo @@ -18,7 +18,7 @@ fn deploy(contract_class_hash: felt252, calldata: Array) -> ContractAdd /// Pop the earliest unpopped logged event for the contract as the requested type /// and checks there's no more data left on the event, preventing unaccounted params. -/// This functions also removes the first key from the event. This is because indexed +/// This function also removes the first key from the event. This is because indexed /// params are set as event keys, but the first event key is always set as the /// event ID. fn pop_log, impl TEvent: starknet::Event>( From c6a24ff9ba3ccf20b1264cfe3a38dee3c9ef2b33 Mon Sep 17 00:00:00 2001 From: Andrew Date: Fri, 29 Sep 2023 02:08:31 -0400 Subject: [PATCH 16/16] remove import --- src/tests/upgrades/test_upgradeable.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/upgrades/test_upgradeable.cairo b/src/tests/upgrades/test_upgradeable.cairo index 68d7f6552..fff1d8ed6 100644 --- a/src/tests/upgrades/test_upgradeable.cairo +++ b/src/tests/upgrades/test_upgradeable.cairo @@ -10,7 +10,6 @@ use openzeppelin::upgrades::upgradeable::Upgradeable::Upgraded; use starknet::ClassHash; use starknet::ContractAddress; use starknet::Felt252TryIntoClassHash; -use starknet::testing; const VALUE: felt252 = 123;