Prevent a malicious validator set from overwriting a notable cosign

Also prevents panics from an invalid Serai node (removing the assumption of an
honest Serai node).

coordinator/cosign/src/evaluator.rs

@@ -74,9 +74,11 @@ impl<D: Db, R: RequestNotableCosigns> ContinuallyRan for CosignEvaluatorTask<D,
 
             let mut weight_cosigned = 0;
             let mut total_weight = 0;
-            let (_, global_session_start_block) = GlobalSessions::get(&txn, global_session).expect(
-              "checking if intended cosign was satisfied within an unrecognized global session",
-            );
+            let (_, global_session_start_block) = GlobalSessions::get(&txn, global_session)
+              .ok_or_else(|| {
+                "checking if intended cosign was satisfied within an unrecognized global session"
+                  .to_string()
+              })?;
             for set in sets {
               // Fetch the weight for this set, as of the start of the global session
               // This simplifies the logic around which set of stakes to use when evaluating
@@ -148,9 +150,10 @@ impl<D: Db, R: RequestNotableCosigns> ContinuallyRan for CosignEvaluatorTask<D,
               let (global_session, sets) =
                 get_latest_global_session_evaluated(&mut txn, &self.serai, parent_hash).await?;
               let (_, global_session_start_block) = GlobalSessions::get(&txn, global_session)
-                .expect(
-                  "checking if intended cosign was satisfied within an unrecognized global session",
-                );
+                .ok_or_else(|| {
+                  "checking if intended cosign was satisfied within an unrecognized global session"
+                    .to_string()
+                })?;
 
               let mut weight_cosigned = 0;
               let mut total_weight = 0;

coordinator/cosign/src/intend.rs

@@ -1,6 +1,6 @@
 use core::future::Future;
 
-use serai_client::{SeraiError, Serai, validator_sets::primitives::ValidatorSet};
+use serai_client::{Serai, validator_sets::primitives::ValidatorSet};
 
 use serai_db::*;
 use serai_task::ContinuallyRan;
@@ -31,18 +31,19 @@ db_channel! {
 async fn block_has_events_justifying_a_cosign(
   serai: &Serai,
   block_number: u64,
-) -> Result<(Block, HasEvents), SeraiError> {
+) -> Result<(Block, HasEvents), String> {
   let block = serai
     .finalized_block_by_number(block_number)
-    .await?
-    .expect("couldn't get block which should've been finalized");
+    .await
+    .map_err(|e| format!("{e:?}"))?
+    .ok_or_else(|| "couldn't get block which should've been finalized".to_string())?;
   let serai = serai.as_of(block.hash());
 
-  if !serai.validator_sets().key_gen_events().await?.is_empty() {
+  if !serai.validator_sets().key_gen_events().await.map_err(|e| format!("{e:?}"))?.is_empty() {
     return Ok((block, HasEvents::Notable));
   }
 
-  if !serai.coins().burn_with_instruction_events().await?.is_empty() {
+  if !serai.coins().burn_with_instruction_events().await.map_err(|e| format!("{e:?}"))?.is_empty() {
     return Ok((block, HasEvents::NonNotable));
   }
 
@@ -79,7 +80,10 @@ impl<D: Db> ContinuallyRan for CosignIntendTask<D> {
             if has_events == HasEvents::Notable {
               let sets = cosigning_sets(&self.serai.as_of(block.hash())).await?;
               let global_session = GlobalSession::new(sets).id();
-              GlobalSessions::set(&mut txn, global_session, &(block.number(), block.hash()));
+              GlobalSessions::set(&mut txn, global_session, &(block_number, block.hash()));
+              if let Some(ending_global_session) = LatestGlobalSessionIntended::get(&txn) {
+                GlobalSessionLastBlock::set(&mut txn, ending_global_session, &block_number);
+              }
               LatestGlobalSessionIntended::set(&mut txn, &global_session);
             }
 

coordinator/cosign/src/lib.rs

@@ -62,6 +62,8 @@ create_db! {
   Cosign {
     // A mapping from a global session's ID to its start block (number, hash).
     GlobalSessions: (global_session: [u8; 32]) -> (u64, [u8; 32]),
+    // The last block to be cosigned by a global session.
+    GlobalSessionLastBlock: (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.
@@ -295,7 +297,7 @@ impl<D: Db> Cosigning<D> {
   /// cosigns, in case of a fault, to induce identification of the fault by others.
   pub fn cosigns_to_rebroadcast(&self) -> Vec<SignedCosign> {
     if let Some(faulted) = FaultedSession::get(&self.db) {
-      let mut cosigns = Faults::get(&self.db, faulted).unwrap();
+      let mut cosigns = Faults::get(&self.db, faulted).expect("faulted with no faults");
       // Also include all of our recognized-as-honest cosigns in an attempt to induce fault
       // identification in those who see the faulty cosigns as honest
       for network in serai_client::primitives::NETWORKS {
@@ -345,19 +347,22 @@ impl<D: Db> Cosigning<D> {
       return Ok(false);
     };
 
-    // Check if we should even bother handling this cosign
-    let mut txn = self.db.txn();
-    // TODO: A malicious validator set can sign a block after their notable block to erase a
-    // notable cosign
-    if let Some(existing) = NetworksLatestCosignedBlock::get(&txn, cosign.global_session, network) {
+    // Check this isn't a dated cosign
+    if let Some(existing) =
+      NetworksLatestCosignedBlock::get(&self.db, cosign.global_session, network)
+    {
       if existing.cosign.block_number >= cosign.block_number {
         return Ok(true);
       }
     }
 
-    // Check we can verify this cosign's signature
+    // Check our finalized (and indexed by intend) blockchain exceeds this block number
+    if cosign.block_number >= intend::ScanCosignFrom::get(&self.db).unwrap_or(0) {
+      return Ok(true);
+    }
+
     let Some((global_session_start_block_number, global_session_start_block_hash)) =
-      GlobalSessions::get(&txn, cosign.global_session)
+      GlobalSessions::get(&self.db, cosign.global_session)
     else {
       // Unrecognized global session
       return Ok(true);
@@ -366,6 +371,10 @@ impl<D: Db> Cosigning<D> {
       // Cosign is for a block predating the global session
       return Ok(false);
     }
+    if Some(cosign.block_number) > GlobalSessionLastBlock::get(&self.db, cosign.global_session) {
+      // Cosign is for a block after the last block this global session should have signed
+      return Ok(false);
+    }
 
     // Check the cosign's signature
     {
@@ -388,23 +397,17 @@ impl<D: Db> Cosigning<D> {
       }
     }
 
-    // Check our finalized blockchain exceeds this block number
-    if self.serai.latest_finalized_block().await.map_err(|e| format!("{e:?}"))?.number() <
-      cosign.block_number
-    {
-      // Unrecognized block number
-      return Ok(true);
-    }
-
     // Since we verified this cosign's signature, and have a chain sufficiently long, handle the
     // cosign
 
+    let mut txn = self.db.txn();
+
     let our_block_hash = self
       .serai
       .block_hash(cosign.block_number)
       .await
       .map_err(|e| format!("{e:?}"))?
-      .expect("requested hash of a finalized block yet received None");
+      .ok_or_else(|| "requested hash of a finalized block yet received None".to_string())?;
     if our_block_hash == cosign.block_hash {
       // If this is for a future global session, we don't acknowledge this cosign at this time
       if global_session_start_block_number > LatestCosignedBlockNumber::get(&txn).unwrap_or(0) {
@@ -445,10 +448,6 @@ impl<D: Db> Cosigning<D> {
         }
 
         // Check if the sum weight means a fault has occurred
-        assert!(
-          total_weight != 0,
-          "evaluating valid cosign when no stake was present in the system"
-        );
         if weight_cosigned >= ((total_weight * 17) / 100) {
           FaultedSession::set(&mut txn, &cosign.global_session);
         }