Skip to content

Commit

Permalink
fix set_arcade_account_impl
Browse files Browse the repository at this point in the history
  • Loading branch information
0xChqrles authored and fracek committed Dec 6, 2023
1 parent 3c3b713 commit 1e886aa
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 16 deletions.
6 changes: 6 additions & 0 deletions demos/arcade-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@

This demo is a Cairo contract allowing [arcade account](https://github.com/BibliothecaDAO/arcade-account) deployment, funding and configuration in a single tx.

### Notes

Usually an Arcade account is initialized with a `public key` and a `master account`, thus the master account will impact the account address. To avoid this, the Arcade factory deploys a custom arcade account for which the master account initialization is done outside of the constructor.

### Hashes

```
factory: 0x0286bb21588bdb808fb52e1e27346413e1f5be82e7de6a173e34677c3531d958
account: 0x0251830adc3d8b4d818c2c309d71f1958308e8c745212480c26e01120c69ee49
```

⚠️ This arcade account is not intended to be used in prod.
3 changes: 2 additions & 1 deletion demos/arcade-factory/src/factory/contract.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ mod ArcadeFactory {
//

#[external(v0)]
fn set_arcade_account_implementation(ref self: ContractState, arcade_account_implementation: starknet::ClassHash) {
// method name cannot be too long :/
fn set_arcade_account_impl(ref self: ContractState, arcade_account_implementation: starknet::ClassHash) {
// only owner
self.ownable.assert_only_owner();

Expand Down
13 changes: 7 additions & 6 deletions demos/arcade-factory/src/factory/factory.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
mod FactoryComponent {
use starknet::SyscallResultTrait;

use openzeppelin::utils::selectors;
use openzeppelin::introspection::interface::ISRC5_ID;

// locals
use arcade_factory::factory::interface;

Expand All @@ -13,8 +16,6 @@ mod FactoryComponent {

const CONTRACT_ADDRESS_PREFIX: felt252 = 'STARKNET_CONTRACT_ADDRESS';

const SUPPORTS_INTERFACE_SELECTOR: felt252 = 0xfe80f537b66d12a00b6d3c072b44afbb716e78dde5c3f0ef116ee93d3e3283;

//
// Storage
//
Expand Down Expand Up @@ -113,14 +114,14 @@ mod FactoryComponent {
) {
// 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
// but we can check if the implementation supports SRC5
let ret_data = starknet::library_call_syscall(
class_hash: arcade_account_implementation,
function_selector: SUPPORTS_INTERFACE_SELECTOR,
calldata: array![ARCADE_ACCOUNT_ID].span()
function_selector: selectors::supports_interface,
calldata: array![ISRC5_ID].span()
).unwrap_syscall();

assert(ret_data.len() == 1, 'Invalid arcade account impl');
assert((ret_data.len() == 1) & (*ret_data.at(0) == true.into()), 'Invalid arcade account impl');

// update implementation
self._arcade_account_implementation.write(arcade_account_implementation);
Expand Down
2 changes: 1 addition & 1 deletion demos/arcade-factory/src/factory/interface.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ trait IFactory<TState> {
trait ArcadeFactoryABI<TState> {
// IFactory
fn arcade_account_implementation(self: @TState) -> starknet::ClassHash;
fn set_arcade_account_implementation(ref self: TState, arcade_account_implementation: starknet::ClassHash);
fn set_arcade_account_impl(ref self: TState, arcade_account_implementation: starknet::ClassHash);
fn compute_address(
self: @TState,
salt: felt252,
Expand Down
37 changes: 35 additions & 2 deletions demos/arcade-factory/src/tests/mocks/arcade_account_mock.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ mod ArcadeAccountMock {
// locals
use super::ArcadeAccountMockABI;

use arcade_factory::account::interface;

component!(path: SRC5Component, storage: src5, event: SRC5Event);

// Components
Expand Down Expand Up @@ -78,6 +76,41 @@ mod ArcadeAccountMock {

#[starknet::contract]
mod ValidArcadeAccountMock {
use openzeppelin::introspection::src5::SRC5Component;

// locals

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
//

#[storage]
struct Storage {
#[substorage(v0)]
src5: SRC5Component::Storage,
}
}

#[starknet::contract]
mod InvalidArcadeAccountMock {
// locals
use arcade_factory::account::interface;

Expand Down
38 changes: 32 additions & 6 deletions demos/arcade-factory/src/tests/test_factory.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use arcade_factory::factory::interface::{ ArcadeFactoryABIDispatcher, ArcadeFact

use super::mocks::arcade_account_mock::{
ValidArcadeAccountMock,
InvalidArcadeAccountMock,
ArcadeAccountMock,
ArcadeAccountMockABIDispatcher,
ArcadeAccountMockABIDispatcherTrait,
Expand Down Expand Up @@ -45,7 +46,7 @@ fn setup() -> ArcadeFactoryABIDispatcher {
#[available_gas(20000000)]
// cannot have another error message from constructor
#[should_panic(expected: ('Result::unwrap failed.',))]
fn test_constructor_invalid_arcade_account_implementation() {
fn test_constructor_unknown_arcade_account_implementation() {
let owner = constants::OWNER();

utils::deploy(
Expand All @@ -54,6 +55,19 @@ fn test_constructor_invalid_arcade_account_implementation() {
);
}

#[test]
#[available_gas(20000000)]
// 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]
);
}

// Arcade account implementation

#[test]
Expand Down Expand Up @@ -117,7 +131,7 @@ fn test_set_arcade_account_implementation() {
let factory = setup();

testing::set_contract_address(constants::OWNER());
factory.set_arcade_account_implementation(
factory.set_arcade_account_impl(
arcade_account_implementation: ValidArcadeAccountMock::TEST_CLASS_HASH.try_into().unwrap()
);

Expand All @@ -127,15 +141,27 @@ fn test_set_arcade_account_implementation() {
);
}

#[test]
#[available_gas(20000000)]
#[should_panic(expected: ('Invalid arcade account impl', 'ENTRYPOINT_FAILED',))]
fn test_set_arcade_account_implementation_invalid() {
let factory = setup();

testing::set_contract_address(constants::OWNER());
factory.set_arcade_account_impl(
arcade_account_implementation: InvalidArcadeAccountMock::TEST_CLASS_HASH.try_into().unwrap()
);
}

#[test]
#[available_gas(20000000)]
// 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() {
fn test_set_arcade_account_implementation_unknown() {
let factory = setup();

testing::set_contract_address(constants::OWNER());
factory.set_arcade_account_implementation(arcade_account_implementation: 'new class hash'.try_into().unwrap());
factory.set_arcade_account_impl(arcade_account_implementation: 'new class hash'.try_into().unwrap());
}

#[test]
Expand All @@ -145,7 +171,7 @@ fn test_set_arcade_account_implementation_unauthorized() {
let factory = setup();

testing::set_contract_address(constants::OTHER());
factory.set_arcade_account_implementation(arcade_account_implementation: 'new class hash'.try_into().unwrap());
factory.set_arcade_account_impl(arcade_account_implementation: 'new class hash'.try_into().unwrap());
}

#[test]
Expand All @@ -155,5 +181,5 @@ fn test_set_arcade_account_implementation_from_zero() {
let factory = setup();

testing::set_contract_address(constants::ZERO());
factory.set_arcade_account_implementation(arcade_account_implementation: 'new class hash'.try_into().unwrap());
factory.set_arcade_account_impl(arcade_account_implementation: 'new class hash'.try_into().unwrap());
}

0 comments on commit 1e886aa

Please sign in to comment.