Document SRC5 migration

CHANGELOG.md

@@ -10,3 +10,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Utilities documentation (#825)
+- Documentation for SRC5 migration (#821)

docs/modules/ROOT/nav.adoc

@@ -25,7 +25,7 @@
 ** xref:/api/security.adoc[API Reference]
 
 * xref:introspection.adoc[Introspection]
-** xref:/guides/src5-migration.adoc[SRC5 Migration]
+** xref:/guides/src5-migration.adoc[Migrating ERC165 to SRC5]
 ** xref:/api/introspection.adoc[API Reference]
 
 // * xref:udc.adoc[Universal Deployer Contract]

docs/modules/ROOT/pages/guides/src5-migration.adoc

@@ -1,37 +1,50 @@
-= SRC5 Migration
+= Migrating ERC165 to SRC5
 
 :eip165: https://eips.ethereum.org/EIPS/eip-165[EIP-165]
 :snip5: https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-5.md[SNIP-5]
 :dual-interface-discussion: https://github.com/OpenZeppelin/cairo-contracts/discussions/640[Dual Introspection Detection]
 :shamans-proposal: https://community.starknet.io/t/starknet-standard-interface-detection/92664[Starknet Shamans proposal]
 
 In the smart contract ecosystem, having the ability to query if a contract supports a given interface is an extremely important feature.
-The initial introspection design in Cairo v0 followed Ethereum's {eip165}.
-Since the language evolved and acquired some stability, we're now migrating to the {snip5} standard for introspection.
+The initial introspection design for Contracts for Cairo before version v0.7.0 followed Ethereum's {eip165}.
+Since the Cairo language evolved introducing native types, we needed an introspection solution tailored to the Cairo ecosystem: the {snip5} standard.
 SNIP-5 allows interface ID calculations to use Cairo types and the Starknet keccak (`sn_keccak`) function.
 For more information on the decision, see the {shamans-proposal} or the {dual-interface-discussion} discussion.
 
-== Migrating to SRC5
+== How to migrate
 
-:src5-component: xref:api/introspection#SRC5Component[SRC5Component]
-:upgradeable-component: xref:api/upgrades#UpgradeableComponent[UpgradeableComponent]
-:isrc6: xref:api/account.adoc#ISRC6[ISRC6]
-:src5-rs: https://github.com/ericnordelo/src5-rs[src5-rs]
+Migrating from ERC165 to SRC5 involves four major steps:
 
-This guide is for already-deployed, upgradeable contracts that support ERC165.
-Migrating to SRC5 requires integrating {src5-component} and {upgradeable-component}.
-The following template assumes that the upgradeable mechanism uses the `upgrade` selector:
+1. Integrate SRC5 into the contract.
+2. Create functions to deregister ERC165 IDs and reregister SRC5 IDs.
+3. Create a `migration_initializer` to call the registration functions.
+4. Upgrade the contract and call `migration_initializer`.
+
+The following guide will go through the steps with examples.
+
+=== Component integration
+
+:src5-component: xref:/api/introspection.adoc#SRC5Component[SRC5Component]
+:upgradeable-component: xref:/api/upgrades.adoc#UpgradeableComponent[UpgradeableComponent]
+:initializable-component: xref:/api/security.adoc#InitializableComponent[InitializableComponent]
+
+The first step is to integrate the necessary components into the new contract.
+The contract should include the new introspection mechanism, {src5-component}.
+It should also include the {upgradeable-component} and {initializable-component} which will be used in the migration initializer.
+Here's the setup:
 
 [,javascript]
 ----
 #[starknet::contract]
 mod MigratingContract {
     use openzeppelin::introspection::src5::SRC5Component;
+    use openzeppelin::security::initializable::InitializableComponent;
     use openzeppelin::upgrades::UpgradeableComponent;
     use starknet::ClassHash;
     use starknet::ContractAddress;
 
     component!(path: SRC5Component, storage: src5, event: SRC5Event);
+    component!(path: InitializableComponent, storage: initializable, event: InitializableEvent);
     component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent);
 
     // SRC5
@@ -41,6 +54,9 @@ mod MigratingContract {
     impl SRC5CamelOnlyImpl = SRC5Component::SRC5CamelImpl<ContractState>;
     impl SRC5InternalImpl = SRC5Component::InternalImpl<ContractState>;
 
+    // Initializable
+    impl InitializableInternalImpl = InitializableComponent::InternalImpl<ContractState>;
+
     // Upgradeable
     impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl<ContractState>;
 
@@ -49,6 +65,8 @@ mod MigratingContract {
         #[substorage(v0)]
         src5: SRC5Component::Storage,
         #[substorage(v0)]
+        initializable: InitializableComponent::Storage,
+        #[substorage(v0)]
         upgradeable: UpgradeableComponent::Storage,
     }
 
@@ -58,40 +76,98 @@ mod MigratingContract {
         #[flat]
         SRC5Event: SRC5Component::Event,
         #[flat]
+        InitializableEvent: InitializableComponent::Event,
+        #[flat]
         UpgradeableEvent: UpgradeableComponent::Event
     }
+}
+----
 
-    #[external(v0)]
-    fn upgrade(ref self: ContractState, new_class_hash: ClassHash) {
-        // Add permissions mechanism
-        self.upgradeable._upgrade(new_class_hash);
-    }
+=== Interface registration
+
+:ierc721: xref:/api/erc721.adoc#IERC721[IERC721]
+:ierc721-metadata: xref:/api/erc721.adoc#IERC721Metadata[IERC721Metadata]
+:deregister-function: xref:/utilities.adoc#utils-deregister_erc165_interface[deregister_erc165_interface]
+:storage-write-syscall: https://docs.starknet.io/documentation/architecture_and_concepts/Smart_Contracts/system-calls-cairo1/#storage_write[storage_write_syscall]
+
+To successfully migrate ERC165 to SRC5, the contract needs to both deregister support for ERC165 interface IDs and reregister support for those interfaces with SRC5.
 
-    // The rest of the contract logic
+For this example, let's say that this contract supports the {ierc721} and {ierc721-metadata} interfaces.
+The contract should implement an `InternalImpl` and add functions to deregister and reregister  support for those interfaces like this:
+
+[,javascript]
+----
+#[starknet::contract]
+mod MigratingContract {
+    use openzeppelin::utils;
+    use openzeppelin::token::erc721::interface::{IERC721_ID, IERC721Metadata_ID};
+
+    (...)
+
+    #[generate_trait]
+    impl InternalImpl of InternalTrait {
+        // Delete ERC165 interface support from storage
+        fn deregister_erc165_interfaces(ref self: ContractState) {
+            let ERC165_IERC721_ID = 0x80ac58cd;
+            let ERC165_IERC721_METADATA_ID = 0x5b5e139f;
+
+            utils::deregister_erc165_interface(ERC165_IERC721_ID);
+            utils::deregister_erc165_interface(ERC165_IERC721_METADATA_ID);
+        }
+
+        // Reregister SRC5 interfaces
+        fn register_src5_interfaces(ref self: ContractState) {
+            self.src5.register_interface(IERC721_ID);
+            self.src5.register_interface(IERC721Metadata_ID);
+        }
+    }
 }
 ----
 
-The contract will also need to register the new SRC5 interface IDs in order to continue declaring support for those interfaces.
-We can create a migration initializer to handle this which needs to be called after the contract upgrade.
+==== Deregistering IDs
+
+A contract normally includes the storage variable in the `Storage` struct in order to access it.
+The new contract, however, will no longer be reading from ERC165's storage address because it's reading from SRC5's storage address.
+To avoid adding ERC165's storage variable to the new contract's storage, the contract can utilize {deregister-function}.
+This utility function uses the {storage-write-syscall} to manually delete the passed `erc165_id` from the ERC165 storage address.
+
+==== Reregistering IDs
+
+:register_interface: xref:/api/introspection.adoc#SRC5Component-register_interface[register_interface]
 
-TIP: OpenZeppelin's Contracts for Cairo already provides the interface IDs of common interfaces in the API section (like {isrc6}).
-For interfaces not provided, tools such as {src5-rs} can help with interface ID calculation.
+Since the new contract integrates `SRC5Component`, it can leverage SRC5's {register_interface} function to reregister supported interfaces.
+
+=== Migration initializer
+
+:access-control: xref:/access.adoc[Access Control]
+
+Next, the contract should define and expose a migration initializer that will invoke the deregister and reregister functions.
+Since the initializer will be publicly callable, it should include some sort of {access-control} so that only permitted addresses can call the function.
+Finally, the initializer should include a reinitialization check to ensure that it cannot be called more than once.
 
 [,javascript]
 ----
 #[starknet::contract]
 mod MigratingContract {
-
     (...)
 
     #[external(v0)]
-    fn migration_initializer(ref self: ContractState, interface_ids: Span<felt252>) {
-        // Add permissions mechanism
-        self.src5.register_interfaces(interface_ids);
+    fn migration_initializer(ref self: ContractState) {
+        // Add appropriate Access Control mechanism
+
+        // Invoke `initialize` so this function can only be called once
+        self.initializable.initialize();
+
+        // Deregister ERC165 IDs
+        self.deregister_erc165_interfaces();
+
+        // Reregister SRC5 IDs
+        self.register_src5_interfaces();
     }
 }
 ----
 
-Note that deployed contracts with upgradeability should also be carefully tested before migrating.
-Upgradeable contracts will likely include some form of a permissions mechanism for upgrading contracts.
-Take extreme care with ensuring admins are not inadvertantly stripped of their permissions.
+=== Execute migration
+
+Once the new contract is prepared for migration and *rigorously tested*, all that's left is to migrate!
+Simply upgrade the contract and then call the `migration_initializer`.

docs/modules/ROOT/pages/utilities.adoc

@@ -22,6 +22,7 @@ Module containing core utilities of the library.
 --
 .Functions
 * xref:#utils-try_selector_with_fallback[`++try_selector_with_fallback(target, selector, fallback, args)++`]
+* xref:#utils-deregister_erc165_interface[`++deregister_erc165_interface(erc165_id)++`]
 
 .Traits
 * xref:#utils-UnwrapAndCast[`++UnwrapAndCast++`]
@@ -58,6 +59,14 @@ error is returned.
 WARNING: The fallback mechanism won't work on live chains (mainnet or testnets) until
 they implement panic handling in their runtime.
 
+[.contract-item]
+[[utils-deregister_erc165_interface]]
+==== `[.contract-item-name]#++deregister_erc165_interface++#++(erc165_id: felt252)++` [.item-kind]#function#
+
+Manually deletes `erc165_id` value from the storage variable `ERC165_supported_interfaces` in the calling contract's storage.
+
+This function should only be used by a migration initializer in the ERC165-to-SRC5 migration process.
+
 [#utils-Traits]
 ==== Traits
 

src/introspection/src5.cairo

@@ -50,20 +50,6 @@ mod SRC5Component {
             self.SRC5_supported_interfaces.write(interface_id, true);
         }
 
-        // Registers the given interfaces as supported by the contract.
-        fn register_interfaces(
-            ref self: ComponentState<TContractState>, mut interface_ids: Span<felt252>
-        ) {
-            loop {
-                if interface_ids.len() == 0 {
-                    break;
-                }
-
-                let id = *interface_ids.pop_front().unwrap();
-                self.register_interface(id);
-            }
-        }
-
         /// Deregisters the given interface as supported by the contract.
         fn deregister_interface(ref self: ComponentState<TContractState>, interface_id: felt252) {
             assert(interface_id != interface::ISRC5_ID, Errors::INVALID_ID);

src/tests/introspection/test_erc165_deregister.cairo

@@ -0,0 +1,42 @@
+use openzeppelin::tests::mocks::erc165_deregister_mock::ERC165DeregisterMock::ERC165DeregisterMockImpl;
+use openzeppelin::tests::mocks::erc165_deregister_mock::ERC165DeregisterMock;
+
+const ID_1: felt252 = 0x12345678;
+const ID_2: felt252 = 0x87654321;
+
+fn STATE() -> ERC165DeregisterMock::ContractState {
+    ERC165DeregisterMock::contract_state_for_testing()
+}
+
+#[test]
+#[available_gas(2000000)]
+fn test_deregister_interface() {
+    let mut state = STATE();
+
+    state.register_interface(ID_1);
+    state.register_interface(ID_2);
+
+    assert(state.supports_interface(ID_1), 'Should support ID_1');
+    assert(state.supports_interface(ID_2), 'Should support ID_2');
+
+    state.deregister_erc165_interface(ID_1);
+    state.deregister_erc165_interface(ID_2);
+
+    assert(!state.supports_interface(ID_1), 'Should not support ID_1');
+    assert(!state.supports_interface(ID_2), 'Should not support ID_1');
+}
+
+#[test]
+#[available_gas(2000000)]
+fn test_deregister_interface_not_registered() {
+    let mut state = STATE();
+
+    assert(!state.supports_interface(ID_1), 'Should not support ID_1');
+    assert(!state.supports_interface(ID_2), 'Should not support ID_1');
+
+    state.deregister_erc165_interface(ID_1);
+    state.deregister_erc165_interface(ID_2);
+
+    assert(!state.supports_interface(ID_1), 'Should not support ID_1');
+    assert(!state.supports_interface(ID_2), 'Should not support ID_1');
+}

src/tests/introspection/test_src5.cairo

@@ -34,31 +34,6 @@ fn test_register_interface() {
     assert(supports_new_interface, 'Should support new interface');
 }
 
-#[test]
-#[available_gas(2000000)]
-fn test_register_interfaces() {
-    let mut state = STATE();
-    let ids = array![OTHER_ID, OTHER_ID_2];
-    state.src5.register_interfaces(ids.span());
-
-    let supports_id = state.src5.supports_interface(OTHER_ID);
-    assert(supports_id, 'Should support new interface');
-
-    let supports_id_2 = state.src5.supports_interface(OTHER_ID_2);
-    assert(supports_id_2, 'Should support new interface');
-}
-
-#[test]
-#[available_gas(2000000)]
-fn test_register_interfaces_one_id() {
-    let mut state = STATE();
-    let ids = array![OTHER_ID];
-    state.src5.register_interfaces(ids.span());
-
-    let supports_id = state.src5.supports_interface(OTHER_ID);
-    assert(supports_id, 'Should support new interface');
-}
-
 #[test]
 #[available_gas(2000000)]
 fn test_deregister_interface() {

src/tests/mocks.cairo

@@ -1,5 +1,6 @@
 mod accesscontrol_mocks;
 mod account_mocks;
+mod erc165_deregister_mock;
 mod erc20_mocks;
 mod erc721_mocks;
 mod erc721_receiver_mocks;

src/tests/mocks/erc165_deregister_mock.cairo

@@ -0,0 +1,29 @@
+#[starknet::contract]
+mod ERC165DeregisterMock {
+    use openzeppelin::utils;
+
+    #[storage]
+    struct Storage {
+        ERC165_supported_interfaces: LegacyMap<felt252, bool>,
+    }
+
+    #[event]
+    #[derive(Drop, starknet::Event)]
+    enum Event {}
+
+    #[generate_trait]
+    #[external(v0)]
+    impl ERC165DeregisterMockImpl of ExternalTrait {
+        fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
+            self.ERC165_supported_interfaces.read(interface_id)
+        }
+
+        fn register_interface(ref self: ContractState, interface_id: felt252) {
+            self.ERC165_supported_interfaces.write(interface_id, true);
+        }
+
+        fn deregister_erc165_interface(ref self: ContractState, interface_id: felt252) {
+            utils::deregister_erc165_interface(interface_id);
+        }
+    }
+}