diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs b/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs index 03b20abcf49..4a5657f0334 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs @@ -95,6 +95,8 @@ where // These are synchronised with respect to an election, so for simplicity in the tests // we just assign them to an election. MockStorageAccess::set_electoral_settings::(setup.electoral_settings.clone()); + MockStorageAccess::set_unsynchronised_state::(setup.unsynchronised_state.clone()); + MockStorageAccess::set_unsynchronised_settings::(setup.unsynchronised_settings.clone()); let election = MockAccess::::new_election( election_identifier_extra, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/mocks/access.rs b/state-chain/pallets/cf-elections/src/electoral_systems/mocks/access.rs index 8c67a36d155..adae194de99 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/mocks/access.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/mocks/access.rs @@ -291,6 +291,7 @@ impl MockStorageAccess { identifier: ElectionIdentifierOf, state: ES::ElectionState, ) { + println!("Setting election state for identifier: {:?}", identifier); ELECTION_STATE.with(|old_state| { let mut state_ref = old_state.borrow_mut(); state_ref.insert(identifier.encode(), state.encode()); @@ -341,6 +342,16 @@ impl MockStorageAccess { }); } + pub fn set_unsynchronised_settings( + unsynchronised_settings: ES::ElectoralUnsynchronisedSettings, + ) { + ELECTORAL_UNSYNCHRONISED_SETTINGS.with(|old_settings| { + let mut settings_ref = old_settings.borrow_mut(); + settings_ref.clear(); + settings_ref.extend_from_slice(&unsynchronised_settings.encode()); + }); + } + pub fn unsynchronised_settings() -> ES::ElectoralUnsynchronisedSettings { ELECTORAL_UNSYNCHRONISED_SETTINGS.with(|old_settings| { let settings_ref = old_settings.borrow(); diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/tests/change.rs b/state-chain/pallets/cf-elections/src/electoral_systems/tests/change.rs index 9eacc88eb93..6af37c8a1b7 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/tests/change.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/tests/change.rs @@ -26,19 +26,19 @@ impl MockHook { type Vote = u64; type SimpleChange = Change<(), Vote, (), MockHook, ()>; -// register_checks! { -// SimpleChange { -// hook_called(_pre, _post) { -// assert!(MockHook::called(), "Hook should have been called!"); -// }, -// hook_not_called(_pre, _post) { -// assert!( -// !MockHook::called(), -// "Hook should not have been called!" -// ); -// }, -// } -// } +register_checks! { + SimpleChange { + hook_called(_pre, _post) { + assert!(MockHook::called(), "Hook should have been called!"); + }, + hook_not_called(_pre, _post) { + assert!( + !MockHook::called(), + "Hook should not have been called!" + ); + }, + } +} const AUTHORITY_COUNT: AuthorityCount = 10; const THRESHOLD: AuthorityCount = cf_utilities::threshold_from_share_count(AUTHORITY_COUNT); @@ -107,29 +107,29 @@ fn minority_cannot_prevent_consensus() { ); } -// #[test] -// fn finalization_only_on_consensus_change() { -// with_default_state() -// .expect_consensus( -// generate_votes(AUTHORITY_COUNT, Vote::default(), 0, 0), -// Some(Vote::default()), -// ) -// .test_on_finalize( -// &(), -// |_| { -// assert!(!MockHook::called()); -// }, -// vec![Check::::hook_not_called(), Check::assert_unchanged()], -// ) -// .expect_consensus( -// generate_votes(AUTHORITY_COUNT, Vote::default() + 1, 0, 0), -// Some(Vote::default() + 1), -// ) -// .test_on_finalize( -// &(), -// |_| { -// assert!(!MockHook::called()); -// }, -// vec![Check::::hook_called(), Check::all_elections_deleted()], -// ); -// } +#[test] +fn finalization_only_on_consensus_change() { + with_default_state() + .expect_consensus( + generate_votes(AUTHORITY_COUNT, Vote::default(), 0, 0), + Some(Vote::default()), + ) + .test_on_finalize( + &(), + |_| { + assert!(!MockHook::called()); + }, + vec![Check::::hook_not_called(), Check::assert_unchanged()], + ) + .expect_consensus( + generate_votes(AUTHORITY_COUNT, Vote::default() + 1, 0, 0), + Some(Vote::default() + 1), + ) + .test_on_finalize( + &(), + |_| { + assert!(!MockHook::called()); + }, + vec![Check::::hook_called(), Check::all_elections_deleted()], + ); +} diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/tests/monotonic_median.rs b/state-chain/pallets/cf-elections/src/electoral_systems/tests/monotonic_median.rs index 2b841a71da7..449ae61ab23 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/tests/monotonic_median.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/tests/monotonic_median.rs @@ -1,5 +1,5 @@ use super::{ - mocks::{Check, TestContext, TestSetup}, + mocks::{Check, MockAccess, TestContext, TestSetup}, register_checks, }; use crate::{ @@ -40,23 +40,22 @@ impl MockHook { } } -// register_checks! { -// MonotonicMedianTest { -// monotonically_increasing_state(pre_finalize, post_finalize) { -// assert!(post_finalize.unsynchronised_state().unwrap() >= -// pre_finalize.unsynchronised_state().unwrap(), "Unsynchronised state can not decrease!"); -// }, -// hook_called(_pre, _post) { -// assert!(MockHook::has_been_called(), "Hook should have been called!"); -// }, -// hook_not_called(_pre, _post) { -// assert!( -// !MockHook::has_been_called(), -// "Hook should not have been called!" -// ); -// }, -// } -// } +register_checks! { + MonotonicMedianTest { + monotonically_increasing_state(pre_finalize, post_finalize) { + assert!(post_finalize.unsynchronised_state >= pre_finalize.unsynchronised_state, "Unsynchronised state can not decrease!"); + }, + hook_called(_pre, _post) { + assert!(MockHook::has_been_called(), "Hook should have been called!"); + }, + hook_not_called(_pre, _post) { + assert!( + !MockHook::has_been_called(), + "Hook should not have been called!" + ); + }, + } +} #[test] fn check_consensus_correctly_calculates_median_when_all_authorities_vote() { @@ -87,81 +86,81 @@ fn too_few_votes_consensus_not_possible() { .expect_consensus(generate_votes(LESS_THAN_SUCCESS_THRESHOLD, AUTHORITY_COUNT), None); } -// #[test] -// fn finalize_election_with_incremented_state() { -// let test = with_default_context(); -// let initial_state = test.access().unsynchronised_state().unwrap(); -// let new_unsynchronised_state = initial_state + 1; - -// test.force_consensus_update(ConsensusStatus::Gained { -// most_recent: None, -// new: new_unsynchronised_state, -// }) -// .test_on_finalize( -// &(), -// |_| { -// assert!( -// !MockHook::has_been_called(), -// "Hook should not have been called before finalization!" -// ); -// }, -// vec![ -// Check::monotonically_increasing_state(), -// Check::::hook_called(), -// Check::new(move |pre, post| { -// assert_eq!(pre.unsynchronised_state().unwrap(), initial_state); -// assert_eq!(post.unsynchronised_state().unwrap(), new_unsynchronised_state); -// }), -// Check::last_election_deleted(), -// Check::election_id_incremented(), -// ], -// ); -// } - -// #[test] -// fn finalize_election_state_can_not_decrease() { -// const INTITIAL_STATE: u64 = 2; - -// #[track_caller] -// fn assert_no_update(new_state: u64) { -// assert!( -// new_state <= INTITIAL_STATE, -// "This test is not valid if the new state is higher than the old." -// ); -// MockHook::reset(); -// with_default_setup() -// .with_unsynchronised_state(INTITIAL_STATE) -// .build_with_initial_election() -// // It's possible for authorities to come to consensus on a lower state, -// // but this should not change the unsynchronised state. -// .force_consensus_update(ConsensusStatus::Gained { most_recent: None, new: new_state }) -// .test_on_finalize( -// &(), -// |_| { -// assert!( -// !MockHook::has_been_called(), -// "Hook should not have been called before finalization!" -// ); -// }, -// vec![ -// Check::monotonically_increasing_state(), -// // The hook should not be called if the state is not updated. -// Check::::hook_not_called(), -// Check::new(|pre, post| { -// assert_eq!(pre.unsynchronised_state().unwrap(), INTITIAL_STATE); -// assert_eq!(post.unsynchronised_state().unwrap(), INTITIAL_STATE); -// }), -// Check::last_election_deleted(), -// Check::election_id_incremented(), -// ], -// ); -// } - -// // Lower state than the initial state should be invalid. -// assert_no_update(INTITIAL_STATE - 1); -// // Equal state to the initial state should be invalid. -// assert_no_update(INTITIAL_STATE); -// } +#[test] +fn finalize_election_with_incremented_state() { + let test = with_default_context(); + let initial_state = MockAccess::::unsynchronised_state().unwrap(); + let new_unsynchronised_state = initial_state + 1; + + test.force_consensus_update(ConsensusStatus::Gained { + most_recent: None, + new: new_unsynchronised_state, + }) + .test_on_finalize( + &(), + |_| { + assert!( + !MockHook::has_been_called(), + "Hook should not have been called before finalization!" + ); + }, + vec![ + Check::monotonically_increasing_state(), + Check::::hook_called(), + Check::new(move |pre, post| { + assert_eq!(pre.unsynchronised_state, initial_state); + assert_eq!(post.unsynchronised_state, new_unsynchronised_state); + }), + Check::last_election_deleted(), + Check::election_id_incremented(), + ], + ); +} + +#[test] +fn finalize_election_state_can_not_decrease() { + const INTITIAL_STATE: u64 = 2; + + #[track_caller] + fn assert_no_update(new_state: u64) { + assert!( + new_state <= INTITIAL_STATE, + "This test is not valid if the new state is higher than the old." + ); + MockHook::reset(); + with_default_setup() + .with_unsynchronised_state(INTITIAL_STATE) + .build_with_initial_election() + // It's possible for authorities to come to consensus on a lower state, + // but this should not change the unsynchronised state. + .force_consensus_update(ConsensusStatus::Gained { most_recent: None, new: new_state }) + .test_on_finalize( + &(), + |_| { + assert!( + !MockHook::has_been_called(), + "Hook should not have been called before finalization!" + ); + }, + vec![ + Check::monotonically_increasing_state(), + // The hook should not be called if the state is not updated. + Check::::hook_not_called(), + Check::new(|pre, post| { + assert_eq!(pre.unsynchronised_state, INTITIAL_STATE); + assert_eq!(post.unsynchronised_state, INTITIAL_STATE); + }), + Check::last_election_deleted(), + Check::election_id_incremented(), + ], + ); + } + + // Lower state than the initial state should be invalid. + assert_no_update(INTITIAL_STATE - 1); + // Equal state to the initial state should be invalid. + assert_no_update(INTITIAL_STATE); +} #[test] fn minority_can_not_influence_consensus() {