fix arcade account implementation validity check

apibara · Dec 6, 2023 · 3c3b713 · 3c3b713
1 parent 4f5ecdc
commit 3c3b713
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 38 deletions.
diff --git a/demos/arcade-factory/src/factory/factory.cairo b/demos/arcade-factory/src/factory/factory.cairo
@@ -111,14 +111,16 @@ mod FactoryComponent {
       ref self: ComponentState<TContractState>,
       arcade_account_implementation: starknet::ClassHash
     ) {
-      // check that the new implementation is a valid arcade account
+      // check that the new implementation is a valid class hash
+      // we cannot check if the implementation register to the arcade account interface
+      // but we can check if the implementation implements the supports_interface method
       let ret_data = starknet::library_call_syscall(
         class_hash: arcade_account_implementation,
         function_selector: SUPPORTS_INTERFACE_SELECTOR,
         calldata: array![ARCADE_ACCOUNT_ID].span()
       ).unwrap_syscall();
 
-      assert((ret_data.len() == 1) & (*ret_data.at(0) == true.into()), 'Invalid arcade account impl');
+      assert(ret_data.len() == 1, 'Invalid arcade account impl');
 
       // update implementation
       self._arcade_account_implementation.write(arcade_account_implementation);

diff --git a/demos/arcade-factory/src/tests/mocks/arcade_account_mock.cairo b/demos/arcade-factory/src/tests/mocks/arcade_account_mock.cairo
@@ -9,11 +9,31 @@ trait ArcadeAccountMockABI<TState> {
 // Using a mock instead of the real impl of the arcade account because of OZ versions conflict
 #[starknet::contract]
 mod ArcadeAccountMock {
+  use openzeppelin::introspection::src5::SRC5Component;
+
   // locals
   use super::ArcadeAccountMockABI;
 
   use arcade_factory::account::interface;
 
+  component!(path: SRC5Component, storage: src5, event: SRC5Event);
+
+  // Components
+
+  #[abi(embed_v0)]
+  impl SRC5Impl = SRC5Component::SRC5Impl<ContractState>;
+
+  //
+  // Events
+  //
+
+  #[event]
+  #[derive(Drop, starknet::Event)]
+  enum Event {
+    #[flat]
+    SRC5Event: SRC5Component::Event,
+  }
+
   //
   // Storage
   //
@@ -22,6 +42,9 @@ mod ArcadeAccountMock {
   struct Storage {
     _public_key: felt252,
     _master_account: starknet::ContractAddress,
+
+    #[substorage(v0)]
+    src5: SRC5Component::Storage,
   }
 
   //
@@ -51,11 +74,6 @@ mod ArcadeAccountMock {
       self._master_account.write(master_account);
     }
   }
-
-  #[external(v0)]
-  fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
-    return interface_id == interface::ARCADE_ACCOUNT_ID;
-  }
 }
 
 #[starknet::contract]
@@ -71,33 +89,11 @@ mod ValidArcadeAccountMock {
   struct Storage { }
 
   //
-  // ArcadeAccountMockABI
-  //
-
-  #[external(v0)]
-  fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
-    return interface_id == interface::ARCADE_ACCOUNT_ID;
-  }
-}
-
-#[starknet::contract]
-mod InvalidArcadeAccountMock {
-  // locals
-  use arcade_factory::account::interface;
-
-  //
-  // Storage
-  //
-
-  #[storage]
-  struct Storage { }
-
-  //
-  // ArcadeAccountMockABI
+  // SRC5
   //
 
   #[external(v0)]
   fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
-    return interface_id != interface::ARCADE_ACCOUNT_ID;
+    false
   }
 }
diff --git a/demos/arcade-factory/src/tests/test_factory.cairo b/demos/arcade-factory/src/tests/test_factory.cairo
@@ -10,7 +10,6 @@ use arcade_factory::factory::interface::{ ArcadeFactoryABIDispatcher, ArcadeFact
 
 use super::mocks::arcade_account_mock::{
   ValidArcadeAccountMock,
-  InvalidArcadeAccountMock,
   ArcadeAccountMock,
   ArcadeAccountMockABIDispatcher,
   ArcadeAccountMockABIDispatcherTrait,
@@ -44,13 +43,14 @@ fn setup() -> ArcadeFactoryABIDispatcher {
 
 #[test]
 #[available_gas(20000000)]
-#[should_panic(expected: ('Result::unwrap failed.',))] // cannot have another error message from constructor
+// cannot have another error message from constructor
+#[should_panic(expected: ('Result::unwrap failed.',))]
 fn test_constructor_invalid_arcade_account_implementation() {
   let owner = constants::OWNER();
 
   utils::deploy(
     contract_class_hash: ArcadeFactory::TEST_CLASS_HASH,
-    calldata: array![owner.into(), InvalidArcadeAccountMock::TEST_CLASS_HASH]
+    calldata: array![owner.into(), 'invalid class hash']
   );
 }
 
@@ -129,14 +129,13 @@ fn test_set_arcade_account_implementation() {
 
 #[test]
 #[available_gas(20000000)]
-#[should_panic(expected: ('Invalid arcade account impl', 'ENTRYPOINT_FAILED',))]
+// cannot have another error message until state revert during tx is supported
+#[should_panic(expected: ('CLASS_HASH_NOT_DECLARED', 'ENTRYPOINT_FAILED',))]
 fn test_set_arcade_account_implementation_invalid() {
   let factory = setup();
 
   testing::set_contract_address(constants::OWNER());
-  factory.set_arcade_account_implementation(
-    arcade_account_implementation: InvalidArcadeAccountMock::TEST_CLASS_HASH.try_into().unwrap()
-  );
+  factory.set_arcade_account_implementation(arcade_account_implementation: 'new class hash'.try_into().unwrap());
 }
 
 #[test]