From c0474d2c9d46ec309026e9bf68d2259cefec14b4 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 10 Mar 2021 17:19:53 -0600 Subject: [PATCH] avoid reshuffling active validators (#2605) --- runtime/parachains/src/inclusion.rs | 12 ++--- runtime/parachains/src/mock.rs | 14 +++++- runtime/parachains/src/session_info.rs | 61 ++++++++++++++++++++++++-- runtime/parachains/src/shared.rs | 12 ++++- 4 files changed, 88 insertions(+), 11 deletions(-) diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 7430b5500c50..c3cf7067e0a9 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -1213,7 +1213,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); let signing_context = SigningContext { @@ -1446,7 +1446,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); let signing_context = SigningContext { @@ -1611,7 +1611,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); run_to_block(5, |_| None); @@ -2098,7 +2098,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); run_to_block(5, |_| None); @@ -2295,7 +2295,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); run_to_block(5, |_| None); @@ -2392,7 +2392,7 @@ mod tests { let validator_public = validator_pubkeys(&validators); new_test_ext(genesis_config(paras)).execute_with(|| { - shared::Module::::set_active_validators(validator_public.clone()); + shared::Module::::set_active_validators_ascending(validator_public.clone()); shared::Module::::set_session_index(5); let validators_new = vec![ diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 433f08813b56..ab22ced8d1b5 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -135,9 +135,21 @@ impl crate::inclusion_inherent::Config for Test { } impl crate::session_info::Config for Test { } +thread_local! { + pub static DISCOVERY_AUTHORITIES: RefCell> = RefCell::new(Vec::new()); +} + +pub fn discovery_authorities() -> Vec { + DISCOVERY_AUTHORITIES.with(|r| r.borrow().clone()) +} + +pub fn set_discovery_authorities(new: Vec) { + DISCOVERY_AUTHORITIES.with(|r| *r.borrow_mut() = new); +} + impl crate::session_info::AuthorityDiscoveryConfig for Test { fn authorities() -> Vec { - Vec::new() + discovery_authorities() } } diff --git a/runtime/parachains/src/session_info.rs b/runtime/parachains/src/session_info.rs index 321c2cd0bb1e..8cc5eaf90ec9 100644 --- a/runtime/parachains/src/session_info.rs +++ b/runtime/parachains/src/session_info.rs @@ -90,8 +90,8 @@ impl Module { let validators = notification.validators.clone(); let discovery_keys = ::authorities(); let assignment_keys = AssignmentKeysUnsafe::get(); - let active_set = >::active_validator_indices(); + let validator_groups = >::validator_groups(); let n_cores = n_parachains + config.parathread_cores; let zeroth_delay_tranche_width = config.zeroth_delay_tranche_width; @@ -118,7 +118,7 @@ impl Module { } // create a new entry in `Sessions` with information about the current session let new_session_info = SessionInfo { - validators: take_active_subset(&active_set, &validators), + validators, // these are from the notification and are thus already correct. discovery_keys: take_active_subset(&active_set, &discovery_keys), assignment_keys: take_active_subset(&active_set, &assignment_keys), validator_groups, @@ -175,7 +175,8 @@ mod tests { use crate::initializer::SessionChangeNotification; use crate::configuration::HostConfiguration; use frame_support::traits::{OnFinalize, OnInitialize}; - use primitives::v1::BlockNumber; + use primitives::v1::{BlockNumber, ValidatorId, ValidatorIndex}; + use keyring::Sr25519Keyring; fn run_to_block( to: BlockNumber, @@ -319,4 +320,58 @@ mod tests { assert_eq!(session.needed_approvals, 42); }) } + + #[test] + fn session_info_active_subsets() { + let unscrambled = vec![ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + Sr25519Keyring::Eve, + ]; + + let active_set = vec![ValidatorIndex(4), ValidatorIndex(0), ValidatorIndex(2)]; + + let unscrambled_validators: Vec + = unscrambled.iter().map(|v| v.public().into()).collect(); + let unscrambled_discovery: Vec + = unscrambled.iter().map(|v| v.public().into()).collect(); + let unscrambled_assignment: Vec + = unscrambled.iter().map(|v| v.public().into()).collect(); + + let validators = take_active_subset(&active_set, &unscrambled_validators); + + new_test_ext(genesis_config()).execute_with(|| { + Shared::set_active_validators_with_indices( + active_set.clone(), + validators.clone(), + ); + + assert_eq!(Shared::active_validator_indices(), active_set); + + AssignmentKeysUnsafe::set(unscrambled_assignment.clone()); + crate::mock::set_discovery_authorities(unscrambled_discovery.clone()); + assert_eq!(::authorities(), unscrambled_discovery); + + // invoke directly, because `run_to_block` will invoke `Shared` and clobber our + // values. + SessionInfo::initializer_on_new_session(&SessionChangeNotification { + session_index: 1, + validators: validators.clone(), + ..Default::default() + }); + let session = Sessions::get(&1).unwrap(); + + assert_eq!(session.validators, validators); + assert_eq!( + session.discovery_keys, + take_active_subset(&active_set, &unscrambled_discovery), + ); + assert_eq!( + session.assignment_keys, + take_active_subset(&active_set, &unscrambled_assignment), + ); + }) + } } diff --git a/runtime/parachains/src/shared.rs b/runtime/parachains/src/shared.rs index 3f89f17f5481..a2eb2548af1b 100644 --- a/runtime/parachains/src/shared.rs +++ b/runtime/parachains/src/shared.rs @@ -116,12 +116,22 @@ impl Module { } #[cfg(test)] - pub(crate) fn set_active_validators(active: Vec) { + pub(crate) fn set_active_validators_ascending(active: Vec) { ActiveValidatorIndices::set( (0..active.len()).map(|i| ValidatorIndex(i as _)).collect() ); ActiveValidatorKeys::set(active); } + + #[cfg(test)] + pub(crate) fn set_active_validators_with_indices( + indices: Vec, + keys: Vec, + ) { + assert_eq!(indices.len(), keys.len()); + ActiveValidatorIndices::set(indices); + ActiveValidatorKeys::set(keys); + } } #[cfg(test)]