Skip to content

Commit 80a50de

Browse files
immrsdimmrsd
authored
Governor: fix state at snapshot (#1606)
* Make state resolving logic consistent with Solidity * Fix timestamp in Governor tests * Add tests checking Governor Pending state at snapshot timestamp * Add changelog entry * Format files * Fix changelog entry * Extract duplicated logic into helper func in tests * Reduce number of fuzzer runs * Fix setup-snfoundry version in tests workflow * Set number of fuzzer runs to 200 * Fix setup-snfoundry version
1 parent 0703869 commit 80a50de

File tree

8 files changed

+104
-31
lines changed

8 files changed

+104
-31
lines changed

‎.github/workflows/test.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ jobs:
3030
FOUNDRY_VERSION=$(grep 'snforge_std = ' Scarb.toml | sed 's/snforge_std = "\(.*\)"/\1/')
3131
echo "FOUNDRY_VERSION=$FOUNDRY_VERSION" >> "$GITHUB_ENV"
3232
33-
- uses: foundry-rs/setup-snfoundry@v4
33+
- uses: foundry-rs/setup-snfoundry@v5.0.0
3434
with:
3535
starknet-foundry-version: ${{ env.FOUNDRY_VERSION }}
3636

@@ -48,7 +48,7 @@ jobs:
4848
run: scarb fmt --check --workspace
4949

5050
- name: Run tests
51-
run: snforge test --workspace --features fuzzing --fuzzer-runs 500
51+
run: snforge test --workspace --features fuzzing --fuzzer-runs 200
5252

5353
- name: Run tests and generate coverage report
5454
run: snforge test --workspace --coverage

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- `MAXIMUM_DEFAULT_ADMIN_TRANSFER_DELAY` constant exposed in the component ImmutableConfig.
1616
- `maximum_default_admin_transfer_delay` getter to the `IAccessControlDefaultAdminRules` interface.
1717

18+
### Changed (Breaking)
19+
20+
- `GovernorComponent` proposal state resolution at snapshot timepoint changed from Active to Pending (#1606)
21+
1822
## 3.0.0-alpha.3 (2025-10-9)
1923

2024
### Added

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -894,13 +894,12 @@ pub mod GovernorComponent {
894894
assert(snapshot.is_non_zero(), Errors::NONEXISTENT_PROPOSAL);
895895

896896
let current_timepoint = self.clock();
897-
if current_timepoint < snapshot {
897+
if snapshot >= current_timepoint {
898898
return ProposalState::Pending;
899899
}
900900

901901
let deadline = self._proposal_deadline(proposal_id);
902-
903-
if current_timepoint <= deadline {
902+
if deadline >= current_timepoint {
904903
return ProposalState::Active;
905904
} else if !self.quorum_reached(proposal_id) || !self.vote_succeeded(proposal_id) {
906905
return ProposalState::Defeated;

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

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,19 @@ fn test_state_pending() {
144144
setup_pending_proposal(ref state, true);
145145
}
146146

147+
#[test]
148+
fn test_state_pending_at_snapshot() {
149+
let mut state = COMPONENT_STATE();
150+
deploy_votes_token();
151+
initialize_votes_component(VOTES_TOKEN);
152+
let (id, proposal) = setup_pending_proposal(ref state, true);
153+
154+
let snapshot = proposal.vote_start;
155+
start_cheat_block_number_global(snapshot);
156+
let current_state = get_state(@state, id, true);
157+
assert_eq!(current_state, ProposalState::Pending);
158+
}
159+
147160
fn test_state_active_external_version(external_state_version: bool) {
148161
let mut state = COMPONENT_STATE();
149162
deploy_votes_token();
@@ -581,8 +594,8 @@ fn test_execute() {
581594

582595
// 2. Cast vote
583596

584-
// Fast forward the vote delay
585-
current_time += GovernorMock::VOTING_DELAY;
597+
// Fast forward to voting start
598+
current_time = resolve_voting_start(current_time);
586599
start_cheat_block_number_global(current_time);
587600

588601
// Cast vote
@@ -634,8 +647,8 @@ fn test_execute_panics() {
634647

635648
// 2. Cast vote
636649

637-
// Fast forward the vote delay
638-
current_time += GovernorMock::VOTING_DELAY;
650+
// Fast forward to voting start
651+
current_time = resolve_voting_start(current_time);
639652
start_cheat_block_number_global(current_time);
640653

641654
// Cast vote
@@ -1230,8 +1243,8 @@ fn prepare_governor_and_signature(
12301243
start_cheat_block_number_global(current_time);
12311244
let proposal_id = governor.propose(calls, description.clone());
12321245

1233-
// 2. Fast forward the vote delay
1234-
current_time += GovernorMock::VOTING_DELAY;
1246+
// 2. Fast forward the voting start
1247+
current_time = resolve_voting_start(current_time);
12351248
start_cheat_block_number_global(current_time);
12361249

12371250
// 3. Generate a key pair and set up an account
@@ -1332,8 +1345,8 @@ fn prepare_governor_and_signature_with_reason_and_params(
13321345
start_cheat_block_number_global(current_time);
13331346
let proposal_id = governor.propose(calls, description.clone());
13341347

1335-
// 2. Fast forward the vote delay
1336-
current_time += GovernorMock::VOTING_DELAY;
1348+
// 2. Fast forward to voting start
1349+
current_time = resolve_voting_start(current_time);
13371350
start_cheat_block_number_global(current_time);
13381351

13391352
// 3. Generate a key pair and set up an account
@@ -2034,7 +2047,7 @@ fn test__cast_vote_pending() {
20342047
}
20352048

20362049
#[test]
2037-
#[should_panic(expected: 'Votes: future Lookup')]
2050+
#[should_panic(expected: 'Unexpected proposal state')]
20382051
fn test__cast_vote_at_vote_start() {
20392052
let mut state = COMPONENT_STATE();
20402053
deploy_votes_token();
@@ -2094,7 +2107,7 @@ fn test__cast_vote_active_with_params() {
20942107
}
20952108

20962109
#[test]
2097-
#[should_panic(expected: 'Votes: future Lookup')]
2110+
#[should_panic(expected: 'Unexpected proposal state')]
20982111
fn test__cast_vote_zero_delay() {
20992112
start_cheat_block_number_global(BLOCK_NUMBER - 1);
21002113
let voter = test_address();
@@ -2209,3 +2222,9 @@ fn delegate_votes_to(delegatee: ContractAddress) {
22092222
let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN };
22102223
votes_dispatcher.delegate(delegatee);
22112224
}
2225+
2226+
fn resolve_voting_start(propose_timepoint: u64) -> u64 {
2227+
let voting_delay = GovernorMock::VOTING_DELAY;
2228+
let snapshot_timepoint = propose_timepoint + voting_delay;
2229+
snapshot_timepoint + 1
2230+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ fn test_state_pending() {
6262
assert_eq!(state, ProposalState::Pending);
6363
}
6464

65+
#[test]
66+
fn test_state_pending_at_snapshot() {
67+
let mut component_state = COMPONENT_STATE();
68+
deploy_votes_token();
69+
initialize_votes_component(VOTES_TOKEN);
70+
let (id, proposal) = setup_pending_proposal(ref component_state, false);
71+
72+
let snapshot = proposal.vote_start;
73+
start_cheat_block_number_global(snapshot);
74+
let state = GovernorExecution::state(@component_state, id);
75+
assert_eq!(state, ProposalState::Pending);
76+
}
77+
6578
#[test]
6679
fn test_state_active() {
6780
let mut component_state = COMPONENT_STATE();

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

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,19 @@ fn test_state_pending() {
144144
setup_pending_proposal(ref state, true);
145145
}
146146

147+
#[test]
148+
fn test_state_pending_at_snapshot() {
149+
let mut state = COMPONENT_STATE();
150+
deploy_votes_token();
151+
initialize_votes_component(VOTES_TOKEN);
152+
let (id, proposal) = setup_pending_proposal(ref state, true);
153+
154+
let snapshot = proposal.vote_start;
155+
start_cheat_block_timestamp_global(snapshot);
156+
let current_state = get_state(@state, id, true);
157+
assert_eq!(current_state, ProposalState::Pending);
158+
}
159+
147160
fn test_state_active_external_version(external_state_version: bool) {
148161
let mut state = COMPONENT_STATE();
149162
deploy_votes_token();
@@ -581,8 +594,8 @@ fn test_execute() {
581594

582595
// 2. Cast vote
583596

584-
// Fast forward the vote delay
585-
current_time += GovernorMock::VOTING_DELAY;
597+
// Fast forward to voting start
598+
current_time = resolve_voting_start(current_time);
586599
start_cheat_block_timestamp_global(current_time);
587600

588601
// Cast vote
@@ -634,8 +647,8 @@ fn test_execute_panics() {
634647

635648
// 2. Cast vote
636649

637-
// Fast forward the vote delay
638-
current_time += GovernorMock::VOTING_DELAY;
650+
// Fast forward to voting start
651+
current_time = resolve_voting_start(current_time);
639652
start_cheat_block_timestamp_global(current_time);
640653

641654
// Cast vote
@@ -934,7 +947,7 @@ fn test_cast_vote_pending() {
934947
}
935948

936949
#[test]
937-
#[should_panic(expected: 'Votes: future Lookup')]
950+
#[should_panic(expected: 'Unexpected proposal state')]
938951
fn test__cast_vote_at_vote_start() {
939952
let mut state = COMPONENT_STATE();
940953
deploy_votes_token();
@@ -968,7 +981,7 @@ fn test_cast_vote_active() {
968981
}
969982

970983
#[test]
971-
#[should_panic(expected: 'Votes: future Lookup')]
984+
#[should_panic(expected: 'Unexpected proposal state')]
972985
fn test__cast_vote_zero_delay() {
973986
start_cheat_block_timestamp_global(TIMESTAMP - 1);
974987
let voter = test_address();
@@ -1286,8 +1299,8 @@ fn prepare_governor_and_signature(
12861299
start_cheat_block_timestamp_global(current_time);
12871300
let proposal_id = governor.propose(calls, description.clone());
12881301

1289-
// 2. Fast forward the vote delay
1290-
current_time += GovernorMock::VOTING_DELAY;
1302+
// 2. Fast forward the voting start
1303+
current_time = resolve_voting_start(current_time);
12911304
start_cheat_block_timestamp_global(current_time);
12921305

12931306
// 3. Generate a key pair and set up an account
@@ -1388,8 +1401,8 @@ fn prepare_governor_and_signature_with_reason_and_params(
13881401
start_cheat_block_timestamp_global(current_time);
13891402
let proposal_id = governor.propose(calls, description.clone());
13901403

1391-
// 2. Fast forward the vote delay
1392-
current_time += GovernorMock::VOTING_DELAY;
1404+
// 2. Fast forward to voting start
1405+
current_time = resolve_voting_start(current_time);
13931406
start_cheat_block_timestamp_global(current_time);
13941407

13951408
// 3. Generate a key pair and set up an account
@@ -2209,3 +2222,9 @@ fn delegate_votes_to(delegatee: ContractAddress) {
22092222
let votes_dispatcher = IVotesDispatcher { contract_address: VOTES_TOKEN };
22102223
votes_dispatcher.delegate(delegatee);
22112224
}
2225+
2226+
fn resolve_voting_start(propose_timepoint: u64) -> u64 {
2227+
let voting_delay = GovernorMock::VOTING_DELAY;
2228+
let snapshot_timepoint = propose_timepoint + voting_delay;
2229+
snapshot_timepoint + 1
2230+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ fn test_state_pending() {
6262
assert_eq!(state, ProposalState::Pending);
6363
}
6464

65+
#[test]
66+
fn test_state_pending_at_snapshot() {
67+
let mut component_state = COMPONENT_STATE();
68+
deploy_votes_token();
69+
initialize_votes_component(VOTES_TOKEN);
70+
let (id, proposal) = setup_pending_proposal(ref component_state, false);
71+
72+
let snapshot = proposal.vote_start;
73+
start_cheat_block_timestamp_global(snapshot);
74+
let state = GovernorExecution::state(@component_state, id);
75+
assert_eq!(state, ProposalState::Pending);
76+
}
77+
6578
#[test]
6679
fn test_state_active() {
6780
let mut component_state = COMPONENT_STATE();

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,8 @@ fn test_execute_operations() {
381381

382382
// 3. Cast vote
383383

384-
// Fast forward the vote delay
385-
current_time += GovernorTimelockedMock::VOTING_DELAY;
384+
// Fast forward to voting start
385+
current_time = resolve_voting_start(current_time);
386386
start_cheat_block_timestamp_global(current_time);
387387

388388
// Cast vote
@@ -458,8 +458,8 @@ fn test_queue_operations() {
458458

459459
// 3. Cast vote
460460

461-
// Fast forward the vote delay
462-
current_time += GovernorTimelockedMock::VOTING_DELAY;
461+
// Fast forward to voting start
462+
current_time = resolve_voting_start(current_time);
463463
start_cheat_block_timestamp_global(current_time);
464464

465465
// Cast vote
@@ -535,8 +535,8 @@ fn test_cancel_operations_queued() {
535535

536536
// 3. Cast vote
537537

538-
// Fast forward the vote delay
539-
current_time += GovernorTimelockedMock::VOTING_DELAY;
538+
// Fast forward to voting start
539+
current_time = resolve_voting_start(current_time);
540540
start_cheat_block_timestamp_global(current_time);
541541

542542
// Cast vote
@@ -691,6 +691,12 @@ fn initialize_votes_component(votes_token: ContractAddress) {
691691
mock_state.governor_votes.initializer(votes_token);
692692
}
693693

694+
fn resolve_voting_start(propose_timepoint: u64) -> u64 {
695+
let voting_delay = GovernorTimelockedMock::VOTING_DELAY;
696+
let snapshot_timepoint = propose_timepoint + voting_delay;
697+
snapshot_timepoint + 1
698+
}
699+
694700
//
695701
// Setup proposals
696702
//

0 commit comments

Comments
 (0)