Bridge the gap between the prior two commits

coordinator/src/main.rs

@@ -36,9 +36,7 @@ use ::tributary::{
 };
 
 mod tributary;
-use crate::tributary::{
-  TributarySpec, SignData, Transaction, scanner::RecognizedIdType, PlanIds,
-};
+use crate::tributary::{TributarySpec, SignData, Transaction, scanner::RecognizedIdType, PlanIds};
 
 mod db;
 use db::MainDb;
@@ -135,14 +133,14 @@ async fn publish_signed_transaction<D: Db, P: P2p>(
 ) {
   log::debug!("publishing transaction {}", hex::encode(tx.hash()));
 
-  let signer = if let TransactionKind::Signed(signed) = tx.kind() {
+  let (order, signer) = if let TransactionKind::Signed(order, signed) = tx.kind() {
     let signer = signed.signer;
 
     // Safe as we should deterministically create transactions, meaning if this is already on-disk,
     // it's what we're saving now
     MainDb::<D>::save_signed_transaction(txn, signed.nonce, tx);
 
-    signer
+    (order, signer)
   } else {
     panic!("non-signed transaction passed to publish_signed_transaction");
   };
@@ -152,7 +150,7 @@ async fn publish_signed_transaction<D: Db, P: P2p>(
   while let Some(tx) = MainDb::<D>::take_signed_transaction(
     txn,
     tributary
-      .next_nonce(signer)
+      .next_nonce(&signer, &order)
       .await
       .expect("we don't have a nonce, meaning we aren't a participant on this tributary"),
   ) {
@@ -697,7 +695,7 @@ async fn handle_processor_message<D: Db, P: P2p>(
             Err(e) => panic!("created an invalid unsigned transaction: {e:?}"),
           }
         }
-        TransactionKind::Signed(_) => {
+        TransactionKind::Signed(_, _) => {
           tx.sign(&mut OsRng, genesis, key);
           publish_signed_transaction(&mut txn, tributary, tx).await;
         }

coordinator/src/tests/tributary/dkg.rs

@@ -50,7 +50,7 @@ async fn dkg_test() {
 
     let mut tx =
       Transaction::DkgCommitments(attempt, vec![commitments], Transaction::empty_signed());
-    tx.sign(&mut OsRng, spec.genesis(), key, 0);
+    tx.sign(&mut OsRng, spec.genesis(), key);
     txs.push(tx);
   }
 
@@ -177,7 +177,7 @@ async fn dkg_test() {
       confirmation_nonces: crate::tributary::dkg_confirmation_nonces(key, &spec, 0),
       signed: Transaction::empty_signed(),
     };
-    tx.sign(&mut OsRng, spec.genesis(), key, 1);
+    tx.sign(&mut OsRng, spec.genesis(), key);
     txs.push(tx);
   }
 
@@ -296,7 +296,7 @@ async fn dkg_test() {
     txn.commit();
 
     let mut tx = Transaction::DkgConfirmed(attempt, share, Transaction::empty_signed());
-    tx.sign(&mut OsRng, spec.genesis(), key, 2);
+    tx.sign(&mut OsRng, spec.genesis(), key);
     txs.push(tx);
   }
   let block_before_tx = tributaries[0].1.tip().await;

coordinator/src/tests/tributary/mod.rs

@@ -5,7 +5,7 @@ use rand_core::{RngCore, OsRng};
 use scale::{Encode, Decode};
 use processor_messages::coordinator::SubstrateSignableId;
 
-use tributary::{ReadWrite, tests::random_signed};
+use tributary::{ReadWrite, tests::random_signed_with_nonce};
 
 use crate::tributary::{SignData, Transaction};
 
@@ -34,6 +34,7 @@ fn random_vec<R: RngCore>(rng: &mut R, limit: usize) -> Vec<u8> {
 fn random_sign_data<R: RngCore, Id: Clone + PartialEq + Eq + Debug + Encode + Decode>(
   rng: &mut R,
   plan: Id,
+  nonce: u32,
 ) -> SignData<Id> {
   SignData {
     plan,
@@ -47,7 +48,7 @@ fn random_sign_data<R: RngCore, Id: Clone + PartialEq + Eq + Debug + Encode + De
       res
     },
 
-    signed: random_signed(&mut OsRng),
+    signed: random_signed_with_nonce(&mut OsRng, nonce),
   }
 }
 
@@ -83,18 +84,40 @@ fn tx_size_limit() {
 
 #[test]
 fn serialize_sign_data() {
+  fn test_read_write<Id: Clone + PartialEq + Eq + Debug + Encode + Decode>(value: SignData<Id>) {
+    let mut buf = vec![];
+    value.write(&mut buf).unwrap();
+    assert_eq!(value, SignData::read(&mut buf.as_slice(), value.signed.nonce).unwrap())
+  }
+
   let mut plan = [0; 3];
   OsRng.fill_bytes(&mut plan);
-  test_read_write(random_sign_data::<_, _>(&mut OsRng, plan));
+  test_read_write(random_sign_data::<_, _>(
+    &mut OsRng,
+    plan,
+    u32::try_from(OsRng.next_u64() >> 32).unwrap(),
+  ));
   let mut plan = [0; 5];
   OsRng.fill_bytes(&mut plan);
-  test_read_write(random_sign_data::<_, _>(&mut OsRng, plan));
+  test_read_write(random_sign_data::<_, _>(
+    &mut OsRng,
+    plan,
+    u32::try_from(OsRng.next_u64() >> 32).unwrap(),
+  ));
   let mut plan = [0; 8];
   OsRng.fill_bytes(&mut plan);
-  test_read_write(random_sign_data::<_, _>(&mut OsRng, plan));
+  test_read_write(random_sign_data::<_, _>(
+    &mut OsRng,
+    plan,
+    u32::try_from(OsRng.next_u64() >> 32).unwrap(),
+  ));
   let mut plan = [0; 24];
   OsRng.fill_bytes(&mut plan);
-  test_read_write(random_sign_data::<_, _>(&mut OsRng, plan));
+  test_read_write(random_sign_data::<_, _>(
+    &mut OsRng,
+    plan,
+    u32::try_from(OsRng.next_u64() >> 32).unwrap(),
+  ));
 }
 
 #[test]
@@ -114,7 +137,7 @@ fn serialize_transaction() {
     test_read_write(Transaction::DkgCommitments(
       random_u32(&mut OsRng),
       commitments,
-      random_signed(&mut OsRng),
+      random_signed_with_nonce(&mut OsRng, 0),
     ));
   }
 
@@ -145,7 +168,7 @@ fn serialize_transaction() {
         OsRng.fill_bytes(&mut nonces);
         nonces
       },
-      signed: random_signed(&mut OsRng),
+      signed: random_signed_with_nonce(&mut OsRng, 1),
     });
   }
 
@@ -165,7 +188,7 @@ fn serialize_transaction() {
       } else {
         Some(random_vec(&mut OsRng, 500)).filter(|blame| !blame.is_empty())
       },
-      signed: random_signed(&mut OsRng),
+      signed: random_signed_with_nonce(&mut OsRng, 2),
     });
   }
 
@@ -176,7 +199,7 @@ fn serialize_transaction() {
       OsRng.fill_bytes(&mut share);
       share
     },
-    random_signed(&mut OsRng),
+    random_signed_with_nonce(&mut OsRng, 2),
   ));
 
   {
@@ -200,6 +223,7 @@ fn serialize_transaction() {
     test_read_write(Transaction::SubstratePreprocess(random_sign_data(
       &mut OsRng,
       SubstrateSignableId::Batch(plan),
+      0,
     )));
   }
   {
@@ -208,18 +232,19 @@ fn serialize_transaction() {
     test_read_write(Transaction::SubstrateShare(random_sign_data(
       &mut OsRng,
       SubstrateSignableId::Batch(plan),
+      1,
     )));
   }
 
   {
     let mut plan = [0; 32];
     OsRng.fill_bytes(&mut plan);
-    test_read_write(Transaction::SignPreprocess(random_sign_data(&mut OsRng, plan)));
+    test_read_write(Transaction::SignPreprocess(random_sign_data(&mut OsRng, plan, 0)));
   }
   {
     let mut plan = [0; 32];
     OsRng.fill_bytes(&mut plan);
-    test_read_write(Transaction::SignShare(random_sign_data(&mut OsRng, plan)));
+    test_read_write(Transaction::SignShare(random_sign_data(&mut OsRng, plan, 1)));
   }
 
   {
@@ -230,8 +255,8 @@ fn serialize_transaction() {
     test_read_write(Transaction::SignCompleted {
       plan,
       tx_hash,
-      first_signer: random_signed(&mut OsRng).signer,
-      signature: random_signed(&mut OsRng).signature,
+      first_signer: random_signed_with_nonce(&mut OsRng, 2).signer,
+      signature: random_signed_with_nonce(&mut OsRng, 2).signature,
     });
   }
 }

coordinator/src/tests/tributary/tx.rs

@@ -41,7 +41,7 @@ async fn tx_test() {
   let block_before_tx = tributaries[sender].1.tip().await;
   let mut tx =
     Transaction::DkgCommitments(attempt, vec![commitments.clone()], Transaction::empty_signed());
-  tx.sign(&mut OsRng, spec.genesis(), &key, 0);
+  tx.sign(&mut OsRng, spec.genesis(), &key);
 
   assert_eq!(tributaries[sender].1.add_transaction(tx.clone()).await, Ok(true));
   let included_in = wait_for_tx_inclusion(&tributaries[sender].1, block_before_tx, tx.hash()).await;

coordinator/src/tributary/handle.rs

@@ -153,7 +153,7 @@ pub(crate) async fn handle_application_tx<
   // Don't handle transactions from fatally slashed participants
   // TODO: Because fatally slashed participants can still publish onto the blockchain, they have
   // a notable DoS ability
-  if let TransactionKind::Signed(signed) = tx.kind() {
+  if let TransactionKind::Signed(_, signed) = tx.kind() {
     if FatallySlashed::get(txn, genesis, signed.signer.to_bytes()).is_some() {
       return;
     }

coordinator/src/tributary/mod.rs

@@ -189,7 +189,7 @@ impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> Debug for SignData<Id
 }
 
 impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> SignData<Id> {
-  fn read<R: io::Read>(reader: &mut R, nonce: u32) -> io::Result<Self> {
+  pub(crate) fn read<R: io::Read>(reader: &mut R, nonce: u32) -> io::Result<Self> {
     let plan = Id::decode(&mut scale::IoReader(&mut *reader))
       .map_err(|_| io::Error::other("invalid plan in SignData"))?;
 
@@ -219,7 +219,7 @@ impl<Id: Clone + PartialEq + Eq + Debug + Encode + Decode> SignData<Id> {
     Ok(SignData { plan, attempt, data, signed })
   }
 
-  fn write<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
+  pub(crate) fn write<W: io::Write>(&self, writer: &mut W) -> io::Result<()> {
     writer.write_all(&self.plan.encode())?;
     writer.write_all(&self.attempt.to_le_bytes())?;
 
@@ -661,28 +661,44 @@ impl TransactionTrait for Transaction {
     match self {
       Transaction::RemoveParticipant(_) => TransactionKind::Provided("remove"),
 
-      Transaction::DkgCommitments(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
-      Transaction::DkgShares { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
-      Transaction::InvalidDkgShare { attempt, signed, .. } => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
-      Transaction::DkgConfirmed(attempt, _, signed) => TransactionKind::Signed((b"dkg", attempt).encode(), signed),
+      Transaction::DkgCommitments(attempt, _, signed) => {
+        TransactionKind::Signed((b"dkg", attempt).encode(), signed)
+      }
+      Transaction::DkgShares { attempt, signed, .. } => {
+        TransactionKind::Signed((b"dkg", attempt).encode(), signed)
+      }
+      Transaction::InvalidDkgShare { attempt, signed, .. } => {
+        TransactionKind::Signed((b"dkg", attempt).encode(), signed)
+      }
+      Transaction::DkgConfirmed(attempt, _, signed) => {
+        TransactionKind::Signed((b"dkg", attempt).encode(), signed)
+      }
 
       Transaction::CosignSubstrateBlock(_) => TransactionKind::Provided("cosign"),
 
       Transaction::Batch(_, _) => TransactionKind::Provided("batch"),
       Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"),
 
-      Transaction::SubstratePreprocess(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed),
-      Transaction::SubstrateShare(data) => TransactionKind::Signed((b"substrate", data.0.plan, data.0.attempt).encode(), &data.signed),
+      Transaction::SubstratePreprocess(data) => {
+        TransactionKind::Signed((b"substrate", data.plan, data.attempt).encode(), &data.signed)
+      }
+      Transaction::SubstrateShare(data) => {
+        TransactionKind::Signed((b"substrate", data.plan, data.attempt).encode(), &data.signed)
+      }
 
-      Transaction::SignPreprocess(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed),
-      Transaction::SignShare(data) => TransactionKind::Signed((b"sign", data.0.plan, data.0.attempt).encode(), &data.signed),
+      Transaction::SignPreprocess(data) => {
+        TransactionKind::Signed((b"sign", data.plan, data.attempt).encode(), &data.signed)
+      }
+      Transaction::SignShare(data) => {
+        TransactionKind::Signed((b"sign", data.plan, data.attempt).encode(), &data.signed)
+      }
       Transaction::SignCompleted { .. } => TransactionKind::Unsigned,
     }
   }
 
   fn hash(&self) -> [u8; 32] {
     let mut tx = self.serialize();
-    if let TransactionKind::Signed(signed) = self.kind() {
+    if let TransactionKind::Signed(_, signed) = self.kind() {
       // Make sure the part we're cutting off is the signature
       assert_eq!(tx.drain((tx.len() - 64) ..).collect::<Vec<_>>(), signed.signature.serialize());
     }
@@ -728,57 +744,61 @@ impl Transaction {
     genesis: [u8; 32],
     key: &Zeroizing<<Ristretto as Ciphersuite>::F>,
   ) {
-    fn signed(tx: &mut Transaction) -> &mut Signed {
-      match tx {
+    fn signed(tx: &mut Transaction) -> (u32, &mut Signed) {
+      let nonce = match tx {
         Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),
 
-        Transaction::DkgCommitments(_, _, ref mut signed) => signed,
-        Transaction::DkgShares { ref mut signed, .. } => signed,
-        Transaction::InvalidDkgShare { ref mut signed, .. } => signed,
-        Transaction::DkgConfirmed(_, _, ref mut signed) => signed,
+        Transaction::DkgCommitments(_, _, _) => 0,
+        Transaction::DkgShares { .. } => 1,
+        Transaction::InvalidDkgShare { .. } => 2,
+        Transaction::DkgConfirmed(_, _, _) => 2,
 
         Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),
 
         Transaction::Batch(_, _) => panic!("signing Batch"),
         Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),
 
-        Transaction::SubstratePreprocess(ref mut data) => &mut data.signed,
-        Transaction::SubstrateShare(ref mut data) => &mut data.signed,
+        Transaction::SubstratePreprocess(_) => 0,
+        Transaction::SubstrateShare(_) => 1,
 
-        Transaction::SignPreprocess(ref mut data) => &mut data.signed,
-        Transaction::SignShare(ref mut data) => &mut data.signed,
+        Transaction::SignPreprocess(_) => 0,
+        Transaction::SignShare(_) => 1,
         Transaction::SignCompleted { .. } => panic!("signing SignCompleted"),
-      }
+      };
+
+      (
+        nonce,
+        match tx {
+          Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),
+
+          Transaction::DkgCommitments(_, _, ref mut signed) => signed,
+          Transaction::DkgShares { ref mut signed, .. } => signed,
+          Transaction::InvalidDkgShare { ref mut signed, .. } => signed,
+          Transaction::DkgConfirmed(_, _, ref mut signed) => signed,
+
+          Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),
+
+          Transaction::Batch(_, _) => panic!("signing Batch"),
+          Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),
+
+          Transaction::SubstratePreprocess(ref mut data) => &mut data.signed,
+          Transaction::SubstrateShare(ref mut data) => &mut data.signed,
+
+          Transaction::SignPreprocess(ref mut data) => &mut data.signed,
+          Transaction::SignShare(ref mut data) => &mut data.signed,
+          Transaction::SignCompleted { .. } => panic!("signing SignCompleted"),
+        },
+      )
     }
 
-    let signed_ref = signed(self);
+    let (nonce, signed_ref) = signed(self);
     signed_ref.signer = Ristretto::generator() * key.deref();
-
-    signed_ref.nonce = match tx {
-      Transaction::RemoveParticipant(_) => panic!("signing RemoveParticipant"),
-
-      Transaction::DkgCommitments(_, _, _) => 0,
-      Transaction::DkgShares { .. } => 1,
-      Transaction::InvalidDkgShare { .. } => 2,
-      Transaction::DkgConfirmed(_, _, _) => 2,
-
-      Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"),
-
-      Transaction::Batch(_, _) => panic!("signing Batch"),
-      Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"),
-
-      Transaction::SubstratePreprocess(_) => 0,
-      Transaction::SubstrateShare(_) => 1,
-
-      Transaction::SignPreprocess(_) => 0,
-      Transaction::SignShare(_) => 1,
-      Transaction::SignCompleted { .. } => panic!("signing SignCompleted"),
-    };
+    signed_ref.nonce = nonce;
 
     let sig_nonce = Zeroizing::new(<Ristretto as Ciphersuite>::F::random(rng));
-    signed(self).signature.R = <Ristretto as Ciphersuite>::generator() * sig_nonce.deref();
+    signed(self).1.signature.R = <Ristretto as Ciphersuite>::generator() * sig_nonce.deref();
     let sig_hash = self.sig_hash(genesis);
-    signed(self).signature = SchnorrSignature::<Ristretto>::sign(key, sig_nonce, sig_hash);
+    signed(self).1.signature = SchnorrSignature::<Ristretto>::sign(key, sig_nonce, sig_hash);
   }
 
   pub fn sign_completed_challenge(&self) -> <Ristretto as Ciphersuite>::F {

coordinator/src/tributary/scanner.rs

@@ -38,8 +38,8 @@ pub(crate) trait RIDTrait<FRid>:
   Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>) -> FRid
 {
 }
-impl<FRid, F: Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>) -> FRid>
-  RIDTrait<FRid> for F
+impl<FRid, F: Clone + Fn(ValidatorSet, [u8; 32], RecognizedIdType, Vec<u8>) -> FRid> RIDTrait<FRid>
+  for F
 {
 }
 

coordinator/tributary/src/tests/transaction/mod.rs

@@ -42,6 +42,12 @@ pub fn random_signed<R: RngCore + CryptoRng>(rng: &mut R) -> Signed {
   }
 }
 
+pub fn random_signed_with_nonce<R: RngCore + CryptoRng>(rng: &mut R, nonce: u32) -> Signed {
+  let mut signed = random_signed(rng);
+  signed.nonce = nonce;
+  signed
+}
+
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct ProvidedTransaction(pub Vec<u8>);