diff --git a/engine/src/elections.rs b/engine/src/elections.rs index 650696ada3..8d7eea006d 100644 --- a/engine/src/elections.rs +++ b/engine/src/elections.rs @@ -13,9 +13,9 @@ use cf_primitives::MILLISECONDS_PER_BLOCK; use cf_utilities::{future_map::FutureMap, task_scope::Scope, UnendingStream}; use futures::{stream, StreamExt, TryStreamExt}; use pallet_cf_elections::{ - electoral_system::{ElectionIdentifierOf, ElectoralSystem}, vote_storage::{AuthorityVote, VoteStorage}, - SharedDataHash, MAXIMUM_VOTES_PER_EXTRINSIC, + CompositeElectionIdentifierOf, ElectoralSystemRunner, SharedDataHash, + MAXIMUM_VOTES_PER_EXTRINSIC, }; use rand::Rng; use std::{ @@ -23,7 +23,7 @@ use std::{ sync::Arc, }; use tracing::{error, info, warn}; -use voter_api::VoterApi; +use voter_api::CompositeVoterApi; const MAXIMUM_CONCURRENT_FILTER_REQUESTS: usize = 16; const LIFETIME_OF_SHARED_DATA_IN_CACHE: std::time::Duration = std::time::Duration::from_secs(90); @@ -34,7 +34,7 @@ const INITIAL_VOTER_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration:: pub struct Voter< Instance: 'static, StateChainClient: ElectoralApi + SignedExtrinsicApi + ChainApi, - VoterClient: VoterApi<>::ElectoralSystem> + Send + Sync + 'static, + VoterClient: CompositeVoterApi<>::ElectoralSystemRunner> + Send + Sync + 'static, > where state_chain_runtime::Runtime: pallet_cf_elections::Config, @@ -47,7 +47,7 @@ pub struct Voter< impl< Instance: Send + Sync + 'static, StateChainClient: ElectoralApi + SignedExtrinsicApi + ChainApi, - VoterClient: VoterApi<>::ElectoralSystem> + Clone + Send + Sync + 'static, + VoterClient: CompositeVoterApi<>::ElectoralSystemRunner> + Clone + Send + Sync + 'static, > Voter where state_chain_runtime::Runtime: @@ -111,17 +111,17 @@ where std::time::Duration::from_millis(MILLISECONDS_PER_BLOCK); let mut submit_interval = tokio::time::interval(BLOCK_TIME); let mut pending_submissions = BTreeMap::< - ElectionIdentifierOf<>::ElectoralSystem>, + CompositeElectionIdentifierOf<>::ElectoralSystemRunner>, ( - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::PartialVote, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::PartialVote, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, ) >::default(); let mut vote_tasks = FutureMap::default(); let mut shared_data_cache = HashMap::< SharedDataHash, ( - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::SharedData, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::SharedData, std::time::Instant, ) >::default(); @@ -163,7 +163,7 @@ where Ok(vote) => { info!("Voting task for election: '{:?}' succeeded.", election_identifier); // Create the partial_vote early so that SharedData can be provided as soon as the vote has been generated, rather than only after it is submitted. - let partial_vote = <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::vote_into_partial_vote(&vote, |shared_data| { + let partial_vote = <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::vote_into_partial_vote(&vote, |shared_data| { let shared_data_hash = SharedDataHash::of(&shared_data); if shared_data_cache.len() > MAXIMUM_SHARED_DATA_CACHE_ITEMS { for shared_data_hash in shared_data_cache.keys().cloned().take(shared_data_cache.len() - MAXIMUM_SHARED_DATA_CACHE_ITEMS).collect::>() { diff --git a/engine/src/elections/voter_api.rs b/engine/src/elections/voter_api.rs index c38a15ada5..0b0dd50a4a 100644 --- a/engine/src/elections/voter_api.rs +++ b/engine/src/elections/voter_api.rs @@ -4,8 +4,10 @@ use frame_support::{ }; use pallet_cf_elections::{ electoral_system::ElectoralSystem, - electoral_systems::composite::{self, Composite}, + electoral_system_runner::RunnerStorageAccessTrait, + electoral_systems::composite::{self, CompositeRunner}, vote_storage::{self, VoteStorage}, + ElectoralSystemRunner, }; #[async_trait::async_trait] @@ -17,33 +19,43 @@ pub trait VoterApi { ) -> Result<<::Vote as VoteStorage>::Vote, anyhow::Error>; } -pub struct CompositeVoter { +pub struct CompositeVoter { voters: Voters, - _phantom: core::marker::PhantomData, + _phantom: core::marker::PhantomData, } -impl Clone for CompositeVoter { +impl Clone for CompositeVoter { fn clone(&self) -> Self { Self { voters: self.voters.clone(), _phantom: Default::default() } } } -impl CompositeVoter { +impl CompositeVoter { pub fn new(voters: Voters) -> Self { Self { voters, _phantom: Default::default() } } } +#[async_trait::async_trait] +pub trait CompositeVoterApi { + async fn vote( + &self, + settings: ::ElectoralSettings, + properties: ::ElectionProperties, + ) -> Result<<::Vote as VoteStorage>::Vote, anyhow::Error>; +} + +// TODO Combine this into the composite macro PRO-1736 macro_rules! generate_voter_api_tuple_impls { ($module:ident: ($(($electoral_system:ident, $voter:ident)),*$(,)?)) => { #[allow(non_snake_case)] #[async_trait::async_trait] - impl<$($voter: VoterApi<$electoral_system> + Send + Sync),*, $($electoral_system : ElectoralSystem + Send + Sync + 'static),*, ValidatorId: MaybeSerializeDeserialize + Member + Parameter, Hooks: Send + Sync + 'static + composite::$module::Hooks<$($electoral_system,)*>> VoterApi> for CompositeVoter, ($($voter,)*)> { + impl<$($voter: VoterApi<$electoral_system> + Send + Sync),*, $($electoral_system : ElectoralSystem + Send + Sync + 'static),*, ValidatorId: MaybeSerializeDeserialize + Member + Parameter, StorageAccess: RunnerStorageAccessTrait> + Send + Sync + 'static, Hooks: Send + Sync + 'static + composite::$module::Hooks<$($electoral_system,)*>> CompositeVoterApi> for CompositeVoter, ($($voter,)*)> { async fn vote( &self, - settings: as ElectoralSystem>::ElectoralSettings, - properties: as ElectoralSystem>::ElectionProperties, + settings: as ElectoralSystemRunner>::ElectoralSettings, + properties: as ElectoralSystemRunner>::ElectionProperties, ) -> Result< - < as ElectoralSystem>::Vote as VoteStorage>::Vote, + < as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, anyhow::Error, > { use vote_storage::composite::$module::CompositeVote; diff --git a/engine/src/state_chain_observer/client/electoral_api.rs b/engine/src/state_chain_observer/client/electoral_api.rs index 3f50d9ea92..61460b7766 100644 --- a/engine/src/state_chain_observer/client/electoral_api.rs +++ b/engine/src/state_chain_observer/client/electoral_api.rs @@ -4,9 +4,8 @@ use crate::state_chain_observer::client::{ }; use codec::{Decode, Encode}; use pallet_cf_elections::{ - electoral_system::{ElectionIdentifierOf, ElectoralSystem}, - vote_storage::VoteStorage, - ElectoralDataFor, + electoral_system_runner::CompositeElectionIdentifierOf, vote_storage::VoteStorage, + ElectoralDataFor, ElectoralSystemRunner, }; use state_chain_runtime::SolanaInstance; use std::collections::{BTreeMap, BTreeSet}; @@ -29,10 +28,10 @@ where fn filter_votes( &self, proposed_votes: BTreeMap< - ElectionIdentifierOf<>::ElectoralSystem>, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, + CompositeElectionIdentifierOf<>::ElectoralSystemRunner>, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, >, - ) -> impl std::future::Future>::ElectoralSystem>>> + Send + 'static; + ) -> impl std::future::Future>::ElectoralSystemRunner>>> + Send + 'static; } impl< @@ -68,10 +67,10 @@ impl< fn filter_votes( &self, proposed_votes: BTreeMap< - ElectionIdentifierOf<>::ElectoralSystem>, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, + CompositeElectionIdentifierOf<>::ElectoralSystemRunner>, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, >, - ) -> impl std::future::Future>::ElectoralSystem>>> + Send + 'static{ + ) -> impl std::future::Future>::ElectoralSystemRunner>>> + Send + 'static{ let base_rpc_client = self.base_rpc_client.clone(); let account_id = self.signed_extrinsic_client.account_id(); async move { @@ -82,10 +81,10 @@ impl< .map_err(anyhow::Error::from) .and_then(|electoral_data| { >::ElectoralSystem, + >>::ElectoralSystemRunner, >, > as Decode>::decode(&mut &electoral_data[..]) .map_err(Into::into) diff --git a/engine/src/witness/sol.rs b/engine/src/witness/sol.rs index 407e373051..8449bf96bf 100644 --- a/engine/src/witness/sol.rs +++ b/engine/src/witness/sol.rs @@ -21,7 +21,7 @@ use futures::FutureExt; use pallet_cf_elections::{electoral_system::ElectoralSystem, vote_storage::VoteStorage}; use state_chain_runtime::{ chainflip::solana_elections::{ - SolanaBlockHeightTracking, SolanaEgressWitnessing, SolanaElectoralSystem, + SolanaBlockHeightTracking, SolanaEgressWitnessing, SolanaElectoralSystemRunner, SolanaFeeTracking, SolanaIngressTracking, SolanaLiveness, SolanaNonceTracking, TransactionSuccessDetails, }, @@ -195,7 +195,7 @@ where crate::elections::Voter::new( scope, state_chain_client, - CompositeVoter::::new(( + CompositeVoter::::new(( SolanaBlockHeightTrackingVoter { client: client.clone() }, SolanaFeeTrackingVoter { client: client.clone() }, SolanaIngressTrackingVoter { client: client.clone() }, diff --git a/state-chain/cf-integration-tests/src/solana.rs b/state-chain/cf-integration-tests/src/solana.rs index 9933045f16..d6753189e9 100644 --- a/state-chain/cf-integration-tests/src/solana.rs +++ b/state-chain/cf-integration-tests/src/solana.rs @@ -27,9 +27,8 @@ use frame_support::{ traits::{OnFinalize, UnfilteredDispatchable}, }; use pallet_cf_elections::{ - electoral_system::{ElectionIdentifierOf, ElectoralSystem}, - vote_storage::{composite::tuple_6_impls::CompositeVote, AuthorityVote, VoteStorage}, - MAXIMUM_VOTES_PER_EXTRINSIC, + vote_storage::{composite::tuple_6_impls::CompositeVote, AuthorityVote}, + CompositeAuthorityVoteOf, CompositeElectionIdentifierOf, MAXIMUM_VOTES_PER_EXTRINSIC, }; use pallet_cf_ingress_egress::{DepositWitness, FetchOrTransfer}; use pallet_cf_validator::RotationPhase; @@ -58,11 +57,12 @@ const BOB: AccountId = AccountId::new([0x44; 32]); const DEPOSIT_AMOUNT: u64 = 5_000_000_000u64; // 5 Sol const FALLBACK_ADDRESS: SolAddress = SolAddress([0xf0; 32]); -type SolanaElectionVote = BoundedBTreeMap::< - ElectionIdentifierOf<>::ElectoralSystem>, - AuthorityVote< - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::PartialVote, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, +type SolanaElectionVote = BoundedBTreeMap< + CompositeElectionIdentifierOf< + >::ElectoralSystemRunner, + >, + CompositeAuthorityVoteOf< + >::ElectoralSystemRunner, >, ConstU32, >; @@ -729,8 +729,8 @@ fn solana_ccm_execution_error_can_trigger_fallback() { let ccm_broadcast_id = pallet_cf_broadcast::PendingBroadcasts::::get().into_iter().next().unwrap(); // Get the election identifier of the Solana egress. - let election_id = SolanaElections::with_electoral_access_and_identifiers( - |_, election_identifiers| { + let election_id = SolanaElections::with_election_identifiers( + |election_identifiers| { Ok(election_identifiers.last().cloned().unwrap()) }, ).unwrap(); diff --git a/state-chain/pallets/cf-elections/src/benchmarking.rs b/state-chain/pallets/cf-elections/src/benchmarking.rs index 079ada4f57..6a0803ff2b 100644 --- a/state-chain/pallets/cf-elections/src/benchmarking.rs +++ b/state-chain/pallets/cf-elections/src/benchmarking.rs @@ -1,8 +1,8 @@ use crate::{ bitmap_components::ElectionBitmapComponents, - electoral_system::{ - AuthorityVoteOf, ElectionIdentifierOf, ElectoralSystem, IndividualComponentOf, - VotePropertiesOf, + electoral_system_runner::{ + CompositeAuthorityVoteOf, CompositeIndividualComponentOf, CompositeVotePropertiesOf, + ElectoralSystemRunner, }, vote_storage::VoteStorage, *, @@ -25,13 +25,13 @@ use crate::Call; #[allow(clippy::multiple_bound_locations)] #[instance_benchmarks( where - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote: BenchmarkValue, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::SharedData: BenchmarkValue, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Properties: BenchmarkValue, - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::IndividualComponent: BenchmarkValue, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote: BenchmarkValue, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::SharedData: BenchmarkValue, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Properties: BenchmarkValue, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::IndividualComponent: BenchmarkValue, InitialStateOf: BenchmarkValue, - ::ElectoralUnsynchronisedSettings: BenchmarkValue, - ::ElectoralSettings: BenchmarkValue, + ::ElectoralUnsynchronisedSettings: BenchmarkValue, + ::ElectoralSettings: BenchmarkValue, )] mod benchmarks { use super::*; @@ -64,8 +64,8 @@ mod benchmarks { fn setup_validators_and_vote, I: 'static>( validator_counts: u32, - vote_value: <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, - ) -> ElectionIdentifierOf { + vote_value: <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, + ) -> CompositeElectionIdentifierOf { // Setup a validator set of 150 as in the case of Mainnet. let validators = ready_validator_for_vote::(validator_counts); let caller = validators[0].clone(); @@ -82,7 +82,9 @@ mod benchmarks { BoundedBTreeMap::try_from( [( election_identifier, - AuthorityVoteOf::::Vote(vote_value.clone()), + CompositeAuthorityVoteOf::::Vote( + vote_value.clone() + ), )] .into_iter() .collect::>(), @@ -106,7 +108,9 @@ mod benchmarks { BoundedBTreeMap::try_from( iter::repeat(( next_election.0, - AuthorityVoteOf::::Vote(BenchmarkValue::benchmark_value()), + CompositeAuthorityVoteOf::::Vote( + BenchmarkValue::benchmark_value(), + ), )) .take(n as usize) .collect::>(), @@ -166,7 +170,9 @@ mod benchmarks { BoundedBTreeMap::try_from( [( next_election.0, - AuthorityVoteOf::::Vote(BenchmarkValue::benchmark_value()), + CompositeAuthorityVoteOf::::Vote( + BenchmarkValue::benchmark_value() + ), )] .into_iter() .collect::>(), @@ -201,7 +207,9 @@ mod benchmarks { BoundedBTreeMap::try_from( [( next_election.0, - AuthorityVoteOf::::Vote(BenchmarkValue::benchmark_value()), + CompositeAuthorityVoteOf::::Vote( + BenchmarkValue::benchmark_value() + ), )] .into_iter() .collect::>(), @@ -239,7 +247,9 @@ mod benchmarks { BoundedBTreeMap::try_from( [( election_identifier, - AuthorityVoteOf::::Vote(BenchmarkValue::benchmark_value()), + CompositeAuthorityVoteOf::::Vote( + BenchmarkValue::benchmark_value() + ), )] .into_iter() .collect::>(), @@ -252,7 +262,7 @@ mod benchmarks { assert_eq!( SharedData::::get(SharedDataHash::of::< - <<>::ElectoralSystem as ElectoralSystem>::Vote as VoteStorage>::Vote, + <<>::ElectoralSystemRunner as ElectoralSystemRunner>::Vote as VoteStorage>::Vote, >(&BenchmarkValue::benchmark_value())), Some(BenchmarkValue::benchmark_value()) ); @@ -351,8 +361,7 @@ mod benchmarks { } assert!(!ElectionConsensusHistoryUpToDate::::contains_key( - access_impls::ElectionAccess::::new(election_identifier) - .unique_monotonic_identifier() + election_identifier.unique_monotonic() )); } @@ -369,8 +378,7 @@ mod benchmarks { }; let epoch = T::EpochInfo::epoch_index(); - let monotonic_identifier = access_impls::ElectionAccess::::new(election_identifier) - .unique_monotonic_identifier(); + let monotonic_identifier = election_identifier.unique_monotonic(); Pallet::::on_finalize(frame_system::Pallet::::block_number()); assert_eq!( @@ -489,7 +497,7 @@ mod benchmarks { (0..b).for_each(|i| { SharedData::::insert( SharedDataHash::of(&i), - <::Vote as VoteStorage>::SharedData::benchmark_value()); + <::Vote as VoteStorage>::SharedData::benchmark_value()); }); (0..c).for_each(|i| { @@ -506,8 +514,8 @@ mod benchmarks { UniqueMonotonicIdentifier::from_u64(i as u64), T::ValidatorId::from(validators[i as usize].clone()), ( - VotePropertiesOf::::benchmark_value(), - IndividualComponentOf::::benchmark_value(), + CompositeVotePropertiesOf::::benchmark_value(), + CompositeIndividualComponentOf::::benchmark_value(), ), ); }); @@ -538,7 +546,7 @@ mod benchmarks { mod tests { use super::*; - use crate::{mock::*, tests::ElectoralSystemTestExt, Instance1}; + use crate::{mock::*, tests::ElectoralSystemRunnerTestExt, Instance1}; macro_rules! benchmark_tests { ( $( $test_name:ident: $test_fn:ident ( $( $arg:expr ),* ) ),+ $(,)? ) => { diff --git a/state-chain/pallets/cf-elections/src/electoral_system.rs b/state-chain/pallets/cf-elections/src/electoral_system.rs index 06583ffca3..4ad624c090 100644 --- a/state-chain/pallets/cf-elections/src/electoral_system.rs +++ b/state-chain/pallets/cf-elections/src/electoral_system.rs @@ -118,7 +118,6 @@ pub trait ElectoralSystem: 'static + Sized { /// to vote in a given Election. Validators are expected to call this indirectly via RPC once /// per state-chain block, for each active election. fn is_vote_desired>( - _election_identifier_with_extra: ElectionIdentifierOf, _election_access: &ElectionAccess, current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, ) -> Result { @@ -155,8 +154,7 @@ pub trait ElectoralSystem: 'static + Sized { /// This is called during the pallet's `on_finalize` callback, if elections aren't paused and /// the CorruptStorage error hasn't occurred. - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, context: &Self::OnFinalizeContext, ) -> Result; @@ -168,10 +166,8 @@ pub trait ElectoralSystem: 'static + Sized { /// You should *NEVER* update the epoch during this call. And in general updating any other /// state of any pallet is ill advised, and should instead be done in the 'on_finalize' /// function. - #[allow(clippy::type_complexity)] fn check_consensus>( - election_identifier: ElectionIdentifierOf, - electoral_access: &ElectionAccess, + election_access: &ElectionAccess, // This is the consensus as of the last time the consensus was checked. Note this is *NOT* // the "last" consensus, i.e. this can be `None` even if on some previous check we had // consensus, but it was subsequently lost. @@ -218,9 +214,12 @@ mod access { use super::{CorruptStorageError, ElectionIdentifierOf, ElectoralSystem}; + #[cfg(test)] + use codec::{Decode, Encode}; + /// Represents the current consensus, and how it has changed since it was last checked (i.e. /// 'check_consensus' was called). - #[cfg_attr(test, derive(Clone, Debug, PartialEq, Eq))] + #[cfg_attr(test, derive(Clone, Debug, PartialEq, Eq, Encode, Decode))] pub enum ConsensusStatus { /// You did not have consensus when previously checked, but now consensus has been gained. Gained { @@ -291,10 +290,7 @@ mod access { &self, ) -> Result<::ElectionState, CorruptStorageError>; - #[cfg(test)] - fn election_identifier( - &self, - ) -> Result, CorruptStorageError>; + fn election_identifier(&self) -> ElectionIdentifierOf; } /// A trait allowing write access to the details about a single election @@ -304,10 +300,10 @@ mod access { /// to ensure that in situations where `check_consensus` depends on the `state` that we will /// correctly recalculate the consensus if needed. fn set_state( - &mut self, + &self, state: ::ElectionState, ) -> Result<(), CorruptStorageError>; - fn clear_votes(&mut self); + fn clear_votes(&self); fn delete(self); /// This will change the `ElectionIdentifierExtra` value of the election, and allows you to /// optionally change the properties. Note the `extra` must be strictly greater than the @@ -322,7 +318,7 @@ mod access { /// will be invalidated by this. fn refresh( &mut self, - extra: ::ElectionIdentifierExtra, + new_extra: ::ElectionIdentifierExtra, properties: ::ElectionProperties, ) -> Result<(), CorruptStorageError>; @@ -330,7 +326,7 @@ mod access { /// votes/state. This also returns information about the difference in the consensus between /// the last call to `check_consensus`. fn check_consensus( - &mut self, + &self, ) -> Result< ConsensusStatus<::Consensus>, CorruptStorageError, @@ -340,28 +336,18 @@ mod access { /// A trait allowing read access to the details about the electoral system and its elections pub trait ElectoralReadAccess { type ElectoralSystem: ElectoralSystem; - type ElectionReadAccess<'a>: ElectionReadAccess - where - Self: 'a; + type ElectionReadAccess: ElectionReadAccess; - fn election( - &self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError>; - fn unsynchronised_settings( - &self, - ) -> Result< + fn election(id: ElectionIdentifierOf) -> Self::ElectionReadAccess; + fn unsynchronised_settings() -> Result< ::ElectoralUnsynchronisedSettings, CorruptStorageError, >; - fn unsynchronised_state( - &self, - ) -> Result< + fn unsynchronised_state() -> Result< ::ElectoralUnsynchronisedState, CorruptStorageError, >; fn unsynchronised_state_map( - &self, key: &::ElectoralUnsynchronisedStateMapKey, ) -> Result< Option< @@ -373,33 +359,27 @@ mod access { /// A trait allowing write access to the details about the electoral system and its elections pub trait ElectoralWriteAccess: ElectoralReadAccess { - type ElectionWriteAccess<'a>: ElectionWriteAccess - where - Self: 'a; + type ElectionWriteAccess: ElectionWriteAccess; fn new_election( - &mut self, extra: ::ElectionIdentifierExtra, properties: ::ElectionProperties, state: ::ElectionState, - ) -> Result, CorruptStorageError>; + ) -> Result; fn election_mut( - &mut self, id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError>; + ) -> Self::ElectionWriteAccess; fn set_unsynchronised_state( - &mut self, unsynchronised_state: ::ElectoralUnsynchronisedState, ) -> Result<(), CorruptStorageError>; /// Inserts or removes a value from the unsynchronised state map of the electoral system. fn set_unsynchronised_state_map( - &mut self, key: ::ElectoralUnsynchronisedStateMapKey, value: Option< ::ElectoralUnsynchronisedStateMapValue, >, - ) -> Result<(), CorruptStorageError>; + ); /// Allows you to mutate the unsynchronised state. This is more efficient than a read /// (`unsynchronised_state`) and then a write (`set_unsynchronised_state`) in the case of @@ -408,16 +388,14 @@ mod access { fn mutate_unsynchronised_state< T, F: for<'a> FnOnce( - &mut Self, &'a mut ::ElectoralUnsynchronisedState, ) -> Result, >( - &mut self, f: F, ) -> Result { - let mut unsynchronised_state = self.unsynchronised_state()?; - let t = f(self, &mut unsynchronised_state)?; - self.set_unsynchronised_state(unsynchronised_state)?; + let mut unsynchronised_state = Self::unsynchronised_state()?; + let t = f(&mut unsynchronised_state)?; + Self::set_unsynchronised_state(unsynchronised_state)?; Ok(t) } } diff --git a/state-chain/pallets/cf-elections/src/electoral_system_runner.rs b/state-chain/pallets/cf-elections/src/electoral_system_runner.rs new file mode 100644 index 0000000000..0a6d1ef1aa --- /dev/null +++ b/state-chain/pallets/cf-elections/src/electoral_system_runner.rs @@ -0,0 +1,323 @@ +use frame_support::{ + pallet_prelude::{MaybeSerializeDeserialize, Member}, + Parameter, +}; +use sp_std::vec::Vec; + +use crate::{ + vote_storage::{AuthorityVote, VoteStorage}, + CorruptStorageError, ElectionIdentifier, +}; + +use crate::electoral_system::ConsensusStatus; + +#[allow(type_alias_bounds)] +pub type CompositeElectionIdentifierOf = + ElectionIdentifier<::ElectionIdentifierExtra>; + +#[allow(type_alias_bounds)] +pub type CompositeAuthorityVoteOf = AuthorityVote< + <::Vote as VoteStorage>::PartialVote, + <::Vote as VoteStorage>::Vote, +>; +#[allow(type_alias_bounds)] +pub type CompositeIndividualComponentOf = + <::Vote as VoteStorage>::IndividualComponent; +#[allow(type_alias_bounds)] +pub type BitmapComponentOf = + <::Vote as VoteStorage>::BitmapComponent; +#[allow(type_alias_bounds)] +pub type CompositeVotePropertiesOf = + <::Vote as VoteStorage>::Properties; + +pub struct CompositeConsensusVote { + // If the validator hasn't voted, they will get a None. + pub vote: Option<(CompositeVotePropertiesOf, ::Vote)>, + pub validator_id: ES::ValidatorId, +} + +pub struct CompositeConsensusVotes { + pub votes: Vec>, +} + +#[cfg(test)] +impl CompositeConsensusVotes { + pub fn active_votes(self) -> Vec<::Vote> { + self.votes + .into_iter() + .filter_map(|CompositeConsensusVote { vote, .. }| vote.map(|v| v.1)) + .collect() + } +} + +/// A trait used to define a runner of electoral systems. An object implementing this trait is +/// injected into an elections pallet, which then executes the necessary logic to run each electoral +/// system's logic. +/// The primary implementation of this trait is the `CompositeRunner`. This should be the *only* +/// implementation of this trait. This ensures that the storage and access is consistent across all +/// electoral systems. i.e. we always wrap the storage types. Which leads to consistent and +/// therefore simpler migration logic. +pub trait ElectoralSystemRunner: 'static + Sized { + type ValidatorId: Parameter + Member + MaybeSerializeDeserialize; + + /// This is intended for storing any internal state of the ElectoralSystem. It is not + /// synchronised and therefore should only be used by the ElectoralSystem, and not be consumed + /// by the engine. + /// + /// Also note that if this state is changed that will not cause election's consensus to be + /// retested. + type ElectoralUnsynchronisedState: Parameter + Member + MaybeSerializeDeserialize; + /// This is intended for storing any internal state of the ElectoralSystem. It is not + /// synchronised and therefore should only be used by the ElectoralSystem, and not be consumed + /// by the engine. + /// + /// Also note that if this state is changed that will not cause election's consensus to be + /// retested. + type ElectoralUnsynchronisedStateMapKey: Parameter + Member; + /// This is intended for storing any internal state of the ElectoralSystem. It is not + /// synchronised and therefore should only be used by the ElectoralSystem, and not be consumed + /// by the engine. + /// + /// Also note that if this state is changed that will not cause election's consensus to be + /// retested. + type ElectoralUnsynchronisedStateMapValue: Parameter + Member; + + /// Settings of the electoral system. These can be changed at any time by governance, and + /// are not synchronised with elections, and therefore there is not a universal mapping from + /// elections to these settings values. Therefore it should only be used for internal + /// state, i.e. the engines should not consume this data. + /// + /// Also note that if these settings are changed that will not cause election's consensus to be + /// retested. + type ElectoralUnsynchronisedSettings: Parameter + Member + MaybeSerializeDeserialize; + + /// Settings of the electoral system. These settings are synchronised with + /// elections, so all engines will have a consistent view of the electoral settings to use for a + /// given election. + type ElectoralSettings: Parameter + Member + MaybeSerializeDeserialize + Eq; + + /// Extra data stored along with the UniqueMonotonicIdentifier as part of the + /// ElectionIdentifier. This is used by composite electoral systems to identify which variant of + /// election it is working with, without needing to reading in further election + /// state/properties/etc. + type ElectionIdentifierExtra: Parameter + Member + Copy + Eq + Ord; + + /// The properties of a single election, for example this could describe which block of the + /// external chain the election is associated with and what needs to be witnessed. + type ElectionProperties: Parameter + Member; + + /// Per-election state needed by the ElectoralSystem. This state is not synchronised across + /// engines, and may change during the lifetime of a election. + type ElectionState: Parameter + Member; + + /// A description of the validator's view of the election's topic. For example a list of all + /// ingresses the validator has observed in the block the election is about. + type Vote: VoteStorage; + + /// This is the information that results from consensus. Typically this will be the same as the + /// `Vote` type, but with more complex consensus models the result of an election may not be + /// sensibly represented in the same form as a single vote. + type Consensus: Parameter + Member + Eq; + + /// This is not used by the pallet, but is used to tell a validator that it should attempt + /// to vote in a given Election. Validators are expected to call this indirectly via RPC once + /// per state-chain block, for each active election. + fn is_vote_desired( + _election_identifier_with_extra: CompositeElectionIdentifierOf, + current_vote: Option<(CompositeVotePropertiesOf, CompositeAuthorityVoteOf)>, + ) -> Result { + Ok(current_vote.is_none()) + } + + /// This is not used by the pallet, but is used to tell a validator if they should submit vote. + /// This is a way to decrease the amount of extrinsics a validator needs to send. + fn is_vote_needed( + _current_vote: ( + CompositeVotePropertiesOf, + ::PartialVote, + CompositeAuthorityVoteOf, + ), + _proposed_vote: ( + ::PartialVote, + ::Vote, + ), + ) -> bool { + true + } + + /// This is used in the vote extrinsic to disallow a validator from providing votes that do not + /// pass this check. It is guaranteed that any vote values provided to + /// `generate_vote_properties`, or `check_consensus` have past this check. + /// + /// We only pass the `PartialVote` into the validity check, instead of the `AuthorityVote` or + /// `Vote`, to ensure the check's logic is consistent regardless of if the authority provides a + /// `Vote` or `PartialVote`. If the check was different depending on if the authority voted with + /// a `PartialVote` or `Vote`, then check only guarantees of the intersection of the two + /// variations. + /// + /// You should *NEVER* update the epoch during this call. And in general updating any other + /// state of any pallet is ill advised, and should instead be done in the 'on_finalize' + /// function. + fn is_vote_valid( + _election_identifier: CompositeElectionIdentifierOf, + _partial_vote: &::PartialVote, + ) -> Result { + Ok(true) + } + + /// This is called every time a vote occurs. It associates the vote with a `Properties` + /// value. + /// + /// You should *NEVER* update the epoch during this call. And in general updating any other + /// state of any pallet is ill advised, and should instead be done in the 'on_finalize' + /// function. + fn generate_vote_properties( + election_identifier: CompositeElectionIdentifierOf, + previous_vote: Option<(CompositeVotePropertiesOf, CompositeAuthorityVoteOf)>, + vote: &::PartialVote, + ) -> Result, CorruptStorageError>; + + /// This is called during the pallet's `on_finalize` callback, if elections aren't paused and + /// the CorruptStorage error hasn't occurred. + fn on_finalize( + election_identifiers: Vec>, + ) -> Result<(), CorruptStorageError>; + + /// This function determines if the votes we have received form a consensus. It is called as + /// part of the Election pallet's `on_finalize` callback when the Election's votes or state have + /// changed since the previous call. + /// + /// You should *NEVER* update the epoch during this call. And in general updating any other + /// state of any pallet is ill advised, and should instead be done in the 'on_finalize' + /// function. + #[allow(clippy::type_complexity)] + fn check_consensus( + election_identifier: CompositeElectionIdentifierOf, + // This is the consensus as of the last time the consensus was checked. Note this is *NOT* + // the "last" consensus, i.e. this can be `None` even if on some previous check we had + // consensus, but it was subsequently lost. + previous_consensus: Option<&Self::Consensus>, + votes: CompositeConsensusVotes, + ) -> Result, CorruptStorageError>; +} + +use crate::UniqueMonotonicIdentifier; + +/// A trait allowing access to a storage layer for electoral sytem runners. +// TODO: rename +pub trait RunnerStorageAccessTrait { + type ElectoralSystemRunner: ElectoralSystemRunner; + + fn electoral_settings_for_election( + unique_monotonic_identifier: UniqueMonotonicIdentifier, + ) -> Result< + ::ElectoralSettings, + CorruptStorageError, + >; + fn election_properties( + election_identifier: CompositeElectionIdentifierOf, + ) -> Result< + ::ElectionProperties, + CorruptStorageError, + >; + fn election_state( + unique_monotonic_identifier: UniqueMonotonicIdentifier, + ) -> Result< + ::ElectionState, + CorruptStorageError, + >; + + /// Sets a new `state` value for the election. This will invalid the current Consensus, and + /// thereby force it to be recalculated, when `check_consensus` is next called. We do this + /// to ensure that in situations where `check_consensus` depends on the `state` that we will + /// correctly recalculate the consensus if needed. + fn set_election_state( + unique_monotonic_identifier: UniqueMonotonicIdentifier, + state: ::ElectionState, + ) -> Result<(), CorruptStorageError>; + + // Clear the votes of a particular election + fn clear_election_votes(unique_monotonic_identifier: UniqueMonotonicIdentifier); + + fn delete_election( + composite_election_identifier: CompositeElectionIdentifierOf, + ); + /// This will change the `ElectionIdentifierExtra` value of the election, and allows you to + /// optionally change the properties. Note the `extra` must be strictly greater than the + /// previous value of this election, this function will return `Err` if it is not. This + /// ensures that all `Self::ElectoralSystemRunner::ElectionIdentifierExtra` ever used by a + /// particular election are unique. The purpose of this function to in effect allow the + /// deletion and recreation of an election so you can change its `Properties`, while + /// efficiently transferring the existing election's votes to the new election. The only + /// difference is that here the elections `Settings` will not be updated to the latest. This + /// could create a problem if you never delete elections, as old `Settings` values will be + /// stored until any elections referencing them are deleted. Any in-flight authority votes + /// will be invalidated by this. + fn refresh_election( + election_identifier: CompositeElectionIdentifierOf, + new_extra: ::ElectionIdentifierExtra, + properties: ::ElectionProperties, + ) -> Result<(), CorruptStorageError>; + + /// This returns the current consensus which will always be up to date with the latest + /// votes/state. This also returns information about the difference in the consensus between + /// the last call to `check_consensus`. + fn check_election_consensus( + election_identifier: CompositeElectionIdentifierOf, + ) -> Result< + ConsensusStatus<::Consensus>, + CorruptStorageError, + >; + + fn unsynchronised_settings() -> Result< + ::ElectoralUnsynchronisedSettings, + CorruptStorageError, + >; + fn unsynchronised_state() -> Result< + ::ElectoralUnsynchronisedState, + CorruptStorageError, + >; + fn unsynchronised_state_map( + key: &::ElectoralUnsynchronisedStateMapKey, + ) -> Option< + ::ElectoralUnsynchronisedStateMapValue, + >; + + fn new_election( + extra: ::ElectionIdentifierExtra, + properties: ::ElectionProperties, + state: ::ElectionState, + ) -> Result, CorruptStorageError>; + + fn set_unsynchronised_state( + unsynchronised_state: ::ElectoralUnsynchronisedState, + ); + + /// Inserts or removes a value from the unsynchronised state map of the electoral system. + fn set_unsynchronised_state_map( + key: ::ElectoralUnsynchronisedStateMapKey, + value: Option< + ::ElectoralUnsynchronisedStateMapValue, + >, + ); + + /// Allows you to mutate the unsynchronised state. This is more efficient than a read + /// (`unsynchronised_state`) and then a write (`set_unsynchronised_state`) in the case of + /// composite ElectoralSystems, as a write from one of the sub-ElectoralSystems internally + /// will require an additional read. Therefore this function should be preferred. + fn mutate_unsynchronised_state< + T, + F: for<'a> FnOnce( + &Self, + &'a ::ElectoralUnsynchronisedState, + ) -> Result, + >( + &self, + f: F, + ) -> Result { + let mut unsynchronised_state = Self::unsynchronised_state()?; + let t = f(self, &mut unsynchronised_state)?; + Self::set_unsynchronised_state(unsynchronised_state); + Ok(t) + } +} diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/blockchain/delta_based_ingress.rs b/state-chain/pallets/cf-elections/src/electoral_systems/blockchain/delta_based_ingress.rs index d92516010c..2868f2391a 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/blockchain/delta_based_ingress.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/blockchain/delta_based_ingress.rs @@ -55,23 +55,22 @@ where ::Account: Ord, ValidatorId: Member + Parameter + Ord + MaybeSerializeDeserialize, { - pub fn open_channel>( + pub fn open_channel + 'static>( election_identifiers: Vec< ElectionIdentifier<::ElectionIdentifierExtra>, >, - electoral_access: &mut ElectoralAccess, channel: Sink::Account, asset: Sink::Asset, close_block: Sink::BlockNumber, ) -> Result<(), CorruptStorageError> { let channel_details = ( OpenChannelDetails { asset, close_block }, - electoral_access.unsynchronised_state_map(&(channel.clone(), asset))?.unwrap_or( + ElectoralAccess::unsynchronised_state_map(&(channel.clone(), asset))?.unwrap_or( ChannelTotalIngressed { block_number: Zero::zero(), amount: Zero::zero() }, ), ); if let Some(election_identifier) = election_identifiers.last() { - let mut election_access = electoral_access.election_mut(*election_identifier)?; + let mut election_access = ElectoralAccess::election_mut(*election_identifier); let mut channels = election_access.properties()?; if channels.len() < MAXIMUM_CHANNELS_PER_ELECTION as usize { channels.insert(channel, channel_details); @@ -86,7 +85,7 @@ where } } - electoral_access.new_election( + ElectoralAccess::new_election( Default::default(), /* We use the lowest value, so we can refresh the elections the * maximum number of times */ [(channel, channel_details)].into_iter().collect(), @@ -139,7 +138,6 @@ where type OnFinalizeReturn = (); fn is_vote_desired>( - _election_identifier_with_extra: ElectionIdentifier, _election_access: &ElectionAccess, _current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, ) -> Result { @@ -168,14 +166,13 @@ where Ok(()) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, chain_tracking: &Self::OnFinalizeContext, ) -> Result { for election_identifier in election_identifiers { let (mut channels, mut pending_ingress_totals, option_consensus) = { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); ( election_access.properties()?, election_access.state()?, @@ -220,11 +217,13 @@ where }; if let Some(ingress_total) = option_ingress_total_before_chain_tracking { - let previous_amount = electoral_access - .unsynchronised_state_map(&(account.clone(), details.asset))? - .map_or(Zero::zero(), |previous_total_ingressed| { - previous_total_ingressed.amount - }); + let previous_amount = ElectoralAccess::unsynchronised_state_map(&( + account.clone(), + details.asset, + ))? + .map_or(Zero::zero(), |previous_total_ingressed| { + previous_total_ingressed.amount + }); match previous_amount.cmp(&ingress_total.amount) { Ordering::Less => { Sink::on_ingress( @@ -234,10 +233,10 @@ where ingress_total.block_number, (), ); - electoral_access.set_unsynchronised_state_map( + ElectoralAccess::set_unsynchronised_state_map( (account.clone(), details.asset), Some(ingress_total), - )?; + ); }, Ordering::Greater => { Sink::on_ingress_reverted( @@ -261,7 +260,7 @@ where } } - let mut election_access = electoral_access.election_mut(election_identifier)?; + let mut election_access = ElectoralAccess::election_mut(election_identifier); if !closed_channels.is_empty() { for closed_channel in closed_channels { pending_ingress_totals.remove(&closed_channel); @@ -291,7 +290,6 @@ where } fn check_consensus>( - _election_identifier: ElectionIdentifier, election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, consensus_votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/composite.rs b/state-chain/pallets/cf-elections/src/electoral_systems/composite.rs index 49c830e30e..8d7d3fdc7f 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/composite.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/composite.rs @@ -1,31 +1,12 @@ -use crate::electoral_system::{ElectoralSystem, ElectoralWriteAccess}; - /// Allows the composition of multiple ElectoralSystems while allowing the ability to configure the /// `on_finalize` behaviour without exposing the internal composite types. -pub struct Composite> { - _phantom: core::marker::PhantomData<(T, ValidatorId, H)>, -} - -pub struct DefaultHooks { - _phantom: core::marker::PhantomData, -} - -pub trait Translator { - type ElectoralSystem: ElectoralSystem; - type ElectionAccess<'a>: ElectoralWriteAccess - where - Self: 'a, - GenericElectoralAccess: 'a; - - fn translate_electoral_access<'a>( - &'a self, - generic_electoral_access: &'a mut GenericElectoralAccess, - ) -> Self::ElectionAccess<'a>; +pub struct CompositeRunner { + _phantom: core::marker::PhantomData<(T, ValidatorId, StorageAccess, H)>, } /// The access wrappers need to impl the access traits once for each variant, /// these tags ensure these trait impls don't overlap. -mod tags { +pub mod tags { pub struct A; pub struct B; pub struct C; @@ -45,9 +26,7 @@ macro_rules! generate_electoral_system_tuple_impls { #[allow(unused_variables)] pub mod $module { use super::{ - Translator, - Composite, - DefaultHooks, + CompositeRunner, tags, }; @@ -55,17 +34,17 @@ macro_rules! generate_electoral_system_tuple_impls { CorruptStorageError, electoral_system::{ ElectoralSystem, - ConsensusStatus, ConsensusVote, ElectionReadAccess, ElectionWriteAccess, ElectoralReadAccess, ElectoralWriteAccess, - AuthorityVoteOf, - VotePropertiesOf, - ElectionIdentifierOf, ConsensusVotes, + ElectionIdentifierOf, + ConsensusStatus, }, + electoral_system_runner::{ElectoralSystemRunner, CompositeAuthorityVoteOf, RunnerStorageAccessTrait, + CompositeVotePropertiesOf, CompositeConsensusVotes, CompositeElectionIdentifierOf, CompositeConsensusVote}, vote_storage::{ AuthorityVote, VoteStorage @@ -78,44 +57,13 @@ macro_rules! generate_electoral_system_tuple_impls { use codec::{Encode, Decode}; use scale_info::TypeInfo; - use core::borrow::Borrow; use sp_std::vec::Vec; - /// This trait specifies the behaviour of the composite's `ElectoralSystem::on_finalize` without that code being exposed to the internals of the composite by using the Translator trait to obtain ElectoralAccess objects that abstract those details. + /// This trait specifies the behaviour of the composite's `ElectoralSystem::on_finalize` function. pub trait Hooks<$($electoral_system: ElectoralSystem,)*> { - /// The `OnFinalizeContext` of the composite's ElectoralSystem implementation. - type OnFinalizeContext; - - /// The 'OnFinalizeReturn' of the composite's ElectoralSystem implementation. - type OnFinalizeReturn; - - fn on_finalize),*>( - generic_electoral_access: &mut GenericElectoralAccess, - electoral_access_translators: ($($electoral_system_alt_name_0,)*), + fn on_finalize( election_identifiers: ($(Vec>,)*), - context: &Self::OnFinalizeContext, - ) -> Result; - } - - impl,)*> Hooks<$($electoral_system,)*> for DefaultHooks { - type OnFinalizeContext = OnFinalizeContext; - type OnFinalizeReturn = (); - - fn on_finalize),*>( - generic_electoral_access: &mut GenericElectoralAccess, - electoral_access_translators: ($($electoral_system_alt_name_0,)*), - election_identifiers: ($(Vec>,)*), - context: &Self::OnFinalizeContext, - ) -> Result { - let ($($electoral_system,)*) = electoral_access_translators; - let ($($electoral_system_alt_name_0,)*) = election_identifiers; - - $( - $electoral_system::on_finalize(&mut $electoral_system.translate_electoral_access(generic_electoral_access), $electoral_system_alt_name_0, &context)?; - )* - - Ok(()) - } + ) -> Result<(), CorruptStorageError>; } #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encode, Decode, TypeInfo)] @@ -143,13 +91,13 @@ macro_rules! generate_electoral_system_tuple_impls { $($electoral_system($electoral_system),)* } - impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static> Composite<($($electoral_system,)*), ValidatorId, H> { + impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, StorageAccess: RunnerStorageAccessTrait + 'static, H: Hooks<$($electoral_system),*> + 'static> CompositeRunner<($($electoral_system,)*), ValidatorId, StorageAccess, H> { pub fn with_identifiers FnOnce( ($( Vec>, )*) ) -> R>( - election_identifiers: Vec>, + election_identifiers: Vec>, f: F, ) -> R { $(let mut $electoral_system_alt_name_0 = Vec::new();)* @@ -166,21 +114,9 @@ macro_rules! generate_electoral_system_tuple_impls { $($electoral_system_alt_name_0,)* )) } - - pub fn with_access_translators FnOnce( - ($( - ElectoralAccessTranslator, - )*) - ) -> R>( - f: F, - ) -> R { - f(( - $(ElectoralAccessTranslator::::new(),)* - )) - } } - impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static> ElectoralSystem for Composite<($($electoral_system,)*), ValidatorId, H> { + impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, StorageAccess: RunnerStorageAccessTrait + 'static, H: Hooks<$($electoral_system),*> + 'static> ElectoralSystemRunner for CompositeRunner<($($electoral_system,)*), ValidatorId, StorageAccess, H> { type ValidatorId = ValidatorId; type ElectoralUnsynchronisedState = ($(<$electoral_system as ElectoralSystem>::ElectoralUnsynchronisedState,)*); type ElectoralUnsynchronisedStateMapKey = CompositeElectoralUnsynchronisedStateMapKey<$(<$electoral_system as ElectoralSystem>::ElectoralUnsynchronisedStateMapKey,)*>; @@ -194,22 +130,17 @@ macro_rules! generate_electoral_system_tuple_impls { type Vote = ($(<$electoral_system as ElectoralSystem>::Vote,)*); type Consensus = CompositeConsensus<$(<$electoral_system as ElectoralSystem>::Consensus,)*>; - type OnFinalizeContext = H::OnFinalizeContext; - type OnFinalizeReturn = H::OnFinalizeReturn; - - fn is_vote_desired>( + fn is_vote_desired( election_identifier: ElectionIdentifier, - election_access: &ElectionAccess, current_vote: Option<( - VotePropertiesOf, - AuthorityVoteOf, + CompositeVotePropertiesOf, + CompositeAuthorityVoteOf, )>, ) -> Result { match *election_identifier.extra() { $(CompositeElectionIdentifierExtra::$electoral_system(extra) => { <$electoral_system as ElectoralSystem>::is_vote_desired( - election_identifier.with_extra(extra), - &CompositeElectionAccess::::new(election_access), + &DerivedElectionAccess::::new(election_identifier.with_extra(extra)), current_vote.map(|(properties, vote)| { Ok(( match properties { @@ -229,7 +160,7 @@ macro_rules! generate_electoral_system_tuple_impls { } fn is_vote_needed( - (current_vote_properties, current_partial_vote, current_authority_vote): (VotePropertiesOf, ::PartialVote, AuthorityVoteOf), + (current_vote_properties, current_partial_vote, current_authority_vote): (CompositeVotePropertiesOf, ::PartialVote, CompositeAuthorityVoteOf), (proposed_partial_vote, proposed_vote): (::PartialVote, ::Vote), ) -> bool { match (current_vote_properties, current_partial_vote, current_authority_vote, proposed_partial_vote, proposed_vote) { @@ -266,11 +197,11 @@ macro_rules! generate_electoral_system_tuple_impls { fn generate_vote_properties( election_identifier: ElectionIdentifier, previous_vote: Option<( - VotePropertiesOf, - AuthorityVoteOf, + CompositeVotePropertiesOf, + CompositeAuthorityVoteOf, )>, partial_vote: &::PartialVote, - ) -> Result, CorruptStorageError> { + ) -> Result, CorruptStorageError> { match (*election_identifier.extra(), partial_vote) { $((CompositeElectionIdentifierExtra::$electoral_system(extra), CompositePartialVote::$electoral_system(partial_vote)) => { <$electoral_system as ElectoralSystem>::generate_vote_properties( @@ -295,34 +226,25 @@ macro_rules! generate_electoral_system_tuple_impls { } } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize( election_identifiers: Vec>, - context: &Self::OnFinalizeContext, - ) -> Result { - Self::with_access_translators(|access_translators| { - Self::with_identifiers(election_identifiers, |election_identifiers| { - H::on_finalize( - electoral_access, - access_translators, - election_identifiers, - context - ) - }) + ) -> Result<(), CorruptStorageError> { + Self::with_identifiers(election_identifiers, |election_identifiers| { + H::on_finalize( + election_identifiers, + ) }) } - fn check_consensus>( + fn check_consensus( election_identifier: ElectionIdentifier, - election_access: &ElectionAccess, previous_consensus: Option<&Self::Consensus>, - consensus_votes: ConsensusVotes, + consensus_votes: CompositeConsensusVotes, ) -> Result, CorruptStorageError> { Ok(match *election_identifier.extra() { $(CompositeElectionIdentifierExtra::$electoral_system(extra) => { <$electoral_system as ElectoralSystem>::check_consensus( - election_identifier.with_extra(extra), - &CompositeElectionAccess::::new(election_access), + &DerivedElectionAccess::::new(election_identifier.with_extra(extra)), previous_consensus.map(|previous_consensus| { match previous_consensus { CompositeConsensus::$electoral_system(previous_consensus) => Ok(previous_consensus), @@ -330,7 +252,7 @@ macro_rules! generate_electoral_system_tuple_impls { } }).transpose()?, ConsensusVotes { - votes: consensus_votes.votes.into_iter().map(|ConsensusVote { vote, validator_id }| { + votes: consensus_votes.votes.into_iter().map(|CompositeConsensusVote { vote, validator_id }| { if let Some((properties, vote)) = vote { match (properties, vote) { ( @@ -357,41 +279,22 @@ macro_rules! generate_electoral_system_tuple_impls { } } - pub struct CompositeElectionAccess { - ea: BorrowEA, - _phantom: core::marker::PhantomData<(Tag, EA)>, - } - impl,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*>, BorrowEA: Borrow, EA: ElectionReadAccess>> CompositeElectionAccess { - fn new(ea: BorrowEA) -> Self { - Self { - ea, - _phantom: Default::default(), - } - } - } - pub struct CompositeElectoralAccess<'a, Tag, EA> { - ea: &'a mut EA, - _phantom: core::marker::PhantomData, - } - impl<'a, Tag, EA> CompositeElectoralAccess<'a, Tag, EA> { - fn new(ea: &'a mut EA) -> Self { - Self { - ea, - _phantom: Default::default(), - } - } + pub struct DerivedElectionAccess { + id: ElectionIdentifierOf, + _phantom: core::marker::PhantomData<(Tag, ES, StorageAccess)>, } - pub struct ElectoralAccessTranslator { - _phantom: core::marker::PhantomData<(Tag, EA)>, - } - impl ElectoralAccessTranslator { - fn new() -> Self { + impl DerivedElectionAccess { + fn new(id: ElectionIdentifierOf) -> Self { Self { + id, _phantom: Default::default(), } } } + pub struct DerivedElectoralAccess { + _phantom: core::marker::PhantomData<(Tag, ES, StorageAccess)>, + } // This macro solves the problem of taking a repeating argument and generating the // product of the arguments elements. As we need to be able to refer to every element @@ -401,15 +304,16 @@ macro_rules! generate_electoral_system_tuple_impls { }; (@ $($previous:ident,)*;: $($electoral_system:ident,)*) => {}; (@ $($previous:ident,)*; $current:ident, $($remaining:ident,)*: $($electoral_system:ident,)*) => { - impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, BorrowEA: Borrow, EA: ElectionReadAccess>> ElectionReadAccess for CompositeElectionAccess { + + impl<'a, $($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, StorageAccess: RunnerStorageAccessTrait> + 'static> ElectionReadAccess for DerivedElectionAccess { type ElectoralSystem = $current; fn settings(&self) -> Result<$current::ElectoralSettings, CorruptStorageError> { - let ($($previous,)* settings, $($remaining,)*) =self.ea.borrow().settings()?; + let ($($previous,)* settings, $($remaining,)*) = StorageAccess::electoral_settings_for_election(*self.id.unique_monotonic())?; Ok(settings) } fn properties(&self) -> Result<$current::ElectionProperties, CorruptStorageError> { - match self.ea.borrow().properties()? { + match StorageAccess::election_properties(self.id.with_extra(CompositeElectionIdentifierExtra::$current(*self.id.extra())))? { CompositeElectionProperties::$current(properties) => { Ok(properties) }, @@ -417,7 +321,7 @@ macro_rules! generate_electoral_system_tuple_impls { } } fn state(&self) -> Result<$current::ElectionState, CorruptStorageError> { - match self.ea.borrow().state()? { + match StorageAccess::election_state(*self.id.unique_monotonic())? { CompositeElectionState::$current(state) => { Ok(state) }, @@ -425,81 +329,73 @@ macro_rules! generate_electoral_system_tuple_impls { } } - #[cfg(test)] - fn election_identifier(&self) -> Result, CorruptStorageError> { - let composite_identifier = self.ea.borrow().election_identifier()?; - let extra = match composite_identifier.extra() { - CompositeElectionIdentifierExtra::$current(extra) => Ok(extra), - _ => Err(CorruptStorageError::new()), - }?; - Ok(composite_identifier.with_extra(*extra)) + fn election_identifier(&self) -> ElectionIdentifierOf { + self.id } } - impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, EA: ElectionWriteAccess>> ElectionWriteAccess for CompositeElectionAccess { - fn set_state(&mut self, state: $current::ElectionState) -> Result<(), CorruptStorageError> { - self.ea.set_state(CompositeElectionState::$current(state)) + + impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, StorageAccess: RunnerStorageAccessTrait> + 'static> ElectionWriteAccess for DerivedElectionAccess { + fn set_state(&self, state: $current::ElectionState) -> Result<(), CorruptStorageError> { + StorageAccess::set_election_state(*self.id.unique_monotonic(), CompositeElectionState::$current(state)) } - fn clear_votes(&mut self) { - self.ea.clear_votes() + fn clear_votes(&self) { + StorageAccess::clear_election_votes(*self.id.unique_monotonic()); } fn delete(self) { - self.ea.delete(); + StorageAccess::delete_election(self.id.with_extra(CompositeElectionIdentifierExtra::$current(*self.id.extra()))); } fn refresh( &mut self, - extra: $current::ElectionIdentifierExtra, + new_extra: $current::ElectionIdentifierExtra, properties: $current::ElectionProperties, ) -> Result<(), CorruptStorageError> { - self.ea.refresh( - CompositeElectionIdentifierExtra::$current(extra), + StorageAccess::refresh_election( + // The current election id + extra that we want to refresh. + self.id.with_extra(CompositeElectionIdentifierExtra::$current(*self.id.extra())), + // The new extra we want to use. + CompositeElectionIdentifierExtra::$current(new_extra), CompositeElectionProperties::$current(properties), - ) + )?; + self.id = self.id.with_extra(new_extra); + Ok(()) } fn check_consensus( - &mut self, + &self, ) -> Result, CorruptStorageError> { - self.ea.check_consensus().and_then(|consensus_status| { + StorageAccess::check_election_consensus(self.id.with_extra(CompositeElectionIdentifierExtra::$current(*self.id.extra()))).and_then(|consensus_status| { consensus_status.try_map(|consensus| { match consensus { CompositeConsensus::$current(consensus) => Ok(consensus), _ => Err(CorruptStorageError::new()), } }) - }) } } - impl<'a, $($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, EA: ElectoralReadAccess>> ElectoralReadAccess for CompositeElectoralAccess<'a, tags::$current, EA> { + + impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, StorageAccess: RunnerStorageAccessTrait> + 'static> ElectoralReadAccess for DerivedElectoralAccess { type ElectoralSystem = $current; - type ElectionReadAccess<'b> = CompositeElectionAccess::ElectionReadAccess<'b>, ::ElectionReadAccess<'b>> - where - Self: 'b; + type ElectionReadAccess = DerivedElectionAccess; fn election( - &self, id: ElectionIdentifier<<$current as ElectoralSystem>::ElectionIdentifierExtra>, - ) -> Result, CorruptStorageError> { - self.ea.election(id.with_extra(CompositeElectionIdentifierExtra::$current(*id.extra()))).map(|election_access| { - CompositeElectionAccess::::ElectionReadAccess<'_>>::new(election_access) - }) + ) -> Self::ElectionReadAccess { + DerivedElectionAccess::::new(id) } fn unsynchronised_settings( - &self, ) -> Result<$current::ElectoralUnsynchronisedSettings, CorruptStorageError> { - let ($($previous,)* unsynchronised_settings, $($remaining,)*) = self.ea.unsynchronised_settings()?; + let ($($previous,)* unsynchronised_settings, $($remaining,)*) = StorageAccess::unsynchronised_settings()?; Ok(unsynchronised_settings) } fn unsynchronised_state( - &self, ) -> Result<$current::ElectoralUnsynchronisedState, CorruptStorageError> { - let ($($previous,)* unsynchronised_state, $($remaining,)*) = self.ea.unsynchronised_state()?; + let ($($previous,)* unsynchronised_state, $($remaining,)*) = StorageAccess::unsynchronised_state()?; Ok(unsynchronised_state) } fn unsynchronised_state_map( - &self, key: &$current::ElectoralUnsynchronisedStateMapKey, ) -> Result, CorruptStorageError> { - match self.ea.unsynchronised_state_map(&CompositeElectoralUnsynchronisedStateMapKey::$current(key.clone()))? { + match StorageAccess::unsynchronised_state_map(&CompositeElectoralUnsynchronisedStateMapKey::$current(key.clone())) { Some(CompositeElectoralUnsynchronisedStateMapValue::$current(value)) => Ok(Some(value)), None => Ok(None), _ => Err(CorruptStorageError::new()), @@ -507,76 +403,57 @@ macro_rules! generate_electoral_system_tuple_impls { } } - impl<'a, $($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, EA: ElectoralWriteAccess>> ElectoralWriteAccess for CompositeElectoralAccess<'a, tags::$current, EA> { - type ElectionWriteAccess<'b> = CompositeElectionAccess::ElectionWriteAccess<'b>, ::ElectionWriteAccess<'b>> - where - Self: 'b; + impl<'a, $($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, StorageAccess: RunnerStorageAccessTrait> + 'static> ElectoralWriteAccess for DerivedElectoralAccess { + type ElectionWriteAccess = DerivedElectionAccess; fn new_election( - &mut self, extra: $current::ElectionIdentifierExtra, properties: $current::ElectionProperties, state: $current::ElectionState, - ) -> Result, CorruptStorageError> { - self.ea.new_election(CompositeElectionIdentifierExtra::$current(extra), CompositeElectionProperties::$current(properties), CompositeElectionState::$current(state)).map(|election_access| { - CompositeElectionAccess::new(election_access) - }) + ) -> Result { + let election_identifier = StorageAccess::new_election(CompositeElectionIdentifierExtra::$current(extra), CompositeElectionProperties::$current(properties), CompositeElectionState::$current(state))?; + Ok(Self::election_mut(election_identifier.with_extra(extra))) } + fn election_mut( - &mut self, id: ElectionIdentifier<$current::ElectionIdentifierExtra>, - ) -> Result, CorruptStorageError> { - self.ea.election_mut(id.with_extra(CompositeElectionIdentifierExtra::$current(*id.extra()))).map(|election_access| { - CompositeElectionAccess::new(election_access) - }) + ) -> Self::ElectionWriteAccess { + Self::ElectionWriteAccess::new(id) } + fn set_unsynchronised_state( - &mut self, unsynchronised_state: $current::ElectoralUnsynchronisedState, ) -> Result<(), CorruptStorageError> { - let ($($previous,)* _, $($remaining,)*) = self.ea.unsynchronised_state()?; - self.ea.set_unsynchronised_state(($($previous,)* unsynchronised_state, $($remaining,)*)) + let ($($previous,)* _, $($remaining,)*) = StorageAccess::unsynchronised_state()?; + StorageAccess::set_unsynchronised_state(($($previous,)* unsynchronised_state, $($remaining,)*)); + Ok(()) } fn set_unsynchronised_state_map( - &mut self, key: $current::ElectoralUnsynchronisedStateMapKey, value: Option<$current::ElectoralUnsynchronisedStateMapValue>, - ) -> Result<(), CorruptStorageError> { - self.ea.set_unsynchronised_state_map( + ) { + StorageAccess::set_unsynchronised_state_map( CompositeElectoralUnsynchronisedStateMapKey::$current(key), value.map(CompositeElectoralUnsynchronisedStateMapValue::$current), - ) + ); } fn mutate_unsynchronised_state< T, F: for<'b> FnOnce( - &mut Self, &'b mut $current::ElectoralUnsynchronisedState, ) -> Result, >( - &mut self, f: F, ) -> Result { - let ($($previous,)* mut unsynchronised_state, $($remaining,)*) = self.ea.unsynchronised_state()?; - let t = f(self, &mut unsynchronised_state)?; - self.ea.set_unsynchronised_state(($($previous,)* unsynchronised_state, $($remaining,)*))?; + let ($($previous,)* mut unsynchronised_state, $($remaining,)*) = StorageAccess::unsynchronised_state()?; + let t = f( &mut unsynchronised_state)?; + StorageAccess::set_unsynchronised_state(($($previous,)* unsynchronised_state, $($remaining,)*)); Ok(t) } } - impl<$($electoral_system: ElectoralSystem,)* ValidatorId: MaybeSerializeDeserialize + Parameter + Member, H: Hooks<$($electoral_system),*> + 'static, EA: ElectoralWriteAccess>> Translator for ElectoralAccessTranslator> { - type ElectoralSystem = $current; - type ElectionAccess<'a> = CompositeElectoralAccess<'a, tags::$current, EA> - where - Self: 'a, EA: 'a; - - fn translate_electoral_access<'a>(&'a self, generic_electoral_access: &'a mut EA) -> Self::ElectionAccess<'a> { - Self::ElectionAccess::<'a>::new(generic_electoral_access) - } - } - generate_electoral_system_tuple_impls!(@ $($previous,)* $current,; $($remaining,)*: $($electoral_system,)*); }; } diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/egress_success.rs b/state-chain/pallets/cf-elections/src/electoral_systems/egress_success.rs index bd8f07989d..4e212c4756 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/egress_success.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/egress_success.rs @@ -31,11 +31,12 @@ impl< ValidatorId: Member + Parameter + Ord + MaybeSerializeDeserialize, > EgressSuccess { - pub fn watch_for_egress>( - electoral_access: &mut ElectoralAccess, + pub fn watch_for_egress< + ElectoralAccess: ElectoralWriteAccess + 'static, + >( identifier: Identifier, ) -> Result<(), CorruptStorageError> { - electoral_access.new_election((), identifier, ())?; + ElectoralAccess::new_election((), identifier, ())?; Ok(()) } } @@ -72,20 +73,18 @@ impl< } fn is_vote_desired>( - _election_identifier: ElectionIdentifierOf, _election_access: &ElectionAccess, _current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, ) -> Result { Ok(true) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, _context: &Self::OnFinalizeContext, ) -> Result { for election_identifier in election_identifiers { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); if let Some(egress_data) = election_access.check_consensus()?.has_consensus() { let identifier = election_access.properties()?; election_access.delete(); @@ -97,7 +96,6 @@ impl< } fn check_consensus>( - _election_identifier: ElectionIdentifierOf, _election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, consensus_votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/liveness.rs b/state-chain/pallets/cf-elections/src/electoral_systems/liveness.rs index 120c0ec808..7d71b0d690 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/liveness.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/liveness.rs @@ -76,15 +76,13 @@ impl< } fn is_vote_desired>( - _election_identifier: ElectionIdentifierOf, _election_access: &ElectionAccess, _current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, ) -> Result { Ok(true) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, (current_sc_block, current_chain_tracking_number): &Self::OnFinalizeContext, ) -> Result { @@ -108,7 +106,7 @@ impl< .at_most_one() .map_err(|_| CorruptStorageError::new())? { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); // Is the block the election started at + the duration we want the check to stay open // for less than or equal to the current SC block? @@ -121,14 +119,14 @@ impl< } } election_access.delete(); - electoral_access.new_election( + ElectoralAccess::new_election( (), block_number_to_check(*current_chain_tracking_number), *current_sc_block, )?; } } else { - electoral_access.new_election( + ElectoralAccess::new_election( (), block_number_to_check(*current_chain_tracking_number), *current_sc_block, @@ -139,7 +137,6 @@ impl< } fn check_consensus>( - _election_identifier: ElectionIdentifierOf, _election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, consensus_votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/mock.rs b/state-chain/pallets/cf-elections/src/electoral_systems/mock.rs index d613a5c15e..f87dcd9a89 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/mock.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/mock.rs @@ -1,14 +1,14 @@ use crate::{ - electoral_system::{ - AuthorityVoteOf, ConsensusStatus, ConsensusVotes, ElectionReadAccess, ElectionWriteAccess, - ElectoralSystem, ElectoralWriteAccess, VotePropertiesOf, - }, + electoral_system::ConsensusStatus, + electoral_system_runner::{CompositeConsensusVotes, RunnerStorageAccessTrait}, mock::Test, vote_storage::{self, VoteStorage}, - CorruptStorageError, ElectionIdentifier, UniqueMonotonicIdentifier, + CompositeAuthorityVoteOf, CompositeElectionIdentifierOf, CompositeVotePropertiesOf, + CorruptStorageError, ElectoralSystemRunner, RunnerStorageAccess, UniqueMonotonicIdentifier, }; use cf_primitives::AuthorityCount; use cf_traits::Chainflip; +use frame_support::instances::Instance1; use sp_std::vec::Vec; use std::{cell::RefCell, collections::BTreeMap}; @@ -32,7 +32,7 @@ thread_local! { /// - `consensus_status` is set to `None` by default. /// /// If assume_consensus is set to `true`, then the consensus value will be the number of votes. -pub struct MockElectoralSystem; +pub struct MockElectoralSystemRunner; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum BehaviourUpdate { @@ -65,7 +65,7 @@ impl BehaviourUpdate { } } -impl MockElectoralSystem { +impl MockElectoralSystemRunner { pub fn vote_desired() -> bool { VOTE_DESIRED.with(|v| *v.borrow()) } @@ -115,7 +115,7 @@ impl MockElectoralSystem { } } -impl ElectoralSystem for MockElectoralSystem { +impl ElectoralSystemRunner for MockElectoralSystemRunner { type ValidatorId = ::ValidatorId; type ElectoralUnsynchronisedState = (); type ElectoralUnsynchronisedStateMapKey = (); @@ -130,40 +130,34 @@ impl ElectoralSystem for MockElectoralSystem { type Vote = vote_storage::individual::Individual<(), vote_storage::individual::shared::Shared<()>>; type Consensus = AuthorityCount; - type OnFinalizeContext = (); - type OnFinalizeReturn = (); fn generate_vote_properties( - _election_identifier: ElectionIdentifier, - _previous_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, + _election_identifier: CompositeElectionIdentifierOf, + _previous_vote: Option<(CompositeVotePropertiesOf, CompositeAuthorityVoteOf)>, _vote: &::PartialVote, ) -> Result<(), CorruptStorageError> { Ok(()) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, - election_identifiers: Vec>, - _context: &Self::OnFinalizeContext, - ) -> Result { + fn on_finalize( + election_identifiers: Vec>, + ) -> Result<(), CorruptStorageError> { for id in election_identifiers { - // Read the current consensus status and save it. - let mut election = electoral_access.election_mut(id)?; - let consensus = election.check_consensus()?; + let consensus = + RunnerStorageAccess::::check_election_consensus(id).unwrap(); Self::set_consensus_status(*id.unique_monotonic(), consensus.clone()); if consensus.has_consensus().is_some() && Self::should_delete_on_finalize_consensus() { - election.delete(); + RunnerStorageAccess::::delete_election(id); } } Ok(()) } - fn check_consensus>( - _election_identifier: ElectionIdentifier, - _election_access: &ElectionAccess, + fn check_consensus( + _election_identifier: CompositeElectionIdentifierOf, _previous_consensus: Option<&Self::Consensus>, - consensus_votes: ConsensusVotes, + consensus_votes: CompositeConsensusVotes, ) -> Result, CorruptStorageError> { Ok(if Self::should_assume_consensus() { Some(consensus_votes.active_votes().len() as AuthorityCount) @@ -172,19 +166,18 @@ impl ElectoralSystem for MockElectoralSystem { }) } - fn is_vote_desired>( - _election_identifier_with_extra: crate::electoral_system::ElectionIdentifierOf, - _election_access: &ElectionAccess, - _current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, + fn is_vote_desired( + _election_identifier: CompositeElectionIdentifierOf, + _current_vote: Option<(CompositeVotePropertiesOf, CompositeAuthorityVoteOf)>, ) -> Result { Ok(Self::vote_desired()) } fn is_vote_needed( _current_vote: ( - VotePropertiesOf, + CompositeVotePropertiesOf, ::PartialVote, - AuthorityVoteOf, + CompositeAuthorityVoteOf, ), _proposed_vote: ( ::PartialVote, 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 4f28f85440..0ea98d014f 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/mocks.rs @@ -1,6 +1,11 @@ -use crate::electoral_system::{ - ConsensusStatus, ConsensusVotes, ElectionIdentifierOf, ElectoralReadAccess, ElectoralSystem, - ElectoralWriteAccess, +use std::collections::BTreeMap; + +use crate::{ + electoral_system::{ + ConsensusStatus, ConsensusVotes, ElectionIdentifierOf, ElectoralReadAccess, + ElectoralSystem, ElectoralWriteAccess, + }, + UniqueMonotonicIdentifier, }; use frame_support::{CloneNoBound, DebugNoBound, EqNoBound, PartialEqNoBound}; @@ -37,7 +42,6 @@ where #[derive(CloneNoBound, DebugNoBound, PartialEqNoBound, EqNoBound)] pub struct TestContext { setup: TestSetup, - electoral_access: MockAccess, } impl TestSetup @@ -79,36 +83,47 @@ where // Useful for testing check_consensus since we already have an election. pub fn build_with_initial_election(self) -> TestContext { let setup = self.clone(); - let mut electoral_access = MockAccess::::new( - self.unsynchronised_state, - self.unsynchronised_settings, - self.electoral_settings, - ); + + // We need to clear the storage at every build so if there are multiple test contexts used + // within a single test they do not conflict. + MockStorageAccess::clear_storage(); let (election_identifier_extra, election_properties, election_state) = self.initial_election_state.unwrap_or_default(); - let election = electoral_access - .new_election(election_identifier_extra, election_properties, election_state) - .unwrap(); + // The ElectoralSettings are synchronised with an election, by election identifier in the + // actual implementation. Here we simplify by storing the settings in the electoral + // system, and upon the creation of new election, we store the ElectoralSettings that were + // in storage with the election directly. This duplicates the settings, but is fine for + // testing. + 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, + election_properties, + election_state, + ) + .unwrap(); // A new election should not have consensus at any authority count. assert_eq!(election.check_consensus(None, ConsensusVotes { votes: vec![] }).unwrap(), None); - TestContext { setup, electoral_access } + TestContext { setup } } // We may want to test initialisation of elections within on finalise, so *don't* want to // initialise an election in the utilities. pub fn build(self) -> TestContext { - TestContext { - setup: self.clone(), - electoral_access: MockAccess::::new( - self.unsynchronised_state, - self.unsynchronised_settings, - self.electoral_settings, - ), - } + // We need to clear the storage at every build so if there are multiple test contexts used + // within a single test they do not conflict. + MockStorageAccess::clear_storage(); + + MockStorageAccess::set_electoral_settings::(self.electoral_settings.clone()); + + TestContext { setup: self.clone() } } } @@ -129,10 +144,7 @@ impl TestContext { // Expect only one election. let current_election_id = self.only_election_id(); - let new_consensus = self - .electoral_access - .election(current_election_id) - .unwrap() + let new_consensus = MockAccess::::election(current_election_id) .check_consensus(None, consensus_votes) .unwrap(); @@ -157,7 +169,7 @@ impl TestContext { } pub fn all_election_ids(&self) -> Vec> { - self.electoral_access.election_identifiers() + MockStorageAccess::election_identifiers::() } /// Update the current consensus without processing any votes. @@ -166,27 +178,15 @@ impl TestContext { self.inner_force_consensus_update(id, new_consensus) } - pub fn access(&self) -> &MockAccess { - &self.electoral_access - } - - pub fn mut_access(&mut self) -> &mut MockAccess { - &mut self.electoral_access - } - #[track_caller] fn inner_force_consensus_update( self, election_id: ElectionIdentifierOf, new_consensus: ConsensusStatus, ) -> Self { - let mut electoral_access = self.electoral_access.clone(); - electoral_access - .election_mut(election_id) - .unwrap() - .set_consensus_status(new_consensus); + MockStorageAccess::set_consensus_status::(election_id, new_consensus); - Self { electoral_access, ..self } + self } /// Test the finalization of the election. @@ -199,18 +199,22 @@ impl TestContext { /// See [register_checks] and #[track_caller] pub fn test_on_finalize( - mut self, + self, on_finalize_context: &ES::OnFinalizeContext, - pre_finalize_checks: impl FnOnce(&MockAccess), + pre_finalize_checks: impl FnOnce(&ElectoralSystemState), post_finalize_checks: impl IntoIterator>, ) -> Self { - let pre_finalize = self.electoral_access.clone(); - // TODO: Move 'hook' static local checks into MockAccess so we can remove this. + let pre_finalize = ElectoralSystemState::::load_state(); + // TODO: Move 'hook' static local checks into ElectoralSystemState so we can remove this. pre_finalize_checks(&pre_finalize); - self.electoral_access.finalize_elections(on_finalize_context).unwrap(); + ES::on_finalize::>( + MockStorageAccess::election_identifiers::(), + on_finalize_context, + ) + .unwrap(); - let post_finalize = self.electoral_access.clone(); + let post_finalize = ElectoralSystemState::::load_state(); for check in post_finalize_checks { check.check(&pre_finalize, &post_finalize); } @@ -295,35 +299,53 @@ register_checks! { assert_eq!(pre_finalize, post_finalize); }, last_election_deleted(pre_finalize, post_finalize) { - let last_election_id = *pre_finalize.election_identifiers().last().expect("Expected an election before finalization."); + let last_election_id = pre_finalize.election_identifiers.last().expect("Expected an election before finalization"); assert!( - post_finalize.election(last_election_id).is_err(), - "Expected election {:?} to be deleted. Elections before: {:?}. After: {:?}", - last_election_id, - pre_finalize.election_identifiers(), - post_finalize.election_identifiers(), + !post_finalize.election_identifiers.contains(last_election_id), + "Last election should have been deleted.", ); }, election_id_incremented(pre_finalize, post_finalize) { assert_eq!( - pre_finalize.next_umi().next_identifier().unwrap(), - post_finalize.next_umi(), + pre_finalize.next_umi.next_identifier().unwrap(), + post_finalize.next_umi, "Expected the election id to be incremented.", ); }, all_elections_deleted(pre_finalize, post_finalize) { assert!( - !pre_finalize.election_identifiers().is_empty(), + !pre_finalize.election_identifiers.is_empty(), "Expected elections before finalization. This check makes no sense otherwise.", ); assert!( - post_finalize.election_identifiers().is_empty(), + post_finalize.election_identifiers.is_empty(), "Expected no elections after finalization.", ); }, } -type CheckFn = Box, &MockAccess)>; +#[derive(CloneNoBound, DebugNoBound, PartialEqNoBound, EqNoBound)] +pub struct ElectoralSystemState { + pub unsynchronised_state: ES::ElectoralUnsynchronisedState, + pub unsynchronised_state_map: BTreeMap, Option>>, + pub unsynchronised_settings: ES::ElectoralUnsynchronisedSettings, + pub election_identifiers: Vec>, + pub next_umi: UniqueMonotonicIdentifier, +} + +impl ElectoralSystemState { + pub fn load_state() -> Self { + Self { + unsynchronised_settings: MockStorageAccess::unsynchronised_settings::(), + unsynchronised_state: MockStorageAccess::unsynchronised_state::(), + unsynchronised_state_map: MockStorageAccess::raw_unsynchronised_state_map(), + election_identifiers: MockStorageAccess::election_identifiers::(), + next_umi: MockStorageAccess::next_umi(), + } + } +} + +type CheckFn = Box, &ElectoralSystemState)>; /// Checks that can be applied post-finalization. pub struct Check { @@ -331,11 +353,17 @@ pub struct Check { } impl Check { - pub fn new(check_fn: impl Fn(&MockAccess, &MockAccess) + 'static) -> Self { + pub fn new( + check_fn: impl Fn(&ElectoralSystemState, &ElectoralSystemState) + 'static, + ) -> Self { Self { check_fn: Box::new(check_fn) } } - pub fn check(&self, pre_finalize: &MockAccess, post_finalize: &MockAccess) { + pub fn check( + &self, + pre_finalize: &ElectoralSystemState, + post_finalize: &ElectoralSystemState, + ) { (self.check_fn)(pre_finalize, post_finalize) } } 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 74500c97f7..af32e582ee 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 @@ -5,55 +5,22 @@ use crate::{ }, CorruptStorageError, ElectionIdentifier, UniqueMonotonicIdentifier, }; -use codec::Encode; -use frame_support::{ - ensure, CloneNoBound, DebugNoBound, EqNoBound, PartialEqNoBound, StorageHasher, Twox64Concat, -}; +use codec::{Decode, Encode}; +use core::cell::RefCell; +use frame_support::{CloneNoBound, DebugNoBound, EqNoBound, PartialEqNoBound}; use std::collections::BTreeMap; -pub struct MockReadAccess<'es, ES: ElectoralSystem> { - election_identifier: ElectionIdentifierOf, - electoral_system: &'es MockAccess, -} -pub struct MockWriteAccess<'es, ES: ElectoralSystem> { +pub struct MockReadAccess { election_identifier: ElectionIdentifierOf, - electoral_system: &'es mut MockAccess, } -#[derive(CloneNoBound, DebugNoBound, PartialEqNoBound, EqNoBound)] -pub struct MockElection { - properties: ES::ElectionProperties, - state: ES::ElectionState, - settings: ES::ElectoralSettings, - consensus_status: ConsensusStatus, +pub struct MockWriteAccess { + election_identifier: ElectionIdentifierOf, } #[derive(CloneNoBound, DebugNoBound, PartialEqNoBound, EqNoBound)] pub struct MockAccess { - electoral_settings: ES::ElectoralSettings, - unsynchronised_state: ES::ElectoralUnsynchronisedState, - unsynchronised_state_map: BTreeMap, Option>, - elections: BTreeMap, MockElection>, - unsynchronised_settings: ES::ElectoralUnsynchronisedSettings, - next_election_id: UniqueMonotonicIdentifier, -} - -impl MockAccess { - fn election_read_access( - &self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - ensure!(self.elections.contains_key(&id), CorruptStorageError::new()); - Ok(MockReadAccess { election_identifier: id, electoral_system: self }) - } - - fn election_write_access( - &mut self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - ensure!(self.elections.contains_key(&id), CorruptStorageError::new()); - Ok(MockWriteAccess { election_identifier: id, electoral_system: self }) - } + _phantom: core::marker::PhantomData, } macro_rules! impl_read_access { @@ -67,7 +34,7 @@ macro_rules! impl_read_access { ::ElectoralSettings, CorruptStorageError, > { - self.with_election(|e| e.settings.clone()) + Ok(MockStorageAccess::electoral_settings_for_election::(self.identifier())) } fn properties( @@ -76,7 +43,7 @@ macro_rules! impl_read_access { ::ElectionProperties, CorruptStorageError, > { - self.with_election(|e| e.properties.clone()) + Ok(MockStorageAccess::election_properties::(self.identifier())) } fn state( @@ -85,27 +52,15 @@ macro_rules! impl_read_access { ::ElectionState, CorruptStorageError, > { - self.with_election(|e| e.state.clone()) + Ok(MockStorageAccess::election_state::(self.identifier())) } - fn election_identifier( - &self, - ) -> Result, CorruptStorageError> { - Ok(self.identifier()) + fn election_identifier(&self) -> ElectionIdentifierOf { + self.identifier() } } impl $t { - fn with_election) -> R, R>( - &self, - f: F, - ) -> Result { - self.electoral_system - .elections - .get(&self.identifier()) - .map(f) - .ok_or_else(CorruptStorageError::new) - } pub fn identifier(&self) -> ElectionIdentifierOf { self.election_identifier } @@ -114,183 +69,386 @@ macro_rules! impl_read_access { previous_consensus: Option<&ES::Consensus>, votes: ConsensusVotes, ) -> Result, CorruptStorageError> { - ES::check_consensus(self.identifier(), self, previous_consensus, votes) + ES::check_consensus(self, previous_consensus, votes) } } }; } -impl_read_access!(MockReadAccess<'_, ES>); -impl_read_access!(MockWriteAccess<'_, ES>); +impl_read_access!(MockReadAccess); +impl_read_access!(MockWriteAccess); -impl MockWriteAccess<'_, ES> { - fn with_election_mut) -> R, R>( - &mut self, - f: F, - ) -> Result { - self.electoral_system - .elections - .get_mut(&self.identifier()) - .map(f) - .ok_or_else(CorruptStorageError::new) - } - pub fn set_consensus_status(&mut self, consensus_status: ConsensusStatus) { - self.with_election_mut(|e| e.consensus_status = consensus_status) - .expect("Cannot set consensus status for non-existent election"); - } +thread_local! { + pub static ELECTION_STATE: RefCell, Vec>> = const { RefCell::new(BTreeMap::new()) }; + pub static ELECTION_PROPERTIES: RefCell, Vec>> = const { RefCell::new(BTreeMap::new()) }; + pub static ELECTORAL_SETTINGS: RefCell> = const { RefCell::new(Vec::new()) }; + // The electoral settings for a particular election + pub static ELECTION_SETTINGS: RefCell, Vec>> = const { RefCell::new(BTreeMap::new()) }; + pub static ELECTORAL_UNSYNCHRONISED_SETTINGS: RefCell> = const { RefCell::new(Vec::new()) }; + pub static ELECTORAL_UNSYNCHRONISED_STATE: RefCell> = const { RefCell::new(Vec::new()) }; + pub static ELECTORAL_UNSYNCHRONISED_STATE_MAP: RefCell, Option>>> = const { RefCell::new(BTreeMap::new()) }; + pub static CONSENSUS_STATUS: RefCell, Vec>> = const { RefCell::new(BTreeMap::new()) }; + pub static NEXT_ELECTION_ID: RefCell = const { RefCell::new(UniqueMonotonicIdentifier::from_u64(0)) }; } -impl ElectionWriteAccess for MockWriteAccess<'_, ES> { +impl ElectionWriteAccess for MockWriteAccess { fn set_state( - &mut self, + &self, state: ::ElectionState, ) -> Result<(), CorruptStorageError> { - self.with_election_mut(|e| e.state = state)?; + MockStorageAccess::set_state::(self.identifier(), state); Ok(()) } - fn clear_votes(&mut self) { + fn clear_votes(&self) { // nothing } fn delete(self) { - self.electoral_system.elections.remove(&self.identifier()); + MockStorageAccess::delete_election::(self.identifier()); } fn refresh( &mut self, - _extra: ::ElectionIdentifierExtra, + new_extra: ::ElectionIdentifierExtra, properties: ::ElectionProperties, ) -> Result<(), CorruptStorageError> { - self.with_election_mut(|e| e.properties = properties)?; + self.election_identifier = self.election_identifier.with_extra(new_extra); + MockStorageAccess::set_election_properties::(self.identifier(), properties); Ok(()) } fn check_consensus( - &mut self, + &self, ) -> Result< ConsensusStatus<::Consensus>, CorruptStorageError, > { - self.with_election_mut(|e| e.consensus_status.clone()) - } -} - -impl MockAccess { - pub fn new( - unsynchronised_state: ES::ElectoralUnsynchronisedState, - unsynchronised_settings: ES::ElectoralUnsynchronisedSettings, - electoral_settings: ES::ElectoralSettings, - ) -> Self { - Self { - electoral_settings, - unsynchronised_state, - unsynchronised_settings, - unsynchronised_state_map: Default::default(), - elections: Default::default(), - next_election_id: Default::default(), - } - } - - pub fn finalize_elections( - &mut self, - context: &ES::OnFinalizeContext, - ) -> Result { - ES::on_finalize(self, self.elections.keys().cloned().collect(), context) - } - - pub fn election_identifiers(&self) -> Vec> { - self.elections.keys().cloned().collect() - } - - pub fn next_umi(&self) -> UniqueMonotonicIdentifier { - self.next_election_id + Ok(MockStorageAccess::consensus_status::(self.identifier())) } } impl ElectoralReadAccess for MockAccess { type ElectoralSystem = ES; - type ElectionReadAccess<'es> = MockReadAccess<'es, ES>; + type ElectionReadAccess = MockReadAccess; - fn election( - &self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - self.election_read_access(id) + fn election(id: ElectionIdentifierOf) -> Self::ElectionReadAccess { + MockReadAccess { election_identifier: id } } - fn unsynchronised_settings( - &self, - ) -> Result< + fn unsynchronised_settings() -> Result< ::ElectoralUnsynchronisedSettings, CorruptStorageError, > { - Ok(self.unsynchronised_settings.clone()) + Ok(MockStorageAccess::unsynchronised_settings::()) } - fn unsynchronised_state( - &self, - ) -> Result< + fn unsynchronised_state() -> Result< ::ElectoralUnsynchronisedState, CorruptStorageError, > { - Ok(self.unsynchronised_state.clone()) + Ok(MockStorageAccess::unsynchronised_state::()) } fn unsynchronised_state_map( - &self, key: &::ElectoralUnsynchronisedStateMapKey, ) -> Result< Option<::ElectoralUnsynchronisedStateMapValue>, CorruptStorageError, > { - self.unsynchronised_state_map - .get(&key.using_encoded(Twox64Concat::hash)) - .ok_or_else(CorruptStorageError::new) - .cloned() + Ok(MockStorageAccess::unsynchronised_state_map::(key)) } } impl ElectoralWriteAccess for MockAccess { - type ElectionWriteAccess<'a> = MockWriteAccess<'a, ES>; + type ElectionWriteAccess = MockWriteAccess; fn new_election( - &mut self, extra: ::ElectionIdentifierExtra, properties: ::ElectionProperties, state: ::ElectionState, - ) -> Result, CorruptStorageError> { - let election_identifier = ElectionIdentifier::new(self.next_election_id, extra); - self.next_election_id = self.next_election_id.next_identifier().unwrap(); - self.elections.insert( - election_identifier, - MockElection { - properties, - state, - settings: self.electoral_settings.clone(), - consensus_status: ConsensusStatus::None, - }, - ); - self.election_write_access(election_identifier) + ) -> Result { + Ok(Self::election_mut(MockStorageAccess::new_election::(extra, properties, state))) } - fn election_mut( - &mut self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - self.election_write_access(id) + fn election_mut(id: ElectionIdentifierOf) -> Self::ElectionWriteAccess { + MockWriteAccess { election_identifier: id } } fn set_unsynchronised_state( - &mut self, unsynchronised_state: ::ElectoralUnsynchronisedState, ) -> Result<(), CorruptStorageError> { - self.unsynchronised_state = unsynchronised_state; + MockStorageAccess::set_unsynchronised_state::(unsynchronised_state); Ok(()) } /// Inserts or removes a value from the unsynchronised state map of the electoral system. fn set_unsynchronised_state_map( - &mut self, key: ::ElectoralUnsynchronisedStateMapKey, value: Option< ::ElectoralUnsynchronisedStateMapValue, >, - ) -> Result<(), CorruptStorageError> { - self.unsynchronised_state_map - .insert(key.using_encoded(Twox64Concat::hash), value); - Ok(()) + ) { + MockStorageAccess::set_unsynchronised_state_map::(key, value); + } +} + +pub struct MockStorageAccess; + +impl MockStorageAccess { + pub fn clear_storage() { + ELECTION_STATE.with(|state| { + let mut state_ref = state.borrow_mut(); + state_ref.clear(); + }); + ELECTION_PROPERTIES.with(|properties| { + let mut properties_ref = properties.borrow_mut(); + properties_ref.clear(); + }); + ELECTORAL_SETTINGS.with(|settings| { + let mut settings_ref = settings.borrow_mut(); + settings_ref.clear(); + }); + ELECTION_SETTINGS.with(|settings| { + let mut settings_ref = settings.borrow_mut(); + settings_ref.clear(); + }); + ELECTORAL_UNSYNCHRONISED_SETTINGS.with(|settings| { + let mut settings_ref = settings.borrow_mut(); + settings_ref.clear(); + }); + ELECTORAL_UNSYNCHRONISED_STATE.with(|state| { + let mut state_ref = state.borrow_mut(); + state_ref.clear(); + }); + ELECTORAL_UNSYNCHRONISED_STATE_MAP.with(|state_map| { + let mut state_map_ref = state_map.borrow_mut(); + state_map_ref.clear(); + }); + CONSENSUS_STATUS.with(|consensus| { + let mut consensus_ref = consensus.borrow_mut(); + consensus_ref.clear(); + }); + NEXT_ELECTION_ID.with(|next_id| { + let mut next_id_ref = next_id.borrow_mut(); + *next_id_ref = UniqueMonotonicIdentifier::from_u64(0); + }); + } + + pub fn next_umi() -> UniqueMonotonicIdentifier { + NEXT_ELECTION_ID.with(|next_id| *next_id.borrow()) + } + + pub fn increment_next_umi() { + NEXT_ELECTION_ID.with(|next_id| { + let mut next_id_ref = next_id.borrow_mut(); + *next_id_ref = next_id_ref.next_identifier().unwrap(); + }); + } + + pub fn set_electoral_settings( + settings: ::ElectoralSettings, + ) { + ELECTORAL_SETTINGS.with(|old_settings| { + let mut settings_ref = old_settings.borrow_mut(); + *settings_ref = settings.encode(); + }); + } + + pub fn electoral_settings() -> ES::ElectoralSettings { + ELECTORAL_SETTINGS.with(|settings| { + let settings_ref = settings.borrow(); + ES::ElectoralSettings::decode(&mut &settings_ref[..]).unwrap() + }) + } + + pub fn set_electoral_settings_for_election( + identifier: ElectionIdentifierOf, + settings: ::ElectoralSettings, + ) { + ELECTION_SETTINGS.with(|old_settings| { + let mut settings_ref = old_settings.borrow_mut(); + settings_ref.insert(identifier.encode(), settings.encode()); + }); + } + + pub fn electoral_settings_for_election( + identifier: ElectionIdentifierOf, + ) -> ::ElectoralSettings { + ELECTION_SETTINGS.with(|settings| { + let settings_ref = settings.borrow(); + settings_ref + .get(&identifier.encode()) + .map(|v| ES::ElectoralSettings::decode(&mut &v[..]).unwrap()) + .unwrap() + }) + } + + pub fn delete_election(identifier: ElectionIdentifierOf) { + ELECTION_PROPERTIES.with(|properties| { + let mut properties_ref = properties.borrow_mut(); + properties_ref.remove(&identifier.encode()); + }); + ELECTION_STATE.with(|state| { + let mut state_ref = state.borrow_mut(); + state_ref.remove(&identifier.encode()); + }); + } + + pub fn set_state( + 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()); + }); + } + + pub fn election_state( + identifier: ElectionIdentifierOf, + ) -> ES::ElectionState { + ELECTION_STATE.with(|old_state| { + let state_ref = old_state.borrow(); + state_ref + .get(&identifier.encode()) + .map(|v| ES::ElectionState::decode(&mut &v[..]).unwrap()) + .unwrap() + }) + } + + pub fn set_election_properties( + identifier: ElectionIdentifierOf, + properties: ES::ElectionProperties, + ) { + ELECTION_PROPERTIES.with(|old_properties| { + let mut properties_ref = old_properties.borrow_mut(); + properties_ref.insert(identifier.encode(), properties.encode()); + }); + } + + pub fn election_properties( + identifier: ElectionIdentifierOf, + ) -> ES::ElectionProperties { + ELECTION_PROPERTIES.with(|old_properties| { + let properties_ref = old_properties.borrow(); + properties_ref + .get(&identifier.encode()) + .map(|v| ES::ElectionProperties::decode(&mut &v[..]).unwrap()) + .unwrap() + }) + } + + pub fn set_unsynchronised_state( + unsynchronised_state: ES::ElectoralUnsynchronisedState, + ) { + ELECTORAL_UNSYNCHRONISED_STATE.with(|old_state| { + let mut state_ref = old_state.borrow_mut(); + state_ref.clear(); + state_ref.extend_from_slice(&unsynchronised_state.encode()); + }); + } + + 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(); + ES::ElectoralUnsynchronisedSettings::decode(&mut &settings_ref[..]).unwrap() + }) + } + + pub fn unsynchronised_state() -> ES::ElectoralUnsynchronisedState { + ELECTORAL_UNSYNCHRONISED_STATE.with(|old_state| { + let state_ref = old_state.borrow(); + ES::ElectoralUnsynchronisedState::decode(&mut &state_ref[..]).unwrap() + }) + } + + pub fn unsynchronised_state_map( + key: &ES::ElectoralUnsynchronisedStateMapKey, + ) -> Option { + ELECTORAL_UNSYNCHRONISED_STATE_MAP.with(|old_state_map| { + let state_map_ref = old_state_map.borrow(); + state_map_ref + .get(&key.encode()) + .expect("Key should exist") + .clone() + .map(|v| ES::ElectoralUnsynchronisedStateMapValue::decode(&mut &v[..]).unwrap()) + }) + } + + pub fn raw_unsynchronised_state_map() -> BTreeMap, Option>> { + ELECTORAL_UNSYNCHRONISED_STATE_MAP.with(|old_state_map| { + let state_map_ref = old_state_map.borrow(); + state_map_ref.clone() + }) + } + + pub fn set_unsynchronised_state_map( + key: ES::ElectoralUnsynchronisedStateMapKey, + value: Option, + ) { + ELECTORAL_UNSYNCHRONISED_STATE_MAP.with(|old_state_map| { + let mut state_map_ref = old_state_map.borrow_mut(); + state_map_ref.insert(key.encode(), value.map(|v| v.encode())); + }); + } + + pub fn election_identifiers() -> Vec> { + ELECTION_PROPERTIES.with(|properties| { + let properties_ref = properties.borrow(); + properties_ref + .keys() + .map(|k| ElectionIdentifierOf::::decode(&mut &k[..]).unwrap()) + .collect() + }) + } + + pub fn set_consensus_status( + identifier: ElectionIdentifierOf, + status: ConsensusStatus, + ) { + println!("Setting consensus status to {:?} for {:?}", status, identifier); + CONSENSUS_STATUS.with(|old_consensus| { + let mut consensus_ref = old_consensus.borrow_mut(); + consensus_ref.insert(identifier.encode(), status.encode()); + }); + } + + pub fn consensus_status( + identifier: ElectionIdentifierOf, + ) -> ConsensusStatus { + CONSENSUS_STATUS + .with(|old_consensus| { + let consensus_ref = old_consensus.borrow(); + consensus_ref + .get(&identifier.encode()) + .map(|v| ConsensusStatus::::decode(&mut &v[..]).unwrap()) + }) + .unwrap_or(ConsensusStatus::None) + } + + pub fn new_election( + extra: ::ElectionIdentifierExtra, + properties: ::ElectionProperties, + state: ::ElectionState, + ) -> ElectionIdentifierOf { + let next_umi = Self::next_umi(); + let election_identifier = ElectionIdentifier::new(next_umi, extra); + Self::increment_next_umi(); + + Self::set_election_properties::(election_identifier, properties); + Self::set_state::(election_identifier, state); + // These are normally stored once and synchronised by election identifier. In the tests we + // simplify this by just storing the electoral settings (that would be fetched by + // resolving the synchronisation) alongside the election. + Self::set_electoral_settings_for_election::( + election_identifier, + Self::electoral_settings::(), + ); + + election_identifier } } diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_change.rs b/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_change.rs index 9ea3054981..3ef6c454c2 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_change.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_change.rs @@ -43,17 +43,12 @@ impl< > MonotonicChange { pub fn watch_for_change>( - electoral_access: &mut ElectoralAccess, identifier: Identifier, previous_value: Value, ) -> Result<(), CorruptStorageError> { let previous_block_height = - electoral_access.unsynchronised_state_map(&identifier)?.unwrap_or_default(); - electoral_access.new_election( - (), - (identifier, previous_value, previous_block_height), - (), - )?; + ElectoralAccess::unsynchronised_state_map(&identifier)?.unwrap_or_default(); + ElectoralAccess::new_election((), (identifier, previous_value, previous_block_height), ())?; Ok(()) } } @@ -82,7 +77,6 @@ impl< type OnFinalizeReturn = (); fn is_vote_desired>( - _election_identifier: ElectionIdentifierOf, _election_access: &ElectionAccess, _current_vote: Option<(VotePropertiesOf, AuthorityVoteOf)>, ) -> Result { @@ -119,12 +113,11 @@ impl< } fn on_finalize>( - electoral_access: &mut ElectoralAccess, election_identifiers: Vec>, _context: &Self::OnFinalizeContext, ) -> Result { for election_identifier in election_identifiers { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); if let Some((value, block_height)) = election_access.check_consensus()?.has_consensus() { let (identifier, previous_value, previous_block_height) = @@ -132,8 +125,7 @@ impl< if previous_value != value && block_height > previous_block_height { election_access.delete(); Hook::on_change(identifier.clone(), value); - electoral_access - .set_unsynchronised_state_map(identifier, Some(block_height))?; + ElectoralAccess::set_unsynchronised_state_map(identifier, Some(block_height)); } else { // We don't expect this to be hit, since we should have filtered out any votes // that would cause this in check_consensus. @@ -150,7 +142,6 @@ impl< } fn check_consensus>( - _election_identifier: ElectionIdentifierOf, election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, consensus_votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_median.rs b/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_median.rs index 9802e7dbba..f674caf880 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_median.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/monotonic_median.rs @@ -63,8 +63,7 @@ impl< Ok(()) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, _context: &Self::OnFinalizeContext, ) -> Result { @@ -73,30 +72,27 @@ impl< .at_most_one() .map_err(|_| CorruptStorageError::new())? { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); if let Some(consensus) = election_access.check_consensus()?.has_consensus() { election_access.delete(); - electoral_access.new_election((), (), ())?; - electoral_access.mutate_unsynchronised_state( - |_electoral_access, unsynchronised_state| { - if consensus > *unsynchronised_state { - *unsynchronised_state = consensus.clone(); - Hook::on_change(consensus); - } + ElectoralAccess::new_election((), (), ())?; + ElectoralAccess::mutate_unsynchronised_state(|unsynchronised_state| { + if consensus > *unsynchronised_state { + *unsynchronised_state = consensus.clone(); + Hook::on_change(consensus); + } - Ok(()) - }, - )?; + Ok(()) + })?; } } else { - electoral_access.new_election((), (), ())?; + ElectoralAccess::new_election((), (), ())?; } - electoral_access.unsynchronised_state() + ElectoralAccess::unsynchronised_state() } fn check_consensus>( - _election_identifier: ElectionIdentifier, _election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, consensus_votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/tests/liveness.rs b/state-chain/pallets/cf-elections/src/electoral_systems/tests/liveness.rs index e602553afd..53a6a7364b 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/tests/liveness.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/tests/liveness.rs @@ -35,8 +35,7 @@ type SimpleLiveness = register_checks! { SimpleLiveness { only_one_election(_pre, post) { - let election_ids = post.election_identifiers(); - assert_eq!(election_ids.len(), 1, "Only one election should exist."); + assert_eq!(post.election_identifiers.len(), 1, "Only one election should exist."); }, hook_called_once(_pre, _post) { assert_eq!(HOOK_CALLED_COUNT.with(|hook_called| hook_called.get()), 1, "Hook should have been called once so far!"); 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 3628ab180b..449ae61ab2 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::{ @@ -43,8 +43,7 @@ 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!"); + 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!"); @@ -90,7 +89,7 @@ fn too_few_votes_consensus_not_possible() { #[test] fn finalize_election_with_incremented_state() { let test = with_default_context(); - let initial_state = test.access().unsynchronised_state().unwrap(); + let initial_state = MockAccess::::unsynchronised_state().unwrap(); let new_unsynchronised_state = initial_state + 1; test.force_consensus_update(ConsensusStatus::Gained { @@ -109,8 +108,8 @@ fn finalize_election_with_incremented_state() { 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); + assert_eq!(pre.unsynchronised_state, initial_state); + assert_eq!(post.unsynchronised_state, new_unsynchronised_state); }), Check::last_election_deleted(), Check::election_id_incremented(), @@ -148,8 +147,8 @@ fn finalize_election_state_can_not_decrease() { // 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); + assert_eq!(pre.unsynchronised_state, INTITIAL_STATE); + assert_eq!(post.unsynchronised_state, INTITIAL_STATE); }), Check::last_election_deleted(), Check::election_id_incremented(), diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/tests/unsafe_median.rs b/state-chain/pallets/cf-elections/src/electoral_systems/tests/unsafe_median.rs index 28f07e626c..dff2e35e31 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/tests/unsafe_median.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/tests/unsafe_median.rs @@ -2,7 +2,7 @@ use cf_primitives::AuthorityCount; use super::{mocks::*, register_checks}; use crate::{ - electoral_system::{ConsensusStatus, ElectoralReadAccess}, + electoral_system::ConsensusStatus, electoral_systems::{tests::utils::generate_votes, unsafe_median::*}, }; @@ -21,23 +21,23 @@ fn with_default_context() -> TestContext { register_checks! { SimpleUnsafeMedian { - started_at_initial_state(pre_finalize, _post) { + started_at_initial_state(pre_finalize, _a) { assert_eq!( - pre_finalize.unsynchronised_state().unwrap(), + pre_finalize.unsynchronised_state, INIT_UNSYNCHRONISED_STATE, "Expected initial state pre-finalization." ); }, ended_at_initial_state(_pre, post_finalize) { assert_eq!( - post_finalize.unsynchronised_state().unwrap(), + post_finalize.unsynchronised_state, INIT_UNSYNCHRONISED_STATE, "Expected initial state post-finalization." ); }, ended_at_new_state(_pre, post_finalize) { assert_eq!( - post_finalize.unsynchronised_state().unwrap(), + post_finalize.unsynchronised_state, NEW_UNSYNCHRONISED_STATE, "Expected new state post-finalization." ); diff --git a/state-chain/pallets/cf-elections/src/electoral_systems/unsafe_median.rs b/state-chain/pallets/cf-elections/src/electoral_systems/unsafe_median.rs index 87fbd6356d..b362cd820d 100644 --- a/state-chain/pallets/cf-elections/src/electoral_systems/unsafe_median.rs +++ b/state-chain/pallets/cf-elections/src/electoral_systems/unsafe_median.rs @@ -58,8 +58,7 @@ impl< Ok(()) } - fn on_finalize>( - electoral_access: &mut ElectoralAccess, + fn on_finalize + 'static>( election_identifiers: Vec>, _context: &Self::OnFinalizeContext, ) -> Result { @@ -68,21 +67,20 @@ impl< .at_most_one() .map_err(|_| CorruptStorageError::new())? { - let mut election_access = electoral_access.election_mut(election_identifier)?; + let election_access = ElectoralAccess::election_mut(election_identifier); if let Some(consensus) = election_access.check_consensus()?.has_consensus() { election_access.delete(); - electoral_access.set_unsynchronised_state(consensus)?; - electoral_access.new_election((), (), ())?; + ElectoralAccess::set_unsynchronised_state(consensus)?; + ElectoralAccess::new_election((), (), ())?; } } else { - electoral_access.new_election((), (), ())?; + ElectoralAccess::new_election((), (), ())?; } Ok(()) } fn check_consensus>( - _election_identifier: ElectionIdentifier, _election_access: &ElectionAccess, _previous_consensus: Option<&Self::Consensus>, votes: ConsensusVotes, diff --git a/state-chain/pallets/cf-elections/src/lib.rs b/state-chain/pallets/cf-elections/src/lib.rs index a294226fff..e921169833 100644 --- a/state-chain/pallets/cf-elections/src/lib.rs +++ b/state-chain/pallets/cf-elections/src/lib.rs @@ -108,6 +108,7 @@ #![doc = include_str!("../../cf-doc-head.md")] pub mod electoral_system; +pub mod electoral_system_runner; pub mod electoral_systems; mod mock; mod tests; @@ -136,14 +137,18 @@ pub mod pallet { use cf_primitives::{AuthorityCount, EpochIndex}; use cf_traits::{AccountRoleRegistry, Chainflip, EpochInfo}; - use crate::electoral_system::{ConsensusVote, ConsensusVotes}; - use access_impls::{ElectionAccess, ElectoralAccess}; + use crate::electoral_system::ConsensusStatus; + pub use access_impls::RunnerStorageAccess; + + use crate::electoral_system_runner::{ + CompositeConsensusVote, CompositeConsensusVotes, RunnerStorageAccessTrait, + }; use bitmap_components::ElectionBitmapComponents; - use electoral_system::{ - AuthorityVoteOf, ConsensusStatus, ElectionIdentifierOf, ElectionReadAccess, - ElectionWriteAccess, ElectoralReadAccess, ElectoralSystem, ElectoralWriteAccess, - IndividualComponentOf, VotePropertiesOf, + pub use electoral_system_runner::{ + CompositeAuthorityVoteOf, CompositeElectionIdentifierOf, CompositeIndividualComponentOf, + CompositeVotePropertiesOf, ElectoralSystemRunner, }; + use frame_support::{ sp_runtime::traits::BlockNumberProvider, storage::bounded_btree_map::BoundedBTreeMap, Deserialize, Serialize, StorageDoubleMap as _, @@ -184,10 +189,10 @@ pub mod pallet { /// when to vote. #[allow(type_alias_bounds)] pub type ElectoralDataFor, I: 'static> = ElectoralData< - ElectionIdentifierOf, - ::ElectoralSettings, - ::ElectionProperties, - AuthorityVoteOf, + CompositeElectionIdentifierOf, + ::ElectoralSettings, + ::ElectionProperties, + CompositeAuthorityVoteOf, BlockNumberFor, >; @@ -203,8 +208,8 @@ pub mod pallet { self.0.checked_add(1).map(Self) } - #[cfg(feature = "runtime-benchmarks")] - pub fn from_u64(value: u64) -> Self { + #[cfg(any(feature = "runtime-benchmarks", test))] + pub const fn from_u64(value: u64) -> Self { Self(value) } } @@ -315,9 +320,9 @@ pub mod pallet { #[allow(type_alias_bounds)] pub type InitialStateOf, I: 'static> = InitialState< - ::ElectoralUnsynchronisedState, - ::ElectoralUnsynchronisedSettings, - ::ElectoralSettings, + ::ElectoralUnsynchronisedState, + ::ElectoralUnsynchronisedSettings, + ::ElectoralSettings, >; #[pallet::genesis_config] @@ -354,8 +359,7 @@ pub mod pallet { type RuntimeEvent: From> + IsType<::RuntimeEvent>; - type ElectoralSystem: ElectoralSystem< - OnFinalizeContext = (), + type ElectoralSystemRunner: ElectoralSystemRunner< ValidatorId = ::ValidatorId, >; @@ -441,7 +445,7 @@ pub mod pallet { _, Identity, SharedDataHash, - <::Vote as VoteStorage>::SharedData, + <::Vote as VoteStorage>::SharedData, OptionQuery, >; @@ -464,10 +468,15 @@ pub mod pallet { UniqueMonotonicIdentifier, Identity, T::ValidatorId, - (VotePropertiesOf, IndividualComponentOf), + ( + CompositeVotePropertiesOf, + CompositeIndividualComponentOf, + ), OptionQuery, >; + // TODO: rename this storage item to be specific to umi. + // election identifier is used elsewhere to mean umi + extra. /// Stores the next valid election identifier. #[pallet::storage] pub(crate) type NextElectionIdentifier, I: 'static = ()> = @@ -478,7 +487,7 @@ pub mod pallet { #[pallet::storage] pub(crate) type ElectoralUnsynchronisedSettings, I: 'static = ()> = StorageValue< _, - ::ElectoralUnsynchronisedSettings, + ::ElectoralUnsynchronisedSettings, OptionQuery, >; @@ -486,7 +495,7 @@ pub mod pallet { #[pallet::storage] pub(crate) type ElectoralUnsynchronisedState, I: 'static = ()> = StorageValue< _, - ::ElectoralUnsynchronisedState, + ::ElectoralUnsynchronisedState, OptionQuery, >; @@ -495,8 +504,8 @@ pub mod pallet { type ElectoralUnsynchronisedStateMap, I: 'static = ()> = StorageMap< _, Twox64Concat, - ::ElectoralUnsynchronisedStateMapKey, - ::ElectoralUnsynchronisedStateMapValue, + ::ElectoralUnsynchronisedStateMapKey, + ::ElectoralUnsynchronisedStateMapValue, OptionQuery, >; @@ -507,7 +516,7 @@ pub mod pallet { _, Twox64Concat, UniqueMonotonicIdentifier, - ::ElectoralSettings, + ::ElectoralSettings, OptionQuery, >; @@ -517,8 +526,8 @@ pub mod pallet { type ElectionProperties, I: 'static = ()> = StorageMap< _, Twox64Concat, - ElectionIdentifierOf, - ::ElectionProperties, + CompositeElectionIdentifierOf, + ::ElectionProperties, OptionQuery, >; @@ -528,19 +537,19 @@ pub mod pallet { _, Twox64Concat, UniqueMonotonicIdentifier, - ::ElectionState, + ::ElectionState, OptionQuery, >; /// Stores the most recent consensus, i.e. the most recent result of - /// `ElectoralSystem::check_consensus` that returned `Some(...)`, and whether it is `current` / - /// has not been `lost` since. + /// `ElectoralSystemRunner::check_consensus` that returned `Some(...)`, and whether it is + /// `current` / has not been `lost` since. #[pallet::storage] type ElectionConsensusHistory, I: 'static = ()> = StorageMap< _, Twox64Concat, UniqueMonotonicIdentifier, - ConsensusHistory<::Consensus>, + ConsensusHistory<::Consensus>, OptionQuery, >; @@ -582,29 +591,43 @@ pub mod pallet { // ---------------------------------------------------------------------------------------- // pub mod access_impls { + use electoral_system_runner::RunnerStorageAccessTrait; + use super::*; - /// Implements traits to allow electoral systems to read/write an Election's details. - pub struct ElectionAccess, I: 'static> { - election_identifier: ElectionIdentifierOf, + // Provides access to the storage layer in a controlled, and consistent way. + pub struct RunnerStorageAccess, I: 'static> { _phantom: core::marker::PhantomData<(T, I)>, } - impl, I: 'static> ElectionAccess { - pub fn new(election_identifier: ElectionIdentifierOf) -> Self { - Self { election_identifier, _phantom: Default::default() } - } - pub(crate) fn unique_monotonic_identifier(&self) -> UniqueMonotonicIdentifier { - *self.election_identifier.unique_monotonic() + impl, I: 'static> RunnerStorageAccessTrait for RunnerStorageAccess { + type ElectoralSystemRunner = T::ElectoralSystemRunner; + + fn new_election( + extra: ::ElectionIdentifierExtra, + properties: ::ElectionProperties, + state: ::ElectionState, + ) -> Result< + CompositeElectionIdentifierOf, + CorruptStorageError, + > { + let unique_monotonic_identifier = NextElectionIdentifier::::get(); + let election_identifier = ElectionIdentifier(unique_monotonic_identifier, extra); + NextElectionIdentifier::::set(UniqueMonotonicIdentifier( + unique_monotonic_identifier + .0 + .checked_add(1) + .ok_or_else(CorruptStorageError::new)?, + )); + ElectionProperties::::insert(election_identifier, properties); + ElectionState::::insert(unique_monotonic_identifier, state); + Ok(election_identifier) } - } - impl, I: 'static> ElectionReadAccess for ElectionAccess { - type ElectoralSystem = T::ElectoralSystem; - fn settings( - &self, + fn electoral_settings_for_election( + unique_monotonic_identifier: UniqueMonotonicIdentifier, ) -> Result< - ::ElectoralSettings, + ::ElectoralSettings, CorruptStorageError, > { let mut settings_boundaries = @@ -613,93 +636,93 @@ pub mod pallet { let settings_boundary = settings_boundaries .iter() .rev() - .find(|settings_boundary| { - **settings_boundary <= self.unique_monotonic_identifier() - }) + .find(|settings_boundary| **settings_boundary <= unique_monotonic_identifier) .ok_or_else(CorruptStorageError::new)?; ElectoralSettings::::get(settings_boundary) .ok_or_else(CorruptStorageError::new) } - fn properties( - &self, + fn election_properties( + election_identifier: CompositeElectionIdentifierOf, ) -> Result< - ::ElectionProperties, + ::ElectionProperties, CorruptStorageError, > { - ElectionProperties::::get(self.election_identifier) + ElectionProperties::::get(election_identifier) .ok_or_else(CorruptStorageError::new) } - fn state( - &self, - ) -> Result<::ElectionState, CorruptStorageError> - { - ElectionState::::get(self.unique_monotonic_identifier()) + fn election_state( + unique_monotonic_identifier: UniqueMonotonicIdentifier, + ) -> Result< + ::ElectionState, + CorruptStorageError, + > { + ElectionState::::get(unique_monotonic_identifier) .ok_or_else(CorruptStorageError::new) } - #[cfg(test)] - fn election_identifier( - &self, - ) -> Result, CorruptStorageError> { - Ok(self.election_identifier) - } - } - impl, I: 'static> ElectionWriteAccess for ElectionAccess { - fn set_state( - &mut self, - state: ::ElectionState, + + fn set_election_state( + unique_monotonic_identifier: UniqueMonotonicIdentifier, + state: ::ElectionState, ) -> Result<(), CorruptStorageError> { - if self.state()? != state { - ElectionState::::insert(self.unique_monotonic_identifier(), state); - ElectionConsensusHistoryUpToDate::::remove( - self.unique_monotonic_identifier(), - ); + if Self::election_state(unique_monotonic_identifier)? != state { + ElectionState::::insert(unique_monotonic_identifier, state); + ElectionConsensusHistoryUpToDate::::remove(unique_monotonic_identifier); } Ok(()) } - fn clear_votes(&mut self) { - let unique_monotonic_identifier = self.unique_monotonic_identifier(); + fn clear_election_votes(unique_monotonic_identifier: UniqueMonotonicIdentifier) { ElectionBitmapComponents::::clear(unique_monotonic_identifier); for (_, (_, individual_component)) in IndividualComponents::::drain_prefix(unique_monotonic_identifier) { - <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component(&individual_component, |shared_data_hash| { - Pallet::::remove_shared_data_reference(shared_data_hash, unique_monotonic_identifier); + <::Vote as + VoteStorage>::visit_shared_data_references_in_individual_component(& + individual_component, |shared_data_hash| { Pallet::::remove_shared_data_reference(shared_data_hash, unique_monotonic_identifier); }); } ElectionConsensusHistoryUpToDate::::remove(unique_monotonic_identifier); } - fn delete(mut self) { - self.clear_votes(); - ElectionProperties::::remove(self.election_identifier); - let unique_monotonic_identifier = self.unique_monotonic_identifier(); + fn delete_election( + composite_election_identifier: CompositeElectionIdentifierOf< + Self::ElectoralSystemRunner, + >, + ) { + let unique_monotonic_identifier = composite_election_identifier.unique_monotonic(); + Self::clear_election_votes(*unique_monotonic_identifier); + ElectionProperties::::remove(composite_election_identifier); ElectionState::::remove(unique_monotonic_identifier); ElectionConsensusHistory::::remove(unique_monotonic_identifier); } - fn refresh( - &mut self, - extra: ::ElectionIdentifierExtra, - properties: ::ElectionProperties, + + fn refresh_election( + election_identifier: CompositeElectionIdentifierOf, + new_extra: ::ElectionIdentifierExtra, + properties: ::ElectionProperties, ) -> Result<(), CorruptStorageError> { - if extra <= *self.election_identifier.extra() { + if new_extra <= *election_identifier.extra() { Err(CorruptStorageError::new()) } else { - ElectionProperties::::remove(self.election_identifier); - self.election_identifier = - ElectionIdentifier::new(self.unique_monotonic_identifier(), extra); - ElectionProperties::::insert(self.election_identifier, properties); + ElectionProperties::::remove(election_identifier); + let new_election_identifier = + CompositeElectionIdentifierOf::::new( + *election_identifier.unique_monotonic(), + new_extra, + ); + ElectionProperties::::insert(new_election_identifier, properties); Ok(()) } } - fn check_consensus( - &mut self, + fn check_election_consensus( + election_identifier: CompositeElectionIdentifierOf, ) -> Result< - ConsensusStatus<::Consensus>, + ConsensusStatus<::Consensus>, CorruptStorageError, > { let epoch_index = T::EpochInfo::epoch_index(); - let unique_monotonic_identifier = self.unique_monotonic_identifier(); + let unique_monotonic_identifier = election_identifier.unique_monotonic(); let option_consensus_history = ElectionConsensusHistory::::get(unique_monotonic_identifier); Ok( @@ -720,7 +743,7 @@ pub mod pallet { let bitmap_components = ElectionBitmapComponents::::with( epoch_index, - unique_monotonic_identifier, + *unique_monotonic_identifier, |election_bitmap_components| { election_bitmap_components.get_all(¤t_authorities) }, @@ -729,18 +752,28 @@ pub mod pallet { IndividualComponents::::iter_prefix(unique_monotonic_identifier) .collect::>(); - let votes = current_authorities.into_iter().map(|validator_id| - ( - VoteComponents { - bitmap_component: bitmap_components.get(&validator_id).cloned(), - individual_component: individual_components.remove(&validator_id), - }, - validator_id, - ) - ).map(|(vote_components, validator_id)| { - if ContributingAuthorities::::contains_key(&validator_id) { - match <::Vote as VoteStorage>::components_into_authority_vote(vote_components, |shared_data_hash| { - // We don't bother to check if the reference has expired, as if we have the data we may as well use it, even if it was provided after the shared data reference expired (But before the reference was cleaned up `on_finalize`). + let votes = current_authorities + .into_iter() + .map(|validator_id| { + ( + VoteComponents { + bitmap_component: bitmap_components + .get(&validator_id) + .cloned(), + individual_component: individual_components + .remove(&validator_id), + }, + validator_id, + ) + }) + .map(|(vote_components, validator_id)| { + if ContributingAuthorities::::contains_key(&validator_id) { + match <::Vote as + VoteStorage>::components_into_authority_vote(vote_components, |shared_data_hash| + { + // We don't bother to check if the reference has expired, as if we have the + // data we may as well use it, even if it was provided after the shared data + // reference expired (But before the reference was cleaned up `on_finalize`). Ok(SharedData::::get(shared_data_hash)) }) { // Only a full vote can count towards consensus. @@ -749,25 +782,26 @@ pub mod pallet { Ok(None) => Ok(None), Err(e) => Err(e), } - } else { - Ok(None) - }.map(|props_and_vote| - ConsensusVote { - vote: props_and_vote, - validator_id + } else { + Ok(None) } - ) - }).collect::, _>>()?; + .map(|props_and_vote| CompositeConsensusVote { + vote: props_and_vote, + validator_id, + }) + }) + .collect::, _>>()?; debug_assert!(votes.len() == current_authorities_count as usize); // Remove individual components from non-authorities for (validator_id, (_, individual_component)) in individual_components { - <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component( + <::Vote as + VoteStorage>::visit_shared_data_references_in_individual_component( &individual_component, |shared_data_hash| { - Pallet::::remove_shared_data_reference(shared_data_hash, unique_monotonic_identifier); - }, + Pallet::::remove_shared_data_reference(shared_data_hash, + *unique_monotonic_identifier); }, ); IndividualComponents::::remove( unique_monotonic_identifier, @@ -776,9 +810,8 @@ pub mod pallet { } let option_new_consensus = - ::check_consensus( - self.election_identifier, - &*self, /* Disallow recursive calls to this function */ + ::check_consensus( + election_identifier, option_consensus_history.as_ref().and_then(|consensus_history| { if consensus_history.lost_since { None @@ -786,7 +819,7 @@ pub mod pallet { Some(&consensus_history.most_recent) } }), - ConsensusVotes { votes }, + CompositeConsensusVotes { votes }, )?; ElectionConsensusHistory::::set( @@ -838,103 +871,44 @@ pub mod pallet { }, ) } - } - - /// Implements traits to allow election systems to read/write multiple Election's details. - pub struct ElectoralAccess, I: 'static> { - _phantom: core::marker::PhantomData<(T, I)>, - } - impl, I: 'static> ElectoralAccess { - pub(super) fn new() -> Self { - Self { _phantom: Default::default() } - } - } - impl, I: 'static> ElectoralReadAccess for ElectoralAccess { - type ElectoralSystem = T::ElectoralSystem; - type ElectionReadAccess<'a> = ElectionAccess; - - fn election( - &self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - Ok(ElectionAccess::new(id)) - } fn unsynchronised_settings( - &self, ) -> Result< - ::ElectoralUnsynchronisedSettings, + ::ElectoralUnsynchronisedSettings, CorruptStorageError, - > { + >{ ElectoralUnsynchronisedSettings::::get().ok_or_else(CorruptStorageError::new) } - fn unsynchronised_state( - &self, - ) -> Result< - ::ElectoralUnsynchronisedState, + fn unsynchronised_state() -> Result< + ::ElectoralUnsynchronisedState, CorruptStorageError, > { ElectoralUnsynchronisedState::::get().ok_or_else(CorruptStorageError::new) } fn unsynchronised_state_map( - &self, - key: &::ElectoralUnsynchronisedStateMapKey, - ) -> Result< + key: &::ElectoralUnsynchronisedStateMapKey, + ) -> Option< - ::ElectoralUnsynchronisedStateMapValue, - >, - CorruptStorageError, - > { - Ok(ElectoralUnsynchronisedStateMap::::get(key)) - } - } - impl, I: 'static> ElectoralWriteAccess for ElectoralAccess { - type ElectionWriteAccess<'a> = ElectionAccess; - - fn new_election( - &mut self, - extra: ::ElectionIdentifierExtra, - properties: ::ElectionProperties, - state: ::ElectionState, - ) -> Result, CorruptStorageError> { - let unique_monotonic_identifier = NextElectionIdentifier::::get(); - let election_identifier = ElectionIdentifier(unique_monotonic_identifier, extra); - NextElectionIdentifier::::set(UniqueMonotonicIdentifier( - unique_monotonic_identifier - .0 - .checked_add(1) - .ok_or_else(CorruptStorageError::new)?, - )); - ElectionProperties::::insert(election_identifier, properties); - ElectionState::::insert(unique_monotonic_identifier, state); - self.election_mut(election_identifier) - } - fn election_mut( - &mut self, - id: ElectionIdentifierOf, - ) -> Result, CorruptStorageError> { - Ok(ElectionAccess::new(id)) + ::ElectoralUnsynchronisedStateMapValue, + >{ + ElectoralUnsynchronisedStateMap::::get(key) } fn set_unsynchronised_state( - &mut self, - unsynchronised_state: ::ElectoralUnsynchronisedState, - ) -> Result<(), CorruptStorageError> { + unsynchronised_state: ::ElectoralUnsynchronisedState, + ) { ElectoralUnsynchronisedState::::put(unsynchronised_state); - Ok(()) } fn set_unsynchronised_state_map( - &mut self, - key: ::ElectoralUnsynchronisedStateMapKey, + key: ::ElectoralUnsynchronisedStateMapKey, value: Option< - ::ElectoralUnsynchronisedStateMapValue, + ::ElectoralUnsynchronisedStateMapValue, >, - ) -> Result<(), CorruptStorageError> { + ) { ElectoralUnsynchronisedStateMap::::set(key, value); - Ok(()) } } } @@ -946,7 +920,7 @@ pub mod pallet { BitmapComponents, Config, CorruptStorageError, Pallet, UniqueMonotonicIdentifier, }; use crate::{ - electoral_system::{BitmapComponentOf, ElectoralSystem}, + electoral_system_runner::{BitmapComponentOf, ElectoralSystemRunner}, vote_storage::VoteStorage, }; use bitvec::prelude::*; @@ -962,7 +936,8 @@ pub mod pallet { pub(crate) struct ElectionBitmapComponents, I: 'static> { epoch: EpochIndex, #[allow(clippy::type_complexity)] - bitmaps: Vec<(BitmapComponentOf, BitVec)>, + bitmaps: + Vec<(BitmapComponentOf, BitVec)>, #[codec(skip)] _phantom: core::marker::PhantomData<(T, I)>, } @@ -1070,7 +1045,7 @@ pub mod pallet { this.bitmaps.retain(|(bitmap_component, bitmap)| { let retain = bitmap.any(); if !retain { - <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( + <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( bitmap_component, |shared_data_hash| { Pallet::::remove_shared_data_reference( @@ -1117,7 +1092,7 @@ pub mod pallet { pub(super) fn add( &mut self, authority_index: AuthorityCount, - bitmap_component: BitmapComponentOf, + bitmap_component: BitmapComponentOf, unique_monotonic_identifier: UniqueMonotonicIdentifier, block_number: BlockNumberFor, ) -> Result<(), CorruptStorageError> { @@ -1140,7 +1115,7 @@ pub mod pallet { true; bitmap })); - <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( + <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( &bitmap_component, |shared_data_hash| { Pallet::::add_shared_data_reference( @@ -1158,7 +1133,8 @@ pub mod pallet { pub(super) fn take( &mut self, authority_index: AuthorityCount, - ) -> Result>, CorruptStorageError> { + ) -> Result>, CorruptStorageError> + { let authority_index = authority_index as usize; Ok(self .bitmaps @@ -1175,7 +1151,8 @@ pub mod pallet { pub(super) fn get( &self, authority_index: AuthorityCount, - ) -> Result>, CorruptStorageError> { + ) -> Result>, CorruptStorageError> + { Ok(self .bitmaps .iter() @@ -1191,7 +1168,7 @@ pub mod pallet { &self, current_authorities: &[T::ValidatorId], ) -> Result< - BTreeMap>, + BTreeMap>, CorruptStorageError, > { self.debug_assert_authorities_in_order_of_indices(current_authorities); @@ -1211,7 +1188,7 @@ pub mod pallet { pub(super) fn clear(unique_monotonic_identifier: UniqueMonotonicIdentifier) { if let Some(this) = BitmapComponents::::get(unique_monotonic_identifier) { for (bitmap_component, _) in this.bitmaps { - <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( + <::Vote as VoteStorage>::visit_shared_data_references_in_bitmap_component( &bitmap_component, |shared_data_hash| { Pallet::::remove_shared_data_reference( @@ -1244,8 +1221,8 @@ pub mod pallet { pub fn vote( origin: OriginFor, authority_votes: BoundedBTreeMap< - ElectionIdentifierOf, - AuthorityVoteOf, + CompositeElectionIdentifierOf, + CompositeAuthorityVoteOf, ConstU32, >, ) -> DispatchResult { @@ -1267,7 +1244,7 @@ pub mod pallet { }, AuthorityVote::Vote(vote) => { ( - <::Vote as VoteStorage>::vote_into_partial_vote( + <::Vote as VoteStorage>::vote_into_partial_vote( &vote, |shared_data| SharedDataHash::of(&shared_data) ), @@ -1282,8 +1259,8 @@ pub mod pallet { &authority, authority_index, |option_existing_vote, election_bitmap_components| { - let components = <::Vote as VoteStorage>::partial_vote_into_components( - ::generate_vote_properties( + let components = <::Vote as VoteStorage>::partial_vote_into_components( + ::generate_vote_properties( election_identifier, option_existing_vote, &partial_vote, @@ -1305,7 +1282,7 @@ pub mod pallet { components.individual_component { // Update shared data reference counts - <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component( + <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component( &individual_component, |shared_data_hash| Self::add_shared_data_reference(shared_data_hash, unique_monotonic_identifier, block_number), ); @@ -1323,7 +1300,7 @@ pub mod pallet { // Insert any `SharedData` provided as part of the `Vote`. if let Some(vote) = option_vote { - <::Vote as VoteStorage>::visit_shared_data_in_vote( + <::Vote as VoteStorage>::visit_shared_data_in_vote( vote, |shared_data| Self::inner_provide_shared_data(shared_data), ) @@ -1345,7 +1322,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::provide_shared_data())] pub fn provide_shared_data( origin: OriginFor, - shared_data: <::Vote as VoteStorage>::SharedData, + shared_data: <::Vote as VoteStorage>::SharedData, ) -> DispatchResult { Self::ensure_can_vote(origin)?; Self::inner_provide_shared_data(shared_data)?; @@ -1381,7 +1358,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::delete_vote())] pub fn delete_vote( origin: OriginFor, - election_identifier: ElectionIdentifierOf, + election_identifier: CompositeElectionIdentifierOf, ) -> DispatchResult { let (epoch_index, authority, authority_index) = Self::ensure_can_vote(origin)?; let unique_monotonic_identifier = Self::ensure_election_exists(election_identifier)?; @@ -1414,9 +1391,11 @@ pub mod pallet { pub fn update_settings( origin: OriginFor, unsynchronised_settings: Option< - ::ElectoralUnsynchronisedSettings, + ::ElectoralUnsynchronisedSettings, + >, + settings: Option< + ::ElectoralSettings, >, - settings: Option<::ElectoralSettings>, ignore_corrupt_storage: CorruptStorageAdherance, ) -> DispatchResult { Self::ensure_governance(origin, ignore_corrupt_storage)?; @@ -1452,7 +1431,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::clear_election_votes())] pub fn clear_election_votes( origin: OriginFor, - election_identifier: ElectionIdentifierOf, + election_identifier: CompositeElectionIdentifierOf, ignore_corrupt_storage: CorruptStorageAdherance, check_election_exists: bool, ) -> DispatchResult { @@ -1461,7 +1440,9 @@ pub mod pallet { Self::ensure_election_exists(election_identifier)?; } - ElectionAccess::::new(election_identifier).clear_votes(); + RunnerStorageAccess::::clear_election_votes( + *election_identifier.unique_monotonic(), + ); Ok(()) } @@ -1470,7 +1451,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::invalidate_election_consensus_cache())] pub fn invalidate_election_consensus_cache( origin: OriginFor, - election_identifier: ElectionIdentifierOf, + election_identifier: CompositeElectionIdentifierOf, ignore_corrupt_storage: CorruptStorageAdherance, check_election_exists: bool, ) -> DispatchResult { @@ -1598,51 +1579,45 @@ pub mod pallet { Self::deposit_event(Event::::CorruptStorage); }, ElectionPalletStatus::Running => { - let _ = Self::with_electoral_access_and_identifiers( - |electoral_access, election_identifiers| { - if Into::::into(block_number) % - BLOCKS_BETWEEN_CLEANUP == sp_core::U256::zero() - { - let minimum_election_identifiers = election_identifiers - .iter() - .copied() - .map(|election_identifier| { - *election_identifier.unique_monotonic() - }) - .min() - .unwrap_or_default(); - let mut settings_boundaries = - ElectoralSettings::::iter_keys().collect::>(); - settings_boundaries.sort(); - for setting_boundary in &settings_boundaries + let _ = Self::with_election_identifiers(|election_identifiers| { + if Into::::into(block_number) % BLOCKS_BETWEEN_CLEANUP == + sp_core::U256::zero() + { + let minimum_election_identifiers = election_identifiers + .iter() + .copied() + .map(|election_identifier| { + *election_identifier.unique_monotonic() + }) + .min() + .unwrap_or_default(); + let mut settings_boundaries = + ElectoralSettings::::iter_keys().collect::>(); + settings_boundaries.sort(); + for setting_boundary in &settings_boundaries [..settings_boundaries[..] .partition_point(|&setting_boundary| { setting_boundary <= minimum_election_identifiers }) .saturating_sub(1) /*Keep the latest settings lower than the minimum election identifier, i.e. the settings referenced by the election with the minimum election identifier*/] - { - ElectoralSettings::::remove(setting_boundary); - } + { + ElectoralSettings::::remove(setting_boundary); + } - let current_authorities = T::EpochInfo::current_authorities(); - for validator in ContributingAuthorities::::iter_keys() - .collect::>() - { - if !current_authorities.contains(&validator) { - ContributingAuthorities::::remove(validator); - } + let current_authorities = T::EpochInfo::current_authorities(); + for validator in + ContributingAuthorities::::iter_keys().collect::>() + { + if !current_authorities.contains(&validator) { + ContributingAuthorities::::remove(validator); } } + } - T::ElectoralSystem::on_finalize( - electoral_access, - election_identifiers, - &(), - )?; + T::ElectoralSystemRunner::on_finalize(election_identifiers)?; - Ok(()) - }, - ); + Ok(()) + }); }, } } @@ -1688,50 +1663,29 @@ pub mod pallet { Ok(()) } - /// Provides access into the ElectoralSystem's storage. - /// - /// Ideally we would avoid introducing re-entrance (also with - /// `with_electoral_access_and_identifiers`), so the ElectoralAccess object can internally - /// cache storage which is possibly helpful particularly in the case of composites. - pub fn with_electoral_access< - R, - F: FnOnce(&mut ElectoralAccess) -> Result, - >( - f: F, - ) -> Result { - if Status::::get().is_some() { - Self::handle_corrupt_storage(f(&mut ElectoralAccess::::new())) - .map_err(Into::into) - } else { - Self::deposit_event(Event::::Uninitialized); - Err(Error::::Uninitialized.into()) - } - } - - /// Provides access into the ElectoralSystem's storage, and also all the current election + /// Provides access into the ElectoralSystem's current election /// identifiers. - /// - /// Ideally we would avoid introducing re-entrance (also with `with_electoral_access`), so - /// the ElectoralAccess object can internally cache storage which is possibly helpful - /// particularly in the case of composites. - pub fn with_electoral_access_and_identifiers< + pub fn with_election_identifiers< R, F: FnOnce( - &mut ElectoralAccess, - Vec>, + Vec>, ) -> Result, >( f: F, ) -> Result { - if Status::::get().is_some() { + Self::with_status_check(|| { let mut election_identifiers = ElectionProperties::::iter_keys().collect::>(); election_identifiers.sort(); - Self::handle_corrupt_storage(f( - &mut ElectoralAccess::::new(), - election_identifiers, - )) - .map_err(Into::into) + f(election_identifiers) + }) + } + + pub fn with_status_check Result>( + f: F, + ) -> Result { + if Status::::get().is_some() { + Self::handle_corrupt_storage(f()).map_err(Into::into) } else { Self::deposit_event(Event::::Uninitialized); Err(Error::::Uninitialized.into()) @@ -1750,8 +1704,8 @@ pub mod pallet { let block_number = frame_system::Pallet::::current_block_number(); Some(ElectoralData { - current_elections: Self::with_electoral_access_and_identifiers( - |electoral_access, election_identifiers| { + current_elections: Self::with_election_identifiers( + |election_identifiers| { election_identifiers .into_iter() .map(|election_identifier| { @@ -1781,27 +1735,23 @@ pub mod pallet { }, )?; - let election_access = - electoral_access.election(election_identifier)?; - Ok(( - election_identifier, - AuthorityElectionData { - settings: election_access.settings()?, - properties: election_access.properties()?, - // We report the vote to the engine even though it is timeouted so the engine - // knows to delete it. As it may still later to reconstructed if the right - // `SharedData` is provided, unless it is delete. - option_existing_vote: option_current_authority_vote.as_ref().map(|(_, authority_vote)| { - authority_vote.clone() - }), - is_vote_desired: ::is_vote_desired( election_identifier, - &election_access, - option_current_authority_vote.filter(|_| !contains_timed_out_shared_data_references), - )?, - }, - )) + AuthorityElectionData { + settings: RunnerStorageAccess::::electoral_settings_for_election(*election_identifier.unique_monotonic())?, + properties: RunnerStorageAccess::::election_properties(election_identifier)?, + // We report the vote to the engine even though it is timeouted so the engine + // knows to delete it. As it may still later to reconstructed if the right + // `SharedData` is provided, unless it is delete. + option_existing_vote: option_current_authority_vote.as_ref().map(|(_, authority_vote)| { + authority_vote.clone() + }), + is_vote_desired: ::is_vote_desired( + election_identifier, + option_current_authority_vote.filter(|_| !contains_timed_out_shared_data_references), + )?, + }, + )) }) .collect::, _>>() }, @@ -1843,10 +1793,10 @@ pub mod pallet { pub fn filter_votes( validator_id: &T::ValidatorId, proposed_votes: BTreeMap< - ElectionIdentifierOf, - <::Vote as VoteStorage>::Vote, + CompositeElectionIdentifierOf, + <::Vote as VoteStorage>::Vote, >, - ) -> BTreeSet> { + ) -> BTreeSet> { use frame_support::traits::OriginTrait; if let Ok((epoch_index, authority, authority_index)) = @@ -1854,8 +1804,8 @@ pub mod pallet { { let block_number = frame_system::Pallet::::current_block_number(); - Self::with_electoral_access_and_identifiers( - |_electoral_access, election_identifiers| { + Self::with_election_identifiers( + |election_identifiers| { election_identifiers .into_iter() .map(|election_identifier| { @@ -1896,12 +1846,12 @@ pub mod pallet { )) = option_current_authority_vote .filter(|_| !contains_timed_out_shared_data_references) { - ::is_vote_needed( + ::is_vote_needed( ( existing_vote_properties, match &existing_authority_vote { AuthorityVote::Vote(existing_vote) => { - <::Vote as VoteStorage>::vote_into_partial_vote( + <::Vote as VoteStorage>::vote_into_partial_vote( existing_vote, |shared_data| SharedDataHash::of(&shared_data) ) @@ -1911,7 +1861,7 @@ pub mod pallet { existing_authority_vote, ), ( - <::Vote as VoteStorage>::vote_into_partial_vote( + <::Vote as VoteStorage>::vote_into_partial_vote( proposed_vote, |shared_data| SharedDataHash::of(&shared_data) ), @@ -1963,7 +1913,10 @@ pub mod pallet { fn take_vote_and_then< R, F: for<'a> FnOnce( - Option<(VotePropertiesOf, AuthorityVoteOf)>, + Option<( + CompositeVotePropertiesOf, + CompositeAuthorityVoteOf, + )>, &'a mut ElectionBitmapComponents, ) -> Result, >( @@ -1981,7 +1934,7 @@ pub mod pallet { IndividualComponents::::take(unique_monotonic_identifier, authority); let r = f( - <::Vote as VoteStorage>::components_into_authority_vote( + <::Vote as VoteStorage>::components_into_authority_vote( VoteComponents { bitmap_component: election_bitmap_components.take(authority_index)?, individual_component: individual_component.clone(), @@ -1994,7 +1947,7 @@ pub mod pallet { // Remove references late to avoid deleting shared data that we will add // references to inside `f`. if let Some((_properties, individual_component)) = individual_component { - <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component( + <::Vote as VoteStorage>::visit_shared_data_references_in_individual_component( &individual_component, |shared_data_hash| Self::remove_shared_data_reference(shared_data_hash, unique_monotonic_identifier), ); @@ -2019,10 +1972,13 @@ pub mod pallet { authority_index: AuthorityCount, mut visit_unprovided_shared_data: VisitUnprovidedSharedData, ) -> Result< - Option<(VotePropertiesOf, AuthorityVoteOf)>, + Option<( + CompositeVotePropertiesOf, + CompositeAuthorityVoteOf, + )>, CorruptStorageError, > { - <::Vote as VoteStorage>::components_into_authority_vote( + <::Vote as VoteStorage>::components_into_authority_vote( VoteComponents { bitmap_component: ElectionBitmapComponents::::with( epoch_index, @@ -2079,7 +2035,7 @@ pub mod pallet { } } fn ensure_election_exists( - election_identifier: ElectionIdentifierOf, + election_identifier: CompositeElectionIdentifierOf, ) -> Result> { ensure!( ElectionProperties::::contains_key(election_identifier), @@ -2152,7 +2108,7 @@ pub mod pallet { } } fn inner_provide_shared_data( - shared_data: <::Vote as VoteStorage>::SharedData, + shared_data: <::Vote as VoteStorage>::SharedData, ) -> Result<(), Error> { let shared_data_hash = SharedDataHash::of(&shared_data); let (unique_monotonic_identifiers, reference_details): (Vec<_>, Vec<_>) = diff --git a/state-chain/pallets/cf-elections/src/mock.rs b/state-chain/pallets/cf-elections/src/mock.rs index 3d3ea08aad..dd67d7257e 100644 --- a/state-chain/pallets/cf-elections/src/mock.rs +++ b/state-chain/pallets/cf-elections/src/mock.rs @@ -24,7 +24,7 @@ impl pallet_cf_elections::Config for Test { type RuntimeEvent = RuntimeEvent; // TODO: Use Settings? - type ElectoralSystem = crate::electoral_systems::mock::MockElectoralSystem; + type ElectoralSystemRunner = crate::electoral_systems::mock::MockElectoralSystemRunner; type WeightInfo = (); } diff --git a/state-chain/pallets/cf-elections/src/tests.rs b/state-chain/pallets/cf-elections/src/tests.rs index 8df579c108..9d4083e8eb 100644 --- a/state-chain/pallets/cf-elections/src/tests.rs +++ b/state-chain/pallets/cf-elections/src/tests.rs @@ -1,10 +1,9 @@ #![cfg(test)] use crate::{mock::*, *}; use cf_primitives::AuthorityCount; -use electoral_system::{ - AuthorityVoteOf, ConsensusStatus, ElectionReadAccess, ElectoralReadAccess, ElectoralWriteAccess, -}; -use electoral_systems::mock::{BehaviourUpdate, MockElectoralSystem}; +use electoral_system::ConsensusStatus; +use electoral_system_runner::RunnerStorageAccessTrait; +use electoral_systems::mock::{BehaviourUpdate, MockElectoralSystemRunner}; use frame_support::traits::OriginTrait; use mock::Test; use std::collections::BTreeMap; @@ -117,7 +116,7 @@ fn ensure_can_vote() { }); } -pub trait ElectoralSystemTestExt: Sized { +pub trait ElectoralSystemRunnerTestExt: Sized { fn update_settings(self, updates: &[BehaviourUpdate]) -> Self; fn expect_consensus_after_next_block(self, expected: ConsensusStatus) -> Self; fn assume_consensus(self) -> Self; @@ -127,39 +126,28 @@ pub trait ElectoralSystemTestExt: Sized { fn submit_votes( self, validator_ids: &[u64], - vote: AuthorityVoteOf, + vote: CompositeAuthorityVoteOf, expected_outcome: Result<(), Error>, ) -> Self where - Test: Config, + Test: Config, ::RuntimeCall: From>; } -impl ElectoralSystemTestExt for TestRunner { +impl ElectoralSystemRunnerTestExt for TestRunner { /// Starts a new election, adding its unique monotonic identifier to the test context. #[track_caller] fn new_election(self) -> Self { self.then_execute_with( #[track_caller] |mut ctx| { - let unique_monotonic_identifier = - *Pallet::::with_electoral_access(|electoral_access| { - electoral_access.new_election((), (), ()) - }) - .expect("New election should not corrupt storage.") - .election_identifier() - .expect("New election should have an identifier.") - .unique_monotonic(); + let identifier = + RunnerStorageAccess::::new_election((), (), ()).unwrap(); + let unique_monotonic_identifier = identifier.unique_monotonic(); assert_eq!(Status::::get(), Some(ElectionPalletStatus::Running)); - Pallet::::with_electoral_access(|electoral_access| { - electoral_access - .election(ElectionIdentifier::new(unique_monotonic_identifier, ())) - }) - .expect("Expected an initial election."); - - ctx.umis.push(unique_monotonic_identifier); + ctx.umis.push(*unique_monotonic_identifier); ctx }, @@ -167,7 +155,7 @@ impl ElectoralSystemTestExt for TestRunner { } fn update_settings(self, updates: &[BehaviourUpdate]) -> Self { - MockElectoralSystem::update(updates); + MockElectoralSystemRunner::update(updates); self } @@ -185,11 +173,11 @@ impl ElectoralSystemTestExt for TestRunner { fn submit_votes( self, validator_ids: &[u64], - vote: AuthorityVoteOf, + vote: CompositeAuthorityVoteOf, expected_outcome: Result<(), Error>, ) -> Self where - Test: Config, + Test: Config, ::RuntimeCall: From>, { self.then_apply_extrinsics( @@ -204,7 +192,9 @@ impl ElectoralSystemTestExt for TestRunner { Call::::vote { authority_votes: BoundedBTreeMap::try_from( sp_std::iter::once(( - ElectionIdentifier::new(*umi, ()), + CompositeElectionIdentifierOf::< + MockElectoralSystemRunner, + >::new(*umi, ()), vote.clone(), )) .collect::>(), @@ -228,7 +218,7 @@ impl ElectoralSystemTestExt for TestRunner { assert!(!umis.is_empty(), "Asserted consensus on empty election set."); for umi in umis { - let actual = MockElectoralSystem::consensus_status(*umi); + let actual = MockElectoralSystemRunner::consensus_status(*umi); assert_eq!( actual, expected, @@ -249,7 +239,7 @@ impl ElectoralSystemTestExt for TestRunner { #[test] fn consensus_state_transitions() { - const VOTE: AuthorityVoteOf = AuthorityVote::Vote(()); + const VOTE: CompositeAuthorityVoteOf = AuthorityVote::Vote(()); election_test_ext(TestSetup { num_non_contributing_authorities: 2, ..Default::default() }) .new_election() @@ -301,7 +291,7 @@ fn consensus_state_transitions() { #[test] fn authority_removes_and_re_adds_itself_from_contributing_set() { - const VOTE: AuthorityVoteOf = AuthorityVote::Vote(()); + const VOTE: CompositeAuthorityVoteOf = AuthorityVote::Vote(()); election_test_ext(Default::default()) .new_election() diff --git a/state-chain/runtime/src/chainflip/solana_elections.rs b/state-chain/runtime/src/chainflip/solana_elections.rs index 19eb845ab5..792e52661b 100644 --- a/state-chain/runtime/src/chainflip/solana_elections.rs +++ b/state-chain/runtime/src/chainflip/solana_elections.rs @@ -3,7 +3,7 @@ use crate::{ SolanaIngressEgress, SolanaThresholdSigner, }; use cf_chains::{ - instances::ChainInstanceAlias, + instances::{ChainInstanceAlias, SolanaInstance}, sol::{ api::SolanaTransactionType, SolAddress, SolAmount, SolHash, SolSignature, SolTrackedData, SolanaCrypto, @@ -22,13 +22,13 @@ use pallet_cf_elections::{ electoral_system::{ElectoralReadAccess, ElectoralSystem}, electoral_systems::{ self, - composite::{tuple_6_impls::Hooks, Composite, Translator}, + composite::{tuple_6_impls::Hooks, CompositeRunner}, egress_success::OnEgressSuccess, liveness::OnCheckComplete, monotonic_change::OnChangeHook, monotonic_median::MedianChangeHook, }, - CorruptStorageError, ElectionIdentifier, InitialState, InitialStateOf, + CorruptStorageError, ElectionIdentifier, InitialState, InitialStateOf, RunnerStorageAccess, }; use scale_info::TypeInfo; @@ -42,7 +42,7 @@ use sol_prim::SlotNumber; type Instance = ::Instance; -pub type SolanaElectoralSystem = Composite< +pub type SolanaElectoralSystemRunner = CompositeRunner< ( SolanaBlockHeightTracking, SolanaFeeTracking, @@ -52,6 +52,7 @@ pub type SolanaElectoralSystem = Composite< SolanaLiveness, ), ::ValidatorId, + RunnerStorageAccess, SolanaElectionHooks, >; @@ -228,34 +229,7 @@ impl SolanaLiveness, > for SolanaElectionHooks { - type OnFinalizeContext = (); - type OnFinalizeReturn = (); - - fn on_finalize< - GenericElectoralAccess, - BlockHeightTranslator: Translator, - FeeTranslator: Translator, - IngressTranslator: Translator, - NonceTrackingTranslator: Translator, - EgressWitnessingTranslator: Translator, - LivenessTranslator: Translator, - >( - generic_electoral_access: &mut GenericElectoralAccess, - ( - block_height_translator, - fee_translator, - ingress_translator, - nonce_tracking_translator, - egress_witnessing_translator, - liveness_translator, - ): ( - BlockHeightTranslator, - FeeTranslator, - IngressTranslator, - NonceTrackingTranslator, - EgressWitnessingTranslator, - LivenessTranslator, - ), + fn on_finalize( ( block_height_identifiers, fee_identifiers, @@ -289,38 +263,45 @@ impl >, Vec::ElectionIdentifierExtra>>, ), - _context: &Self::OnFinalizeContext, - ) -> Result { - let block_height = SolanaBlockHeightTracking::on_finalize( - &mut block_height_translator.translate_electoral_access(generic_electoral_access), - block_height_identifiers, - &(), - )?; - SolanaLiveness::on_finalize( - &mut liveness_translator.translate_electoral_access(generic_electoral_access), - liveness_identifiers, - &(crate::System::block_number(), block_height), - )?; - SolanaFeeTracking::on_finalize( - &mut fee_translator.translate_electoral_access(generic_electoral_access), - fee_identifiers, - &(), - )?; - SolanaNonceTracking::on_finalize( - &mut nonce_tracking_translator.translate_electoral_access(generic_electoral_access), - nonce_tracking_identifiers, - &(), - )?; - SolanaEgressWitnessing::on_finalize( - &mut egress_witnessing_translator.translate_electoral_access(generic_electoral_access), - egress_witnessing_identifiers, - &(), - )?; - SolanaIngressTracking::on_finalize( - &mut ingress_translator.translate_electoral_access(generic_electoral_access), - ingress_identifiers, - &block_height, - )?; + ) -> Result<(), CorruptStorageError> { + let block_height = SolanaBlockHeightTracking::on_finalize::< + DerivedElectoralAccess< + _, + SolanaBlockHeightTracking, + RunnerStorageAccess, + >, + >(block_height_identifiers, &())?; + SolanaLiveness::on_finalize::< + DerivedElectoralAccess<_, SolanaLiveness, RunnerStorageAccess>, + >(liveness_identifiers, &(crate::System::block_number(), block_height))?; + SolanaFeeTracking::on_finalize::< + DerivedElectoralAccess< + _, + SolanaFeeTracking, + RunnerStorageAccess, + >, + >(fee_identifiers, &())?; + SolanaNonceTracking::on_finalize::< + DerivedElectoralAccess< + _, + SolanaNonceTracking, + RunnerStorageAccess, + >, + >(nonce_tracking_identifiers, &())?; + SolanaEgressWitnessing::on_finalize::< + DerivedElectoralAccess< + _, + SolanaEgressWitnessing, + RunnerStorageAccess, + >, + >(egress_witnessing_identifiers, &())?; + SolanaIngressTracking::on_finalize::< + DerivedElectoralAccess< + _, + SolanaIngressTracking, + RunnerStorageAccess, + >, + >(ingress_identifiers, &block_height)?; Ok(()) } } @@ -353,19 +334,16 @@ impl BenchmarkValue for SolanaIngressSettings { } } +use pallet_cf_elections::electoral_systems::composite::tuple_6_impls::DerivedElectoralAccess; + pub struct SolanaChainTrackingProvider; impl GetBlockHeight for SolanaChainTrackingProvider { fn get_block_height() -> ::ChainBlockNumber { - pallet_cf_elections::Pallet::::with_electoral_access( - |electoral_access| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (access_translator, ..) = &access_translators; - access_translator - .translate_electoral_access(electoral_access) - .unsynchronised_state() - }) - }, - ) + DerivedElectoralAccess::< + _, + SolanaBlockHeightTracking, + RunnerStorageAccess, + >::unsynchronised_state() .unwrap_or_else(|err| { log_or_panic!("Failed to obtain Solana block height: '{err:?}'."); // We use default in error case as it is preferable to panicking, and in @@ -375,19 +353,15 @@ impl GetBlockHeight for SolanaChainTrackingProvider { }) } } + impl SolanaChainTrackingProvider { pub fn priority_fee() -> Option<::ChainAmount> { - pallet_cf_elections::Pallet::::with_electoral_access( - |electoral_access| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (_, access_translator, ..) = &access_translators; - let electoral_access = - access_translator.translate_electoral_access(electoral_access); - electoral_access.unsynchronised_state() - }) - }, - ) - .ok() + DerivedElectoralAccess::< + _, + SolanaFeeTracking, + RunnerStorageAccess, + >::unsynchronised_state() + .ok() } fn with_tracked_data_then_apply_fee_multiplier< @@ -395,25 +369,26 @@ impl SolanaChainTrackingProvider { >( f: F, ) -> ::ChainAmount { - pallet_cf_elections::Pallet::::with_electoral_access( - |electoral_access| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (_, access_translator, ..) = &access_translators; - let electoral_access = - access_translator.translate_electoral_access(electoral_access); - Ok(electoral_access - .unsynchronised_settings()? - .fee_multiplier - .saturating_mul_int(f(SolTrackedData { - priority_fee: electoral_access.unsynchronised_state()?, - }))) + DerivedElectoralAccess::< + _, + SolanaFeeTracking, + RunnerStorageAccess, + >::unsynchronised_state() + .and_then(|priority_fee| { + DerivedElectoralAccess::< + _, + SolanaFeeTracking, + RunnerStorageAccess, + >::unsynchronised_settings().map(|fees| { + { + fees.fee_multiplier.saturating_mul_int(f(SolTrackedData { priority_fee })) + } }) - }, - ) - .unwrap_or_else(|err| { - log_or_panic!("Failed to obtain Solana fee: '{err:?}'."); - Default::default() - }) + }) + .unwrap_or_else(|err| { + log_or_panic!("Failed to obtain Solana fee: '{err:?}'."); + Default::default() + }) } } impl AdjustedFeeEstimationApi for SolanaChainTrackingProvider { @@ -441,22 +416,19 @@ impl IngressSource for SolanaIngress { asset: ::ChainAsset, close_block: ::ChainBlockNumber, ) -> DispatchResult { - pallet_cf_elections::Pallet::::with_electoral_access_and_identifiers( - |electoral_access, election_identifiers| { - SolanaElectoralSystem::with_identifiers( - election_identifiers, - |election_identifiers| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (_, _, access_translator, ..) = &access_translators; - let (_, _, election_identifiers, ..) = election_identifiers; - SolanaIngressTracking::open_channel( - election_identifiers, - &mut access_translator.translate_electoral_access(electoral_access), - channel, - asset, - close_block, - ) - }) + pallet_cf_elections::Pallet::::with_election_identifiers( + |composite_election_identifiers| { + SolanaElectoralSystemRunner::with_identifiers( + composite_election_identifiers, + |grouped_election_identifiers| { + let (_, _, election_identifiers, ..) = grouped_election_identifiers; + SolanaIngressTracking::open_channel::< + DerivedElectoralAccess< + _, + SolanaIngressTracking, + RunnerStorageAccess, + >, + >(election_identifiers, channel, asset, close_block) }, ) }, @@ -471,20 +443,15 @@ impl SolanaNonceWatch for SolanaNonceTrackingTrigger { nonce_account: SolAddress, previous_nonce_value: SolHash, ) -> DispatchResult { - pallet_cf_elections::Pallet::::with_electoral_access( - |electoral_access| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (_, _, _, access_translator, ..) = &access_translators; - let mut electoral_access = - access_translator.translate_electoral_access(electoral_access); - SolanaNonceTracking::watch_for_change( - &mut electoral_access, - nonce_account, - previous_nonce_value, - ) - }) - }, - ) + pallet_cf_elections::Pallet::::with_status_check(|| { + SolanaNonceTracking::watch_for_change::< + DerivedElectoralAccess< + _, + SolanaNonceTracking, + RunnerStorageAccess, + >, + >(nonce_account, previous_nonce_value) + }) } } @@ -494,16 +461,14 @@ impl ElectionEgressWitnesser for SolanaEgressWitnessingTrigger { type Chain = SolanaCrypto; fn watch_for_egress_success(signature: SolSignature) -> DispatchResult { - pallet_cf_elections::Pallet::::with_electoral_access( - |electoral_access| { - SolanaElectoralSystem::with_access_translators(|access_translators| { - let (_, _, _, _, access_translator, ..) = &access_translators; - let mut electoral_access = - access_translator.translate_electoral_access(electoral_access); - - SolanaEgressWitnessing::watch_for_egress(&mut electoral_access, signature) - }) - }, - ) + pallet_cf_elections::Pallet::::with_status_check(|| { + SolanaEgressWitnessing::watch_for_egress::< + DerivedElectoralAccess< + _, + SolanaEgressWitnessing, + RunnerStorageAccess, + >, + >(signature) + }) } } diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index c70a746158..7373f1d4f8 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -1000,7 +1000,7 @@ impl pallet_cf_chain_tracking::Config for Runtime { impl pallet_cf_elections::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type ElectoralSystem = chainflip::solana_elections::SolanaElectoralSystem; + type ElectoralSystemRunner = chainflip::solana_elections::SolanaElectoralSystemRunner; type WeightInfo = pallet_cf_elections::weights::PalletWeight; }