diff --git a/starknet/src/authenticators/eth_sig.cairo b/starknet/src/authenticators/eth_sig.cairo index 8144a340..ee22916d 100644 --- a/starknet/src/authenticators/eth_sig.cairo +++ b/starknet/src/authenticators/eth_sig.cairo @@ -10,9 +10,9 @@ trait IEthSigAuthenticator { v: u256, target: ContractAddress, author: EthAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, salt: u256, ); fn authenticate_vote( @@ -67,9 +67,9 @@ mod EthSigAuthenticator { v: u256, target: ContractAddress, author: EthAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, salt: u256, ) { signatures::verify_propose_sig( @@ -88,9 +88,9 @@ mod EthSigAuthenticator { ISpaceDispatcher { contract_address: target } .propose( UserAddress::Ethereum(author), + metadata_uri, execution_strategy, user_proposal_validation_params, - metadata_uri ); } @@ -126,7 +126,7 @@ mod EthSigAuthenticator { proposal_id, choice, user_voting_strategies, - metadata_uri + metadata_uri, ); } diff --git a/starknet/src/authenticators/eth_tx.cairo b/starknet/src/authenticators/eth_tx.cairo index b51747e5..f91dc904 100644 --- a/starknet/src/authenticators/eth_tx.cairo +++ b/starknet/src/authenticators/eth_tx.cairo @@ -7,9 +7,9 @@ trait IEthTxAuthenticator { ref self: TContractState, target: ContractAddress, author: EthAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array ); fn authenticate_vote( ref self: TContractState, @@ -53,17 +53,17 @@ mod EthTxAuthenticator { ref self: ContractState, target: ContractAddress, author: EthAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array ) { let mut payload = array![]; target.serialize(ref payload); PROPOSE_SELECTOR.serialize(ref payload); author.serialize(ref payload); + metadata_uri.serialize(ref payload); execution_strategy.serialize(ref payload); user_proposal_validation_params.serialize(ref payload); - metadata_uri.serialize(ref payload); let payload_hash = poseidon::poseidon_hash_span(payload.span()); consume_commit(ref self, payload_hash, author); @@ -71,9 +71,9 @@ mod EthTxAuthenticator { ISpaceDispatcher { contract_address: target } .propose( UserAddress::Ethereum(author), + metadata_uri, execution_strategy, user_proposal_validation_params, - metadata_uri ); } @@ -104,7 +104,7 @@ mod EthTxAuthenticator { proposal_id, choice, user_voting_strategies, - metadata_uri + metadata_uri, ); } diff --git a/starknet/src/authenticators/stark_sig.cairo b/starknet/src/authenticators/stark_sig.cairo index ae2a4cdb..64dff69b 100644 --- a/starknet/src/authenticators/stark_sig.cairo +++ b/starknet/src/authenticators/stark_sig.cairo @@ -8,9 +8,9 @@ trait IStarkSigAuthenticator { signature: Array, target: ContractAddress, author: ContractAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, salt: felt252, account_type: felt252 ); @@ -60,9 +60,9 @@ mod StarkSigAuthenticator { signature: Array, target: ContractAddress, author: ContractAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, salt: felt252, account_type: felt252 ) { @@ -84,9 +84,9 @@ mod StarkSigAuthenticator { ISpaceDispatcher { contract_address: target } .propose( UserAddress::Starknet(author), + metadata_uri, execution_strategy, user_proposal_validation_params, - metadata_uri ); } @@ -121,7 +121,7 @@ mod StarkSigAuthenticator { proposal_id, choice, user_voting_strategies, - metadata_uri + metadata_uri, ); } diff --git a/starknet/src/authenticators/stark_tx.cairo b/starknet/src/authenticators/stark_tx.cairo index 82b1dcfb..74751df2 100644 --- a/starknet/src/authenticators/stark_tx.cairo +++ b/starknet/src/authenticators/stark_tx.cairo @@ -7,9 +7,9 @@ trait IStarkTxAuthenticator { ref self: TContractState, space: ContractAddress, author: ContractAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array ); fn authenticate_vote( ref self: TContractState, @@ -48,18 +48,18 @@ mod StarkTxAuthenticator { ref self: ContractState, space: ContractAddress, author: ContractAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array ) { assert(info::get_caller_address() == author, 'Invalid Caller'); ISpaceDispatcher { contract_address: space } .propose( UserAddress::Starknet(author), + metadata_uri, execution_strategy, user_proposal_validation_params, - metadata_uri ); } @@ -80,7 +80,7 @@ mod StarkTxAuthenticator { proposal_id, choice, user_voting_strategies, - metadata_uri + metadata_uri, ); } diff --git a/starknet/src/factory/factory.cairo b/starknet/src/factory/factory.cairo index 195c1db5..6eab25d1 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 sx::utils::constants::INITIALIZE_SELECTOR; @@ -42,14 +42,13 @@ 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( @@ -58,7 +57,7 @@ mod Factory { ) ); - space_address + Result::Ok(space_address) } } } diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index e9ccfa3d..f6736622 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -1,4 +1,4 @@ -use starknet::{ClassHash, ContractAddress}; +use starknet::{ClassHash, ContractAddress, SyscallResult}; use sx::types::{ UserAddress, Strategy, Proposal, IndexedStrategy, Choice, UpdateSettingsCalldata, ProposalStatus }; @@ -9,6 +9,7 @@ trait ISpace { fn owner(self: @TContractState) -> ContractAddress; fn max_voting_duration(self: @TContractState) -> u32; fn min_voting_duration(self: @TContractState) -> u32; + fn dao_uri(self: @TContractState) -> Array; fn next_proposal_id(self: @TContractState) -> u256; fn voting_delay(self: @TContractState) -> u32; fn authenticators(self: @TContractState, account: ContractAddress) -> bool; @@ -16,10 +17,8 @@ trait ISpace { fn active_voting_strategies(self: @TContractState) -> u256; fn next_voting_strategy_index(self: @TContractState) -> u8; fn proposal_validation_strategy(self: @TContractState) -> Strategy; - // #[view] fn vote_power(self: @TContractState, proposal_id: u256, choice: Choice) -> u256; - // #[view] - // fn vote_registry(proposal_id: u256, voter: ContractAddress) -> bool; + fn vote_registry(self: @TContractState, proposal_id: u256, voter: UserAddress) -> bool; fn proposals(self: @TContractState, proposal_id: u256) -> Proposal; fn get_proposal_status(self: @TContractState, proposal_id: u256) -> ProposalStatus; @@ -45,9 +44,9 @@ trait ISpace { fn propose( ref self: TContractState, author: UserAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, ); fn vote( ref self: TContractState, @@ -65,10 +64,10 @@ trait ISpace { execution_strategy: Strategy, metadata_uri: Array, ); - fn cancel_proposal(ref self: TContractState, proposal_id: u256); + fn cancel(ref self: TContractState, proposal_id: u256); fn upgrade( ref self: TContractState, class_hash: ClassHash, initialize_calldata: Array - ); + ) -> SyscallResult<()>; } #[starknet::contract] @@ -76,7 +75,7 @@ mod Space { use super::ISpace; use starknet::{ storage_access::{StorePacking, StoreUsingPacking}, ClassHash, ContractAddress, info, Store, - syscalls + syscalls, SyscallResult, }; use sx::{ interfaces::{ @@ -86,8 +85,8 @@ mod Space { }, types::{ UserAddress, Choice, FinalizationStatus, Strategy, IndexedStrategy, Proposal, - ProposalStatus, PackedProposal, IndexedStrategyTrait, IndexedStrategyImpl, - UpdateSettingsCalldata, NoUpdateTrait, NoUpdateString, + PackedProposal, IndexedStrategyTrait, IndexedStrategyImpl, UpdateSettingsCalldata, + NoUpdateTrait, NoUpdateString, strategy::StoreFelt252Array, ProposalStatus, }, utils::{ reinitializable::{Reinitializable}, ReinitializableImpl, bits::BitSetter, @@ -105,6 +104,7 @@ mod Space { _max_voting_duration: u32, _next_proposal_id: u256, _voting_delay: u32, + _dao_uri: Array, _active_voting_strategies: u256, _voting_strategies: LegacyMap::, _next_voting_strategy_index: u8, @@ -158,8 +158,8 @@ mod Space { proposal_id: u256, author: UserAddress, proposal: Proposal, - payload: Span, metadata_uri: Span, + payload: Span, } #[derive(Drop, PartialEq, starknet::Event)] @@ -289,16 +289,21 @@ 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); Ownable::transfer_ownership(ref state, owner); + _set_dao_uri(ref self, dao_uri); _set_max_voting_duration( ref self, max_voting_duration ); // Need to set max before min, or else `max == 0` and set_min will revert _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); + _set_voting_delay(ref self, voting_delay); _add_voting_strategies(ref self, voting_strategies.span()); _add_authenticators(ref self, authenticators.span()); self._next_proposal_id.write(1_u256); @@ -307,13 +312,12 @@ mod Space { fn propose( ref self: ContractState, author: UserAddress, + metadata_uri: Array, execution_strategy: Strategy, user_proposal_validation_params: Array, - metadata_uri: Array, ) { assert_only_authenticator(@self); assert(author.is_non_zero(), 'Zero Address'); - let proposal_id = self._next_proposal_id.read(); // Proposal Validation let proposal_validation_strategy = self._proposal_validation_strategy.read(); @@ -349,11 +353,11 @@ mod Space { finalization_status: FinalizationStatus::Pending(()), active_voting_strategies: self._active_voting_strategies.read() }; - let clone_proposal = proposal.clone(); - self._proposals.write(proposal_id, proposal); + let proposal_id = self._next_proposal_id.read(); + self._proposals.write(proposal_id, proposal.clone()); - self._next_proposal_id.write(proposal_id + 1_u256); + self._next_proposal_id.write(proposal_id + 1); self .emit( @@ -361,9 +365,9 @@ mod Space { ProposalCreated { proposal_id: proposal_id, author: author, - proposal: clone_proposal, // TODO: use span, remove clone + proposal, + metadata_uri: metadata_uri.span(), payload: execution_strategy.params.span(), - metadata_uri: metadata_uri.span() } ) ); @@ -394,6 +398,9 @@ mod Space { self._vote_registry.read((proposal_id, voter)) == false, 'Voter has already voted' ); + // Written here to prevent re-entrency attacks via malicious voting strategies + self._vote_registry.write((proposal_id, voter), true); + let voting_power = _get_cumulative_power( @self, voter, @@ -401,16 +408,17 @@ mod Space { user_voting_strategies.span(), proposal.active_voting_strategies ); + assert(voting_power > 0, 'User has no voting power'); - assert(voting_power > 0_u256, 'User has no voting power'); self ._vote_power .write( (proposal_id, choice), self._vote_power.read((proposal_id, choice)) + voting_power ); - self._vote_registry.write((proposal_id, voter), true); + // Contrary to the SX-EVM implementation, we don't differentiate between `VoteCast` and `VoteCastWithMetadata` + // because calldata is free. self .emit( Event::VoteCast( @@ -425,6 +433,7 @@ mod Space { ); } + // TODO: missing `nonReentrant` modifier fn execute(ref self: ContractState, proposal_id: u256, execution_payload: Array) { let mut proposal = self._proposals.read(proposal_id); assert_proposal_exists(@proposal); @@ -438,20 +447,39 @@ mod Space { proposal.finalization_status == FinalizationStatus::Pending(()), 'Already finalized' ); - IExecutionStrategyDispatcher { contract_address: proposal.execution_strategy } + // We cache the proposal to prevent re-entrency attacks by setting + // the finalization status to `Executed` before calling the `execute` function. + let cached_proposal = proposal.clone(); + proposal.finalization_status = FinalizationStatus::Executed(()); + + self._proposals.write(proposal_id, proposal); + + IExecutionStrategyDispatcher { contract_address: cached_proposal.execution_strategy } .execute( - proposal.clone(), + cached_proposal, self._vote_power.read((proposal_id, Choice::For(()))), self._vote_power.read((proposal_id, Choice::Against(()))), self._vote_power.read((proposal_id, Choice::Abstain(()))), execution_payload ); - proposal.finalization_status = FinalizationStatus::Executed(()); + self.emit(Event::ProposalExecuted(ProposalExecuted { proposal_id: proposal_id })); + } + fn cancel(ref self: ContractState, proposal_id: u256) { + //TODO: temporary component syntax + let state = Ownable::unsafe_new_contract_state(); + Ownable::assert_only_owner(@state); + + let mut proposal = self._proposals.read(proposal_id); + assert_proposal_exists(@proposal); + assert( + proposal.finalization_status == FinalizationStatus::Pending(()), 'Already finalized' + ); + proposal.finalization_status = FinalizationStatus::Cancelled(()); self._proposals.write(proposal_id, proposal); - self.emit(Event::ProposalExecuted(ProposalExecuted { proposal_id: proposal_id })); + self.emit(Event::ProposalCancelled(ProposalCancelled { proposal_id: proposal_id })); } fn update_proposal( @@ -465,17 +493,19 @@ mod Space { assert(author.is_non_zero(), 'Zero Address'); let mut proposal = self._proposals.read(proposal_id); assert_proposal_exists(@proposal); - assert(proposal.author == author, 'Only Author'); + assert( + proposal.finalization_status == FinalizationStatus::Pending(()), 'Already finalized' + ); + assert(proposal.author == author, 'Invalid caller'); assert( info::get_block_timestamp() < proposal.start_timestamp.into(), 'Voting period started' ); - proposal.execution_strategy = execution_strategy.address; - proposal .execution_payload_hash = poseidon::poseidon_hash_span(execution_strategy.params.span()); + proposal.execution_strategy = execution_strategy.address; self._proposals.write(proposal_id, proposal); @@ -491,29 +521,14 @@ mod Space { ); } - fn cancel_proposal(ref self: ContractState, proposal_id: u256) { - //TODO: temporary component syntax - let state = Ownable::unsafe_new_contract_state(); - Ownable::assert_only_owner(@state); - let mut proposal = self._proposals.read(proposal_id); - assert_proposal_exists(@proposal); - assert( - proposal.finalization_status == FinalizationStatus::Pending(()), 'Already Finalized' - ); - proposal.finalization_status = FinalizationStatus::Cancelled(()); - self._proposals.write(proposal_id, proposal); - - self.emit(Event::ProposalCancelled(ProposalCancelled { proposal_id: proposal_id })); - } - fn upgrade( ref self: ContractState, class_hash: ClassHash, initialize_calldata: Array - ) { + ) -> SyscallResult<()> { 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'); - starknet::replace_class_syscall(class_hash).unwrap(); + starknet::replace_class_syscall(class_hash)?; // Allowing initializer to be called again. let mut state: Reinitializable::ContractState = @@ -523,8 +538,7 @@ mod Space { // Call initializer on the new version. syscalls::call_contract_syscall( info::get_contract_address(), INITIALIZE_SELECTOR, initialize_calldata.span() - ) - .unwrap(); + )?; self .emit( @@ -534,6 +548,7 @@ mod Space { } ) ); + SyscallResult::Ok(()) } fn owner(self: @ContractState) -> ContractAddress { @@ -558,6 +573,10 @@ mod Space { self._voting_delay.read() } + fn dao_uri(self: @ContractState) -> Array { + self._dao_uri.read() + } + fn authenticators(self: @ContractState, account: ContractAddress) -> bool { self._authenticators.read(account) } @@ -662,6 +681,20 @@ mod Space { ); } + if NoUpdateString::should_update((@input).metadata_uri) { + self + .emit( + Event::MetadataUriUpdated( + MetadataUriUpdated { metadata_uri: input.metadata_uri.span() } + ) + ); + } + + if NoUpdateString::should_update((@input).dao_uri) { + _set_dao_uri(ref self, input.dao_uri.clone()); + self.emit(Event::DaoUriUpdated(DaoUriUpdated { dao_uri: input.dao_uri.span() })); + } + if input.proposal_validation_strategy.should_update() { _set_proposal_validation_strategy( ref self, input.proposal_validation_strategy.clone() @@ -706,6 +739,14 @@ mod Space { } if input.voting_strategies_to_add.should_update() { + assert( + input + .voting_strategies_to_add + .len() == input + .voting_strategies_metadata_uris_to_add + .len(), + 'len mismatch' + ); _add_voting_strategies(ref self, input.voting_strategies_to_add.span()); self .emit( @@ -731,21 +772,10 @@ mod Space { ) ); } + } - // TODO: test once #506 is merged - if NoUpdateString::should_update((@input).metadata_uri) { - self - .emit( - Event::MetadataUriUpdated( - MetadataUriUpdated { metadata_uri: input.metadata_uri.span() } - ) - ); - } - - // TODO: test once #506 is merged - if NoUpdateString::should_update((@input).dao_uri) { - self.emit(Event::DaoUriUpdated(DaoUriUpdated { dao_uri: input.dao_uri.span() })); - } + fn vote_registry(self: @ContractState, proposal_id: u256, voter: UserAddress) -> bool { + self._vote_registry.read((proposal_id, voter)) } fn vote_power(self: @ContractState, proposal_id: u256, choice: Choice) -> u256 { @@ -777,7 +807,9 @@ mod Space { } fn assert_proposal_exists(proposal: @Proposal) { - assert(!(*proposal.start_timestamp).is_zero(), 'Proposal does not exist'); + assert( + *proposal.start_timestamp != 0, 'Proposal does not exist' + ); // TODO: test this assertion } fn _get_cumulative_power( @@ -825,6 +857,10 @@ mod Space { self._voting_delay.write(_voting_delay); } + fn _set_dao_uri(ref self: ContractState, _dao_uri: Array) { + self._dao_uri.write(_dao_uri); + } + fn _set_proposal_validation_strategy( ref self: ContractState, _proposal_validation_strategy: Strategy ) { diff --git a/starknet/src/tests/setup/setup.cairo b/starknet/src/tests/setup/setup.cairo index 08a8035f..6a372e7a 100644 --- a/starknet/src/tests/setup/setup.cairo +++ b/starknet/src/tests/setup/setup.cairo @@ -13,6 +13,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, Serde)] struct Config { @@ -97,7 +98,7 @@ mod setup { ); let proposal_validation_strategy_metadata_uri = array!['https:://rick.roll']; - let voting_strategies_metadata_uris = array![]; + let voting_strategies_metadata_uris = array![array![]]; let dao_uri = array!['https://dao.uri']; let metadata_uri = array!['https://metadata.uri']; @@ -120,16 +121,31 @@ 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 }; let mut initializer_calldata = config.get_initialize_calldata(); - 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 848f3ea8..237a3982 100644 --- a/starknet/src/tests/test_factory.cairo +++ b/starknet/src/tests/test_factory.cairo @@ -13,6 +13,7 @@ mod tests { use openzeppelin::tests::utils; use sx::space::space::Space::SpaceCreated; use sx::factory::factory::Factory::NewContractDeployed; + use debug::PrintTrait; use traits::{PartialEq}; use clone::Clone; @@ -80,10 +81,17 @@ mod tests { fn 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 9b5381c5..0fbbeb33 100644 --- a/starknet/src/tests/test_space.cairo +++ b/starknet/src/tests/test_space.cairo @@ -27,9 +27,9 @@ mod tests { UserAddress, Strategy, IndexedStrategy, Choice, FinalizationStatus, Proposal, UpdateSettingsCalldataImpl }; - use sx::tests::utils::strategy_trait::{StrategyImpl}; + use sx::tests::utils::strategy_trait::{StrategyImpl, StrategyDefault}; use sx::utils::constants::{PROPOSE_SELECTOR, VOTE_SELECTOR, UPDATE_PROPOSAL_SELECTOR}; - use traits::Default; + use sx::tests::mocks::executor::ExecutorWithoutTxExecutionStrategy; use openzeppelin::tests::utils; use Space::Space as SpaceImpl; @@ -139,7 +139,7 @@ mod tests { vanilla_proposal_validation_strategy.clone(), array![], voting_strategies, - array![], + array![array![]], authenticators, array![], array![] @@ -155,6 +155,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 = StrategyDefault::default(); + 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 = StrategyDefault::default(); + let proposal_validation_strategy_metadata_uri = array![]; + let voting_strategies = array![StrategyDefault::default()]; + 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 = StrategyDefault::default(); + let proposal_validation_strategy_metadata_uri = array![]; + let voting_strategies = array![StrategyDefault::default()]; + 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'))] @@ -225,7 +324,7 @@ mod tests { vanilla_proposal_validation_strategy.clone(), array![], voting_strategies.clone(), - array![], + array![array![]], authenticators.clone(), array![], array![] @@ -278,11 +377,11 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array![]; author.serialize(ref propose_calldata); + let metadata_uri: Array = array![]; + metadata_uri.serialize(ref propose_calldata); vanilla_execution_strategy.serialize(ref propose_calldata); let user_proposal_validation_params: Array = array![]; user_proposal_validation_params.serialize(ref propose_calldata); - let metadata_uri: Array = array![]; - metadata_uri.serialize(ref propose_calldata); // Create Proposal authenticator.authenticate(space_contract_address, PROPOSE_SELECTOR, propose_calldata); @@ -412,8 +511,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array![]; author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Try to create Proposal @@ -448,8 +547,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array![]; author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -486,7 +585,8 @@ mod tests { #[test] #[available_gas(10000000000)] - fn get_proposal_status() { + #[should_panic(expected: ('Invalid payload hash', 'ENTRYPOINT_FAILED'))] + fn execute_invalid_payload() { let config = setup(); let (factory, space) = deploy(@config); @@ -494,7 +594,41 @@ mod tests { contract_address: *config.authenticators.at(0), }; - let quorum = u256_from_felt252(1); + let (executor_address, _) = deploy_syscall( + ExecutorWithoutTxExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + array![].span(), + false + ) + .unwrap(); + let execution_strategy = StrategyImpl::from_address(executor_address); + let author = UserAddress::Starknet(contract_address_const::<0x5678>()); + let mut propose_calldata = array![]; + author.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + testing::set_block_timestamp( + config.voting_delay.into() + config.max_voting_duration.into() + ); + + // Execute Proposal + space.execute(1, array!['random', 'stuff']); + } + + #[test] + #[available_gas(10000000000)] + fn get_proposal_status() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + let quorum = 1_u256; let mut constructor_calldata = array![]; quorum.serialize(ref constructor_calldata); @@ -511,8 +645,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array![]; author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -547,25 +681,19 @@ mod tests { contract_address: *config.authenticators.at(0) }; - let quorum = u256_from_felt252(1); - let mut constructor_calldata = array![]; - quorum.serialize(ref constructor_calldata); - - let (vanilla_execution_strategy_address, _) = deploy_syscall( - VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + let (executor_address, _) = deploy_syscall( + ExecutorWithoutTxExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), 0, - constructor_calldata.span(), + array![].span(), false ) .unwrap(); - let vanilla_execution_strategy = StrategyImpl::from_address( - vanilla_execution_strategy_address - ); + let execution_strategy = StrategyImpl::from_address(executor_address); let mut propose_calldata = array![]; let author = UserAddress::Starknet(contract_address_const::<0x5678>()); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -579,7 +707,7 @@ mod tests { // Cancel Proposal testing::set_caller_address(config.owner); testing::set_contract_address(config.owner); - space.cancel_proposal(proposal_id); + space.cancel(proposal_id); let proposal = space.proposals(proposal_id); assert(proposal.finalization_status == FinalizationStatus::Cancelled(()), 'cancelled'); @@ -597,6 +725,73 @@ mod tests { authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); } + #[test] + #[available_gas(1000000000)] + #[should_panic(expected: ('Caller is not the owner',))] + fn cancel_unauthorized() { + let mut state = Space::unsafe_new_contract_state(); + + testing::set_caller_address(contract_address_const::<'random'>()); + Space::Space::cancel(ref state, 0); + } + + #[test] + #[available_gas(1000000000)] + #[should_panic( + expected: ('Unknown enum indicator:', 0, 'ENTRYPOINT_FAILED') + )] // TODO: replace once `default` works on Proposal + // #[should_panic(expected: ('Proposal does not exist', 'ENTRYPOINT_FAILED'))] + fn cancel_inexistent_proposal() { + let config = setup(); + let (factory, space) = deploy(@config); + + testing::set_contract_address(config.owner); + space.cancel(0); + } + + #[test] + #[available_gas(1000000000)] + #[should_panic(expected: ('Already finalized', 'ENTRYPOINT_FAILED'))] + fn cancel_already_finalized() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let (executor_address, _) = deploy_syscall( + ExecutorWithoutTxExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + 0, + array![].span(), + false + ) + .unwrap(); + + let execution_strategy = StrategyImpl::from_address(executor_address); + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + let mut propose_calldata = array![]; + author.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + testing::set_block_timestamp( + config.voting_delay.into() + config.max_voting_duration.into() + ); + + // Execute Proposal + space.execute(1, array![]); + + testing::set_contract_address(config.owner); + // Cancel the proposal + space.cancel(1); + } + + #[test] #[available_gas(10000000000)] #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED'))] @@ -626,8 +821,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x0>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -645,26 +840,13 @@ mod tests { contract_address: *config.authenticators.at(0), }; - let quorum = u256_from_felt252(1); - let mut constructor_calldata = ArrayTrait::::new(); - quorum.serialize(ref constructor_calldata); - - let (vanilla_execution_strategy_address, _) = deploy_syscall( - VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), - 0, - constructor_calldata.span(), - false - ) - .unwrap(); - let vanilla_execution_strategy = StrategyImpl::from_address( - vanilla_execution_strategy_address - ); + let execution_strategy = StrategyDefault::default(); // author is the zero address let author = UserAddress::Starknet(contract_address_const::<0x0>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -676,22 +858,20 @@ mod tests { let proposal_id = u256_from_felt252(1); proposal_id.serialize(ref update_calldata); // Keeping the same execution strategy contract but changing the payload - let mut new_payload = ArrayTrait::::new(); - new_payload.append(1); - let execution_strategy = Strategy { - address: vanilla_execution_strategy.address, params: new_payload - }; - execution_strategy.serialize(ref update_calldata); + let mut new_execution_strategy = execution_strategy; + new_execution_strategy.params = array!['random', 'stuff']; + new_execution_strategy.serialize(ref update_calldata); ArrayTrait::::new().serialize(ref update_calldata); authenticator .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); } + #[test] #[available_gas(10000000000)] - #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED'))] - fn vote_zero_address() { + #[should_panic(expected: ('Already finalized', 'ENTRYPOINT_FAILED'))] + fn update_already_finalized() { let config = setup(); let (factory, space) = deploy(@config); @@ -699,46 +879,194 @@ mod tests { contract_address: *config.authenticators.at(0), }; - let quorum = u256_from_felt252(1); - let mut constructor_calldata = ArrayTrait::::new(); - quorum.serialize(ref constructor_calldata); - - let (vanilla_execution_strategy_address, _) = deploy_syscall( - VanillaExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), + let (executor_address, _) = deploy_syscall( + ExecutorWithoutTxExecutionStrategy::TEST_CLASS_HASH.try_into().unwrap(), 0, - constructor_calldata.span(), + array![].span(), false ) .unwrap(); - let vanilla_execution_strategy = StrategyImpl::from_address( - vanilla_execution_strategy_address - ); - let author = UserAddress::Starknet(contract_address_const::<0x5678>()); + let execution_strategy = StrategyImpl::from_address(executor_address); + + let author = UserAddress::Starknet(contract_address_const::<'author'>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); - // Update Proposal + // Execute proposal + space.execute(1, array![]); + + // Try to update Proposal let mut update_calldata = array::ArrayTrait::::new(); author.serialize(ref update_calldata); let proposal_id = u256_from_felt252(1); proposal_id.serialize(ref update_calldata); // Keeping the same execution strategy contract but changing the payload - let mut new_payload = ArrayTrait::::new(); - new_payload.append(1); - let execution_strategy = Strategy { - address: vanilla_execution_strategy.address, params: new_payload + let mut new_execution_strategy = execution_strategy; + new_execution_strategy.params = array!['random', 'stuff']; + new_execution_strategy.serialize(ref update_calldata); + ArrayTrait::::new().serialize(ref update_calldata); + + authenticator + .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Invalid caller', 'ENTRYPOINT_FAILED'))] + fn update_invalid_caller() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), }; - execution_strategy.serialize(ref update_calldata); + + let execution_strategy = StrategyDefault::default(); + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + // Try to update Proposal + let mut update_calldata = array::ArrayTrait::::new(); + + // author is different this time + let author = UserAddress::Starknet(contract_address_const::<'author2'>()); + author.serialize(ref update_calldata); + let proposal_id = u256_from_felt252(1); + proposal_id.serialize(ref update_calldata); + // Keeping the same execution strategy contract but changing the payload + let mut new_execution_strategy = execution_strategy; + new_execution_strategy.params = array!['random', 'stuff']; + new_execution_strategy.serialize(ref update_calldata); ArrayTrait::::new().serialize(ref update_calldata); authenticator .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Voting period started', 'ENTRYPOINT_FAILED'))] + fn update_voting_period_started() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let execution_strategy = StrategyDefault::default(); + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + // Skip voting delay + testing::set_block_timestamp(config.voting_delay.into()); + + // Try to update Proposal + let mut update_calldata = array::ArrayTrait::::new(); + author.serialize(ref update_calldata); + let proposal_id = u256_from_felt252(1); + proposal_id.serialize(ref update_calldata); + + // Keeping the same execution strategy contract but changing the payload + let mut new_execution_strategy = execution_strategy; + new_execution_strategy.params = array!['random', 'stuff']; + new_execution_strategy.serialize(ref update_calldata); + ArrayTrait::::new().serialize(ref update_calldata); + + authenticator + .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic( + expected: ('Unknown enum indicator:', 'ENTRYPOINT_FAILED') + )] // TODO: replace once `default` works on Proposal + // #[should_panic(expected: ('Proposal does not exist', 'ENTRYPOINT_FAILED'))] + fn update_inexistent_proposal() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + // Update Proposal + let mut update_calldata = array::ArrayTrait::::new(); + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + author.serialize(ref update_calldata); + let proposal_id = 1_u256; + proposal_id.serialize(ref update_calldata); + // Keeping the same execution strategy contract but changing the payload + let new_execution_strategy = StrategyDefault::default(); + new_execution_strategy.serialize(ref update_calldata); + ArrayTrait::::new().serialize(ref update_calldata); + + authenticator + .authenticate(space.contract_address, UPDATE_PROPOSAL_SELECTOR, update_calldata); + } + + #[test] + #[available_gas(1000000000)] + #[should_panic(expected: ('Caller is not an authenticator',))] + fn update_unauthorized() { + let mut state = Space::unsafe_new_contract_state(); + + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + let proposal_id = 1; + let execution_strategy = StrategyDefault::default(); + let metadata_uri = array![]; + testing::set_caller_address(contract_address_const::<'random'>()); + Space::Space::update_proposal( + ref state, author, proposal_id, execution_strategy, metadata_uri + ); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('Zero Address', 'ENTRYPOINT_FAILED'))] + fn vote_zero_address() { + let config = setup(); + let (factory, space) = deploy(@config); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + let quorum = u256_from_felt252(1); + let mut constructor_calldata = ArrayTrait::::new(); + quorum.serialize(ref constructor_calldata); + + let vanilla_execution_strategy = StrategyDefault::default(); + let author = UserAddress::Starknet(contract_address_const::<'author'>()); + let mut propose_calldata = array::ArrayTrait::::new(); + author.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); // Increasing block block_number by 1 to pass voting delay testing::set_block_number(1_u64); @@ -747,13 +1075,13 @@ mod tests { // Voter is the zero address let voter = UserAddress::Starknet(contract_address_const::<0x0>()); voter.serialize(ref vote_calldata); - let proposal_id = u256_from_felt252(1); + let proposal_id = 1_u256; proposal_id.serialize(ref vote_calldata); let choice = Choice::For(()); choice.serialize(ref vote_calldata); - let mut user_voting_strategies = ArrayTrait::::new(); - user_voting_strategies - .append(IndexedStrategy { index: 0_u8, params: ArrayTrait::::new() }); + let mut user_voting_strategies = array![ + IndexedStrategy { index: 0_u8, params: ArrayTrait::::new() } + ]; user_voting_strategies.serialize(ref vote_calldata); ArrayTrait::::new().serialize(ref vote_calldata); diff --git a/starknet/src/tests/test_stark_tx_auth.cairo b/starknet/src/tests/test_stark_tx_auth.cairo index 30c18e46..592a4c80 100644 --- a/starknet/src/tests/test_stark_tx_auth.cairo +++ b/starknet/src/tests/test_stark_tx_auth.cairo @@ -72,7 +72,7 @@ mod tests { testing::set_contract_address(author); authenticator .authenticate_propose( - space.contract_address, author, vanilla_execution_strategy, array![], array![] + space.contract_address, author, array![], vanilla_execution_strategy, array![] ); assert(space.next_proposal_id() == 2_u256, 'next_proposal_id should be 2'); @@ -89,7 +89,7 @@ mod tests { testing::set_contract_address(author); authenticator .authenticate_update_proposal( - space.contract_address, author, proposal_id, new_execution_strategy, array![] + space.contract_address, author, proposal_id, new_execution_strategy, array![], ); // Increasing block timestamp by 1 to pass voting delay @@ -140,7 +140,7 @@ mod tests { testing::set_contract_address(config.owner); authenticator .authenticate_propose( - space.contract_address, author, vanilla_execution_strategy, array![], array![] + space.contract_address, author, array![], vanilla_execution_strategy, array![], ); } @@ -172,7 +172,7 @@ mod tests { testing::set_contract_address(author); authenticator .authenticate_propose( - space.contract_address, author, vanilla_execution_strategy, array![], array![] + space.contract_address, author, array![], vanilla_execution_strategy, array![], ); assert(space.next_proposal_id() == 2_u256, 'next_proposal_id should be 2'); @@ -222,7 +222,7 @@ mod tests { testing::set_contract_address(author); authenticator .authenticate_propose( - space.contract_address, author, vanilla_execution_strategy, array![], array![] + space.contract_address, author, array![], vanilla_execution_strategy, array![], ); assert(space.next_proposal_id() == 2_u256, 'next_proposal_id should be 2'); diff --git a/starknet/src/tests/test_update_settings.cairo b/starknet/src/tests/test_update_settings.cairo index 28829ad9..bcb07aa2 100644 --- a/starknet/src/tests/test_update_settings.cairo +++ b/starknet/src/tests/test_update_settings.cairo @@ -3,7 +3,7 @@ mod tests { use sx::space::space::{Space, ISpaceDispatcher, ISpaceDispatcherTrait}; use sx::tests::setup::setup::setup::{setup, deploy, Config}; use sx::types::{UpdateSettingsCalldata, UpdateSettingsCalldataImpl}; - use sx::tests::utils::strategy_trait::{StrategyImpl}; + use sx::tests::utils::strategy_trait::{StrategyImpl, StrategyDefault}; use starknet::testing; use starknet::info; use starknet::{contract_address_const, ContractAddress}; @@ -185,11 +185,10 @@ mod tests { fn dao_uri() { let (config, space) = setup_update_settings(); let mut input = UpdateSettingsCalldataImpl::default(); - let mut arr = array![]; - 'hello!'.serialize(ref arr); - input.dao_uri = arr; + input.dao_uri = array!['hello!']; space.update_settings(input.clone()); + assert(space.dao_uri() == input.dao_uri, 'dao uri not updated'); let expected = DaoUriUpdated { dao_uri: input.dao_uri.span() }; assert_correct_event::(space.contract_address, expected); } @@ -270,6 +269,29 @@ mod tests { let mut arr = array![vs1.clone(), vs2.clone()]; input.voting_strategies_to_add = arr; + input.voting_strategies_metadata_uris_to_add = array![array![], array![]]; + + space.update_settings(input); + + assert(space.voting_strategies(1) == vs1, 'Voting strategy 1 not added'); + assert(space.voting_strategies(2) == vs2, 'Voting strategy 2 not added'); + assert(space.active_voting_strategies() == 0b111, 'Voting strategies not active'); + // TODO: check event once it's been added + } + + #[test] + #[available_gas(10000000000)] + #[should_panic(expected: ('len mismatch', 'ENTRYPOINT_FAILED'))] + fn add_voting_strategies_mismatch() { + let (config, space) = setup_update_settings(); + let mut input = UpdateSettingsCalldataImpl::default(); + + let vs1 = StrategyImpl::from_address(contract_address_const::<'votingStrategy1'>()); + let vs2 = StrategyImpl::from_address(contract_address_const::<'votingStrategy2'>()); + + let mut arr = array![vs1.clone(), vs2.clone()]; + input.voting_strategies_to_add = arr; + input.voting_strategies_metadata_uris_to_add = array![array![]]; // missing one uri! space.update_settings(input.clone()); @@ -294,6 +316,7 @@ mod tests { let vs1 = StrategyImpl::from_address(contract_address_const::<'votingStrategy1'>()); let mut arr = array![vs1.clone()]; input.voting_strategies_to_add = arr; + input.voting_strategies_metadata_uris_to_add = array![array![]]; space.update_settings(input); assert(space.voting_strategies(1) == vs1, 'Voting strategy 1 not added'); assert(space.active_voting_strategies() == 0b11, 'Voting strategy not active'); diff --git a/starknet/src/tests/test_upgrade.cairo b/starknet/src/tests/test_upgrade.cairo index ec95abd7..ab972039 100644 --- a/starknet/src/tests/test_upgrade.cairo +++ b/starknet/src/tests/test_upgrade.cairo @@ -24,7 +24,7 @@ mod tests { UserAddress, Strategy, IndexedStrategy, Choice, FinalizationStatus, Proposal, UpdateSettingsCalldataImpl }; - use sx::utils::constants::{PROPOSE_SELECTOR, VOTE_SELECTOR, UPDATE_PROPOSAL_SELECTOR}; + use sx::utils::constants::{PROPOSE_SELECTOR}; use sx::tests::setup::setup::setup::{setup, deploy}; use sx::interfaces::{ IProposalValidationStrategyDispatcher, IProposalValidationStrategyDispatcherTrait @@ -94,8 +94,8 @@ mod tests { let mut propose_calldata = array![]; UserAddress::Starknet(contract_address_const::<0x7676>()).serialize(ref propose_calldata); - execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); let authenticator = IVanillaAuthenticatorDispatcher { diff --git a/starknet/src/tests/utils/strategy_trait.cairo b/starknet/src/tests/utils/strategy_trait.cairo index 5964fc37..53f34685 100644 --- a/starknet/src/tests/utils/strategy_trait.cairo +++ b/starknet/src/tests/utils/strategy_trait.cairo @@ -2,15 +2,16 @@ use sx::types::Strategy; use starknet::{ContractAddress, contract_address_const}; trait StrategyTrait { - fn value() -> Strategy; fn from_address(addr: ContractAddress) -> Strategy; } -impl StrategyImpl of StrategyTrait { - fn value() -> Strategy { - Strategy { address: contract_address_const::<0x5c011>(), params: array![], } +impl StrategyDefault of Default { + fn default() -> Strategy { + Strategy { address: contract_address_const::<'snapshot'>(), params: array![], } } +} +impl StrategyImpl of StrategyTrait { fn from_address(addr: ContractAddress) -> Strategy { Strategy { address: addr, params: array![], } } diff --git a/starknet/src/tests/vote.cairo b/starknet/src/tests/vote.cairo index 0fe3db4e..59ef34c4 100644 --- a/starknet/src/tests/vote.cairo +++ b/starknet/src/tests/vote.cairo @@ -53,8 +53,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array![]; author.serialize(ref propose_calldata); - execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create Proposal @@ -110,6 +110,7 @@ mod tests { utils::drop_events(space.contract_address, 4); authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); + assert(space.vote_registry(proposal_id, voter) == true, 'vote registry incorrect'); assert(space.vote_power(proposal_id, Choice::For(())) == 1, 'Vote power should be 1'); assert(space.vote_power(proposal_id, Choice::Against(())) == 0, 'Vote power should be 0'); assert(space.vote_power(proposal_id, Choice::Abstain(())) == 0, 'Vote power should be 0'); @@ -150,6 +151,7 @@ mod tests { utils::drop_events(space.contract_address, 4); authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); + assert(space.vote_registry(proposal_id, voter) == true, 'vote registry incorrect'); assert(space.vote_power(proposal_id, Choice::For(())) == 0, 'Vote power should be 0'); assert(space.vote_power(proposal_id, Choice::Against(())) == 1, 'Vote power should be 1'); assert(space.vote_power(proposal_id, Choice::Abstain(())) == 0, 'Vote power should be 0'); @@ -189,6 +191,7 @@ mod tests { // empty events queue utils::drop_events(space.contract_address, 4); authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); + assert(space.vote_registry(proposal_id, voter) == true, 'vote registry incorrect'); assert(space.vote_power(proposal_id, Choice::For(())) == 0, 'Vote power should be 0'); assert(space.vote_power(proposal_id, Choice::Against(())) == 0, 'Vote power should be 0'); assert(space.vote_power(proposal_id, Choice::Abstain(())) == 1, 'Vote power should be 1'); @@ -380,6 +383,7 @@ mod tests { let mut input = UpdateSettingsCalldataImpl::default(); input.voting_strategies_to_add = array![no_voting_power_strategy]; + input.voting_strategies_metadata_uris_to_add = array![array![]]; testing::set_contract_address(config.owner); space.update_settings(input); @@ -404,4 +408,38 @@ mod tests { authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); } + + #[test] + #[available_gas(10000000000)] + #[should_panic( + expected: ('Unknown enum indicator:', 'ENTRYPOINT_FAILED') + )] // TODO: replace once `default` works on Proposal + fn vote_inexistant_proposal() { + let config = setup(); + let (factory, space) = deploy(@config); + + let execution_strategy = get_execution_strategy(); + + let authenticator = IVanillaAuthenticatorDispatcher { + contract_address: *config.authenticators.at(0), + }; + + create_proposal(authenticator, space, execution_strategy); + + // Increasing block timestamp pass voting delay + testing::set_block_timestamp(config.voting_delay.into()); + + let mut vote_calldata = array![]; + let voter = UserAddress::Starknet(contract_address_const::<0x8765>()); + voter.serialize(ref vote_calldata); + let proposal_id = 42_u256; // inexistent proposal + proposal_id.serialize(ref vote_calldata); + let choice = Choice::For(()); + choice.serialize(ref vote_calldata); + let mut user_voting_strategies = array![IndexedStrategy { index: 0_u8, params: array![] }]; + user_voting_strategies.serialize(ref vote_calldata); + ArrayTrait::::new().serialize(ref vote_calldata); + + authenticator.authenticate(space.contract_address, VOTE_SELECTOR, vote_calldata); + } } diff --git a/starknet/src/tests/voting_strategies/erc20_votes.cairo b/starknet/src/tests/voting_strategies/erc20_votes.cairo index b231f9d7..fd5411ee 100644 --- a/starknet/src/tests/voting_strategies/erc20_votes.cairo +++ b/starknet/src/tests/voting_strategies/erc20_votes.cairo @@ -77,6 +77,7 @@ mod tests { let to_add = array![erc20_voting_strategy]; let mut settings = UpdateSettingsCalldataImpl::default(); settings.voting_strategies_to_add = to_add; + settings.voting_strategies_metadata_uris_to_add = array![array![]]; settings.voting_strategies_to_remove = to_remove; testing::set_contract_address(config.owner); @@ -165,8 +166,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create a proposal @@ -212,8 +213,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create proposal @@ -262,8 +263,8 @@ mod tests { let author = UserAddress::Starknet(contract_address_const::<0x5678>()); let mut propose_calldata = array::ArrayTrait::::new(); author.serialize(ref propose_calldata); - vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); + vanilla_execution_strategy.serialize(ref propose_calldata); ArrayTrait::::new().serialize(ref propose_calldata); // Create proposal diff --git a/starknet/src/types/update_settings_calldata.cairo b/starknet/src/types/update_settings_calldata.cairo index 5b08e8fd..d15b754c 100644 --- a/starknet/src/types/update_settings_calldata.cairo +++ b/starknet/src/types/update_settings_calldata.cairo @@ -87,12 +87,11 @@ impl NoUpdateString of NoUpdateTrait> { Option::Some(e) => { *e.unbox() != 'No update' }, - Option::None => false, + Option::None => true, } } } -// TODO: find a way for "Strings" impl NoUpdateArray of NoUpdateTrait> { fn no_update() -> Array { array![] @@ -109,14 +108,14 @@ impl UpdateSettingsCalldataImpl of UpdateSettingsCalldataTrait { min_voting_duration: NoUpdateU32::no_update(), max_voting_duration: NoUpdateU32::no_update(), voting_delay: NoUpdateU32::no_update(), - metadata_uri: NoUpdateArray::no_update(), // TODO: string - dao_uri: NoUpdateArray::no_update(), // TODO: string + metadata_uri: NoUpdateString::no_update(), + dao_uri: NoUpdateString::no_update(), proposal_validation_strategy: NoUpdateStrategy::no_update(), - proposal_validation_strategy_metadata_uri: NoUpdateArray::no_update(), // TODO: string + proposal_validation_strategy_metadata_uri: NoUpdateString::no_update(), authenticators_to_add: NoUpdateArray::no_update(), authenticators_to_remove: NoUpdateArray::no_update(), voting_strategies_to_add: NoUpdateArray::no_update(), - voting_strategies_metadata_uris_to_add: NoUpdateArray::no_update(), // TODO: string + voting_strategies_metadata_uris_to_add: NoUpdateArray::no_update(), voting_strategies_to_remove: NoUpdateArray::no_update(), } } diff --git a/starknet/test/l1-execution.test.ts b/starknet/test/l1-execution.test.ts index 5f655f49..868386df 100644 --- a/starknet/test/l1-execution.test.ts +++ b/starknet/test/l1-execution.test.ts @@ -77,7 +77,7 @@ describe('L1 Avatar Execution', function () { }, _proposal_validation_strategy_metadata_URI: [], _voting_strategies: [{ address: vanillaVotingStrategy.address, params: [] }], - _voting_strategies_metadata_URI: [], + _voting_strategies_metadata_URI: [[]], _authenticators: [starkTxAuthenticator.address], _metadata_URI: [], _dao_URI: [], @@ -140,12 +140,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -243,12 +243,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -341,12 +341,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -441,12 +441,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -538,12 +538,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -643,12 +643,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -740,12 +740,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -837,12 +837,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); @@ -922,12 +922,12 @@ describe('L1 Avatar Execution', function () { CallData.compile({ space: space.address, author: account.address, + metadataURI: [], executionStrategy: { address: ethRelayer.address, params: executionPayload, }, userProposalValidationParams: [], - metadataURI: [], }), { rawInput: true }, ); diff --git a/starknet/test/stark-sig.test.ts b/starknet/test/stark-sig.test.ts index 0e7a22aa..483b0320 100644 --- a/starknet/test/stark-sig.test.ts +++ b/starknet/test/stark-sig.test.ts @@ -105,7 +105,7 @@ describe('Starknet Signature Authenticator', () => { }, _proposal_validation_strategy_metadata_URI: [], _voting_strategies: [{ address: vanillaVotingStrategyAddress, params: [] }], - _voting_strategies_metadata_URI: [], + _voting_strategies_metadata_URI: [[]], _authenticators: [starkSigAuthAddress], _metadata_URI: [], _dao_URI: [], @@ -125,12 +125,12 @@ describe('Starknet Signature Authenticator', () => { const proposeMsg: Propose = { space: spaceAddress, author: address0, + metadataURI: ['0x1', '0x2', '0x3', '0x4'], executionStrategy: { address: '0x0000000000000000000000000000000000001234', params: ['0x5', '0x6', '0x7', '0x8'], }, userProposalValidationParams: ['0x1', '0x2', '0x3', '0x4'], - metadataURI: ['0x1', '0x2', '0x3', '0x4'], salt: '0x0', };