diff --git a/src/governor.cairo b/src/governor.cairo index f71b5c1..e878ed5 100644 --- a/src/governor.cairo +++ b/src/governor.cairo @@ -52,9 +52,6 @@ pub trait IGovernor { // Cancel the proposal with the given ID. Only callable by the proposer. fn cancel(ref self: TContractState, id: felt252); - // Execute the given proposal. - fn execute(ref self: TContractState, id: felt252, calls: Span) -> Span>; - // Attaches the given text to the proposal. Simply emits an event containing the proposal description. fn describe(ref self: TContractState, id: felt252, description: ByteArray); @@ -103,14 +100,14 @@ pub mod Governor { use governance::staker::{IStakerDispatcherTrait}; use starknet::{ get_block_timestamp, get_caller_address, contract_address_const, get_contract_address, - syscalls::{replace_class_syscall} + syscalls::{replace_class_syscall}, account::{AccountContract}, get_tx_info, VALIDATED }; + use super::{ IStakerDispatcher, ContractAddress, Array, IGovernor, Config, ProposalInfo, Call, ExecutionState, ByteArray, ClassHash }; - #[derive(starknet::Event, Drop)] pub struct Proposed { pub id: felt252, @@ -346,60 +343,6 @@ pub mod Governor { self.emit(Canceled { id }); } - fn execute( - ref self: ContractState, id: felt252, mut calls: Span - ) -> Span> { - let calls_hash = hash_calls(@calls); - - let (mut proposal, config) = self.get_proposal_with_config(id); - - assert(proposal.calls_hash == calls_hash, 'CALLS_HASH_MISMATCH'); - assert(proposal.proposer.is_non_zero(), 'DOES_NOT_EXIST'); - assert(proposal.execution_state.executed.is_zero(), 'ALREADY_EXECUTED'); - assert(proposal.execution_state.canceled.is_zero(), 'PROPOSAL_CANCELED'); - - let timestamp_current = get_block_timestamp(); - // we cannot tell if a proposal is executed if it is executed at timestamp 0 - // this can only happen in testing, but it makes this method locally correct - assert(timestamp_current.is_non_zero(), 'TIMESTAMP_ZERO'); - - let voting_end = proposal.execution_state.created - + config.voting_start_delay - + config.voting_period; - assert(timestamp_current >= voting_end, 'VOTING_NOT_ENDED'); - - let window_start = voting_end + config.execution_delay; - assert(timestamp_current >= window_start, 'EXECUTION_WINDOW_NOT_STARTED'); - - let window_end = window_start + config.execution_window; - assert(timestamp_current < window_end, 'EXECUTION_WINDOW_OVER'); - - assert(proposal.yea >= config.quorum, 'QUORUM_NOT_MET'); - assert(proposal.yea >= proposal.nay, 'NO_MAJORITY'); - - proposal - .execution_state = - ExecutionState { - created: proposal.execution_state.created, - executed: timestamp_current, - canceled: Zero::zero() - }; - - self.proposals.write(id, proposal); - - let mut results: Array> = array![]; - - while let Option::Some(call) = calls.pop_front() { - results.append(call.execute()); - }; - - let result_span = results.span(); - - self.emit(Executed { id, result_data: result_span }); - - result_span - } - fn get_config(self: @ContractState) -> Config { self.get_config_version(self.latest_config_version.read()) } @@ -451,4 +394,80 @@ pub mod Governor { replace_class_syscall(class_hash).unwrap(); } } + + fn get_proposal_id_from_signature() -> felt252 { + let mut tx_info = get_tx_info().unbox(); + *tx_info.signature.pop_front().expect('PROPOSAL_ID_IN_SIGNATURE') + } + + #[abi(embed_v0)] + impl GovernorAccountContract of AccountContract { + fn __validate_declare__(self: @ContractState, class_hash: felt252) -> felt252 { + panic!("Declare not supported"); + 0 + } + + fn __validate__(ref self: ContractState, calls: Array) -> felt252 { + let id = get_proposal_id_from_signature(); + + let mut calls_span = calls.span(); + let calls_hash = hash_calls(@calls_span); + + let (proposal, config) = self.get_proposal_with_config(id); + + assert(proposal.calls_hash == calls_hash, 'CALLS_HASH_MISMATCH'); + assert(proposal.proposer.is_non_zero(), 'DOES_NOT_EXIST'); + assert(proposal.execution_state.executed.is_zero(), 'ALREADY_EXECUTED'); + assert(proposal.execution_state.canceled.is_zero(), 'PROPOSAL_CANCELED'); + + let timestamp_current = get_block_timestamp(); + // we cannot tell if a proposal is executed if it is executed at timestamp 0 + // this can only happen in testing, but it makes this method locally correct + assert(timestamp_current.is_non_zero(), 'TIMESTAMP_ZERO'); + + let voting_end = proposal.execution_state.created + + config.voting_start_delay + + config.voting_period; + assert(timestamp_current >= voting_end, 'VOTING_NOT_ENDED'); + + let window_start = voting_end + config.execution_delay; + assert(timestamp_current >= window_start, 'EXECUTION_WINDOW_NOT_STARTED'); + + let window_end = window_start + config.execution_window; + assert(timestamp_current < window_end, 'EXECUTION_WINDOW_OVER'); + + assert(proposal.yea >= config.quorum, 'QUORUM_NOT_MET'); + assert(proposal.yea >= proposal.nay, 'NO_MAJORITY'); + + VALIDATED + } + + fn __execute__(ref self: ContractState, calls: Array) -> Array> { + let id = get_proposal_id_from_signature(); + let mut calls_span = calls.span(); + + let mut proposal = self.get_proposal(id); + + proposal + .execution_state = + ExecutionState { + created: proposal.execution_state.created, + executed: get_block_timestamp(), + canceled: Zero::zero() + }; + + self.proposals.write(id, proposal); + + let mut results: Array> = array![]; + + while let Option::Some(call) = calls_span + .pop_front() { + results.append(call.execute()); + }; + + self.emit(Executed { id, result_data: results.span() }); + + results + } + } } diff --git a/src/governor_test.cairo b/src/governor_test.cairo index 3a53cad..c6e4af9 100644 --- a/src/governor_test.cairo +++ b/src/governor_test.cairo @@ -5,6 +5,7 @@ use core::option::{OptionTrait}; use core::result::{Result, ResultTrait}; use core::serde::Serde; +use core::starknet::account::AccountContractDispatcherTrait; use core::traits::{TryInto}; use governance::execution_state::{ExecutionState}; @@ -19,9 +20,24 @@ use starknet::account::{Call}; use starknet::{ get_contract_address, syscalls::deploy_syscall, ClassHash, contract_address_const, ContractAddress, get_block_timestamp, - testing::{set_block_timestamp, set_contract_address, pop_log} + testing::{set_block_timestamp, set_contract_address, pop_log, set_signature}, + account::{AccountContractDispatcher} }; +#[generate_trait] +impl GovernorExecuteImpl of GovernorExecuteTrait { + fn validate(self: IGovernorDispatcher, id: felt252, calls: Array) -> felt252 { + set_signature(array![id].span()); + AccountContractDispatcher { contract_address: self.contract_address }.__validate__(calls) + } + fn execute(self: IGovernorDispatcher, id: felt252, calls: Array) -> Span> { + set_signature(array![id].span()); + AccountContractDispatcher { contract_address: self.contract_address } + .__execute__(calls) + .span() + } +} + pub(crate) fn recipient() -> ContractAddress { 'recipient'.try_into().unwrap() @@ -381,7 +397,7 @@ fn test_describe_proposal_fails_if_executed() { governor.vote(id, true); advance_time(config.voting_period + config.execution_delay); set_contract_address(anyone()); - governor.execute(id, array![transfer_call(token, anyone(), amount: 0)].span()); + governor.execute(id, array![transfer_call(token, anyone(), amount: 0)]); set_contract_address(proposer()); governor.describe(id, "This proposal is already executed"); @@ -605,10 +621,7 @@ fn test_execute_valid_proposal() { set_contract_address(anyone()); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.execute(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); let proposal = governor.get_proposal(id); assert(proposal.execution_state.executed.is_non_zero(), 'execute failed'); @@ -625,10 +638,7 @@ fn test_canceled_proposal_cannot_be_executed() { advance_time(config.voting_start_delay); advance_time(config.voting_period); set_contract_address(anyone()); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -640,10 +650,7 @@ fn test_execute_before_voting_ends_should_fail() { advance_time(config.voting_start_delay); // Execute the proposal. The vote is still active, this should fail. - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } @@ -657,10 +664,7 @@ fn test_execute_quorum_not_met_should_fail() { set_contract_address(proposer()); // Execute the proposal. The quorum was not met, this should fail. - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -699,10 +703,7 @@ fn test_execute_no_majority_should_fail() { advance_time(config.voting_period + config.execution_delay); // Execute the proposal. The majority of votes are no, this should fail. - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -723,10 +724,7 @@ fn test_execute_before_execution_window_begins() { governor.vote(id, true); advance_time(config.voting_period); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } @@ -748,10 +746,7 @@ fn test_execute_after_execution_window_ends() { governor.vote(id, true); advance_time(config.voting_period + config.execution_delay + config.execution_window); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -801,10 +796,7 @@ fn test_verify_votes_are_counted_over_voting_weight_smoothing_duration_from_star advance_time(config.voting_period + config.execution_delay); // Execute the proposal. The quorum was not met, this should fail. - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -831,10 +823,7 @@ fn test_quorum_counts_only_yes_votes_not_met() { advance_time(config.voting_period + config.execution_delay); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); } #[test] @@ -843,8 +832,7 @@ fn test_quorum_counts_only_yes_votes_exactly_met() { let calls = array![ transfer_call(token: token, recipient: recipient(), amount: 150), transfer_call(token: token, recipient: recipient(), amount: 50) - ] - .span(); + ]; // so execution can succeed token.transfer(governor.contract_address, 200); @@ -856,7 +844,7 @@ fn test_quorum_counts_only_yes_votes_exactly_met() { advance_time(config.voting_weight_smoothing_duration); set_contract_address(voter1()); - let id = governor.propose(calls); + let id = governor.propose(calls.span()); advance_time(config.voting_start_delay); governor.vote(id, true); @@ -875,8 +863,7 @@ fn test_execute_emits_logs_from_data() { let calls = array![ transfer_call(token: token, recipient: recipient(), amount: 150), transfer_call(token: token, recipient: recipient(), amount: 50) - ] - .span(); + ]; // so execution can succeed token.transfer(governor.contract_address, 200); @@ -888,7 +875,7 @@ fn test_execute_emits_logs_from_data() { advance_time(config.voting_weight_smoothing_duration); set_contract_address(voter1()); - let id = governor.propose(calls); + let id = governor.propose(calls.span()); advance_time(config.voting_start_delay); governor.vote(id, true); @@ -928,13 +915,10 @@ fn test_execute_already_executed_should_fail() { advance_time(config.voting_period + config.execution_delay); set_contract_address(anyone()); + governor.execute(id, array![transfer_call(token: token, recipient: recipient(), amount: 100)]); governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() - ); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 100)].span() + .validate( + id, array![transfer_call(token: token, recipient: recipient(), amount: 100)] ); // Try to execute again } @@ -955,10 +939,7 @@ fn test_execute_invalid_call_id() { advance_time(config.voting_period); set_contract_address(anyone()); - governor - .execute( - id, array![transfer_call(token: token, recipient: recipient(), amount: 101)].span() - ); + governor.validate(id, array![transfer_call(token: token, recipient: recipient(), amount: 101)]); } #[test] @@ -1004,7 +985,6 @@ fn test_upgrade_succeeds_self_call() { calldata: array![Governor::TEST_CLASS_HASH].span() } ] - .span() ); } @@ -1073,7 +1053,6 @@ fn test_reconfigure_succeeds_self_call() { calldata: args.span() } ] - .span() ); pop_log::(governor.contract_address).unwrap();