Skip to content

Commit 0703869

Browse files
immrsdimmrsd
authored
Clarify Governor voting start (#1598)
* Add doc requirement clarifying that voting starts only after snapshot * Mint initial supply at deployment for Votes mocks * Add additional test cases for block-number-based governor voting * Add additional test cases for timestamp-based governor voting * Format files
1 parent 66bceaf commit 0703869

File tree

6 files changed

+167
-16
lines changed

6 files changed

+167
-16
lines changed

‎packages/governance/src/governor/governor.cairo‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,7 @@ pub mod GovernorComponent {
562562
/// Requirements:
563563
///
564564
/// - The proposal must be active.
565+
/// - The current timepoint must be greater than the proposal's snapshot timepoint.
565566
///
566567
/// Emits a `VoteCast` event.
567568
fn cast_vote(
@@ -576,6 +577,7 @@ pub mod GovernorComponent {
576577
/// Requirements:
577578
///
578579
/// - The proposal must be active.
580+
/// - The current timepoint must be greater than the proposal's snapshot timepoint.
579581
///
580582
/// Emits a `VoteCast` event.
581583
fn cast_vote_with_reason(
@@ -593,6 +595,7 @@ pub mod GovernorComponent {
593595
/// Requirements:
594596
///
595597
/// - The proposal must be active.
598+
/// - The current timepoint must be greater than the proposal's snapshot timepoint.
596599
///
597600
/// Emits either:
598601
/// - `VoteCast` event if no params are provided.
@@ -613,6 +616,7 @@ pub mod GovernorComponent {
613616
/// Requirements:
614617
///
615618
/// - The proposal must be active.
619+
/// - The current timepoint must be greater than the proposal's snapshot timepoint.
616620
/// - The nonce in the signed message must match the account's current nonce.
617621
/// - `voter` must implement `SRC6::is_valid_signature`.
618622
/// - `signature` should be valid for the message hash.
@@ -645,6 +649,7 @@ pub mod GovernorComponent {
645649
/// Requirements:
646650
///
647651
/// - The proposal must be active.
652+
/// - The current timepoint must be greater than the proposal's snapshot timepoint.
648653
/// - The nonce in the signed message must match the account's current nonce.
649654
/// - `voter` must implement `SRC6::is_valid_signature`.
650655
/// - `signature` should be valid for the message hash.

‎packages/governance/src/tests/governor/block_number/common.cairo‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use openzeppelin_interfaces::governor::{IGovernor, ProposalState};
55
use openzeppelin_test_common::mocks::governor::GovernorMock::SNIP12MetadataImpl;
66
use openzeppelin_test_common::mocks::governor::{GovernorMock, GovernorTimelockedMock};
77
use openzeppelin_testing as utils;
8-
use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN};
8+
use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, VOTES_TOKEN};
99
use openzeppelin_utils::bytearray::ByteArrayExtTrait;
10+
use openzeppelin_utils::serde::SerializedAppend;
1011
use snforge_std::{start_cheat_block_number_global, start_mock_call};
1112
use starknet::ContractAddress;
1213
use starknet::account::Call;
@@ -45,10 +46,16 @@ pub fn hash_proposal(calls: Span<Call>, description_hash: felt252) -> felt252 {
4546

4647
pub fn get_proposal_info() -> (felt252, ProposalCore) {
4748
let calls = get_calls(OTHER, false);
48-
get_proposal_with_id(calls, @"proposal description")
49+
build_proposal_with_id(calls, @"proposal description")
4950
}
5051

51-
pub fn get_proposal_with_id(calls: Span<Call>, description: @ByteArray) -> (felt252, ProposalCore) {
52+
pub fn get_proposal_info_with_custom_delay(delay: u64) -> (felt252, ProposalCore) {
53+
let (id, mut proposal) = get_proposal_info();
54+
proposal.vote_start = starknet::get_block_number() + delay;
55+
(id, proposal)
56+
}
57+
58+
fn build_proposal_with_id(calls: Span<Call>, description: @ByteArray) -> (felt252, ProposalCore) {
5259
let block_number = starknet::get_block_number();
5360
let vote_start = block_number + GovernorMock::VOTING_DELAY;
5461
let vote_duration = GovernorMock::VOTING_PERIOD;
@@ -105,7 +112,9 @@ pub fn set_executor(
105112
}
106113

107114
pub fn deploy_votes_token() -> IERC20Dispatcher {
108-
utils::declare_and_deploy_at("ERC20BlockNumberVotesMock", VOTES_TOKEN, array![]);
115+
let mut calldata = array![];
116+
calldata.append_serde(SUPPLY);
117+
utils::declare_and_deploy_at("ERC20BlockNumberVotesMock", VOTES_TOKEN, calldata);
109118
IERC20Dispatcher { contract_address: VOTES_TOKEN }
110119
}
111120

‎packages/governance/src/tests/governor/block_number/test_governor.cairo‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use core::num::traits::Bounded;
2+
use openzeppelin_interfaces::governance::votes::{IVotesDispatcher, IVotesDispatcherTrait};
23
use openzeppelin_interfaces::governor::{
34
IGOVERNOR_ID, IGovernor, IGovernorDispatcher, IGovernorDispatcherTrait, ProposalState,
45
};
@@ -9,7 +10,7 @@ use openzeppelin_test_common::mocks::timelock::{
910
IMockContractDispatcher, IMockContractDispatcherTrait,
1011
};
1112
use openzeppelin_testing as utils;
12-
use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN, ZERO};
13+
use openzeppelin_testing::constants::{ADMIN, BLOCK_NUMBER, OTHER, SUPPLY, VOTES_TOKEN, ZERO};
1314
use openzeppelin_testing::{AsAddressTrait, spy_events};
1415
use openzeppelin_utils::bytearray::ByteArrayExtTrait;
1516
use openzeppelin_utils::cryptography::snip12::OffchainMessageHash;
@@ -27,8 +28,9 @@ use crate::governor::vote::{Vote, VoteWithReasonAndParams};
2728
use crate::governor::{DefaultConfig, ProposalCore};
2829
use crate::tests::governor::block_number::common::{
2930
COMPONENT_STATE, CONTRACT_STATE, deploy_votes_token, get_calls, get_mock_state,
30-
get_proposal_info, get_state, hash_proposal, setup_active_proposal, setup_canceled_proposal,
31-
setup_defeated_proposal, setup_executed_proposal, setup_pending_proposal, setup_queued_proposal,
31+
get_proposal_info, get_proposal_info_with_custom_delay, get_state, hash_proposal,
32+
setup_active_proposal, setup_canceled_proposal, setup_defeated_proposal,
33+
setup_executed_proposal, setup_pending_proposal, setup_queued_proposal,
3234
setup_succeeded_proposal,
3335
};
3436
use crate::tests::governor::common::GovernorSpyHelpersImpl;
@@ -436,10 +438,10 @@ fn test_has_voted() {
436438

437439
fn test_propose_external_version(external_state_version: bool) {
438440
let mut state = COMPONENT_STATE();
439-
let mut spy = spy_events();
440441
let contract_address = test_address();
441442
deploy_votes_token();
442443
initialize_votes_component(VOTES_TOKEN);
444+
let mut spy = spy_events();
443445

444446
let calls = get_calls(OTHER, false);
445447
let proposer = ADMIN;
@@ -2031,6 +2033,19 @@ fn test__cast_vote_pending() {
20312033
state._cast_vote(id, OTHER, 0, "", params);
20322034
}
20332035

2036+
#[test]
2037+
#[should_panic(expected: 'Votes: future Lookup')]
2038+
fn test__cast_vote_at_vote_start() {
2039+
let mut state = COMPONENT_STATE();
2040+
deploy_votes_token();
2041+
initialize_votes_component(VOTES_TOKEN);
2042+
let (id, proposal) = setup_pending_proposal(ref state, false);
2043+
2044+
start_cheat_block_number_global(proposal.vote_start);
2045+
let params = array![].span();
2046+
state._cast_vote(id, OTHER, 0, "", params);
2047+
}
2048+
20342049
#[test]
20352050
fn test__cast_vote_active_no_params() {
20362051
let mut state = COMPONENT_STATE();
@@ -2078,6 +2093,49 @@ fn test__cast_vote_active_with_params() {
20782093
);
20792094
}
20802095

2096+
#[test]
2097+
#[should_panic(expected: 'Votes: future Lookup')]
2098+
fn test__cast_vote_zero_delay() {
2099+
start_cheat_block_number_global(BLOCK_NUMBER - 1);
2100+
let voter = test_address();
2101+
let mut state = COMPONENT_STATE();
2102+
deploy_votes_token();
2103+
delegate_votes_to(voter);
2104+
initialize_votes_component(VOTES_TOKEN);
2105+
2106+
// Setup proposal with 0 delay
2107+
start_cheat_block_number_global(BLOCK_NUMBER);
2108+
let (id, proposal) = get_proposal_info_with_custom_delay(0);
2109+
state.Governor_proposals.write(id, proposal);
2110+
2111+
// Try to cast vote
2112+
let reason = "reason";
2113+
let params = array![].span();
2114+
state._cast_vote(id, OTHER, 0, reason, params);
2115+
}
2116+
2117+
#[test]
2118+
fn test__cast_vote_zero_delay_in_next_block() {
2119+
start_cheat_block_number_global(BLOCK_NUMBER - 1);
2120+
let voter = test_address();
2121+
let mut state = COMPONENT_STATE();
2122+
deploy_votes_token();
2123+
delegate_votes_to(voter);
2124+
initialize_votes_component(VOTES_TOKEN);
2125+
2126+
// Setup proposal with 0 delay
2127+
start_cheat_block_number_global(BLOCK_NUMBER);
2128+
let (id, proposal) = get_proposal_info_with_custom_delay(0);
2129+
state.Governor_proposals.write(id, proposal);
2130+
2131+
// Cast vote
2132+
start_cheat_block_number_global(BLOCK_NUMBER + 1);
2133+
let reason = "reason";
2134+
let params = array![].span();
2135+
let weight = state._cast_vote(id, voter, 0, reason, params);
2136+
assert_eq!(weight, SUPPLY);
2137+
}
2138+
20812139
#[test]
20822140
#[should_panic(expected: 'Unexpected proposal state')]
20832141
fn test__cast_vote_defeated() {
@@ -2146,3 +2204,8 @@ fn initialize_votes_component(votes_token: ContractAddress) {
21462204
let mut mock_state = CONTRACT_STATE();
21472205
mock_state.governor_votes.initializer(votes_token);
21482206
}
2207+
2208+
fn delegate_votes_to(delegatee: ContractAddress) {
2209+
let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN };
2210+
votes_dispatcher.delegate(delegatee);
2211+
}

‎packages/governance/src/tests/governor/timestamp/common.cairo‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use openzeppelin_interfaces::governor::{IGovernor, ProposalState};
55
use openzeppelin_test_common::mocks::governor::GovernorMock::SNIP12MetadataImpl;
66
use openzeppelin_test_common::mocks::governor::{GovernorMock, GovernorTimelockedMock};
77
use openzeppelin_testing as utils;
8-
use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN};
8+
use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, VOTES_TOKEN};
99
use openzeppelin_utils::bytearray::ByteArrayExtTrait;
10+
use openzeppelin_utils::serde::SerializedAppend;
1011
use snforge_std::{start_cheat_block_timestamp_global, start_mock_call};
1112
use starknet::ContractAddress;
1213
use starknet::account::Call;
@@ -48,6 +49,12 @@ pub fn get_proposal_info() -> (felt252, ProposalCore) {
4849
get_proposal_with_id(calls, @"proposal description")
4950
}
5051

52+
pub fn get_proposal_info_with_custom_delay(delay: u64) -> (felt252, ProposalCore) {
53+
let (id, mut proposal) = get_proposal_info();
54+
proposal.vote_start = starknet::get_block_timestamp() + delay;
55+
(id, proposal)
56+
}
57+
5158
pub fn get_proposal_with_id(calls: Span<Call>, description: @ByteArray) -> (felt252, ProposalCore) {
5259
let timestamp = starknet::get_block_timestamp();
5360
let vote_start = timestamp + GovernorMock::VOTING_DELAY;
@@ -105,7 +112,9 @@ pub fn set_executor(
105112
}
106113

107114
pub fn deploy_votes_token() -> IERC20Dispatcher {
108-
utils::declare_and_deploy_at("ERC20TimestampVotesMock", VOTES_TOKEN, array![]);
115+
let mut calldata = array![];
116+
calldata.append_serde(SUPPLY);
117+
utils::declare_and_deploy_at("ERC20TimestampVotesMock", VOTES_TOKEN, calldata);
109118
IERC20Dispatcher { contract_address: VOTES_TOKEN }
110119
}
111120

‎packages/governance/src/tests/governor/timestamp/test_governor.cairo‎

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use core::num::traits::Bounded;
2+
use openzeppelin_interfaces::governance::votes::{IVotesDispatcher, IVotesDispatcherTrait};
23
use openzeppelin_interfaces::governor::{
34
IGOVERNOR_ID, IGovernor, IGovernorDispatcher, IGovernorDispatcherTrait, ProposalState,
45
};
@@ -9,7 +10,7 @@ use openzeppelin_test_common::mocks::timelock::{
910
IMockContractDispatcher, IMockContractDispatcherTrait,
1011
};
1112
use openzeppelin_testing as utils;
12-
use openzeppelin_testing::constants::{ADMIN, OTHER, VOTES_TOKEN, ZERO};
13+
use openzeppelin_testing::constants::{ADMIN, OTHER, SUPPLY, TIMESTAMP, VOTES_TOKEN, ZERO};
1314
use openzeppelin_testing::{AsAddressTrait, spy_events};
1415
use openzeppelin_utils::bytearray::ByteArrayExtTrait;
1516
use openzeppelin_utils::cryptography::snip12::OffchainMessageHash;
@@ -28,8 +29,9 @@ use crate::governor::{DefaultConfig, ProposalCore};
2829
use crate::tests::governor::common::GovernorSpyHelpersImpl;
2930
use crate::tests::governor::timestamp::common::{
3031
COMPONENT_STATE, CONTRACT_STATE, deploy_votes_token, get_calls, get_mock_state,
31-
get_proposal_info, get_state, hash_proposal, setup_active_proposal, setup_canceled_proposal,
32-
setup_defeated_proposal, setup_executed_proposal, setup_pending_proposal, setup_queued_proposal,
32+
get_proposal_info, get_proposal_info_with_custom_delay, get_state, hash_proposal,
33+
setup_active_proposal, setup_canceled_proposal, setup_defeated_proposal,
34+
setup_executed_proposal, setup_pending_proposal, setup_queued_proposal,
3335
setup_succeeded_proposal,
3436
};
3537

@@ -436,10 +438,10 @@ fn test_has_voted() {
436438

437439
fn test_propose_external_version(external_state_version: bool) {
438440
let mut state = COMPONENT_STATE();
439-
let mut spy = spy_events();
440441
let contract_address = test_address();
441442
deploy_votes_token();
442443
initialize_votes_component(VOTES_TOKEN);
444+
let mut spy = spy_events();
443445

444446
let calls = get_calls(OTHER, false);
445447
let proposer = ADMIN;
@@ -931,6 +933,19 @@ fn test_cast_vote_pending() {
931933
state.cast_vote(id, 0);
932934
}
933935

936+
#[test]
937+
#[should_panic(expected: 'Votes: future Lookup')]
938+
fn test__cast_vote_at_vote_start() {
939+
let mut state = COMPONENT_STATE();
940+
deploy_votes_token();
941+
initialize_votes_component(VOTES_TOKEN);
942+
let (id, proposal) = setup_pending_proposal(ref state, false);
943+
944+
start_cheat_block_timestamp_global(proposal.vote_start);
945+
let params = array![].span();
946+
state._cast_vote(id, OTHER, 0, "", params);
947+
}
948+
934949
#[test]
935950
fn test_cast_vote_active() {
936951
let mut state = COMPONENT_STATE();
@@ -952,6 +967,49 @@ fn test_cast_vote_active() {
952967
spy.assert_only_event_vote_cast(contract_address, OTHER, id, 0, expected_weight, @"");
953968
}
954969

970+
#[test]
971+
#[should_panic(expected: 'Votes: future Lookup')]
972+
fn test__cast_vote_zero_delay() {
973+
start_cheat_block_timestamp_global(TIMESTAMP - 1);
974+
let voter = test_address();
975+
let mut state = COMPONENT_STATE();
976+
deploy_votes_token();
977+
delegate_votes_to(voter);
978+
initialize_votes_component(VOTES_TOKEN);
979+
980+
// Setup proposal with 0 delay
981+
start_cheat_block_timestamp_global(TIMESTAMP);
982+
let (id, proposal) = get_proposal_info_with_custom_delay(0);
983+
state.Governor_proposals.write(id, proposal);
984+
985+
// Try to cast vote
986+
let reason = "reason";
987+
let params = array![].span();
988+
state._cast_vote(id, voter, 0, reason, params);
989+
}
990+
991+
#[test]
992+
fn test__cast_vote_zero_delay_after_some_time() {
993+
start_cheat_block_timestamp_global(TIMESTAMP - 1);
994+
let voter = test_address();
995+
let mut state = COMPONENT_STATE();
996+
deploy_votes_token();
997+
delegate_votes_to(voter);
998+
initialize_votes_component(VOTES_TOKEN);
999+
1000+
// Setup proposal with 0 delay
1001+
start_cheat_block_timestamp_global(TIMESTAMP);
1002+
let (id, proposal) = get_proposal_info_with_custom_delay(0);
1003+
state.Governor_proposals.write(id, proposal);
1004+
1005+
// Cast vote
1006+
start_cheat_block_timestamp_global(TIMESTAMP + 1);
1007+
let reason = "reason";
1008+
let params = array![].span();
1009+
let weight = state._cast_vote(id, voter, 0, reason, params);
1010+
assert_eq!(weight, SUPPLY);
1011+
}
1012+
9551013
#[test]
9561014
#[should_panic(expected: 'Unexpected proposal state')]
9571015
fn test_cast_vote_defeated() {
@@ -2146,3 +2204,8 @@ fn initialize_votes_component(votes_token: ContractAddress) {
21462204
let mut mock_state = CONTRACT_STATE();
21472205
mock_state.governor_votes.initializer(votes_token);
21482206
}
2207+
2208+
fn delegate_votes_to(delegatee: ContractAddress) {
2209+
let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN };
2210+
votes_dispatcher.delegate(delegatee);
2211+
}

‎packages/test_common/src/mocks/votes.cairo‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ pub mod ERC20TimestampVotesMock {
128128
}
129129

130130
#[constructor]
131-
fn constructor(ref self: ContractState) {
131+
fn constructor(ref self: ContractState, total_supply: u256) {
132132
self.erc20.initializer("MyToken", "MTK");
133+
self.erc20.mint(starknet::get_caller_address(), total_supply);
133134
}
134135
}
135136

@@ -179,8 +180,9 @@ pub mod ERC20BlockNumberVotesMock {
179180
}
180181

181182
#[constructor]
182-
fn constructor(ref self: ContractState) {
183+
fn constructor(ref self: ContractState, total_supply: u256) {
183184
self.erc20.initializer("MyToken", "MTK");
185+
self.erc20.mint(starknet::get_caller_address(), total_supply);
184186
}
185187
}
186188

0 commit comments

Comments
 (0)