From 95d326a064eb4912428a4a4b724e100a1b34b6e2 Mon Sep 17 00:00:00 2001 From: Patrice Tisserand Date: Tue, 23 Apr 2024 00:52:11 +0200 Subject: [PATCH 1/4] feat(starknet): add `set_base_uri` and `set_token_uri` in ERC721Bridgeable contract --- .../src/token/erc721_bridgeable.cairo | 112 +++++++++++++++++- .../starknet/src/token/interfaces.cairo | 7 ++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/apps/blockchain/starknet/src/token/erc721_bridgeable.cairo b/apps/blockchain/starknet/src/token/erc721_bridgeable.cairo index b6938697..9ec9419c 100644 --- a/apps/blockchain/starknet/src/token/erc721_bridgeable.cairo +++ b/apps/blockchain/starknet/src/token/erc721_bridgeable.cairo @@ -9,7 +9,7 @@ mod erc721_bridgeable { use openzeppelin::token::erc721::ERC721Component; use openzeppelin::access::ownable::OwnableComponent; - use starklane::token::interfaces::{IERC721Bridgeable, IERC721Mintable}; + use starklane::token::interfaces::{IERC721Bridgeable, IERC721Mintable, IERC721Uri}; use starklane::interfaces::IUpgradeable; component!(path: ERC721Component, storage: erc721, event: ERC721Event); @@ -154,6 +154,24 @@ mod erc721_bridgeable { self.token_uris.write(token_id, token_uri); } } + + #[abi(embed_v0)] + impl ERC721UriImpl of IERC721Uri { + fn base_uri(self: @ContractState) -> ByteArray { + self.erc721._base_uri() + } + + fn set_base_uri(ref self: ContractState, base_uri: ByteArray) { + self.ownable.assert_only_owner(); + self.erc721._set_base_uri(base_uri); + } + + fn set_token_uri(ref self: ContractState, token_id: u256, token_uri: ByteArray) { + self.ownable.assert_only_owner(); + assert(self.erc721._exists(token_id), 'ERC721: invalid token ID'); + self.token_uris.write(token_id, token_uri); + } + } } #[cfg(test)] @@ -168,6 +186,7 @@ mod tests { IERC721BridgeableDispatcher, IERC721BridgeableDispatcherTrait, IERC721Dispatcher, IERC721DispatcherTrait, IERC721MintableDispatcher, IERC721MintableDispatcherTrait, + IERC721UriDispatcher, IERC721UriDispatcherTrait, }; use starklane::token::collection_manager; @@ -402,4 +421,95 @@ mod tests { ownable.transfer_ownership(BOB); stop_prank(CheatTarget::One(contract_address)); } + + #[test] + fn test_set_base_uri() { + let COLLECTION_OWNER = collection_owner_addr_mock(); + let new_uri = "https://this.is.a.test.com"; + let contract_address = deploy_everai_collection(); + + let contract = IERC721UriDispatcher { contract_address}; + assert_eq!(contract.base_uri(), "https://my.base.uri"); + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + contract.set_base_uri(new_uri.clone()); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(contract.base_uri(), new_uri); + } + + #[test] + #[should_panic(expected: ('Caller is not the owner',))] + fn should_panic_set_base_uri_not_owner() { + let COLLECTION_OWNER = collection_owner_addr_mock(); + let ALICE = starknet::contract_address_const::<'alice'>(); + + let contract_address = deploy_everai_collection(); + + let ownable = IOwnableTwoStepDispatcher { contract_address }; + assert_eq!(ownable.owner(), COLLECTION_OWNER, "bad owner"); + + start_prank(CheatTarget::One(contract_address), ALICE); + IERC721UriDispatcher { contract_address}.set_base_uri("https://this.is.a.test.com"); + stop_prank(CheatTarget::One(contract_address)); + } + + #[test] + fn test_set_token_uri() { + let COLLECTION_OWNER = collection_owner_addr_mock(); + let ALICE = starknet::contract_address_const::<'alice'>(); + let new_uri = "https://this.is.a.test.com/68"; + let contract_address = deploy_everai_collection(); + let token_id = 42_u256; + + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + IERC721MintableDispatcher { contract_address}.mint(ALICE, token_id); + stop_prank(CheatTarget::One(contract_address)); + assert!(IERC721Dispatcher {contract_address}.token_uri(token_id) != new_uri.clone()); + + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + IERC721UriDispatcher { contract_address}.set_token_uri(token_id, new_uri.clone()); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(IERC721Dispatcher {contract_address}.token_uri(token_id), new_uri); + } + + #[test] + #[should_panic(expected: 'ERC721: invalid token ID',)] + fn test_set_token_uri_for_invalid_token_id() { + let COLLECTION_OWNER = collection_owner_addr_mock(); + let ALICE = starknet::contract_address_const::<'alice'>(); + let new_uri = "https://this.is.a.test.com/68"; + let contract_address = deploy_everai_collection(); + let token_id = 42_u256; + let invalid_token_id = 68_u256; + + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + IERC721MintableDispatcher { contract_address}.mint(ALICE, token_id); + stop_prank(CheatTarget::One(contract_address)); + assert!(IERC721Dispatcher {contract_address}.token_uri(token_id) != new_uri.clone()); + + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + IERC721UriDispatcher { contract_address}.set_token_uri(invalid_token_id, new_uri.clone()); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(IERC721Dispatcher {contract_address}.token_uri(token_id), new_uri); + } + + #[test] + #[should_panic(expected: 'Caller is not the owner',)] + fn should_panic_set_token_uri_not_owner_of_collection() { + let COLLECTION_OWNER = collection_owner_addr_mock(); + let ALICE = starknet::contract_address_const::<'alice'>(); + let new_uri = "https://this.is.a.test.com/68"; + let contract_address = deploy_everai_collection(); + let token_id = 42_u256; + + start_prank(CheatTarget::One(contract_address), COLLECTION_OWNER); + IERC721MintableDispatcher { contract_address}.mint(ALICE, token_id); + stop_prank(CheatTarget::One(contract_address)); + assert!(IERC721Dispatcher {contract_address}.token_uri(token_id) != new_uri.clone()); + + start_prank(CheatTarget::One(contract_address), ALICE); + IERC721UriDispatcher { contract_address}.set_token_uri(token_id, new_uri.clone()); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(IERC721Dispatcher {contract_address}.token_uri(token_id), new_uri); + } + } diff --git a/apps/blockchain/starknet/src/token/interfaces.cairo b/apps/blockchain/starknet/src/token/interfaces.cairo index 0fd8814c..5d27fcf2 100644 --- a/apps/blockchain/starknet/src/token/interfaces.cairo +++ b/apps/blockchain/starknet/src/token/interfaces.cairo @@ -13,6 +13,13 @@ trait IERC721 { fn approve(ref self: T, to: ContractAddress, token_id: u256); } +#[starknet::interface] +trait IERC721Uri { + fn base_uri(self: @T) -> ByteArray; + fn set_base_uri(ref self: T, base_uri: ByteArray); + fn set_token_uri(ref self: T, token_id: u256, token_uri: ByteArray); +} + #[starknet::interface] trait IERC721Mintable { fn mint(ref self: T, to: ContractAddress, token_id: u256); From 6677e332886ddb13bba8e675134234ce5f46e406 Mon Sep 17 00:00:00 2001 From: Patrice Tisserand Date: Tue, 23 Apr 2024 02:26:44 +0200 Subject: [PATCH 2/4] feat(starknet): use OZ two steps Ownable implementation for bridge contract --- apps/blockchain/starknet/src/bridge.cairo | 20 ++++++--- .../starknet/src/tests/bridge_t.cairo | 44 ++++++++++++++++++- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/apps/blockchain/starknet/src/bridge.cairo b/apps/blockchain/starknet/src/bridge.cairo index a578d4e8..d9ccd124 100644 --- a/apps/blockchain/starknet/src/bridge.cairo +++ b/apps/blockchain/starknet/src/bridge.cairo @@ -14,6 +14,7 @@ mod bridge { use starknet::contract_address::ContractAddressZeroable; use starknet::eth_address::EthAddressZeroable; + use openzeppelin::access::ownable::OwnableComponent; use openzeppelin::access::ownable::interface::{ IOwnableDispatcher, IOwnableDispatcherTrait }; @@ -49,10 +50,15 @@ mod bridge { use poseidon::poseidon_hash_span; + component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); + + #[abi(embed_v0)] + impl OwnableTwoStepMixinImpl = OwnableComponent::OwnableTwoStepMixinImpl; + + impl OwnableInternalImpl = OwnableComponent::InternalImpl; + #[storage] struct Storage { - // Bridge administrator. - bridge_admin: ContractAddress, // Bridge address on L1 (to allow it to consume messages). bridge_l1_address: EthAddress, // The class to deploy for ERC721 tokens. @@ -72,6 +78,9 @@ mod bridge { // Bridge enabled flag enabled: bool, + + #[substorage(v0)] + ownable: OwnableComponent::Storage, } #[constructor] @@ -81,8 +90,7 @@ mod bridge { bridge_l1_address: EthAddress, erc721_bridgeable_class: ClassHash, ) { - self.bridge_admin.write(bridge_admin); - + self.ownable.initializer(bridge_admin); // TODO: add validation of inputs. self.bridge_l1_address.write(bridge_l1_address); self.erc721_bridgeable_class.write(erc721_bridgeable_class); @@ -98,6 +106,8 @@ mod bridge { WithdrawRequestCompleted: WithdrawRequestCompleted, ReplacedClassHash: ReplacedClassHash, BridgeEnabled: BridgeEnabled, + #[flat] + OwnableEvent: OwnableComponent::Event, } @@ -348,7 +358,7 @@ mod bridge { /// Ensures the caller is the bridge admin. Revert if it's not. fn ensure_is_admin(self: @ContractState) { - assert(starknet::get_caller_address() == self.bridge_admin.read(), 'Unauthorized call'); + self.ownable.assert_only_owner(); } /// Ensures the bridge is enabled diff --git a/apps/blockchain/starknet/src/tests/bridge_t.cairo b/apps/blockchain/starknet/src/tests/bridge_t.cairo index a43db473..d6443631 100644 --- a/apps/blockchain/starknet/src/tests/bridge_t.cairo +++ b/apps/blockchain/starknet/src/tests/bridge_t.cairo @@ -709,7 +709,7 @@ mod tests { } #[test] - #[should_panic] + #[should_panic(expected: ('Caller is not the owner',))] fn upgrade_as_not_admin() { let erc721b_contract_class = declare("erc721_bridgeable"); @@ -724,6 +724,46 @@ mod tests { stop_prank(CheatTarget::One(bridge_address)); } + #[test] + fn support_two_step_transfer_ownership() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + let ALICE = starknet::contract_address_const::<'alice'>(); + let contract_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let ownable = IOwnableTwoStepDispatcher { contract_address}; + + assert_eq!(ownable.owner(), BRIDGE_ADMIN, "bad owner"); + start_prank(CheatTarget::One(contract_address), BRIDGE_ADMIN); + ownable.transfer_ownership(ALICE); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(ownable.owner(), BRIDGE_ADMIN, "bad owner"); + assert_eq!(ownable.pending_owner(), ALICE, "bad pending owner"); + + start_prank(CheatTarget::One(contract_address), ALICE); + ownable.accept_ownership(); + stop_prank(CheatTarget::One(contract_address)); + assert_eq!(ownable.owner(), ALICE, "bad owner"); + } + + #[test] + #[should_panic(expected: ('Caller is not the owner',))] + fn should_panic_transfer_not_owner() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + let ALICE = starknet::contract_address_const::<'alice'>(); + let contract_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let ownable = IOwnableTwoStepDispatcher { contract_address}; + + assert_eq!(ownable.owner(), BRIDGE_ADMIN, "bad owner"); + start_prank(CheatTarget::One(contract_address), ALICE); + ownable.transfer_ownership(ALICE); + stop_prank(CheatTarget::One(contract_address)); + } + #[test] fn collection_upgrade_as_admin() { let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; @@ -790,7 +830,7 @@ mod tests { } #[test] - #[should_panic(expected: ('Unauthorized call',))] + #[should_panic(expected: ('Caller is not the owner',))] fn collection_transfer_ownership_as_not_admin() { let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; let OWNER_L1: EthAddress = 0xe00.try_into().unwrap(); From 1436145c63084a865d17c3f6293e304230411fb7 Mon Sep 17 00:00:00 2001 From: Patrice Tisserand Date: Wed, 24 Apr 2024 15:58:11 +0200 Subject: [PATCH 3/4] feat(starknet): add function to retrieve white list --- apps/blockchain/starknet/src/bridge.cairo | 87 ++++++++- apps/blockchain/starknet/src/interfaces.cairo | 1 + .../starknet/src/tests/bridge_t.cairo | 166 ++++++++++++++++++ 3 files changed, 248 insertions(+), 6 deletions(-) diff --git a/apps/blockchain/starknet/src/bridge.cairo b/apps/blockchain/starknet/src/bridge.cairo index d9ccd124..10559e65 100644 --- a/apps/blockchain/starknet/src/bridge.cairo +++ b/apps/blockchain/starknet/src/bridge.cairo @@ -73,9 +73,11 @@ mod bridge { // White list enabled flag white_list_enabled: bool, + // Registry of whitelisted collections - white_list: LegacyMap::, - + white_listed_list: LegacyMap::, + white_listed_head: ContractAddress, + // Bridge enabled flag enabled: bool, @@ -312,13 +314,31 @@ mod bridge { fn white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) { ensure_is_admin(@self); - self.white_list.write(collection, enabled); + _white_list_collection(ref self, collection, enabled); } fn is_white_listed(self: @ContractState, collection: ContractAddress) -> bool { _is_white_listed(self, collection) } + fn get_white_listed_collections(self: @ContractState) -> Span { + let mut white_listed = array![]; + let mut current = self.white_listed_head.read(); + loop { + if current.is_zero() { + break; + } + let (enabled, next) = self.white_listed_list.read(current); + if !enabled { + break; + } else { + white_listed.append(current); + current = next; + } + }; + white_listed.span() + } + fn enable(ref self: ContractState, enable: bool) { ensure_is_admin(@self); self.enabled.write(enable); @@ -442,8 +462,9 @@ mod bridge { ); // update whitelist if needed - if self.white_list.read(l2_addr_from_deploy) != true { - self.white_list.write(l2_addr_from_deploy, true); + let (already_white_listed, _) = self.white_listed_list.read(l2_addr_from_deploy); + if already_white_listed != true { + _white_list_collection(ref self, l2_addr_from_deploy, true); } l2_addr_from_deploy } @@ -451,8 +472,62 @@ mod bridge { fn _is_white_listed(self: @ContractState, collection: ContractAddress) -> bool { let enabled = self.white_list_enabled.read(); if (enabled) { - return self.white_list.read(collection); + let (ret, _) = self.white_listed_list.read(collection); + return ret; } true } + + fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) { + let no_value = starknet::contract_address_const::<0>(); + let (current, _) = self.white_listed_list.read(collection); + if current != enabled { + let mut prev = self.white_listed_head.read(); + if enabled { + self.white_listed_list.write(collection, (enabled, no_value)); + if prev.is_zero() { + self.white_listed_head.write(collection); + return; + } + // find last element + loop { + let (_, next) = self.white_listed_list.read(prev); + if next.is_zero() { + break; + } + let (active, _) = self.white_listed_list.read(next); + if !active { + break; + } + prev = next; + }; + self.white_listed_list.write(prev, (true, collection)); + } else { + // change head + if prev == collection { + let (_, next) = self.white_listed_list.read(prev); + self.white_listed_list.write(collection, (false, no_value)); + self.white_listed_head.write(next); + return; + } + // removed element from linked list + loop { + let (active, next) = self.white_listed_list.read(prev); + if next.is_zero() { + // end of list + break; + } + if !active { + break; + } + if next == collection { + let (_, target) = self.white_listed_list.read(collection); + self.white_listed_list.write(prev, (active, target)); + break; + } + }; + self.white_listed_list.write(collection, (false, no_value)); + } + } + } } diff --git a/apps/blockchain/starknet/src/interfaces.cairo b/apps/blockchain/starknet/src/interfaces.cairo index d5cb9df0..4aa939f9 100644 --- a/apps/blockchain/starknet/src/interfaces.cairo +++ b/apps/blockchain/starknet/src/interfaces.cairo @@ -28,6 +28,7 @@ trait IStarklane { fn is_white_list_enabled(self: @T) -> bool; fn white_list_collection(ref self: T, collection: ContractAddress, enabled: bool); fn is_white_listed(self: @T, collection: ContractAddress) -> bool; + fn get_white_listed_collections(self: @T) -> Span; fn enable(ref self: T, enable: bool); fn is_enabled(self: @T) -> bool; diff --git a/apps/blockchain/starknet/src/tests/bridge_t.cairo b/apps/blockchain/starknet/src/tests/bridge_t.cairo index d6443631..07303513 100644 --- a/apps/blockchain/starknet/src/tests/bridge_t.cairo +++ b/apps/blockchain/starknet/src/tests/bridge_t.cairo @@ -513,6 +513,8 @@ mod tests { assert!(erc721.owner_of(1) == OWNER_L2, "Wrong owner after req"); assert!(bridge.is_white_listed(deployed_address), "Collection shall be whitelisted"); + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(*white_listed.at(0), deployed_address, "Collection whitelisted shall be in list"); } #[test] @@ -605,6 +607,170 @@ mod tests { stop_prank(CheatTarget::One(bridge_address)); } + #[test] + fn whitelist_collection_is_empty_by_default() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + + let bridge_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let bridge = IStarklaneDispatcher { contract_address: bridge_address }; + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.enable_white_list(true); + stop_prank(CheatTarget::One(bridge_address)); + assert!(bridge.get_white_listed_collections().is_empty(), "White list shall be empty by default"); + } + + #[test] + fn whitelist_collection_is_updated_when_collection_is_added() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + + let bridge_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let bridge = IStarklaneDispatcher { contract_address: bridge_address }; + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.enable_white_list(true); + stop_prank(CheatTarget::One(bridge_address)); + + let collection1 = starknet::contract_address_const::<'collection1'>(); + let collection2 = starknet::contract_address_const::<'collection2'>(); + let collection3 = starknet::contract_address_const::<'collection3'>(); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 1, "White list shall contain 1 element"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection2, true); + bridge.white_list_collection(collection3, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 3, "White list shall contain 3 elements"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection2, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(2), collection3, "Wrong collection address in white list"); + assert!(bridge.is_white_listed(collection1), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection2), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection2, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 3, "White list shall contain 3 elements"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection2, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(2), collection3, "Wrong collection address in white list"); + assert!(bridge.is_white_listed(collection1), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection2), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + } + + #[test] + fn whitelist_collection_is_updated_when_collection_is_removed() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + + let bridge_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let bridge = IStarklaneDispatcher { contract_address: bridge_address }; + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.enable_white_list(true); + stop_prank(CheatTarget::One(bridge_address)); + + let collection1 = starknet::contract_address_const::<'collection1'>(); + let collection2 = starknet::contract_address_const::<'collection2'>(); + let collection3 = starknet::contract_address_const::<'collection3'>(); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, true); + bridge.white_list_collection(collection2, true); + bridge.white_list_collection(collection3, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 3, "White list shall contain 3 elements"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection2, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(2), collection3, "Wrong collection address in white list"); + assert!(bridge.is_white_listed(collection1), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection2), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection2, false); + stop_prank(CheatTarget::One(bridge_address)); + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 2, "White list shall contain 2 elements"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection3, "Wrong collection address in white list"); + assert!(bridge.is_white_listed(collection1), "Collection1 should be whitelisted"); + assert!(!bridge.is_white_listed(collection2), "Collection1 should not be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, false); + bridge.white_list_collection(collection3, false); + stop_prank(CheatTarget::One(bridge_address)); + let white_listed = bridge.get_white_listed_collections(); + assert!(white_listed.is_empty(), "White list shall be empty"); + assert!(!bridge.is_white_listed(collection1), "Collection1 should not be whitelisted"); + assert!(!bridge.is_white_listed(collection2), "Collection1 should not be whitelisted"); + assert!(!bridge.is_white_listed(collection3), "Collection1 should not be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, true); + bridge.white_list_collection(collection3, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 2, "White list shall contain 2 elements"); + assert_eq!(*white_listed.at(0), collection1, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection3, "Wrong collection address in white list"); + assert!(bridge.is_white_listed(collection1), "Collection1 should be whitelisted"); + assert!(!bridge.is_white_listed(collection2), "Collection1 should not be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, false); + bridge.white_list_collection(collection2, true); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert_eq!(white_listed.len(), 2, "White list shall contain 2 elements"); + assert_eq!(*white_listed.at(0), collection3, "Wrong collection address in white list"); + assert_eq!(*white_listed.at(1), collection2, "Wrong collection address in white list"); + assert!(!bridge.is_white_listed(collection1), "Collection1 should not be whitelisted"); + assert!(bridge.is_white_listed(collection2), "Collection1 should be whitelisted"); + assert!(bridge.is_white_listed(collection3), "Collection1 should be whitelisted"); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection2, false); + bridge.white_list_collection(collection3, false); + bridge.white_list_collection(collection1, false); + bridge.white_list_collection(collection1, false); + stop_prank(CheatTarget::One(bridge_address)); + + let white_listed = bridge.get_white_listed_collections(); + assert!(white_listed.is_empty(), "White list shall be empty"); + assert!(!bridge.is_white_listed(collection1), "Collection1 should not be whitelisted"); + assert!(!bridge.is_white_listed(collection2), "Collection1 should not be whitelisted"); + assert!(!bridge.is_white_listed(collection3), "Collection1 should not be whitelisted"); + } + #[test] #[should_panic] fn deposit_token_not_enabled() { From 1dc38d14073fffb4e75b5e2b6906beca994e0e66 Mon Sep 17 00:00:00 2001 From: Patrice Tisserand Date: Wed, 24 Apr 2024 17:20:22 +0200 Subject: [PATCH 4/4] feat(starknet): emit event when collection whitelist list is updated --- apps/blockchain/starknet/src/bridge.cairo | 10 ++++ apps/blockchain/starknet/src/interfaces.cairo | 9 ++- .../starknet/src/tests/bridge_t.cairo | 60 +++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/apps/blockchain/starknet/src/bridge.cairo b/apps/blockchain/starknet/src/bridge.cairo index 10559e65..23cbf8af 100644 --- a/apps/blockchain/starknet/src/bridge.cairo +++ b/apps/blockchain/starknet/src/bridge.cairo @@ -29,6 +29,7 @@ mod bridge { CollectionDeployedFromL1, ReplacedClassHash, BridgeEnabled, + CollectionWhiteListUpdated, }; use starklane::request::{ @@ -108,6 +109,7 @@ mod bridge { WithdrawRequestCompleted: WithdrawRequestCompleted, ReplacedClassHash: ReplacedClassHash, BridgeEnabled: BridgeEnabled, + CollectionWhiteListUpdated: CollectionWhiteListUpdated, #[flat] OwnableEvent: OwnableComponent::Event, } @@ -315,6 +317,10 @@ mod bridge { fn white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) { ensure_is_admin(@self); _white_list_collection(ref self, collection, enabled); + self.emit(CollectionWhiteListUpdated { + collection, + enabled, + }); } fn is_white_listed(self: @ContractState, collection: ContractAddress) -> bool { @@ -465,6 +471,10 @@ mod bridge { let (already_white_listed, _) = self.white_listed_list.read(l2_addr_from_deploy); if already_white_listed != true { _white_list_collection(ref self, l2_addr_from_deploy, true); + self.emit(CollectionWhiteListUpdated { + collection: l2_addr_from_deploy, + enabled: true, + }); } l2_addr_from_deploy } diff --git a/apps/blockchain/starknet/src/interfaces.cairo b/apps/blockchain/starknet/src/interfaces.cairo index 4aa939f9..33264712 100644 --- a/apps/blockchain/starknet/src/interfaces.cairo +++ b/apps/blockchain/starknet/src/interfaces.cairo @@ -99,4 +99,11 @@ struct L1L2CollectionMappingUpdated { collection_l1: EthAddress, #[key] collection_l2: ContractAddress -} \ No newline at end of file +} + +#[derive(Drop, starknet::Event)] +struct CollectionWhiteListUpdated { + #[key] + collection: ContractAddress, + enabled: bool, +} \ No newline at end of file diff --git a/apps/blockchain/starknet/src/tests/bridge_t.cairo b/apps/blockchain/starknet/src/tests/bridge_t.cairo index 07303513..c7d1043b 100644 --- a/apps/blockchain/starknet/src/tests/bridge_t.cairo +++ b/apps/blockchain/starknet/src/tests/bridge_t.cairo @@ -771,6 +771,66 @@ mod tests { assert!(!bridge.is_white_listed(collection3), "Collection1 should not be whitelisted"); } + #[test] + fn whitelist_collection_update_events() { + let erc721b_contract_class = declare("erc721_bridgeable"); + + let BRIDGE_ADMIN = starknet::contract_address_const::<'starklane'>(); + let BRIDGE_L1 = EthAddress { address: 'starklane_l1' }; + + let bridge_address = deploy_starklane(BRIDGE_ADMIN, BRIDGE_L1, erc721b_contract_class.class_hash); + let bridge = IStarklaneDispatcher { contract_address: bridge_address }; + + let collection1 = starknet::contract_address_const::<'collection1'>(); + let collection2 = starknet::contract_address_const::<'collection2'>(); + let collection3 = starknet::contract_address_const::<'collection3'>(); + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.enable_white_list(true); + stop_prank(CheatTarget::One(bridge_address)); + + let mut spy = spy_events(SpyOn::One(bridge_address)); + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection1, true); + stop_prank(CheatTarget::One(bridge_address)); + spy.assert_emitted(@array![ + ( + bridge_address, + bridge::Event::CollectionWhiteListUpdated( + bridge::CollectionWhiteListUpdated { + collection: collection1, + enabled: true, + } + ) + ) + ]); + + start_prank(CheatTarget::One(bridge_address), BRIDGE_ADMIN); + bridge.white_list_collection(collection2, true); + bridge.white_list_collection(collection1, false); + stop_prank(CheatTarget::One(bridge_address)); + spy.assert_emitted(@array![ + ( + bridge_address, + bridge::Event::CollectionWhiteListUpdated( + bridge::CollectionWhiteListUpdated { + collection: collection2, + enabled: true, + } + ) + ), + ( + bridge_address, + bridge::Event::CollectionWhiteListUpdated( + bridge::CollectionWhiteListUpdated { + collection: collection1, + enabled: false, + } + ) + ) + ]); + + } + #[test] #[should_panic] fn deposit_token_not_enabled() {