diff --git a/packages/governance/src/governor/governor.cairo b/packages/governance/src/governor/governor.cairo index b1ed21eff..3fbbda05d 100644 --- a/packages/governance/src/governor/governor.cairo +++ b/packages/governance/src/governor/governor.cairo @@ -562,6 +562,7 @@ pub mod GovernorComponent { /// Requirements: /// /// - The proposal must be active. + /// - The current timepoint must be greater than the proposal's snapshot timepoint. /// /// Emits a `VoteCast` event. fn cast_vote( @@ -576,6 +577,7 @@ pub mod GovernorComponent { /// Requirements: /// /// - The proposal must be active. + /// - The current timepoint must be greater than the proposal's snapshot timepoint. /// /// Emits a `VoteCast` event. fn cast_vote_with_reason( @@ -593,6 +595,7 @@ pub mod GovernorComponent { /// Requirements: /// /// - The proposal must be active. + /// - The current timepoint must be greater than the proposal's snapshot timepoint. /// /// Emits either: /// - `VoteCast` event if no params are provided. @@ -613,6 +616,7 @@ pub mod GovernorComponent { /// Requirements: /// /// - The proposal must be active. + /// - The current timepoint must be greater than the proposal's snapshot timepoint. /// - The nonce in the signed message must match the account's current nonce. /// - `voter` must implement `SRC6::is_valid_signature`. /// - `signature` should be valid for the message hash. @@ -645,6 +649,7 @@ pub mod GovernorComponent { /// Requirements: /// /// - The proposal must be active. + /// - The current timepoint must be greater than the proposal's snapshot timepoint. /// - The nonce in the signed message must match the account's current nonce. /// - `voter` must implement `SRC6::is_valid_signature`. /// - `signature` should be valid for the message hash. diff --git a/packages/governance/src/tests/governor/block_number/common.cairo b/packages/governance/src/tests/governor/block_number/common.cairo index 277b38ecf..6fccda9ee 100644 --- a/packages/governance/src/tests/governor/block_number/common.cairo +++ b/packages/governance/src/tests/governor/block_number/common.cairo @@ -5,8 +5,9 @@ use openzeppelin_interfaces::governor::{IGovernor, ProposalState}; use openzeppelin_test_common::mocks::governor::GovernorMock::SNIP12MetadataImpl; use openzeppelin_test_common::mocks::governor::{GovernorMock, GovernorTimelockedMock}; use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN}; +use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, VOTES_TOKEN}; use openzeppelin_utils::bytearray::ByteArrayExtTrait; +use openzeppelin_utils::serde::SerializedAppend; use snforge_std::{start_cheat_block_number_global, start_mock_call}; use starknet::ContractAddress; use starknet::account::Call; @@ -45,10 +46,16 @@ pub fn hash_proposal(calls: Span, description_hash: felt252) -> felt252 { pub fn get_proposal_info() -> (felt252, ProposalCore) { let calls = get_calls(OTHER, false); - get_proposal_with_id(calls, @"proposal description") + build_proposal_with_id(calls, @"proposal description") } -pub fn get_proposal_with_id(calls: Span, description: @ByteArray) -> (felt252, ProposalCore) { +pub fn get_proposal_info_with_custom_delay(delay: u64) -> (felt252, ProposalCore) { + let (id, mut proposal) = get_proposal_info(); + proposal.vote_start = starknet::get_block_number() + delay; + (id, proposal) +} + +fn build_proposal_with_id(calls: Span, description: @ByteArray) -> (felt252, ProposalCore) { let block_number = starknet::get_block_number(); let vote_start = block_number + GovernorMock::VOTING_DELAY; let vote_duration = GovernorMock::VOTING_PERIOD; @@ -105,7 +112,9 @@ pub fn set_executor( } pub fn deploy_votes_token() -> IERC20Dispatcher { - utils::declare_and_deploy_at("ERC20BlockNumberVotesMock", VOTES_TOKEN, array![]); + let mut calldata = array![]; + calldata.append_serde(SUPPLY); + utils::declare_and_deploy_at("ERC20BlockNumberVotesMock", VOTES_TOKEN, calldata); IERC20Dispatcher { contract_address: VOTES_TOKEN } } diff --git a/packages/governance/src/tests/governor/block_number/test_governor.cairo b/packages/governance/src/tests/governor/block_number/test_governor.cairo index 81a822bc5..f26c7875c 100644 --- a/packages/governance/src/tests/governor/block_number/test_governor.cairo +++ b/packages/governance/src/tests/governor/block_number/test_governor.cairo @@ -1,4 +1,5 @@ use core::num::traits::Bounded; +use openzeppelin_interfaces::governance::votes::{IVotesDispatcher, IVotesDispatcherTrait}; use openzeppelin_interfaces::governor::{ IGOVERNOR_ID, IGovernor, IGovernorDispatcher, IGovernorDispatcherTrait, ProposalState, }; @@ -9,7 +10,7 @@ use openzeppelin_test_common::mocks::timelock::{ IMockContractDispatcher, IMockContractDispatcherTrait, }; use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN, ZERO}; +use openzeppelin_testing::constants::{ADMIN, BLOCK_NUMBER, OTHER, SUPPLY, VOTES_TOKEN, ZERO}; use openzeppelin_testing::{AsAddressTrait, spy_events}; use openzeppelin_utils::bytearray::ByteArrayExtTrait; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; @@ -27,8 +28,9 @@ use crate::governor::vote::{Vote, VoteWithReasonAndParams}; use crate::governor::{DefaultConfig, ProposalCore}; use crate::tests::governor::block_number::common::{ COMPONENT_STATE, CONTRACT_STATE, deploy_votes_token, get_calls, get_mock_state, - get_proposal_info, get_state, hash_proposal, setup_active_proposal, setup_canceled_proposal, - setup_defeated_proposal, setup_executed_proposal, setup_pending_proposal, setup_queued_proposal, + get_proposal_info, get_proposal_info_with_custom_delay, get_state, hash_proposal, + setup_active_proposal, setup_canceled_proposal, setup_defeated_proposal, + setup_executed_proposal, setup_pending_proposal, setup_queued_proposal, setup_succeeded_proposal, }; use crate::tests::governor::common::GovernorSpyHelpersImpl; @@ -436,10 +438,10 @@ fn test_has_voted() { fn test_propose_external_version(external_state_version: bool) { let mut state = COMPONENT_STATE(); - let mut spy = spy_events(); let contract_address = test_address(); deploy_votes_token(); initialize_votes_component(VOTES_TOKEN); + let mut spy = spy_events(); let calls = get_calls(OTHER, false); let proposer = ADMIN; @@ -2031,6 +2033,19 @@ fn test__cast_vote_pending() { state._cast_vote(id, OTHER, 0, "", params); } +#[test] +#[should_panic(expected: 'Votes: future Lookup')] +fn test__cast_vote_at_vote_start() { + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + initialize_votes_component(VOTES_TOKEN); + let (id, proposal) = setup_pending_proposal(ref state, false); + + start_cheat_block_number_global(proposal.vote_start); + let params = array![].span(); + state._cast_vote(id, OTHER, 0, "", params); +} + #[test] fn test__cast_vote_active_no_params() { let mut state = COMPONENT_STATE(); @@ -2078,6 +2093,49 @@ fn test__cast_vote_active_with_params() { ); } +#[test] +#[should_panic(expected: 'Votes: future Lookup')] +fn test__cast_vote_zero_delay() { + start_cheat_block_number_global(BLOCK_NUMBER - 1); + let voter = test_address(); + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + delegate_votes_to(voter); + initialize_votes_component(VOTES_TOKEN); + + // Setup proposal with 0 delay + start_cheat_block_number_global(BLOCK_NUMBER); + let (id, proposal) = get_proposal_info_with_custom_delay(0); + state.Governor_proposals.write(id, proposal); + + // Try to cast vote + let reason = "reason"; + let params = array![].span(); + state._cast_vote(id, OTHER, 0, reason, params); +} + +#[test] +fn test__cast_vote_zero_delay_in_next_block() { + start_cheat_block_number_global(BLOCK_NUMBER - 1); + let voter = test_address(); + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + delegate_votes_to(voter); + initialize_votes_component(VOTES_TOKEN); + + // Setup proposal with 0 delay + start_cheat_block_number_global(BLOCK_NUMBER); + let (id, proposal) = get_proposal_info_with_custom_delay(0); + state.Governor_proposals.write(id, proposal); + + // Cast vote + start_cheat_block_number_global(BLOCK_NUMBER + 1); + let reason = "reason"; + let params = array![].span(); + let weight = state._cast_vote(id, voter, 0, reason, params); + assert_eq!(weight, SUPPLY); +} + #[test] #[should_panic(expected: 'Unexpected proposal state')] fn test__cast_vote_defeated() { @@ -2146,3 +2204,8 @@ fn initialize_votes_component(votes_token: ContractAddress) { let mut mock_state = CONTRACT_STATE(); mock_state.governor_votes.initializer(votes_token); } + +fn delegate_votes_to(delegatee: ContractAddress) { + let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN }; + votes_dispatcher.delegate(delegatee); +} diff --git a/packages/governance/src/tests/governor/timestamp/common.cairo b/packages/governance/src/tests/governor/timestamp/common.cairo index a71eafeb8..87337a5e5 100644 --- a/packages/governance/src/tests/governor/timestamp/common.cairo +++ b/packages/governance/src/tests/governor/timestamp/common.cairo @@ -5,8 +5,9 @@ use openzeppelin_interfaces::governor::{IGovernor, ProposalState}; use openzeppelin_test_common::mocks::governor::GovernorMock::SNIP12MetadataImpl; use openzeppelin_test_common::mocks::governor::{GovernorMock, GovernorTimelockedMock}; use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN}; +use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, VOTES_TOKEN}; use openzeppelin_utils::bytearray::ByteArrayExtTrait; +use openzeppelin_utils::serde::SerializedAppend; use snforge_std::{start_cheat_block_timestamp_global, start_mock_call}; use starknet::ContractAddress; use starknet::account::Call; @@ -48,6 +49,12 @@ pub fn get_proposal_info() -> (felt252, ProposalCore) { get_proposal_with_id(calls, @"proposal description") } +pub fn get_proposal_info_with_custom_delay(delay: u64) -> (felt252, ProposalCore) { + let (id, mut proposal) = get_proposal_info(); + proposal.vote_start = starknet::get_block_timestamp() + delay; + (id, proposal) +} + pub fn get_proposal_with_id(calls: Span, description: @ByteArray) -> (felt252, ProposalCore) { let timestamp = starknet::get_block_timestamp(); let vote_start = timestamp + GovernorMock::VOTING_DELAY; @@ -105,7 +112,9 @@ pub fn set_executor( } pub fn deploy_votes_token() -> IERC20Dispatcher { - utils::declare_and_deploy_at("ERC20TimestampVotesMock", VOTES_TOKEN, array![]); + let mut calldata = array![]; + calldata.append_serde(SUPPLY); + utils::declare_and_deploy_at("ERC20TimestampVotesMock", VOTES_TOKEN, calldata); IERC20Dispatcher { contract_address: VOTES_TOKEN } } diff --git a/packages/governance/src/tests/governor/timestamp/test_governor.cairo b/packages/governance/src/tests/governor/timestamp/test_governor.cairo index 21ded3c38..75fcb60cd 100644 --- a/packages/governance/src/tests/governor/timestamp/test_governor.cairo +++ b/packages/governance/src/tests/governor/timestamp/test_governor.cairo @@ -1,4 +1,5 @@ use core::num::traits::Bounded; +use openzeppelin_interfaces::governance::votes::{IVotesDispatcher, IVotesDispatcherTrait}; use openzeppelin_interfaces::governor::{ IGOVERNOR_ID, IGovernor, IGovernorDispatcher, IGovernorDispatcherTrait, ProposalState, }; @@ -9,7 +10,7 @@ use openzeppelin_test_common::mocks::timelock::{ IMockContractDispatcher, IMockContractDispatcherTrait, }; use openzeppelin_testing as utils; -use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN, ZERO}; +use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, TIMESTAMP, VOTES_TOKEN, ZERO}; use openzeppelin_testing::{AsAddressTrait, spy_events}; use openzeppelin_utils::bytearray::ByteArrayExtTrait; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; @@ -28,8 +29,9 @@ use crate::governor::{DefaultConfig, ProposalCore}; use crate::tests::governor::common::GovernorSpyHelpersImpl; use crate::tests::governor::timestamp::common::{ COMPONENT_STATE, CONTRACT_STATE, deploy_votes_token, get_calls, get_mock_state, - get_proposal_info, get_state, hash_proposal, setup_active_proposal, setup_canceled_proposal, - setup_defeated_proposal, setup_executed_proposal, setup_pending_proposal, setup_queued_proposal, + get_proposal_info, get_proposal_info_with_custom_delay, get_state, hash_proposal, + setup_active_proposal, setup_canceled_proposal, setup_defeated_proposal, + setup_executed_proposal, setup_pending_proposal, setup_queued_proposal, setup_succeeded_proposal, }; @@ -436,10 +438,10 @@ fn test_has_voted() { fn test_propose_external_version(external_state_version: bool) { let mut state = COMPONENT_STATE(); - let mut spy = spy_events(); let contract_address = test_address(); deploy_votes_token(); initialize_votes_component(VOTES_TOKEN); + let mut spy = spy_events(); let calls = get_calls(OTHER, false); let proposer = ADMIN; @@ -931,6 +933,19 @@ fn test_cast_vote_pending() { state.cast_vote(id, 0); } +#[test] +#[should_panic(expected: 'Votes: future Lookup')] +fn test__cast_vote_at_vote_start() { + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + initialize_votes_component(VOTES_TOKEN); + let (id, proposal) = setup_pending_proposal(ref state, false); + + start_cheat_block_timestamp_global(proposal.vote_start); + let params = array![].span(); + state._cast_vote(id, OTHER, 0, "", params); +} + #[test] fn test_cast_vote_active() { let mut state = COMPONENT_STATE(); @@ -952,6 +967,49 @@ fn test_cast_vote_active() { spy.assert_only_event_vote_cast(contract_address, OTHER, id, 0, expected_weight, @""); } +#[test] +#[should_panic(expected: 'Votes: future Lookup')] +fn test__cast_vote_zero_delay() { + start_cheat_block_timestamp_global(TIMESTAMP - 1); + let voter = test_address(); + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + delegate_votes_to(voter); + initialize_votes_component(VOTES_TOKEN); + + // Setup proposal with 0 delay + start_cheat_block_timestamp_global(TIMESTAMP); + let (id, proposal) = get_proposal_info_with_custom_delay(0); + state.Governor_proposals.write(id, proposal); + + // Try to cast vote + let reason = "reason"; + let params = array![].span(); + state._cast_vote(id, voter, 0, reason, params); +} + +#[test] +fn test__cast_vote_zero_delay_after_some_time() { + start_cheat_block_timestamp_global(TIMESTAMP - 1); + let voter = test_address(); + let mut state = COMPONENT_STATE(); + deploy_votes_token(); + delegate_votes_to(voter); + initialize_votes_component(VOTES_TOKEN); + + // Setup proposal with 0 delay + start_cheat_block_timestamp_global(TIMESTAMP); + let (id, proposal) = get_proposal_info_with_custom_delay(0); + state.Governor_proposals.write(id, proposal); + + // Cast vote + start_cheat_block_timestamp_global(TIMESTAMP + 1); + let reason = "reason"; + let params = array![].span(); + let weight = state._cast_vote(id, voter, 0, reason, params); + assert_eq!(weight, SUPPLY); +} + #[test] #[should_panic(expected: 'Unexpected proposal state')] fn test_cast_vote_defeated() { @@ -2146,3 +2204,8 @@ fn initialize_votes_component(votes_token: ContractAddress) { let mut mock_state = CONTRACT_STATE(); mock_state.governor_votes.initializer(votes_token); } + +fn delegate_votes_to(delegatee: ContractAddress) { + let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN }; + votes_dispatcher.delegate(delegatee); +} diff --git a/packages/test_common/src/mocks/votes.cairo b/packages/test_common/src/mocks/votes.cairo index 5abf19ca7..104d3179a 100644 --- a/packages/test_common/src/mocks/votes.cairo +++ b/packages/test_common/src/mocks/votes.cairo @@ -128,8 +128,9 @@ pub mod ERC20TimestampVotesMock { } #[constructor] - fn constructor(ref self: ContractState) { + fn constructor(ref self: ContractState, total_supply: u256) { self.erc20.initializer("MyToken", "MTK"); + self.erc20.mint(starknet::get_caller_address(), total_supply); } } @@ -179,8 +180,9 @@ pub mod ERC20BlockNumberVotesMock { } #[constructor] - fn constructor(ref self: ContractState) { + fn constructor(ref self: ContractState, total_supply: u256) { self.erc20.initializer("MyToken", "MTK"); + self.erc20.mint(starknet::get_caller_address(), total_supply); } }