diff --git a/substrate/validator-sets/pallet/src/lib.rs b/substrate/validator-sets/pallet/src/lib.rs index c852c4ce..505782b8 100644 --- a/substrate/validator-sets/pallet/src/lib.rs +++ b/substrate/validator-sets/pallet/src/lib.rs @@ -643,8 +643,9 @@ pub mod pallet { // Checks if this session has completed the handover from the prior session. fn handover_completed(network: NetworkId, session: Session) -> bool { let Some(current_session) = Self::session(network) else { return false }; - // No handover occurs on genesis - if current_session.0 == 0 { + + // If the session we've been queried about is old, it must have completed its handover + if current_session.0 > session.0 { return true; } // If the session we've been queried about has yet to start, it can't have completed its @@ -652,19 +653,21 @@ pub mod pallet { if current_session.0 < session.0 { return false; } - if current_session.0 == session.0 { - // Handover is automatically complete for Serai as it doesn't have a handover protocol - // If not Serai, check the prior session had its keys cleared, which happens once its - // retired - return (network == NetworkId::Serai) || - (!Keys::::contains_key(ValidatorSet { - network, - session: Session(current_session.0 - 1), - })); + + // Handover is automatically complete for Serai as it doesn't have a handover protocol + if network == NetworkId::Serai { + return true; } - // We're currently in a future session, meaning this session definitely performed itself - // handover - true + + // The current session must have set keys for its handover to be completed + if !Keys::::contains_key(ValidatorSet { network, session }) { + return false; + } + + // This must be the first session (which has set keys) OR the prior session must have been + // retired (signified by its keys no longer being present) + (session.0 == 0) || + (!Keys::::contains_key(ValidatorSet { network, session: Session(session.0 - 1) })) } fn new_session() { @@ -682,6 +685,8 @@ pub mod pallet { } } + // TODO: This is called retire_set, yet just starts retiring the set + // Update the nomenclature within this function pub fn retire_set(set: ValidatorSet) { // If the prior prior set didn't report, emit they're retired now if PendingSlashReport::::get(set.network).is_some() { diff --git a/tests/coordinator/src/tests/key_gen.rs b/tests/coordinator/src/tests/key_gen.rs index 8b97132d..17bf43e7 100644 --- a/tests/coordinator/src/tests/key_gen.rs +++ b/tests/coordinator/src/tests/key_gen.rs @@ -10,7 +10,6 @@ use ciphersuite::{ group::{ff::Field, GroupEncoding}, Ciphersuite, Ristretto, Secp256k1, }; -use dkg::ThresholdParams; use serai_client::{ primitives::NetworkId, @@ -41,8 +40,8 @@ pub async fn key_gen( shares, }) => { assert_eq!(id, *this_id); - assert_eq!(params.t(), u16::try_from(((coordinators * 2) / 3) + 1).unwrap(),); - assert_eq!(params.n(), u16::try_from(coordinators).unwrap(),); + assert_eq!(params.t(), u16::try_from(((coordinators * 2) / 3) + 1).unwrap()); + assert_eq!(params.n(), u16::try_from(coordinators).unwrap()); assert_eq!(*shares, 1); participant_is.push(params.i()); break; diff --git a/tests/coordinator/src/tests/rotation.rs b/tests/coordinator/src/tests/rotation.rs index 3d3fc40b..4a8b95ff 100644 --- a/tests/coordinator/src/tests/rotation.rs +++ b/tests/coordinator/src/tests/rotation.rs @@ -83,46 +83,45 @@ async fn deallocate_stake( publish_tx(serai, &tx).await } -async fn wait_till_next_epoch(serai: &Serai, current_epoch: u32) -> Session { - let mut session = Session(current_epoch); - while session.0 < current_epoch + 1 { +async fn get_session(serai: &Serai, network: NetworkId) -> Session { + serai + .as_of_latest_finalized_block() + .await + .unwrap() + .validator_sets() + .session(network) + .await + .unwrap() + .unwrap() +} + +async fn wait_till_next_epoch(serai: &Serai) -> Session { + let starting_session = get_session(serai, NetworkId::Serai).await; + + let mut session = starting_session; + while session == starting_session { sleep(Duration::from_secs(6)).await; - session = serai - .as_of_latest_finalized_block() - .await - .unwrap() - .validator_sets() - .session(NetworkId::Serai) - .await - .unwrap() - .unwrap(); + session = get_session(serai, NetworkId::Serai).await; } session } -async fn get_session(serai: &Serai, block: [u8; 32], network: NetworkId) -> Session { - serai.as_of(block).validator_sets().session(network).await.unwrap().unwrap() -} - -async fn new_set_events( - serai: &Serai, - session: Session, - network: NetworkId, -) -> Vec { +async fn most_recent_new_set_event(serai: &Serai, network: NetworkId) -> ValidatorSetsEvent { let mut current_block = serai.latest_finalized_block().await.unwrap(); - let mut current_session = get_session(serai, current_block.hash(), network).await; - - while current_session == session { + loop { let events = serai.as_of(current_block.hash()).validator_sets().new_set_events().await.unwrap(); - if !events.is_empty() { - return events; + for event in events { + match event { + ValidatorSetsEvent::NewSet { set } => { + if set.network == network { + return event; + } + } + _ => panic!("new_set_events gave non-NewSet event: {event:?}"), + } } - current_block = serai.block(current_block.header.parent_hash.0).await.unwrap().unwrap(); - current_session = get_session(serai, current_block.hash(), network).await; } - - panic!("can't find the new set events for session: {} ", session.0); } #[tokio::test] @@ -133,30 +132,29 @@ async fn set_rotation_test() { let excluded = processors.pop().unwrap(); assert_eq!(processors.len(), COORDINATORS); - // genesis keygen - let _ = key_gen::(&mut processors, Session(0)).await; - let pair5 = insecure_pair_from_name("Eve"); let network = NetworkId::Bitcoin; let amount = Amount(1_000_000 * 10_u64.pow(8)); let serai = processors[0].serai().await; // add the last participant into validator set for btc network - let block = allocate_stake(&serai, network, amount, &pair5, 0).await; + allocate_stake(&serai, network, amount, &pair5, 0).await; + + // genesis keygen + let _ = key_gen::(&mut processors, Session(0)).await; // wait until next session to see the effect on coordinator - let current_epoch = get_session(&serai, block, NetworkId::Serai).await; - let session = wait_till_next_epoch(&serai, current_epoch.0).await; + wait_till_next_epoch(&serai).await; // verfiy that coordinator received new_set - let events = new_set_events(&serai, session, network).await; - assert!( - events.contains(&ValidatorSetsEvent::NewSet { set: ValidatorSet { session, network } }) + assert_eq!( + most_recent_new_set_event(&serai, network).await, + ValidatorSetsEvent::NewSet { set: ValidatorSet { session: Session(1), network } }, ); // add the last participant & do the keygen processors.push(excluded); - let _ = key_gen::(&mut processors, session).await; + let _ = key_gen::(&mut processors, Session(1)).await; }, true, )