From 2240a50a0cdce4f01fcca8d41f48dbf36cd8226b Mon Sep 17 00:00:00 2001
From: Luke Parker <lukeparker5132@gmail.com>
Date: Tue, 31 Dec 2024 17:17:12 -0500
Subject: [PATCH] Rebroadcast cosigns for the currently evaluated session, not
 the latest intended

If Substrate has a block 500 with a key gen, and a block 600 with a key gen,
and the session starting on 500 never cosigns everything, everyone up-to-date
will want the cosigns for the session starting on block 500. Everyone
up-to-date will also be rebroadcasting the non-existent cosigns for the session
which has yet to start. This wouldn't cause a stall as eventually, each
individual set would cosign the latest notable block, and then that would be
explicitly synced, but it's still not the intended behavior.

We also won't even intake the cosigns for the latest intended session if it
exceeds the session we're currently evaluating. This does mean those behind on
the cosigning protocol wouldn't have rebroadcasted their historical cosigns,
and now will, but that's valuable as we don't actually know if we're behind or
up-to-date (per above posited issue).
---
 coordinator/cosign/src/evaluator.rs | 17 +++++++++--------
 coordinator/cosign/src/lib.rs       |  6 ++----
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/coordinator/cosign/src/evaluator.rs b/coordinator/cosign/src/evaluator.rs
index fc606ecc..db286a4f 100644
--- a/coordinator/cosign/src/evaluator.rs
+++ b/coordinator/cosign/src/evaluator.rs
@@ -24,7 +24,7 @@ db_channel!(
 );
 
 // This is a strict function which won't panic, even with a malicious Serai node, so long as:
-// - It's called incrementally
+// - It's called incrementally (with an increment of 1)
 // - It's only called for block numbers we've completed indexing on within the intend task
 // - It's only called for block numbers after a global session has started
 // - The global sessions channel is populated as the block declaring the session is indexed
@@ -69,6 +69,10 @@ fn currently_evaluated_global_session_strict(
   res
 }
 
+pub(crate) fn currently_evaluated_global_session(getter: &impl Get) -> Option<[u8; 32]> {
+  CurrentlyEvaluatedGlobalSession::get(getter).map(|(id, _info)| id)
+}
+
 /// A task to determine if a block has been cosigned and we should handle it.
 pub(crate) struct CosignEvaluatorTask<D: Db, R: RequestNotableCosigns> {
   pub(crate) db: D,
@@ -87,13 +91,14 @@ impl<D: Db, R: RequestNotableCosigns> ContinuallyRan for CosignEvaluatorTask<D,
           break;
         };
 
+        // Fetch the global session information
+        let (global_session, global_session_info) =
+          currently_evaluated_global_session_strict(&mut txn, block_number);
+
         match has_events {
           // Because this had notable events, we require an explicit cosign for this block by a
           // supermajority of the prior block's validator sets
           HasEvents::Notable => {
-            let (global_session, global_session_info) =
-              currently_evaluated_global_session_strict(&mut txn, block_number);
-
             let mut weight_cosigned = 0;
             for set in global_session_info.sets {
               // Check if we have the cosign from this set
@@ -145,10 +150,6 @@ impl<D: Db, R: RequestNotableCosigns> ContinuallyRan for CosignEvaluatorTask<D,
                 is during the latest global session we've evaluated the start of.
               */
 
-              // Get the global session for this block
-              let (global_session, global_session_info) =
-                currently_evaluated_global_session_strict(&mut txn, block_number);
-
               let mut weight_cosigned = 0;
               let mut lowest_common_block: Option<u64> = None;
               for set in global_session_info.sets {
diff --git a/coordinator/cosign/src/lib.rs b/coordinator/cosign/src/lib.rs
index 076b56c1..6409b56f 100644
--- a/coordinator/cosign/src/lib.rs
+++ b/coordinator/cosign/src/lib.rs
@@ -308,14 +308,12 @@ impl<D: Db> Cosigning<D> {
       }
       cosigns
     } else {
-      let Some(latest_global_session) = LatestGlobalSessionIntended::get(&self.db) else {
+      let Some(global_session) = evaluator::currently_evaluated_global_session(&self.db) else {
         return vec![];
       };
       let mut cosigns = Vec::with_capacity(serai_client::primitives::NETWORKS.len());
       for network in serai_client::primitives::NETWORKS {
-        if let Some(cosign) =
-          NetworksLatestCosignedBlock::get(&self.db, latest_global_session, network)
-        {
+        if let Some(cosign) = NetworksLatestCosignedBlock::get(&self.db, global_session, network) {
           cosigns.push(cosign);
         }
       }