From b60e3c25247076d793f9bf10af8cf66457f938f6 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 14 Dec 2023 15:53:41 -0500 Subject: [PATCH] Replace PSTTrait and PstTxType with PublishSeraiTransaction --- coordinator/src/tests/tributary/dkg.rs | 87 +++++---- coordinator/src/tests/tributary/mod.rs | 22 ++- coordinator/src/tributary/handle.rs | 39 ++-- coordinator/src/tributary/scanner.rs | 235 ++++++++++++++----------- 4 files changed, 210 insertions(+), 173 deletions(-) diff --git a/coordinator/src/tests/tributary/dkg.rs b/coordinator/src/tests/tributary/dkg.rs index 89d1558d..21792533 100644 --- a/coordinator/src/tests/tributary/dkg.rs +++ b/coordinator/src/tests/tributary/dkg.rs @@ -8,7 +8,10 @@ use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; use frost::Participant; use sp_runtime::traits::Verify; -use serai_client::validator_sets::primitives::{ValidatorSet, KeyPair}; +use serai_client::{ + primitives::{SeraiAddress, Signature}, + validator_sets::primitives::{ValidatorSet, KeyPair}, +}; use tokio::time::sleep; @@ -24,7 +27,7 @@ use tributary::{TransactionTrait, Tributary}; use crate::{ tributary::{ Transaction, TributarySpec, - scanner::{PstTxType, handle_new_blocks}, + scanner::{PublishSeraiTransaction, handle_new_blocks}, }, tests::{ MemProcessors, LocalP2p, @@ -104,7 +107,7 @@ async fn dkg_test() { panic!("provided TX caused recognized_id to be called in new_processors") }, &processors, - &|_, _, _| async { panic!("test tried to publish a new Serai TX in new_processors") }, + &(), &|_| async { panic!( "test tried to publish a new Tributary TX from handle_application_tx in new_processors" @@ -135,7 +138,7 @@ async fn dkg_test() { panic!("provided TX caused recognized_id to be called after Commitments") }, &processors, - &|_, _, _| async { panic!("test tried to publish a new Serai TX after Commitments") }, + &(), &|_| async { panic!( "test tried to publish a new Tributary TX from handle_application_tx after Commitments" @@ -221,7 +224,7 @@ async fn dkg_test() { panic!("provided TX caused recognized_id to be called after some shares") }, &processors, - &|_, _, _| async { panic!("test tried to publish a new Serai TX after some shares") }, + &(), &|_| async { panic!( "test tried to publish a new Tributary TX from handle_application_tx after some shares" @@ -273,7 +276,7 @@ async fn dkg_test() { key, &|_, _, _, _| async { panic!("provided TX caused recognized_id to be called after shares") }, &processors, - &|_, _, _| async { panic!("test tried to publish a new Serai TX") }, + &(), &|_| async { panic!("test tried to publish a new Tributary TX from handle_application_tx") }, &spec, &tributaries[i].1.reader(), @@ -338,6 +341,39 @@ async fn dkg_test() { wait_for_tx_inclusion(&tributaries[0].1, block_before_tx, tx.hash()).await; } + struct CheckPublishSetKeys { + spec: TributarySpec, + key_pair: KeyPair, + } + #[async_trait::async_trait] + impl PublishSeraiTransaction for CheckPublishSetKeys { + async fn publish_set_keys(&self, set: ValidatorSet, key_pair: KeyPair, signature: Signature) { + assert_eq!(set, self.spec.set()); + assert_eq!(self.key_pair, key_pair); + assert!(signature.verify( + &*serai_client::validator_sets::primitives::set_keys_message(&set, &key_pair), + &serai_client::Public( + frost::dkg::musig::musig_key::( + &serai_client::validator_sets::primitives::musig_context(set), + &self.spec.validators().into_iter().map(|(validator, _)| validator).collect::>() + ) + .unwrap() + .to_bytes() + ), + )); + } + + async fn publish_remove_participant( + &self, + set: ValidatorSet, + removing: [u8; 32], + signers: Vec, + signature: Signature, + ) { + ().publish_remove_participant(set, removing, signers, signature).await + } + } + // The scanner should successfully try to publish a transaction with a validly signed signature handle_new_blocks::<_, _, _, _, _, LocalP2p>( &mut dbs[0], @@ -346,44 +382,7 @@ async fn dkg_test() { panic!("provided TX caused recognized_id to be called after DKG confirmation") }, &processors, - &|set: ValidatorSet, tx_type, tx: serai_client::Transaction| { - assert_eq!(tx_type, PstTxType::SetKeys); - - let spec = spec.clone(); - let key_pair = key_pair.clone(); - async move { - assert_eq!(tx.signature, None); - match tx.call { - serai_client::abi::Call::ValidatorSets( - serai_client::abi::validator_sets::Call::set_keys { - network, - key_pair: set_key_pair, - signature, - }, - ) => { - assert_eq!(set, spec.set()); - assert_eq!(set.network, network); - assert_eq!(key_pair, set_key_pair); - assert!(signature.verify( - &*serai_client::validator_sets::primitives::set_keys_message(&set, &key_pair), - &serai_client::Public( - frost::dkg::musig::musig_key::( - &serai_client::validator_sets::primitives::musig_context(set), - &spec - .validators() - .into_iter() - .map(|(validator, _)| validator) - .collect::>() - ) - .unwrap() - .to_bytes() - ), - )); - } - _ => panic!("Serai TX wasn't to set_keys"), - } - } - }, + &CheckPublishSetKeys { spec: spec.clone(), key_pair: key_pair.clone() }, &|_| async { panic!("test tried to publish a new Tributary TX from handle_application_tx") }, &spec, &tributaries[0].1.reader(), diff --git a/coordinator/src/tests/tributary/mod.rs b/coordinator/src/tests/tributary/mod.rs index 84c22343..3f84378f 100644 --- a/coordinator/src/tests/tributary/mod.rs +++ b/coordinator/src/tests/tributary/mod.rs @@ -3,11 +3,15 @@ use core::fmt::Debug; use rand_core::{RngCore, OsRng}; use scale::{Encode, Decode}; +use serai_client::{ + primitives::{SeraiAddress, Signature}, + validator_sets::primitives::{ValidatorSet, KeyPair}, +}; use processor_messages::coordinator::SubstrateSignableId; use tributary::{ReadWrite, tests::random_signed_with_nonce}; -use crate::tributary::{Label, SignData, Transaction}; +use crate::tributary::{Label, SignData, Transaction, scanner::PublishSeraiTransaction}; mod chain; pub use chain::*; @@ -20,6 +24,22 @@ mod dkg; mod handle_p2p; mod sync; +#[async_trait::async_trait] +impl PublishSeraiTransaction for () { + async fn publish_set_keys(&self, _set: ValidatorSet, _key_pair: KeyPair, _signature: Signature) { + panic!("publish_set_keys was called in test") + } + async fn publish_remove_participant( + &self, + _set: ValidatorSet, + _removing: [u8; 32], + _signers: Vec, + _signature: Signature, + ) { + panic!("publish_remove_participant was called in test") + } +} + fn random_u32(rng: &mut R) -> u32 { u32::try_from(rng.next_u64() >> 32).unwrap() } diff --git a/coordinator/src/tributary/handle.rs b/coordinator/src/tributary/handle.rs index 4ab63066..e02cda19 100644 --- a/coordinator/src/tributary/handle.rs +++ b/coordinator/src/tributary/handle.rs @@ -9,9 +9,7 @@ use ciphersuite::{group::GroupEncoding, Ciphersuite, Ristretto}; use frost::dkg::Participant; use scale::{Encode, Decode}; -use serai_client::{ - Public, SeraiAddress, Signature, validator_sets::primitives::KeyPair, SeraiValidatorSets, -}; +use serai_client::{Public, Signature, validator_sets::primitives::KeyPair}; use tributary::{Signed, TransactionKind, TransactionTrait}; @@ -28,7 +26,9 @@ use crate::{ tributary::{ *, signing_protocol::{DkgConfirmer, DkgRemoval}, - scanner::{RecognizedIdType, RIDTrait, PstTxType, PSTTrait, PTTTrait, TributaryBlockHandler}, + scanner::{ + RecognizedIdType, RIDTrait, PublishSeraiTransaction, PTTTrait, TributaryBlockHandler, + }, }, P2p, }; @@ -106,8 +106,14 @@ fn unflatten( } } -impl - TributaryBlockHandler<'_, T, Pro, PST, PTT, RID, P> +impl< + T: DbTxn, + Pro: Processors, + PST: PublishSeraiTransaction, + PTT: PTTTrait, + RID: RIDTrait, + P: P2p, + > TributaryBlockHandler<'_, T, Pro, PST, PTT, RID, P> { fn accumulate( &mut self, @@ -576,14 +582,7 @@ impl { panic!("wasn't a participant in DKG confirmination shares") @@ -671,16 +670,14 @@ impl, + signature: Signature, ); } -#[async_trait::async_trait] -impl< - FPst: Send + Future, - F: Sync + Fn(ValidatorSet, PstTxType, serai_client::Transaction) -> FPst, - > PSTTrait for F -{ - async fn publish_serai_tx( - &self, - set: ValidatorSet, - kind: PstTxType, - tx: serai_client::Transaction, - ) { - (self)(set, kind, tx).await + +mod impl_pst_for_serai { + use super::*; + + use serai_client::SeraiValidatorSets; + + // Uses a macro because Rust can't resolve the lifetimes/generics around the check function + // check is expected to return true if the effect has already occurred + // The generated publish function will return true if *we* published the transaction + macro_rules! common_pst { + ($Meta: ty, $check: ident) => { + async fn publish( + serai: &Serai, + set: ValidatorSet, + tx: serai_client::Transaction, + meta: $Meta, + ) -> bool { + loop { + match serai.publish(&tx).await { + Ok(_) => return true, + // This is assumed to be some ephemeral error due to the assumed fault-free + // creation + // TODO2: Differentiate connection errors from invariants + Err(e) => { + if let Ok(serai) = serai.as_of_latest_finalized_block().await { + let serai = serai.validator_sets(); + + // The following block is irrelevant, and can/likely will fail, if we're publishing + // a TX for an old session + // If we're on a newer session, move on + if let Ok(Some(current_session)) = serai.session(set.network).await { + if current_session.0 > set.session.0 { + log::warn!( + "trying to publish a TX relevant to set {set:?} which isn't the latest" + ); + return false; + } + } + + // Check if someone else published the TX in question + if $check(serai, set, meta).await { + return false; + } + + log::error!("couldn't connect to Serai node to publish TX: {e:?}"); + tokio::time::sleep(core::time::Duration::from_secs(5)).await; + } + } + } + } + } + }; + } + + #[async_trait::async_trait] + impl PublishSeraiTransaction for Serai { + async fn publish_set_keys(&self, set: ValidatorSet, key_pair: KeyPair, signature: Signature) { + let tx = SeraiValidatorSets::set_keys(set.network, key_pair, signature); + async fn check(serai: SeraiValidatorSets<'_>, set: ValidatorSet, _: ()) -> bool { + if matches!(serai.keys(set).await, Ok(Some(_))) { + log::info!("another coordinator set key pair for {:?}", set); + return true; + } + false + } + common_pst!((), check); + if publish(self, set, tx, ()).await { + log::info!("published set keys for {set:?}"); + } + } + + async fn publish_remove_participant( + &self, + set: ValidatorSet, + removing: [u8; 32], + signers: Vec, + signature: Signature, + ) { + let tx = SeraiValidatorSets::remove_participant( + set.network, + SeraiAddress(removing), + signers, + signature, + ); + async fn check(serai: SeraiValidatorSets<'_>, set: ValidatorSet, removing: [u8; 32]) -> bool { + if let Ok(Some(_)) = serai.keys(set).await { + log::info!( + "keys were set before we personally could publish the removal for {}", + hex::encode(removing) + ); + return true; + } + + if let Ok(Some(participants)) = serai.participants(set.network).await { + if !participants.iter().any(|(participant, _)| participant.0 == removing) { + log::info!("another coordinator published removal for {:?}", hex::encode(removing),); + return true; + } + } + false + } + common_pst!([u8; 32], check); + if publish(self, set, tx, removing).await { + log::info!("published remove participant for {}", hex::encode(removing)); + } + } } } @@ -113,7 +205,7 @@ pub struct TributaryBlockHandler< 'a, T: DbTxn, Pro: Processors, - PST: PSTTrait, + PST: PublishSeraiTransaction, PTT: PTTTrait, RID: RIDTrait, P: P2p, @@ -130,8 +222,14 @@ pub struct TributaryBlockHandler< _p2p: PhantomData

, } -impl - TributaryBlockHandler<'_, T, Pro, PST, PTT, RID, P> +impl< + T: DbTxn, + Pro: Processors, + PST: PublishSeraiTransaction, + PTT: PTTTrait, + RID: RIDTrait, + P: P2p, + > TributaryBlockHandler<'_, T, Pro, PST, PTT, RID, P> { async fn attempt_dkg_removal(&mut self, removing: [u8; 32], attempt: u32) { let genesis = self.spec.genesis(); @@ -391,7 +489,7 @@ impl { - log::info!("set key pair for {set:?}"); - break; - } - // This is assumed to be some ephemeral error due to the assumed fault-free - // creation - // TODO2: Differentiate connection errors from invariants - Err(e) => { - if let Ok(serai) = serai.as_of_latest_finalized_block().await { - let serai = serai.validator_sets(); - - // The following block is irrelevant, and can/likely will fail, if - // we're publishing a TX for an old session - // If we're on a newer session, move on - if let Ok(Some(current_session)) = - serai.session(spec.set().network).await - { - if current_session.0 > spec.set().session.0 { - log::warn!( - "trying to publish a TX relevant to a set {} {:?}", - "which isn't the latest", - set - ); - break; - } - } - - // Check if someone else published the TX in question - match tx_type { - PstTxType::SetKeys => { - if matches!(serai.keys(spec.set()).await, Ok(Some(_))) { - log::info!("another coordinator set key pair for {:?}", set); - break; - } - } - PstTxType::RemoveParticipant(removed) => { - if let Ok(Some(_)) = serai.keys(spec.set()).await { - log::info!( - "keys were set before we {} {:?}", - "personally could publish the removal for", - hex::encode(removed) - ); - break; - } - - if let Ok(Some(participants)) = - serai.participants(spec.set().network).await - { - if !participants - .iter() - .any(|(participant, _)| participant.0 == removed) - { - log::info!( - "another coordinator published removal for {:?}", - hex::encode(removed) - ); - break; - } - } - } - } - } - - log::error!( - "couldn't connect to Serai node to publish {tx_type:?} TX: {:?}", - e - ); - tokio::time::sleep(core::time::Duration::from_secs(10)).await; - } - } - } - } - }, + &*serai, &|tx: Transaction| { let tributary = tributary.clone(); async move {