From 10f373d1607f83123eb1fabf326c8781a20c6c8b Mon Sep 17 00:00:00 2001 From: pscott <30843220+pscott@users.noreply.github.com> Date: Tue, 5 Sep 2023 18:07:48 +0200 Subject: [PATCH] feat: add get_proposal_status (#527) * feat: add get_proposal_status * expose get_proposal_status on space * unit test simple quorum execution strategy --- .../execution_strategies/eth_relayer.cairo | 13 +- .../execution_strategies/simple_quorum.cairo | 230 +++++++++++++++++- .../src/execution_strategies/vanilla.cairo | 23 +- .../src/interfaces/i_execution_strategy.cairo | 10 +- starknet/src/space/space.cairo | 24 +- starknet/src/tests/mocks/executor.cairo | 22 ++ starknet/src/tests/test_space.cairo | 53 +++- starknet/src/types/proposal.cairo | 17 +- starknet/src/types/proposal_status.cairo | 3 +- starknet/src/types/user_address.cairo | 2 +- 10 files changed, 378 insertions(+), 19 deletions(-) diff --git a/starknet/src/execution_strategies/eth_relayer.cairo b/starknet/src/execution_strategies/eth_relayer.cairo index 888a8e17..88c6eff1 100644 --- a/starknet/src/execution_strategies/eth_relayer.cairo +++ b/starknet/src/execution_strategies/eth_relayer.cairo @@ -6,7 +6,7 @@ mod EthRelayerExecutionStrategy { use serde::Serde; use starknet::{info, syscalls, EthAddress}; use sx::interfaces::IExecutionStrategy; - use sx::types::Proposal; + use sx::types::{Proposal, ProposalStatus}; #[storage] struct Storage {} @@ -53,5 +53,16 @@ mod EthRelayerExecutionStrategy { fn get_strategy_type(self: @ContractState) -> felt252 { 'EthRelayer' } + + fn get_proposal_status( + self: @ContractState, + proposal: Proposal, + votes_for: u256, + votes_against: u256, + votes_abstain: u256, + ) -> ProposalStatus { + panic_with_felt252('unimplemented'); + ProposalStatus::Cancelled(()) + } } } diff --git a/starknet/src/execution_strategies/simple_quorum.cairo b/starknet/src/execution_strategies/simple_quorum.cairo index 2c6bf603..b3737622 100644 --- a/starknet/src/execution_strategies/simple_quorum.cairo +++ b/starknet/src/execution_strategies/simple_quorum.cairo @@ -27,7 +27,7 @@ mod SimpleQuorumExecutionStrategy { proposal: @Proposal, votes_for: u256, votes_against: u256, - votes_abstain: u256 + votes_abstain: u256, ) -> ProposalStatus { let accepted = _quorum_reached(self._quorum.read(), votes_for, votes_against, votes_abstain) & _supported(votes_for, votes_against); @@ -67,3 +67,231 @@ mod SimpleQuorumExecutionStrategy { votes_for > votes_against } } + +#[cfg(test)] +mod tests { + use super::SimpleQuorumExecutionStrategy; + use super::SimpleQuorumExecutionStrategy::{get_proposal_status, initializer}; + use sx::types::{Proposal, proposal::ProposalDefault, FinalizationStatus, ProposalStatus}; + + #[test] + #[available_gas(10000000)] + fn cancelled() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + + let mut proposal = ProposalDefault::default(); + proposal.finalization_status = FinalizationStatus::Cancelled(()); + let votes_for = 0; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Cancelled(()), 'failed cancelled'); + } + + #[test] + #[available_gas(10000000)] + fn executed() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + + let mut proposal = ProposalDefault::default(); + proposal.finalization_status = FinalizationStatus::Executed(()); + let votes_for = 0; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Executed(()), 'failed executed'); + } + + #[test] + #[available_gas(10000000)] + fn voting_delay() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + + let mut proposal = ProposalDefault::default(); + proposal.start_timestamp = 42424242; + let votes_for = 0; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingDelay(()), 'failed voting_delay'); + } + + #[test] + #[available_gas(10000000)] + fn voting_period() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + + let mut proposal = ProposalDefault::default(); + proposal.min_end_timestamp = 42424242; + let votes_for = 0; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingPeriod(()), 'failed min_end_timestamp'); + } + + #[test] + #[available_gas(10000000)] + fn shortcut_accepted() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + proposal.max_end_timestamp = 10; + let votes_for = quorum; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingPeriodAccepted(()), 'failed shortcut_accepted'); + } + + #[test] + #[available_gas(10000000)] + fn shortcut_only_abstains() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + proposal.max_end_timestamp = 10; + let votes_for = 0; + let votes_against = 0; + let votes_abstain = quorum; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_only_abstains'); + } + + #[test] + #[available_gas(10000000)] + fn shortcut_only_againsts() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + proposal.max_end_timestamp = 10; + let votes_for = 0; + let votes_against = quorum; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_only_againsts'); + } + + #[test] + #[available_gas(10000000)] + fn shortcut_balanced() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + proposal.max_end_timestamp = 10; + let votes_for = quorum; + let votes_against = quorum; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::VotingPeriod(()), 'failed shortcut_balanced'); + } + + #[test] + #[available_gas(10000000)] + fn balanced() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + let votes_for = quorum; + let votes_against = quorum; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Rejected(()), 'failed balanced'); + } + + #[test] + #[available_gas(10000000)] + fn accepted() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 2; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + let votes_for = quorum; + let votes_against = quorum - 1; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Accepted(()), 'failed accepted'); + } + + #[test] + #[available_gas(10000000)] + fn accepted_with_abstains() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 5; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + let votes_for = 2; + let votes_against = 1; + let votes_abstain = 10; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Accepted(()), 'failed accepted abstains'); + } + + #[test] + #[available_gas(10000000)] + fn rejected_only_againsts() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 0; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + let votes_for = 0; + let votes_against = 1; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Rejected(()), 'failed rejected'); + } + + #[test] + #[available_gas(10000000)] + fn quorum_not_reached() { + let mut state = SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + let quorum = 3; + initializer(ref state, quorum); + + let mut proposal = ProposalDefault::default(); + let votes_for = 2; + let votes_against = 0; + let votes_abstain = 0; + let result = get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain + ); + assert(result == ProposalStatus::Rejected(()), 'failed quorum_not_reached'); + } +} diff --git a/starknet/src/execution_strategies/vanilla.cairo b/starknet/src/execution_strategies/vanilla.cairo index 9a5399b1..4899370e 100644 --- a/starknet/src/execution_strategies/vanilla.cairo +++ b/starknet/src/execution_strategies/vanilla.cairo @@ -29,12 +29,8 @@ mod VanillaExecutionStrategy { votes_abstain: u256, payload: Array ) { - let mut state: SimpleQuorumExecutionStrategy::ContractState = - SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); - - let proposal_status = SimpleQuorumExecutionStrategy::get_proposal_status( - @state, @proposal, votes_for, votes_against, votes_abstain - ); + let proposal_status = self + .get_proposal_status(proposal, votes_for, votes_against, votes_abstain,); assert( (proposal_status == ProposalStatus::Accepted(())) | (proposal_status == ProposalStatus::VotingPeriodAccepted(())), @@ -43,6 +39,21 @@ mod VanillaExecutionStrategy { self._num_executed.write(self._num_executed.read() + 1); } + fn get_proposal_status( + self: @ContractState, + proposal: Proposal, + votes_for: u256, + votes_against: u256, + votes_abstain: u256, + ) -> ProposalStatus { + let mut state: SimpleQuorumExecutionStrategy::ContractState = + SimpleQuorumExecutionStrategy::unsafe_new_contract_state(); + + SimpleQuorumExecutionStrategy::get_proposal_status( + @state, @proposal, votes_for, votes_against, votes_abstain, + ) + } + fn get_strategy_type(self: @ContractState) -> felt252 { 'SimpleQuorumVanilla' } diff --git a/starknet/src/interfaces/i_execution_strategy.cairo b/starknet/src/interfaces/i_execution_strategy.cairo index 9de50e13..7aff4265 100644 --- a/starknet/src/interfaces/i_execution_strategy.cairo +++ b/starknet/src/interfaces/i_execution_strategy.cairo @@ -1,4 +1,4 @@ -use sx::types::Proposal; +use sx::types::{Proposal, ProposalStatus}; #[starknet::interface] trait IExecutionStrategy { @@ -11,5 +11,13 @@ trait IExecutionStrategy { payload: Array ); + fn get_proposal_status( + self: @TContractState, + proposal: Proposal, + votes_for: u256, + votes_against: u256, + votes_abstain: u256, + ) -> ProposalStatus; + fn get_strategy_type(self: @TContractState) -> felt252; } diff --git a/starknet/src/space/space.cairo b/starknet/src/space/space.cairo index b60fa8cc..e9ccfa3d 100644 --- a/starknet/src/space/space.cairo +++ b/starknet/src/space/space.cairo @@ -1,5 +1,7 @@ use starknet::{ClassHash, ContractAddress}; -use sx::types::{UserAddress, Strategy, Proposal, IndexedStrategy, Choice, UpdateSettingsCalldata}; +use sx::types::{ + UserAddress, Strategy, Proposal, IndexedStrategy, Choice, UpdateSettingsCalldata, ProposalStatus +}; #[starknet::interface] trait ISpace { @@ -19,8 +21,7 @@ trait ISpace { // #[view] // fn vote_registry(proposal_id: u256, voter: ContractAddress) -> bool; fn proposals(self: @TContractState, proposal_id: u256) -> Proposal; - // #[view] - // fn get_proposal_status(proposal_id: u256) -> u8; + fn get_proposal_status(self: @TContractState, proposal_id: u256) -> ProposalStatus; // Owner Actions fn update_settings(ref self: TContractState, input: UpdateSettingsCalldata); @@ -85,8 +86,8 @@ mod Space { }, types::{ UserAddress, Choice, FinalizationStatus, Strategy, IndexedStrategy, Proposal, - PackedProposal, IndexedStrategyTrait, IndexedStrategyImpl, UpdateSettingsCalldata, - NoUpdateTrait, NoUpdateString, + ProposalStatus, PackedProposal, IndexedStrategyTrait, IndexedStrategyImpl, + UpdateSettingsCalldata, NoUpdateTrait, NoUpdateString, }, utils::{ reinitializable::{Reinitializable}, ReinitializableImpl, bits::BitSetter, @@ -98,7 +99,6 @@ mod Space { external::ownable::Ownable }; - #[storage] struct Storage { _min_voting_duration: u32, @@ -582,6 +582,18 @@ mod Space { self._proposals.read(proposal_id) } + fn get_proposal_status(self: @ContractState, proposal_id: u256) -> ProposalStatus { + let proposal = self._proposals.read(proposal_id); + assert_proposal_exists(@proposal); + + let votes_for = self._vote_power.read((proposal_id, Choice::For(()))); + let votes_against = self._vote_power.read((proposal_id, Choice::Against(()))); + let votes_abstain = self._vote_power.read((proposal_id, Choice::Abstain(()))); + + IExecutionStrategyDispatcher { contract_address: proposal.execution_strategy } + .get_proposal_status(proposal, votes_for, votes_against, votes_abstain) + } + fn update_settings(ref self: ContractState, input: UpdateSettingsCalldata) { //TODO: temporary component syntax let state = Ownable::unsafe_new_contract_state(); diff --git a/starknet/src/tests/mocks/executor.cairo b/starknet/src/tests/mocks/executor.cairo index ed2b2ef2..8d210076 100644 --- a/starknet/src/tests/mocks/executor.cairo +++ b/starknet/src/tests/mocks/executor.cairo @@ -31,6 +31,17 @@ mod ExecutorExecutionStrategy { call_contract_syscall(tx.target, tx.selector, tx.data.span()).unwrap(); } + fn get_proposal_status( + self: @ContractState, + proposal: Proposal, + votes_for: u256, + votes_against: u256, + votes_abstain: u256, + ) -> ProposalStatus { + panic_with_felt252('unimplemented'); + ProposalStatus::Cancelled(()) + } + fn get_strategy_type(self: @ContractState) -> felt252 { 'Executor' } @@ -62,6 +73,17 @@ mod ExecutorWithoutTxExecutionStrategy { payload: Array ) {} + fn get_proposal_status( + self: @ContractState, + proposal: Proposal, + votes_for: u256, + votes_against: u256, + votes_abstain: u256, + ) -> ProposalStatus { + panic_with_felt252('unimplemented'); + ProposalStatus::Cancelled(()) + } + fn get_strategy_type(self: @ContractState) -> felt252 { 'ExecutorWithoutTx' } diff --git a/starknet/src/tests/test_space.cairo b/starknet/src/tests/test_space.cairo index ef0d1ff3..9b5381c5 100644 --- a/starknet/src/tests/test_space.cairo +++ b/starknet/src/tests/test_space.cairo @@ -29,8 +29,8 @@ mod tests { }; use sx::tests::utils::strategy_trait::{StrategyImpl}; use sx::utils::constants::{PROPOSE_SELECTOR, VOTE_SELECTOR, UPDATE_PROPOSAL_SELECTOR}; + use traits::Default; use openzeppelin::tests::utils; - use debug::PrintTrait; use Space::Space as SpaceImpl; @@ -484,6 +484,57 @@ mod tests { space.execute(u256_from_felt252(1), vanilla_execution_strategy.params.clone()); } + #[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 = 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(), + 0, + constructor_calldata.span(), + false + ) + .unwrap(); + let vanilla_execution_strategy = StrategyImpl::from_address( + vanilla_execution_strategy_address + ); + 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); + ArrayTrait::::new().serialize(ref propose_calldata); + + // Create Proposal + authenticator.authenticate(space.contract_address, PROPOSE_SELECTOR, propose_calldata); + + // We don't check the proposal status, simply call to make sure it doesn't revert + space.get_proposal_status(1); + } + + #[test] + #[available_gas(10000000000)] + #[should_panic( + expected: ('Unknown enum indicator:', 0, 'ENTRYPOINT_FAILED') + )] // #default not working the the `ProposalStatus` enum + // #[should_panic(expected: ('Proposal does not exist', 'ENTRYPOINT_FAILED'))] // replace with this once default works + fn get_proposal_status_invalid_proposal_id() { + let config = setup(); + let (factory, space) = deploy(@config); + + space.get_proposal_status(0); + } + #[test] #[available_gas(10000000000)] #[should_panic(expected: ('Proposal has been finalized', 'ENTRYPOINT_FAILED'))] diff --git a/starknet/src/types/proposal.cairo b/starknet/src/types/proposal.cairo index b461a132..76750027 100644 --- a/starknet/src/types/proposal.cairo +++ b/starknet/src/types/proposal.cairo @@ -1,4 +1,4 @@ -use starknet::{ContractAddress, storage_access::StorePacking, Store}; +use starknet::{ContractAddress, storage_access::StorePacking, Store, contract_address_const}; use sx::{ utils::math::pow, types::{FinalizationStatus, UserAddress, user_address::UserAddressTrait} }; @@ -28,6 +28,21 @@ struct Proposal { active_voting_strategies: u256 } +impl ProposalDefault of Default { + fn default() -> Proposal { + Proposal { + start_timestamp: 0, + min_end_timestamp: 0, + max_end_timestamp: 0, + finalization_status: FinalizationStatus::Pending(()), + execution_payload_hash: 0, + execution_strategy: contract_address_const::<0>(), + author: UserAddress::Starknet(contract_address_const::<0>()), + active_voting_strategies: 0, + } + } +} + #[derive(Drop, starknet::Store)] struct PackedProposal { timestamps_and_finalization_status: u128, // In order: start, min, max, finalization_status diff --git a/starknet/src/types/proposal_status.cairo b/starknet/src/types/proposal_status.cairo index cf719f28..c986aa68 100644 --- a/starknet/src/types/proposal_status.cairo +++ b/starknet/src/types/proposal_status.cairo @@ -1,5 +1,6 @@ -#[derive(Copy, Drop, Serde, PartialEq)] +#[derive(Copy, Drop, Default, Serde, PartialEq)] enum ProposalStatus { + #[default] VotingDelay: (), VotingPeriod: (), VotingPeriodAccepted: (), diff --git a/starknet/src/types/user_address.cairo b/starknet/src/types/user_address.cairo index 0d653a51..26be46ee 100644 --- a/starknet/src/types/user_address.cairo +++ b/starknet/src/types/user_address.cairo @@ -1,4 +1,4 @@ -use starknet::{ContractAddress, EthAddress}; +use starknet::{ContractAddress, EthAddress, contract_address_const}; #[derive(Copy, Drop, Serde, PartialEq, starknet::Store)] enum UserAddress {