From cef5bc95b0e7b0e4f85d04feefa146cc03cbe6c8 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 26 Dec 2024 00:15:49 -0500 Subject: [PATCH] Revert prior commit An archive of all GlobalSessions is necessary to check for faults. The storage cost is also minimal. While it should be avoided if it can be, it can't be here. --- coordinator/cosign/src/evaluator.rs | 27 +++------------- coordinator/cosign/src/intend.rs | 19 ++++++------ coordinator/cosign/src/lib.rs | 48 ++++++++++++++++------------- 3 files changed, 40 insertions(+), 54 deletions(-) diff --git a/coordinator/cosign/src/evaluator.rs b/coordinator/cosign/src/evaluator.rs index 8f72b536..91f92b44 100644 --- a/coordinator/cosign/src/evaluator.rs +++ b/coordinator/cosign/src/evaluator.rs @@ -5,7 +5,7 @@ use serai_task::ContinuallyRan; use crate::{ HasEvents, GlobalSession, NetworksLatestCosignedBlock, RequestNotableCosigns, - intend::{GlobalSessions, BlockEventData, BlockEvents}, + intend::{GlobalSessionsChannel, BlockEventData, BlockEvents}, }; create_db!( @@ -34,8 +34,8 @@ fn currently_evaluated_global_session_strict( let existing = match CurrentlyEvaluatedGlobalSession::get(txn) { Some(existing) => existing, None => { - let first = - GlobalSessions::try_recv(txn).expect("fetching latest global session yet none declared"); + let first = GlobalSessionsChannel::try_recv(txn) + .expect("fetching latest global session yet none declared"); CurrentlyEvaluatedGlobalSession::set(txn, &first); first } @@ -47,14 +47,14 @@ fn currently_evaluated_global_session_strict( existing }; - if let Some(next) = GlobalSessions::peek(txn) { + if let Some(next) = GlobalSessionsChannel::peek(txn) { assert!( block_number <= next.1.start_block_number, "currently_evaluated_global_session_strict wasn't called incrementally" ); // If it's time for this session to activate, take it from the channel and set it if block_number == next.1.start_block_number { - GlobalSessions::try_recv(txn).unwrap(); + GlobalSessionsChannel::try_recv(txn).unwrap(); CurrentlyEvaluatedGlobalSession::set(txn, &next); res = next; } @@ -63,23 +63,6 @@ fn currently_evaluated_global_session_strict( res } -// This is a non-strict function which won't panic, and also won't increment the session as needed. -pub(crate) fn currently_evaluated_global_session( - getter: &impl Get, -) -> Option<([u8; 32], GlobalSession)> { - // If there's a next session... - if let Some(next_global_session) = GlobalSessions::peek(getter) { - // and we've already evaluated the cosigns for the block declaring it... - if LatestCosignedBlockNumber::get(getter) == Some(next_global_session.1.start_block_number - 1) - { - // return it as the current session. - return Some(next_global_session); - } - } - // Else, return the current session - CurrentlyEvaluatedGlobalSession::get(getter) -} - /// A task to determine if a block has been cosigned and we should handle it. pub(crate) struct CosignEvaluatorTask { pub(crate) db: D, diff --git a/coordinator/cosign/src/intend.rs b/coordinator/cosign/src/intend.rs index 38d8b9a8..7466ae5a 100644 --- a/coordinator/cosign/src/intend.rs +++ b/coordinator/cosign/src/intend.rs @@ -26,7 +26,7 @@ pub(crate) struct BlockEventData { db_channel! { CosignIntendChannels { - GlobalSessions: () -> ([u8; 32], GlobalSession), + GlobalSessionsChannel: () -> ([u8; 32], GlobalSession), BlockEvents: () -> BlockEventData, IntendedCosigns: (set: ValidatorSet) -> CosignIntent, } @@ -128,14 +128,12 @@ impl ContinuallyRan for CosignIntendTask { stakes, total_stake, }; - if let Some((ending_global_session, _ending_global_session_info)) = global_session_for_this_block { + GlobalSessions::set(&mut txn, global_session, &global_session_info); + if let Some(ending_global_session) = global_session_for_this_block { GlobalSessionsLastBlock::set(&mut txn, ending_global_session, &block_number); } - LatestGlobalSessionIntended::set( - &mut txn, - &(global_session, global_session_info.clone()), - ); - GlobalSessions::send(&mut txn, &(global_session, global_session_info)); + LatestGlobalSessionIntended::set(&mut txn, &global_session); + GlobalSessionsChannel::send(&mut txn, &(global_session, global_session_info)); } // If there isn't anyone available to cosign this block, meaning it'll never be cosigned, @@ -147,9 +145,10 @@ impl ContinuallyRan for CosignIntendTask { match has_events { HasEvents::Notable | HasEvents::NonNotable => { - let (global_session_for_this_block, global_session_info) = - global_session_for_this_block - .expect("global session for this block was None but still attempting to cosign it"); + let global_session_for_this_block = global_session_for_this_block + .expect("global session for this block was None but still attempting to cosign it"); + let global_session_info = GlobalSessions::get(&txn, global_session_for_this_block) + .expect("last global session intended wasn't saved to the database"); // Tell each set of their expectation to cosign this block for set in global_session_info.sets { diff --git a/coordinator/cosign/src/lib.rs b/coordinator/cosign/src/lib.rs index 3ef802e6..aacd4c96 100644 --- a/coordinator/cosign/src/lib.rs +++ b/coordinator/cosign/src/lib.rs @@ -45,7 +45,7 @@ pub const COSIGN_CONTEXT: &[u8] = b"serai-cosign"; have validator sets follow two distinct global sessions without breaking the bounds of the cosigning protocol. */ -#[derive(Clone, Debug, BorshSerialize, BorshDeserialize)] +#[derive(Debug, BorshSerialize, BorshDeserialize)] pub(crate) struct GlobalSession { pub(crate) start_block_number: u64, pub(crate) sets: Vec, @@ -66,12 +66,14 @@ create_db! { // An index of Substrate blocks SubstrateBlocks: (block_number: u64) -> [u8; 32], + // A mapping from a global session's ID to its relevant information. + GlobalSessions: (global_session: [u8; 32]) -> GlobalSession, // The last block to be cosigned by a global session. GlobalSessionsLastBlock: (global_session: [u8; 32]) -> u64, // The latest global session intended. // // This is distinct from the latest global session for which we've evaluated the cosigns for. - LatestGlobalSessionIntended: () -> ([u8; 32], GlobalSession), + LatestGlobalSessionIntended: () -> [u8; 32], // The following are managed by the `intake_cosign` function present in this file @@ -285,9 +287,7 @@ impl Cosigning { } cosigns } else { - let Some((latest_global_session, _latest_global_session_info)) = - LatestGlobalSessionIntended::get(&self.db) - else { + let Some(latest_global_session) = LatestGlobalSessionIntended::get(&self.db) else { return vec![]; }; let mut cosigns = Vec::with_capacity(serai_client::primitives::NETWORKS.len()); @@ -320,6 +320,12 @@ impl Cosigning { let cosign = &signed_cosign.cosign; let network = cosign.cosigner; + // Check our indexed blockchain includes a block with this block number + let Some(our_block_hash) = SubstrateBlocks::get(&self.db, cosign.block_number) else { + return Ok(true); + }; + let faulty = our_block_hash == cosign.block_hash; + // Check this isn't a dated cosign within its global session (as it would be if rebroadcasted) if let Some(existing) = NetworksLatestCosignedBlock::get(&self.db, cosign.global_session, network) @@ -329,24 +335,13 @@ impl Cosigning { } } - // Check our indexed blockchain includes a block with this block number - let Some(our_block_hash) = SubstrateBlocks::get(&self.db, cosign.block_number) else { + let Some(global_session) = GlobalSessions::get(&self.db, cosign.global_session) else { + // Unrecognized global session return Ok(true); }; - // Check the cosign aligns with the global session we're currently working on - let Some((global_session, global_session_info)) = - evaluator::currently_evaluated_global_session(&self.db) - else { - // We haven't recognized any global sessions yet - return Ok(true); - }; - if cosign.global_session != global_session { - return Ok(true); - } - // Check the cosigned block number is in range to the global session - if cosign.block_number < global_session_info.start_block_number { + if cosign.block_number < global_session.start_block_number { // Cosign is for a block predating the global session return Ok(false); } @@ -360,7 +355,7 @@ impl Cosigning { // Check the cosign's signature { let key = Public::from({ - let Some(key) = global_session_info.keys.get(&network) else { + let Some(key) = global_session.keys.get(&network) else { return Ok(false); }; *key @@ -377,6 +372,15 @@ impl Cosigning { let mut txn = self.db.txn(); if our_block_hash == cosign.block_hash { + // If this is for a future global session, we don't acknowledge this cosign at this time + let latest_cosigned_block_number = LatestCosignedBlockNumber::get(&txn).unwrap_or(0); + // This global session starts the block *after* its declaration, so we want to check if the + // block declaring it was cosigned + if (global_session.start_block_number - 1) > latest_cosigned_block_number { + drop(txn); + return Ok(true); + } + NetworksLatestCosignedBlock::set(&mut txn, cosign.global_session, network, signed_cosign); } else { let mut faults = Faults::get(&txn, cosign.global_session).unwrap_or(vec![]); @@ -387,14 +391,14 @@ impl Cosigning { let mut weight_cosigned = 0; for fault in &faults { - let Some(stake) = global_session_info.stakes.get(&fault.cosign.cosigner) else { + let Some(stake) = global_session.stakes.get(&fault.cosign.cosigner) else { Err("cosigner with recognized key didn't have a stake entry saved".to_string())? }; weight_cosigned += stake; } // Check if the sum weight means a fault has occurred - if weight_cosigned >= ((global_session_info.total_stake * 17) / 100) { + if weight_cosigned >= ((global_session.total_stake * 17) / 100) { FaultedSession::set(&mut txn, &cosign.global_session); } }