Remove needless transposition in coordinator

coordinator/src/cosign_evaluator.rs

@@ -257,7 +257,6 @@ impl<D: Db> CosignEvaluator<D> {
       }
     } else {
       let mut latest_cosigns = self.latest_cosigns.write().await;
-
       latest_cosigns.insert(cosign.network, (block.number(), cosign));
       self.update_latest_cosign().await;
     }

coordinator/src/main.rs

@@ -431,24 +431,19 @@ async fn handle_processor_message<D: Db, P: P2p>(
             .i(pub_key)
             .expect("processor message to DKG for a session we aren't a validator in");
 
-          // TODO: This is [receiver_share][sender_share] and is later transposed to
-          // [sender_share][receiver_share]. Make this [sender_share][receiver_share] from the
-          // start?
           // `tx_shares` needs to be done here as while it can be serialized from the HashMap
           // without further context, it can't be deserialized without context
           let mut tx_shares = Vec::with_capacity(shares.len());
-          for i in 1 ..= spec.n() {
-            let i = Participant::new(i).unwrap();
-            if our_i.contains(&i) {
-              for shares in &shares {
+          for shares in &mut shares {
+            tx_shares.push(vec![]);
+            for i in 1 ..= spec.n() {
+              let i = Participant::new(i).unwrap();
+              if our_i.contains(&i) {
                 if shares.contains_key(&i) {
                   panic!("processor sent us our own shares");
                 }
+                continue;
               }
-              continue;
-            }
-            tx_shares.push(vec![]);
-            for shares in &mut shares {
               tx_shares.last_mut().unwrap().push(
                 shares.remove(&i).expect("processor didn't send share for another validator"),
               );

coordinator/src/substrate/mod.rs

@@ -456,15 +456,11 @@ async fn handle_new_blocks<D: Db, Pro: Processors>(
     let maximally_latent_cosign_block =
       skipped_block.map(|skipped_block| skipped_block + COSIGN_DISTANCE);
     for block in (last_intended_to_cosign_block + 1) ..= latest_number {
-      SeraiBlockNumber::set(
-        &mut txn,
-        serai
-          .block_by_number(block)
-          .await?
-          .expect("couldn't get block which should've been finalized")
-          .hash(),
-        &block,
-      );
+      let actual_block = serai
+        .block_by_number(block)
+        .await?
+        .expect("couldn't get block which should've been finalized");
+      SeraiBlockNumber::set(&mut txn, actual_block.hash(), &block);
 
       let mut set = false;
 
@@ -494,10 +490,6 @@ async fn handle_new_blocks<D: Db, Pro: Processors>(
         // Get the keys as of the prior block
         // That means if this block is setting new keys (which won't lock in until we process this
         // block), we won't freeze up waiting for the yet-to-be-processed keys to sign this block
-        let actual_block = serai
-          .block_by_number(block)
-          .await?
-          .expect("couldn't get block which should've been finalized");
         let serai = serai.as_of(actual_block.header().parent_hash.into());
 
         has_no_cosigners = Some(actual_block.clone());

coordinator/src/tests/tributary/dkg.rs

@@ -161,12 +161,12 @@ async fn dkg_test() {
   for (k, key) in keys.iter().enumerate() {
     let attempt = 0;
 
-    let mut shares = vec![];
+    let mut shares = vec![vec![]];
     for i in 0 .. keys.len() {
       if i != k {
         let mut share = vec![0; 256];
         OsRng.fill_bytes(&mut share);
-        shares.push(vec![share]);
+        shares.last_mut().unwrap().push(share);
       }
     }
 
@@ -225,7 +225,7 @@ async fn dkg_test() {
               let relative_i = i - (if i > l { 1 } else { 0 });
               Some((
                 Participant::new((l + 1).try_into().unwrap()).unwrap(),
-                shares[relative_i][0].clone(),
+                shares[0][relative_i].clone(),
               ))
             }
           } else {

coordinator/src/tributary/handle.rs

@@ -279,38 +279,39 @@ pub(crate) async fn handle_application_tx<
         .expect("transaction added to tributary by signer who isn't a participant");
       let sender_is_len = u16::from(sender_i.end) - u16::from(sender_i.start);
 
-      if shares.len() != (usize::from(spec.n() - sender_is_len)) {
-        fatal_slash::<D>(txn, genesis, signed.signer.to_bytes(), "invalid amount of DKG shares");
+      if shares.len() != usize::from(sender_is_len) {
+        fatal_slash::<D>(
+          txn,
+          genesis,
+          signed.signer.to_bytes(),
+          "invalid amount of DKG shares by key shares",
+        );
         return;
       }
       for shares in &shares {
-        if shares.len() != usize::from(sender_is_len) {
-          fatal_slash::<D>(
-            txn,
-            genesis,
-            signed.signer.to_bytes(),
-            "invalid amount of DKG shares by key shares",
-          );
+        if shares.len() != (usize::from(spec.n() - sender_is_len)) {
+          fatal_slash::<D>(txn, genesis, signed.signer.to_bytes(), "invalid amount of DKG shares");
           return;
         }
       }
 
       // Save each share as needed for blame
       {
-        let from = spec.i(signed.signer).unwrap();
-        for (to, shares) in shares.iter().enumerate() {
-          // 0-indexed (the enumeration) to 1-indexed (Participant)
-          let mut to = u16::try_from(to).unwrap() + 1;
-          // Adjust for the omission of the sender's own shares
-          if to >= u16::from(from.start) {
-            to += u16::from(from.end) - u16::from(from.start);
-          }
-          let to = Participant::new(to).unwrap();
+        let from_range = spec.i(signed.signer).unwrap();
+        for (from_offset, shares) in shares.iter().enumerate() {
+          let from =
+            Participant::new(u16::from(from_range.start) + u16::try_from(from_offset).unwrap())
+              .unwrap();
+
+          for (to_offset, share) in shares.iter().enumerate() {
+            // 0-indexed (the enumeration) to 1-indexed (Participant)
+            let mut to = u16::try_from(to_offset).unwrap() + 1;
+            // Adjust for the omission of the sender's own shares
+            if to >= u16::from(from_range.start) {
+              to += u16::from(from_range.end) - u16::from(from_range.start);
+            }
+            let to = Participant::new(to).unwrap();
 
-          for (sender_share, share) in shares.iter().enumerate() {
-            let from =
-              Participant::new(u16::from(from.start) + u16::try_from(sender_share).unwrap())
-                .unwrap();
             TributaryDb::<D>::save_share_for_blame(txn, &genesis, from, to, share);
           }
         }
@@ -331,21 +332,17 @@ pub(crate) async fn handle_application_tx<
           our_i_pos -= sender_is_len;
         }
         let our_i_pos = usize::from(our_i_pos);
-        let shares = shares
-          .drain(
-            our_i_pos .. (our_i_pos + usize::from(u16::from(our_i.end) - u16::from(our_i.start))),
-          )
-          .collect::<Vec<_>>();
-
-        // Transpose from our shares -> sender shares -> shares to
-        // sender shares -> our shares -> shares
-        let mut transposed = vec![vec![]; shares[0].len()];
-        for shares in shares {
-          for (sender_index, share) in shares.into_iter().enumerate() {
-            transposed[sender_index].push(share);
-          }
-        }
-        transposed
+        shares
+          .iter_mut()
+          .map(|shares| {
+            shares
+              .drain(
+                our_i_pos ..
+                  (our_i_pos + usize::from(u16::from(our_i.end) - u16::from(our_i.start))),
+              )
+              .collect::<Vec<_>>()
+          })
+          .collect()
       };
       // Drop shares as it's been mutated into invalidity
       drop(shares);
@@ -519,6 +516,8 @@ pub(crate) async fn handle_application_tx<
         };
         break key_pair.0.into();
       };
+      let block_number = SeraiBlockNumber::get(txn, hash)
+        .expect("CosignSubstrateBlock yet didn't save Serai block number");
       processors
         .send(
           spec.set().network,
@@ -528,8 +527,7 @@ pub(crate) async fn handle_application_tx<
               id: SubstrateSignableId::CosigningSubstrateBlock(hash),
               attempt: 0,
             },
-            block_number: SeraiBlockNumber::get(txn, hash)
-              .expect("CosignSubstrateBlock yet didn't save Serai block number"),
+            block_number,
           },
         )
         .await;

coordinator/src/tributary/mod.rs

@@ -243,7 +243,7 @@ pub enum Transaction {
   DkgCommitments(u32, Vec<Vec<u8>>, Signed),
   DkgShares {
     attempt: u32,
-    // Receiving Participant, Sending Participant, Share
+    // Sending Participant, Receiving Participant, Share
     shares: Vec<Vec<Vec<u8>>>,
     confirmation_nonces: [u8; 64],
     signed: Signed,

tests/coordinator/src/tests/cosign.rs

@@ -22,7 +22,7 @@ pub async fn potentially_cosign(
 ) -> CoordinatorMessage {
   let msg = processors[primary_processor].recv_message().await;
   let messages::CoordinatorMessage::Coordinator(
-    messages::coordinator::CoordinatorMessage::CosignSubstrateBlock { id },
+    messages::coordinator::CoordinatorMessage::CosignSubstrateBlock { block_number, id },
   ) = msg.clone()
   else {
     return msg;
@@ -85,7 +85,7 @@ pub async fn potentially_cosign(
       participants.insert(known_signer_i);
       participants
     }
-    _ => panic!("coordinator didn't send back SubstratePreprocesses"),
+    other => panic!("coordinator didn't send back SubstratePreprocesses: {:?}", other),
   };
 
   for i in participants.clone() {
@@ -152,7 +152,7 @@ pub async fn potentially_cosign(
   let signature = Signature(
     schnorrkel::keys::Keypair::from_bytes(&schnorrkel_key_pair)
       .unwrap()
-      .sign_simple(b"substrate", &cosign_block_msg(block))
+      .sign_simple(b"substrate", &cosign_block_msg(block_number, block))
       .to_bytes(),
   );
 
@@ -162,6 +162,7 @@ pub async fn potentially_cosign(
     }
     processor
       .send_message(messages::coordinator::ProcessorMessage::CosignedBlock {
+        block_number,
         block,
         signature: signature.0.to_vec(),
       })