From 7ab6b7c7199aa1e15f1e6ad518156fa734b8cb77 Mon Sep 17 00:00:00 2001 From: Scott Piriou <30843220+pscott@users.noreply.github.com> Date: Wed, 30 Aug 2023 17:07:28 +0200 Subject: [PATCH] add assert in initializer function; test them --- starknet/src/factory/factory.cairo | 15 ++-- starknet/src/space/space.cairo | 4 + starknet/src/tests/setup/setup.cairo | 41 ++++++++-- starknet/src/tests/test_factory.cairo | 16 +++- starknet/src/tests/test_space.cairo | 103 +++++++++++++++++++++++++- 5 files changed, 158 insertions(+), 21 deletions(-) diff --git a/starknet/src/factory/factory.cairo b/starknet/src/factory/factory.cairo index cf6e1130..89867278 100644 --- a/starknet/src/factory/factory.cairo +++ b/starknet/src/factory/factory.cairo @@ -1,4 +1,4 @@ -use starknet::{ContractAddress, ClassHash}; +use starknet::{ContractAddress, ClassHash, SyscallResult}; #[starknet::interface] trait IFactory { @@ -7,7 +7,7 @@ trait IFactory { class_hash: ClassHash, contract_address_salt: felt252, initialize_calldata: Span - ) -> ContractAddress; + ) -> SyscallResult; } @@ -16,7 +16,7 @@ mod Factory { use super::IFactory; use starknet::{ ContractAddress, ClassHash, contract_address_const, - syscalls::{deploy_syscall, call_contract_syscall} + syscalls::{deploy_syscall, call_contract_syscall}, SyscallResult }; use result::ResultTrait; use array::{ArrayTrait, SpanTrait}; @@ -44,18 +44,17 @@ mod Factory { class_hash: ClassHash, contract_address_salt: felt252, initialize_calldata: Span - ) -> ContractAddress { + ) -> SyscallResult { let (space_address, _) = deploy_syscall( class_hash, contract_address_salt, array![].span(), false - ) - .unwrap(); + )?; // Call initializer. - call_contract_syscall(space_address, INITIALIZE_SELECTOR, initialize_calldata).unwrap(); + call_contract_syscall(space_address, INITIALIZE_SELECTOR, initialize_calldata)?; self.emit(Event::SpaceDeployed(SpaceDeployed { class_hash, space_address })); - space_address + Result::Ok(space_address) } } } diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index 85503810..ef49312a 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -294,6 +294,10 @@ mod Space { Reinitializable::unsafe_new_contract_state(); ReinitializableImpl::initialize(ref state); + assert(voting_strategies.len() != 0, 'empty voting strategies'); + assert(authenticators.len() != 0, 'empty authenticators'); + assert(voting_strategies.len() == voting_strategy_metadata_URIs.len(), 'len mismatch'); + //TODO: temporary component syntax let mut state = Ownable::unsafe_new_contract_state(); Ownable::initializer(ref state); diff --git a/starknet/src/tests/setup/setup.cairo b/starknet/src/tests/setup/setup.cairo index 38d4e412..3d060936 100644 --- a/starknet/src/tests/setup/setup.cairo +++ b/starknet/src/tests/setup/setup.cairo @@ -18,6 +18,7 @@ mod setup { use sx::factory::factory::{Factory, IFactoryDispatcher, IFactoryDispatcherTrait}; use starknet::ClassHash; use sx::space::space::{Space, ISpaceDispatcher, ISpaceDispatcherTrait}; + use debug::PrintTrait; #[derive(Drop)] struct Config { @@ -114,7 +115,18 @@ mod setup { proposal_validation_strategy.serialize(ref initializer_calldata); ArrayTrait::::new().serialize(ref initializer_calldata); voting_strategies.serialize(ref initializer_calldata); - ArrayTrait::::new().serialize(ref initializer_calldata); + let mut voting_strategies_metadata_uris: Array> = array![]; + + let mut i = 0; + loop { + if i >= voting_strategies.len() { + break; + } + voting_strategies_metadata_uris.append(array![]); + i += 1; + }; + + voting_strategies_metadata_uris.serialize(ref initializer_calldata); authenticators.serialize(ref initializer_calldata); ArrayTrait::::new().serialize(ref initializer_calldata); ArrayTrait::::new().serialize(ref initializer_calldata); @@ -126,10 +138,17 @@ mod setup { let space_class_hash: ClassHash = Space::TEST_CLASS_HASH.try_into().unwrap(); let contract_address_salt = 0; - let (factory_address, _) = deploy_syscall( - Factory::TEST_CLASS_HASH.try_into().unwrap(), 0, array![].span(), false - ) - .unwrap(); + let factory_address = + match deploy_syscall( + Factory::TEST_CLASS_HASH.try_into().unwrap(), 0, array![].span(), false + ) { + Result::Ok((address, _)) => address, + Result::Err(e) => { + e.print(); + panic_with_felt252('deploy failed'); + contract_address_const::<0>() + } + }; let factory = IFactoryDispatcher { contract_address: factory_address }; @@ -142,8 +161,16 @@ mod setup { config.voting_strategies, config.authenticators ); - let space_address = factory - .deploy(space_class_hash, contract_address_salt, initializer_calldata.span()); + let space_address = + match factory + .deploy(space_class_hash, contract_address_salt, initializer_calldata.span()) { + Result::Ok(address) => address, + Result::Err(e) => { + e.print(); + panic_with_felt252('deploy failed'); + contract_address_const::<0>() + }, + }; let space = ISpaceDispatcher { contract_address: space_address }; diff --git a/starknet/src/tests/test_factory.cairo b/starknet/src/tests/test_factory.cairo index b5f0bd75..42a75b33 100644 --- a/starknet/src/tests/test_factory.cairo +++ b/starknet/src/tests/test_factory.cairo @@ -9,6 +9,7 @@ mod tests { use sx::space::space::Space; use sx::types::Strategy; use starknet::ClassHash; + use debug::PrintTrait; use sx::tests::setup::setup::setup::{setup, get_initialize_calldata, deploy}; @@ -28,10 +29,17 @@ mod tests { fn test_deploy_reuse_salt() { let mut constructor_calldata = array![]; - let (factory_address, _) = deploy_syscall( - Factory::TEST_CLASS_HASH.try_into().unwrap(), 0, constructor_calldata.span(), false - ) - .unwrap(); + let factory_address = + match deploy_syscall( + Factory::TEST_CLASS_HASH.try_into().unwrap(), 0, constructor_calldata.span(), false + ) { + Result::Ok((address, _)) => address, + Result::Err(e) => { + e.print(); + panic_with_felt252('deploy failed'); + contract_address_const::<0>() + }, + }; let factory = IFactoryDispatcher { contract_address: factory_address }; diff --git a/starknet/src/tests/test_space.cairo b/starknet/src/tests/test_space.cairo index ba651290..3f1919a6 100644 --- a/starknet/src/tests/test_space.cairo +++ b/starknet/src/tests/test_space.cairo @@ -87,7 +87,7 @@ mod tests { vanilla_proposal_validation_strategy.clone(), array![], voting_strategies, - array![], + array![array![]], authenticators, array![], array![] @@ -103,6 +103,105 @@ mod tests { ); } + #[test] + #[available_gas(10000000)] + #[should_panic(expected: ('empty voting strategies',))] + fn empty_voting_strategies() { + let mut state = Space::unsafe_new_contract_state(); + let owner = contract_address_const::<0x123456789>(); + let min_voting_duration = 1_u32; + let max_voting_duration = 2_u32; + let voting_delay = 1_u32; + let proposal_validation_strategy = StrategyImpl::test_value(); + let proposal_validation_strategy_metadata_uri = array![]; + let voting_strategies = array![]; + let voting_strategies_metadata_uris = array![]; + let authenticators = array![contract_address_const::<0>()]; + let metadata_uri = array![]; + let dao_uri = array![]; + + Space::Space::initialize( + ref state, + owner, + min_voting_duration, + max_voting_duration, + voting_delay, + proposal_validation_strategy, + proposal_validation_strategy_metadata_uri, + voting_strategies, + voting_strategies_metadata_uris, + authenticators, + metadata_uri, + dao_uri, + ) + } + + #[test] + #[available_gas(10000000)] + #[should_panic(expected: ('empty authenticators',))] + fn empty_authenticators() { + let mut state = Space::unsafe_new_contract_state(); + let owner = contract_address_const::<0x123456789>(); + let min_voting_duration = 1_u32; + let max_voting_duration = 2_u32; + let voting_delay = 1_u32; + let proposal_validation_strategy = StrategyImpl::test_value(); + let proposal_validation_strategy_metadata_uri = array![]; + let voting_strategies = array![StrategyImpl::test_value()]; + let voting_strategies_metadata_uris = array![array![]]; + let authenticators = array![]; + let metadata_uri = array![]; + let dao_uri = array![]; + + Space::Space::initialize( + ref state, + owner, + min_voting_duration, + max_voting_duration, + voting_delay, + proposal_validation_strategy, + proposal_validation_strategy_metadata_uri, + voting_strategies, + voting_strategies_metadata_uris, + authenticators, + metadata_uri, + dao_uri, + ) + } + + #[test] + #[available_gas(10000000)] + #[should_panic(expected: ('len mismatch',))] + fn voting_strategies_and_metadata_uris_mismatch() { + let mut state = Space::unsafe_new_contract_state(); + let owner = contract_address_const::<0x123456789>(); + let min_voting_duration = 1_u32; + let max_voting_duration = 2_u32; + let voting_delay = 1_u32; + let proposal_validation_strategy = StrategyImpl::test_value(); + let proposal_validation_strategy_metadata_uri = array![]; + let voting_strategies = array![StrategyImpl::test_value()]; + let voting_strategies_metadata_uris = array![array![], array![]]; + let authenticators = array![contract_address_const::<0>()]; + let metadata_uri = array![]; + let dao_uri = array![]; + + Space::Space::initialize( + ref state, + owner, + min_voting_duration, + max_voting_duration, + voting_delay, + proposal_validation_strategy, + proposal_validation_strategy_metadata_uri, + voting_strategies, + voting_strategies_metadata_uris, + authenticators, + metadata_uri, + dao_uri, + ) + } + #[test] #[available_gas(100000000)] #[should_panic(expected: ('Already Initialized', 'ENTRYPOINT_FAILED'))] @@ -173,7 +272,7 @@ mod tests { vanilla_proposal_validation_strategy.clone(), array![], voting_strategies.clone(), - array![], + array![array![]], authenticators.clone(), array![], array![]