Skip to content

Commit

Permalink
feat: upgradeability with initializer (#497)
Browse files Browse the repository at this point in the history 
* refactor: replace constructor with initializer and call when upgrading

* chore: updated tests

* chore: test reinitialization

* refactor: moved reinitializeable logic to component

* feat: add initializer params to upgrade event

* refactor: use reinitializable in spacev2

---------

Co-authored-by: Orlando <[email protected]>
  • Loading branch information
@Orland0x
Orland0x and Orlando authored Aug 18, 2023
1 parent dda48da commit 3871e5f
Show file tree
Hide file tree
Showing 11 changed files with 331 additions and 120 deletions.
14 changes: 10 additions & 4 deletions starknet/src/factory/factory.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ trait IFactory<TContractState> {
self: @TContractState,
class_hash: ClassHash,
contract_address_salt: felt252,
calldata: Span<felt252>
initialize_calldata: Span<felt252>
) -> ContractAddress;
}

Expand All @@ -17,9 +17,11 @@ mod Factory {
use super::IFactory;
use starknet::ContractAddress;
use starknet::contract_address_const;
use starknet::syscalls::deploy_syscall;
use starknet::syscalls::{deploy_syscall, call_contract_syscall};
use starknet::ClassHash;
use result::ResultTrait;
use array::{ArrayTrait, SpanTrait};
use sx::utils::constants::INITIALIZE_SELECTOR;

#[event]
fn SpaceDeployed(class_hash: ClassHash, space_address: ContractAddress) {}
Expand All @@ -33,13 +35,17 @@ mod Factory {
self: @ContractState,
class_hash: ClassHash,
contract_address_salt: felt252,
calldata: Span<felt252>
initialize_calldata: Span<felt252>
) -> ContractAddress {
let (space_address, _) = deploy_syscall(
class_hash, contract_address_salt, calldata, false
class_hash, contract_address_salt, array![].span(), false
)
.unwrap();

// Call initializer.
call_contract_syscall(space_address, INITIALIZE_SELECTOR, initialize_calldata)
.unwrap_syscall();

SpaceDeployed(class_hash, space_address);

space_address
Expand Down
138 changes: 86 additions & 52 deletions starknet/src/space/space.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ trait ISpace<TContractState> {
fn transfer_ownership(ref self: TContractState, new_owner: ContractAddress);
fn renounce_ownership(ref self: TContractState);
// Actions
fn initialize(
ref self: TContractState,
owner: ContractAddress,
min_voting_duration: u32,
max_voting_duration: u32,
voting_delay: u32,
proposal_validation_strategy: Strategy,
proposal_validation_strategy_metadata_URI: Array<felt252>,
voting_strategies: Array<Strategy>,
voting_strategy_metadata_URIs: Array<Array<felt252>>,
authenticators: Array<ContractAddress>,
metadata_URI: Array<felt252>,
dao_URI: Array<felt252>,
);
fn propose(
ref self: TContractState,
author: UserAddress,
Expand All @@ -53,13 +67,15 @@ trait ISpace<TContractState> {
metadata_URI: Array<felt252>,
);
fn cancel_proposal(ref self: TContractState, proposal_id: u256);
fn upgrade(ref self: TContractState, class_hash: ClassHash);
fn upgrade(
ref self: TContractState, class_hash: ClassHash, initialize_calldata: Array<felt252>
);
}

#[starknet::contract]
mod Space {
use super::ISpace;
use starknet::{ClassHash, ContractAddress, info, Store};
use starknet::{ClassHash, ContractAddress, info, Store, syscalls};
use zeroable::Zeroable;
use array::{ArrayTrait, SpanTrait};
use clone::Clone;
Expand All @@ -77,14 +93,17 @@ mod Space {
IndexedStrategyTrait, IndexedStrategyImpl, UpdateSettingsCalldata, NoUpdateU32,
NoUpdateStrategy, NoUpdateArray
};
use sx::utils::reinitializable::Reinitializable;
use sx::utils::ReinitializableImpl;
use sx::utils::bits::BitSetter;
use sx::utils::legacy_hash::LegacyHashChoice;
use sx::external::ownable::Ownable;
use sx::utils::constants::INITIALIZE_SELECTOR;

#[storage]
struct Storage {
_max_voting_duration: u32,
_min_voting_duration: u32,
_max_voting_duration: u32,
_next_proposal_id: u256,
_voting_delay: u32,
_active_voting_strategies: u256,
Expand All @@ -101,9 +120,9 @@ mod Space {
fn SpaceCreated(
_space: ContractAddress,
_owner: ContractAddress,
_voting_delay: u32,
_min_voting_duration: u32,
_max_voting_duration: u32,
_voting_delay: u32,
_proposal_validation_strategy: @Strategy,
_proposal_validation_strategy_metadata_URI: @Array<felt252>,
_voting_strategies: @Array<Strategy>,
Expand Down Expand Up @@ -179,10 +198,56 @@ mod Space {
fn VotingDelayUpdated(_new_voting_delay: u32) {}

#[event]
fn Upgraded(class_hash: ClassHash) {}
fn Upgraded(class_hash: ClassHash, initialize_calldata: Array<felt252>) {}

#[external(v0)]
impl Space of ISpace<ContractState> {
fn initialize(
ref self: ContractState,
owner: ContractAddress,
min_voting_duration: u32,
max_voting_duration: u32,
voting_delay: u32,
proposal_validation_strategy: Strategy,
proposal_validation_strategy_metadata_URI: Array<felt252>,
voting_strategies: Array<Strategy>,
voting_strategy_metadata_URIs: Array<Array<felt252>>,
authenticators: Array<ContractAddress>,
metadata_URI: Array<felt252>,
dao_URI: Array<felt252>,
) {
SpaceCreated(
info::get_contract_address(),
owner,
min_voting_duration,
max_voting_duration,
voting_delay,
@proposal_validation_strategy,
@proposal_validation_strategy_metadata_URI,
@voting_strategies,
@voting_strategy_metadata_URIs,
@authenticators,
@metadata_URI,
@dao_URI
);
// Checking that the contract is not already initialized
//TODO: temporary component syntax (see imports too)
let mut state: Reinitializable::ContractState =
Reinitializable::unsafe_new_contract_state();
ReinitializableImpl::initialize(ref state);

//TODO: temporary component syntax
let mut state: Ownable::ContractState = Ownable::unsafe_new_contract_state();
Ownable::initializer(ref state);
Ownable::transfer_ownership(ref state, owner);
_set_min_voting_duration(ref self, min_voting_duration);
_set_max_voting_duration(ref self, max_voting_duration);
_set_voting_delay(ref self, voting_delay);
_set_proposal_validation_strategy(ref self, proposal_validation_strategy);
_add_voting_strategies(ref self, voting_strategies);
_add_authenticators(ref self, authenticators);
self._next_proposal_id.write(1_u256);
}
fn propose(
ref self: ContractState,
author: UserAddress,
Expand Down Expand Up @@ -344,15 +409,26 @@ mod Space {
ProposalCancelled(proposal_id);
}

fn upgrade(ref self: ContractState, class_hash: ClassHash) {
fn upgrade(
ref self: ContractState, class_hash: ClassHash, initialize_calldata: Array<felt252>
) {
let state: Ownable::ContractState = Ownable::unsafe_new_contract_state();
Ownable::assert_only_owner(@state);

assert(
class_hash.is_non_zero(), 'Class Hash cannot be zero'
); // TODO: not sure this is needed
assert(class_hash.is_non_zero(), 'Class Hash cannot be zero');
starknet::replace_class_syscall(class_hash).unwrap_syscall();
Upgraded(class_hash);

// Allowing initializer to be called again.
let mut state: Reinitializable::ContractState =
Reinitializable::unsafe_new_contract_state();
ReinitializableImpl::reinitialize(ref state);

// Call initializer on the new version.
syscalls::call_contract_syscall(
info::get_contract_address(), INITIALIZE_SELECTOR, initialize_calldata.span()
)
.unwrap_syscall();
Upgraded(class_hash, initialize_calldata);
}

fn owner(self: @ContractState) -> ContractAddress {
Expand Down Expand Up @@ -485,48 +561,6 @@ mod Space {
}
}

#[constructor]
fn constructor(
ref self: ContractState,
_owner: ContractAddress,
_max_voting_duration: u32,
_min_voting_duration: u32,
_voting_delay: u32,
_proposal_validation_strategy: Strategy,
_proposal_validation_strategy_metadata_URI: Array<felt252>,
_voting_strategies: Array<Strategy>,
_voting_strategies_metadata_URIs: Array<Array<felt252>>,
_authenticators: Array<ContractAddress>,
_metadata_URI: Array<felt252>,
_dao_URI: Array<felt252>
) {
//TODO: temporary component syntax
let mut state: Ownable::ContractState = Ownable::unsafe_new_contract_state();
Ownable::initializer(ref state);
Ownable::transfer_ownership(ref state, _owner);
_set_max_voting_duration(ref self, _max_voting_duration);
_set_min_voting_duration(ref self, _min_voting_duration);
_set_voting_delay(ref self, _voting_delay);
_set_proposal_validation_strategy(ref self, _proposal_validation_strategy.clone());
_add_voting_strategies(ref self, _voting_strategies.clone());
_add_authenticators(ref self, _authenticators.clone());
self._next_proposal_id.write(1_u256);
SpaceCreated(
info::get_contract_address(),
_owner,
_voting_delay,
_min_voting_duration,
_max_voting_duration,
@_proposal_validation_strategy,
@_proposal_validation_strategy_metadata_URI,
@_voting_strategies,
@_voting_strategies_metadata_URIs,
@_authenticators,
@_metadata_URI,
@_dao_URI
);
}

///
/// Internals
///
Expand Down
1 change: 1 addition & 0 deletions starknet/src/tests/mocks.cairo
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
mod proposal_validation_always_fail;
mod executor;
mod space_v2;
30 changes: 30 additions & 0 deletions starknet/src/tests/mocks/space_v2.cairo
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#[starknet::interface]
trait ISpaceV2<TContractState> {
fn initialize(ref self: TContractState, var: felt252);
fn get_var(self: @TContractState) -> felt252;
}

#[starknet::contract]
mod SpaceV2 {
use super::ISpaceV2;
use sx::utils::reinitializable::Reinitializable;
use sx::utils::ReinitializableImpl;
#[storage]
struct Storage {
_var: felt252
}

#[external(v0)]
impl SpaceV2 of ISpaceV2<ContractState> {
fn initialize(ref self: ContractState, var: felt252) {
// TODO: Temp component syntax
let mut state: Reinitializable::ContractState =
Reinitializable::unsafe_new_contract_state();
ReinitializableImpl::initialize(ref state);
self._var.write(var);
}
fn get_var(self: @ContractState) -> felt252 {
self._var.read()
}
}
}
40 changes: 20 additions & 20 deletions starknet/src/tests/setup/setup.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ mod setup {
.append(Strategy { address: vanilla_voting_strategy_address, params: array![] });

// Deploy Vanilla Execution Strategy
let mut constructor_calldata = array![];
quorum.serialize(ref constructor_calldata);
let mut initializer_calldata = array![];
quorum.serialize(ref initializer_calldata);
let (vanilla_execution_strategy_address, _) = deploy_syscall(
VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(),
0,
constructor_calldata.span(),
initializer_calldata.span(),
false
)
.unwrap();
Expand All @@ -96,7 +96,7 @@ mod setup {
}
}

fn get_constructor_calldata(
fn get_initialize_calldata(
owner: @ContractAddress,
min_voting_duration: @u64,
max_voting_duration: @u64,
Expand All @@ -106,20 +106,20 @@ mod setup {
authenticators: @Array<ContractAddress>
) -> Array<felt252> {
// Using empty arrays for all the metadata fields
let mut constructor_calldata = array![];
constructor_calldata.append((*owner).into());
constructor_calldata.append((*max_voting_duration).into());
constructor_calldata.append((*min_voting_duration).into());
constructor_calldata.append((*voting_delay).into());
proposal_validation_strategy.serialize(ref constructor_calldata);
ArrayTrait::<felt252>::new().serialize(ref constructor_calldata);
voting_strategies.serialize(ref constructor_calldata);
ArrayTrait::<felt252>::new().serialize(ref constructor_calldata);
authenticators.serialize(ref constructor_calldata);
ArrayTrait::<felt252>::new().serialize(ref constructor_calldata);
ArrayTrait::<felt252>::new().serialize(ref constructor_calldata);

constructor_calldata
let mut initializer_calldata = array![];
owner.serialize(ref initializer_calldata);
min_voting_duration.serialize(ref initializer_calldata);
max_voting_duration.serialize(ref initializer_calldata);
voting_delay.serialize(ref initializer_calldata);
proposal_validation_strategy.serialize(ref initializer_calldata);
ArrayTrait::<felt252>::new().serialize(ref initializer_calldata);
voting_strategies.serialize(ref initializer_calldata);
ArrayTrait::<felt252>::new().serialize(ref initializer_calldata);
authenticators.serialize(ref initializer_calldata);
ArrayTrait::<felt252>::new().serialize(ref initializer_calldata);
ArrayTrait::<felt252>::new().serialize(ref initializer_calldata);

initializer_calldata
}

fn deploy(config: @Config) -> (IFactoryDispatcher, ISpaceDispatcher) {
Expand All @@ -133,7 +133,7 @@ mod setup {

let factory = IFactoryDispatcher { contract_address: factory_address };

let constructor_calldata = get_constructor_calldata(
let initializer_calldata = get_initialize_calldata(
config.owner,
config.min_voting_duration,
config.max_voting_duration,
Expand All @@ -143,7 +143,7 @@ mod setup {
config.authenticators
);
let space_address = factory
.deploy(space_class_hash, contract_address_salt, constructor_calldata.span());
.deploy(space_class_hash, contract_address_salt, initializer_calldata.span());

let space = ISpaceDispatcher { contract_address: space_address };

Expand Down
4 changes: 2 additions & 2 deletions starknet/src/tests/test_factory.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod tests {
use sx::types::Strategy;
use starknet::ClassHash;

use sx::tests::setup::setup::setup::{setup, get_constructor_calldata, deploy};
use sx::tests::setup::setup::setup::{setup, get_initialize_calldata, deploy};

#[test]
#[available_gas(10000000000)]
Expand Down Expand Up @@ -39,7 +39,7 @@ mod tests {
let contract_address_salt = 0;

let config = setup();
let constructor_calldata = get_constructor_calldata(
let constructor_calldata = get_initialize_calldata(
@config.owner,
@config.min_voting_duration,
@config.max_voting_duration,
Expand Down
Loading

0 comments on commit 3871e5f

Please sign in to comment.