Remo/bump tools

.github/workflows/on-pull-request.yaml

@@ -13,10 +13,10 @@ jobs:
       - uses: actions/checkout@v4
       - uses: foundry-rs/setup-snfoundry@v3
         with:
-          starknet-foundry-version: "0.55.0"
+          starknet-foundry-version: "0.60.0"
       - uses: software-mansion/setup-scarb@v1
         with:
-          scarb-version: "2.15.1"
+          scarb-version: "2.18.0"
 
       - name: Install cairo-coverage
         run: |

.github/workflows/publish-testing.yml

@@ -22,7 +22,7 @@ jobs:
 
       - uses: software-mansion/setup-scarb@v1
         with:
-          scarb-version: "2.11.4"
+          scarb-version: "2.18.0"
 
       - name: Extract current versions
         working-directory: ./packages/testing

.github/workflows/publish.yml

@@ -22,7 +22,7 @@ jobs:
 
       - uses: software-mansion/setup-scarb@v1
         with:
-          scarb-version: "2.11.4"
+          scarb-version: "2.18.0"
 
       - name: Extract publish version
         run: |

.tool-versions

@@ -1,2 +1,2 @@
-scarb 2.15.1
-starknet-foundry 0.55.0
+scarb 2.18.0
+starknet-foundry 0.60.0

Scarb.lock

@@ -141,15 +141,15 @@ dependencies = [
 
 [[package]]
 name = "snforge_scarb_plugin"
-version = "0.55.0"
+version = "0.60.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:638535780a23d1491c2438e64045c479d16de6a69e41ad17ac065272c485873b"
+checksum = "sha256:924358bf316e502923f6733b50e239ea37585a05dc24c5fc8dd9e45f88cf7339"
 
 [[package]]
 name = "snforge_std"
-version = "0.55.0"
+version = "0.60.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:a04b0bf731f02307506dad368713099e701565edd9b98b044ca54b932c29ef74"
+checksum = "sha256:32e6baabec4f9af21089bc7ca685ffea5e4164497340ecbdb99314e568029195"
 dependencies = [
  "snforge_scarb_plugin",
 ]

Scarb.toml

@@ -17,16 +17,16 @@ repository = "https://github.com/starkware-libs/starkware-starknet-utils"
 version = "1.0.0"
 
 [workspace.dependencies]
-assert_macros = "2.15.1"
+assert_macros = "2.18.0"
 openzeppelin = "3.0.0"
-snforge_std = "0.55.0"
-starknet = "2.15.1"
+snforge_std = "0.60.0"
+starknet = "2.18.0"
 
 [workspace.tool.fmt]
 sort-module-level-items = true
 
 [workspace.tool.scarb]
-allow-prebuilt-plugins = ["snforge_std"]
+allow-prebuilt-plugins = ["snforge_std", "snforge_scarb_plugin"]
 
 [profile.dev.cairo]
 inlining-strategy = "avoid"

packages/testing/Scarb.toml

@@ -29,7 +29,9 @@ test = "SNFORGE_BACKTRACE=1 snforge test"
 
 [tool]
 fmt.workspace = true
-scarb.workspace = true
+
+[tool.scarb]
+allow-prebuilt-plugins = ["snforge_std", "snforge_scarb_plugin"]
 
 [profile.dev.cairo]
 unstable-add-statements-functions-debug-info = true

packages/utils/Scarb.toml

@@ -27,7 +27,9 @@ name = "starkware_utils_unittest"
 
 [tool]
 fmt.workspace = true
-scarb.workspace = true
+
+[tool.scarb]
+allow-prebuilt-plugins = ["snforge_std", "snforge_scarb_plugin"]
 
 [profile.dev.cairo]
 unstable-add-statements-functions-debug-info = true

packages/utils/src/components/replaceability.cairo

@@ -13,6 +13,9 @@ mod eic_test_contract;
 #[cfg(test)]
 pub(crate) mod mock;
 
+#[cfg(test)]
+pub(crate) mod mock_v2;
+
 #[cfg(test)]
 mod test;
 

packages/utils/src/components/replaceability/errors.cairo

@@ -4,23 +4,29 @@ use starkware_utils::errors::{Describable, ErrorDisplay};
 pub(crate) enum ReplaceErrors {
     ALREADY_INITIALIZED,
     FINALIZED,
+    FINALIZE_IS_UNSAFE,
     UNKNOWN_IMPLEMENTATION,
     NOT_ENABLED_YET,
     IMPLEMENTATION_EXPIRED,
     EIC_LIB_CALL_FAILED,
     REPLACE_CLASS_HASH_FAILED,
+    FAILED_REPLACE_CLASS_HASH_A2B,
+    FAILED_REPLACE_CLASS_HASH_B2A,
 }
 
 impl DescribableError of Describable<ReplaceErrors> {
     fn describe(self: @ReplaceErrors) -> ByteArray {
         match self {
             ReplaceErrors::ALREADY_INITIALIZED => "ALREADY_INITIALIZED",
             ReplaceErrors::FINALIZED => "FINALIZED",
+            ReplaceErrors::FINALIZE_IS_UNSAFE => "FINALIZE_IS_UNSAFE",
             ReplaceErrors::UNKNOWN_IMPLEMENTATION => "UNKNOWN_IMPLEMENTATION",
             ReplaceErrors::NOT_ENABLED_YET => "NOT_ENABLED_YET",
             ReplaceErrors::IMPLEMENTATION_EXPIRED => "IMPLEMENTATION_EXPIRED",
             ReplaceErrors::EIC_LIB_CALL_FAILED => "EIC_LIB_CALL_FAILED",
             ReplaceErrors::REPLACE_CLASS_HASH_FAILED => "REPLACE_CLASS_HASH_FAILED",
+            ReplaceErrors::FAILED_REPLACE_CLASS_HASH_A2B => "FAILED_REPLACE_CLASS_HASH_A2B",
+            ReplaceErrors::FAILED_REPLACE_CLASS_HASH_B2A => "FAILED_REPLACE_CLASS_HASH_B2A",
         }
     }
 }

packages/utils/src/components/replaceability/interface.cairo

@@ -1,5 +1,9 @@
 use starknet::class_hash::ClassHash;
 
+/// Sentinel panic value used by `validate_upgradeability` to signal a successful dry-run.
+/// Any other panic from a dry-run is treated as a real failure.
+pub(crate) const UPGRADEABILITY_VALIDATION_SUCCESS: felt252 = 'UPGRADEABILITY_OK';
+
 /// Holds EIC data.
 /// * eic_hash is the EIC class hash.
 /// * eic_init_data is a span of the EIC init args.
@@ -39,8 +43,12 @@ pub trait IReplaceable<TContractState> {
         self: @TContractState, implementation_data: ImplementationData,
     ) -> u64;
     fn add_new_implementation(ref self: TContractState, implementation_data: ImplementationData);
+    fn add_new_implementation_unsafe(
+        ref self: TContractState, implementation_data: ImplementationData,
+    );
     fn remove_implementation(ref self: TContractState, implementation_data: ImplementationData);
     fn replace_to(ref self: TContractState, implementation_data: ImplementationData);
+    fn validate_upgradeability(ref self: TContractState, implementation_data: ImplementationData);
 }
 
 #[derive(Copy, Drop, PartialEq, starknet::Event)]

packages/utils/src/components/replaceability/mock_v2.cairo

@@ -0,0 +1,52 @@
+// Variant of `ReplaceabilityMock` with a class hash distinctly different than `mock.cairo`.
+#[starknet::contract]
+pub(crate) mod ReplaceabilityMockV2 {
+    use CommonRolesComponent::InternalTrait as CommonRolesInternalTrait;
+    use openzeppelin::access::accesscontrol::AccessControlComponent;
+    use openzeppelin::introspection::src5::SRC5Component;
+    use starknet::ContractAddress;
+    use starkware_utils::components::common_roles::CommonRolesComponent;
+    use starkware_utils::components::replaceability::ReplaceabilityComponent;
+    use starkware_utils::components::replaceability::ReplaceabilityComponent::InternalReplaceabilityTrait;
+
+    component!(path: ReplaceabilityComponent, storage: replaceability, event: ReplaceabilityEvent);
+    component!(path: CommonRolesComponent, storage: common_roles, event: CommonRolesEvent);
+    component!(path: AccessControlComponent, storage: accesscontrol, event: AccessControlEvent);
+    component!(path: SRC5Component, storage: src5, event: SRC5Event);
+
+    #[storage]
+    struct Storage {
+        #[substorage(v0)]
+        replaceability: ReplaceabilityComponent::Storage,
+        #[substorage(v0)]
+        common_roles: CommonRolesComponent::Storage,
+        #[substorage(v0)]
+        accesscontrol: AccessControlComponent::Storage,
+        #[substorage(v0)]
+        src5: SRC5Component::Storage,
+        // Extra field to produce a different class hash from ReplaceabilityMock.
+        _v2_marker: bool,
+    }
+
+    #[event]
+    #[derive(Drop, starknet::Event)]
+    pub(crate) enum Event {
+        ReplaceabilityEvent: ReplaceabilityComponent::Event,
+        CommonRolesEvent: CommonRolesComponent::Event,
+        AccessControlEvent: AccessControlComponent::Event,
+        SRC5Event: SRC5Component::Event,
+    }
+
+    #[constructor]
+    fn constructor(ref self: ContractState, upgrade_delay: u64, governance_admin: ContractAddress) {
+        self.common_roles.initialize(:governance_admin);
+        self.replaceability.initialize(:upgrade_delay);
+    }
+
+    #[abi(embed_v0)]
+    impl ReplaceabilityImpl =
+        ReplaceabilityComponent::ReplaceabilityImpl<ContractState>;
+
+    #[abi(embed_v0)]
+    impl CommonRolesImpl = CommonRolesComponent::CommonRolesImpl<ContractState>;
+}

packages/utils/src/components/replaceability/replaceability.cairo

@@ -4,18 +4,22 @@ pub(crate) mod ReplaceabilityComponent {
     use core::poseidon;
     use openzeppelin::access::accesscontrol::AccessControlComponent;
     use openzeppelin::introspection::src5::SRC5Component;
-    use starknet::get_block_timestamp;
     use starknet::storage::{
         Map, StorageMapReadAccess, StorageMapWriteAccess, StoragePointerReadAccess,
         StoragePointerWriteAccess,
     };
-    use starknet::syscalls::{library_call_syscall, replace_class_syscall};
+    use starknet::syscalls::{
+        get_class_hash_at_syscall, library_call_syscall, replace_class_syscall,
+    };
+    use starknet::{SyscallResultTrait, get_block_timestamp, get_contract_address};
     use starkware_utils::components::common_roles::CommonRolesComponent;
     use starkware_utils::components::common_roles::CommonRolesComponent::InternalTrait;
     use starkware_utils::components::replaceability::errors::ReplaceErrors;
     use starkware_utils::components::replaceability::interface::{
-        EIC_INITIALIZE_SELECTOR, IMPLEMENTATION_EXPIRATION, IReplaceable, ImplementationAdded,
+        EIC_INITIALIZE_SELECTOR, IMPLEMENTATION_EXPIRATION, IReplaceable,
+        IReplaceableDispatcherTrait, IReplaceableLibraryDispatcher, ImplementationAdded,
         ImplementationData, ImplementationFinalized, ImplementationRemoved, ImplementationReplaced,
+        UPGRADEABILITY_VALIDATION_SUCCESS,
     };
 
 
@@ -69,17 +73,24 @@ pub(crate) mod ReplaceabilityComponent {
             self.impl_activation_time.read(impl_key)
         }
 
+        // Schedule a new implementation upgrade and validates the implementation upgradeability.
         fn add_new_implementation(
             ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
         ) {
-            // The call is restricted to the upgrade governor.
+            assert!(!implementation_data.final, "{}", ReplaceErrors::FINALIZE_IS_UNSAFE);
+            self.invoke_upgradeability_validation(implementation_data);
+            self.add_new_implementation_unsafe(implementation_data);
+        }
+
+        // Schedule a new implementation upgrade without validation.
+        fn add_new_implementation_unsafe(
+            ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
+        ) {
             let common_roles = get_dep_component!(@self, CommonRoles);
             common_roles.only_upgrade_governor();
 
             let activation_time = get_block_timestamp() + self.get_upgrade_delay();
             let expiration_time = activation_time + IMPLEMENTATION_EXPIRATION;
-            // TODO(Yaniv, 01/08/2024) -  add an assertion that the `implementation_data.impl_hash`
-            // is declared.
             self.set_impl_activation_time(:implementation_data, :activation_time);
             self.set_impl_expiration_time(:implementation_data, :expiration_time);
             self.emit(ImplementationAdded { implementation_data });
@@ -88,7 +99,6 @@ pub(crate) mod ReplaceabilityComponent {
         fn remove_implementation(
             ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
         ) {
-            // The call is restricted to the upgrade governor.
             let common_roles = get_dep_component!(@self, CommonRoles);
             common_roles.only_upgrade_governor();
 
@@ -102,8 +112,8 @@ pub(crate) mod ReplaceabilityComponent {
             }
         }
 
-        // Replaces the non-finalized current implementation to one that was previously added and
-        // whose activation time had passed.
+        // Replaces the class hash to a previously-added implementation whose activation time
+        // has passed.
         fn replace_to(
             ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
         ) {
@@ -126,10 +136,8 @@ pub(crate) mod ReplaceabilityComponent {
 
             assert!(impl_activation_time <= now, "{}", ReplaceErrors::NOT_ENABLED_YET);
             assert!(now <= impl_expiration_time, "{}", ReplaceErrors::IMPLEMENTATION_EXPIRED);
-            // We emit now so that finalize emits last (if it does).
             self.emit(ImplementationReplaced { implementation_data });
 
-            // Finalize implementation, if needed.
             if (implementation_data.final) {
                 self.finalize();
                 self.emit(ImplementationFinalized { impl_hash: implementation_data.impl_hash });
@@ -159,7 +167,50 @@ pub(crate) mod ReplaceabilityComponent {
             self.set_impl_activation_time(:implementation_data, activation_time: 0);
             self.set_impl_expiration_time(:implementation_data, expiration_time: 0);
         }
+
+        // Dry-run a full upgrade cycle:
+        // 1. User planned upgrade (A->B).
+        // 2. From target implementation (B->A).
+        // Always panics to revert side-effects:
+        // `UPGRADEABILITY_VALIDATION_SUCCESS` on success, or the underlying error otherwise.
+        fn validate_upgradeability(
+            ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
+        ) {
+            // impl_a_hash: current class-hash. impl_b_hash: target class-hash.
+            let impl_a_hash = get_class_hash_at_syscall(get_contract_address()).unwrap_syscall();
+            let impl_b_hash = implementation_data.impl_hash;
+
+            // Step 1 (A->B): User planned upgrade.
+            // Zero the upgrade delay to allow instant add & replace.
+            self.upgrade_delay.write(0);
+            self.add_new_implementation_unsafe(implementation_data);
+            self.replace_to(implementation_data);
+            assert!(
+                get_class_hash_at_syscall(get_contract_address()).unwrap_syscall() == impl_b_hash,
+                "{}",
+                ReplaceErrors::FAILED_REPLACE_CLASS_HASH_A2B,
+            );
+
+            // Step 2 (B->A): Upgrade back to impl_a using target class code.
+            // Re-zero upgrade delay in case EIC altered it.
+            self.upgrade_delay.write(0);
+            let back2a_impl_data = ImplementationData {
+                impl_hash: impl_a_hash, eic_data: Option::None, final: false,
+            };
+            let impl_b_dispatcher = IReplaceableLibraryDispatcher { class_hash: impl_b_hash };
+
+            impl_b_dispatcher.add_new_implementation_unsafe(implementation_data: back2a_impl_data);
+            impl_b_dispatcher.replace_to(implementation_data: back2a_impl_data);
+            assert!(
+                get_class_hash_at_syscall(get_contract_address()).unwrap_syscall() == impl_a_hash,
+                "{}",
+                ReplaceErrors::FAILED_REPLACE_CLASS_HASH_B2A,
+            );
+
+            core::panic_with_felt252(UPGRADEABILITY_VALIDATION_SUCCESS);
+        }
     }
+
     #[generate_trait]
     pub impl InternalReplaceabilityImpl<
         TContractState, +HasComponent<TContractState>, +Drop<TContractState>,
@@ -175,6 +226,30 @@ pub(crate) mod ReplaceabilityComponent {
     impl PrivateReplaceabilityImpl<
         TContractState, +HasComponent<TContractState>, +Drop<TContractState>,
     > of PrivateReplaceabilityTrait<TContractState> {
+        // Invoke `validate_upgradeability` via library_call on the current class.
+        // Returns on the success sentinel; propagates any other panic.
+        fn invoke_upgradeability_validation(
+            ref self: ComponentState<TContractState>, implementation_data: ImplementationData,
+        ) {
+            let current_class_hash = get_class_hash_at_syscall(get_contract_address())
+                .unwrap_syscall();
+
+            let mut calldata = array![];
+            implementation_data.serialize(ref calldata);
+            let result = library_call_syscall(
+                class_hash: current_class_hash,
+                function_selector: selector!("validate_upgradeability"),
+                calldata: calldata.span(),
+            );
+
+            // validate_upgradeability always panics.
+            let panic_data = result.expect_err('VALIDATION_DID_NOT_PANIC');
+            // Catch the success sentinel panic, re-throw any other panic.
+            if panic_data != array![UPGRADEABILITY_VALIDATION_SUCCESS, 'ENTRYPOINT_FAILED'] {
+                core::panics::panic(panic_data);
+            }
+        }
+
         fn is_finalized(self: @ComponentState<TContractState>) -> bool {
             self.finalized.read()
         }