diff --git a/coordinator/src/main.rs b/coordinator/src/main.rs
index 19208cb3..ca150656 100644
--- a/coordinator/src/main.rs
+++ b/coordinator/src/main.rs
@@ -12,7 +12,11 @@ use std::{
use zeroize::{Zeroize, Zeroizing};
use rand_core::OsRng;
-use ciphersuite::{group::ff::PrimeField, Ciphersuite, Ristretto};
+use ciphersuite::{
+ group::ff::{Field, PrimeField},
+ Ciphersuite, Ristretto,
+};
+use schnorr::SchnorrSignature;
use serai_db::{DbTxn, Db};
use serai_env as env;
@@ -476,6 +480,10 @@ pub async fn handle_processors<D: Db, Pro: Processors, P: P2p>(
loop {
// TODO: Dispatch this message to a task dedicated to handling this processor, preventing one
// processor from holding up all the others
+ // In order to do so, we need to locally track if a message was handled, even if its ID is
+ // greater than what the message-queue tracks as ack'd
+ // TODO: Shouldn't the processor also perform such checks? That its ack doesn't exceed the
+ // queue's?
let msg = processors.recv().await;
// TODO2: This is slow, and only works as long as a network only has a single Tributary
@@ -565,7 +573,23 @@ pub async fn handle_processors<D: Db, Pro: Processors, P: P2p>(
signed: Transaction::empty_signed(),
})),
sign::ProcessorMessage::Completed { key: _, id, tx } => {
- Some(Transaction::SignCompleted(id, tx, Transaction::empty_signed()))
+ let r = Zeroizing::new(<Ristretto as Ciphersuite>::F::random(&mut OsRng));
+ #[allow(non_snake_case)]
+ let R = <Ristretto as Ciphersuite>::generator() * r.deref();
+ let mut tx = Transaction::SignCompleted {
+ plan: id,
+ tx_hash: tx,
+ first_signer: pub_key,
+ signature: SchnorrSignature { R, s: <Ristretto as Ciphersuite>::F::ZERO },
+ };
+ let signed = SchnorrSignature::sign(&key, r, tx.sign_completed_challenge());
+ match &mut tx {
+ Transaction::SignCompleted { signature, .. } => {
+ *signature = signed;
+ }
+ _ => unreachable!(),
+ }
+ Some(tx)
}
},
ProcessorMessage::Coordinator(inner_msg) => match inner_msg {
diff --git a/coordinator/src/substrate/mod.rs b/coordinator/src/substrate/mod.rs
index b6bb3608..d114b4a5 100644
--- a/coordinator/src/substrate/mod.rs
+++ b/coordinator/src/substrate/mod.rs
@@ -101,7 +101,6 @@ async fn handle_key_gen<Pro: Processors>(
key_pair: KeyPair,
) -> Result<(), SeraiError> {
if in_set(key, serai, set).await?.expect("KeyGen occurred for a set which doesn't exist") {
- // TODO: Check how the processor handles this being fired multiple times
processors
.send(
set.network,
@@ -196,7 +195,6 @@ async fn handle_batch_and_burns<Pro: Processors>(
.expect("network had a batch/burn yet never set a latest block")
};
- // TODO: Check how the processor handles this being fired multiple times
processors
.send(
network,
diff --git a/coordinator/src/tests/tributary/mod.rs b/coordinator/src/tests/tributary/mod.rs
index 62ea82e7..449fc3bc 100644
--- a/coordinator/src/tests/tributary/mod.rs
+++ b/coordinator/src/tests/tributary/mod.rs
@@ -116,4 +116,17 @@ fn serialize_transaction() {
test_read_write(Transaction::SignPreprocess(random_sign_data(&mut OsRng)));
test_read_write(Transaction::SignShare(random_sign_data(&mut OsRng)));
+
+ {
+ let mut plan = [0; 32];
+ OsRng.fill_bytes(&mut plan);
+ let mut tx_hash = vec![0; (OsRng.next_u64() % 64).try_into().unwrap()];
+ OsRng.fill_bytes(&mut tx_hash);
+ test_read_write(Transaction::SignCompleted {
+ plan,
+ tx_hash,
+ first_signer: random_signed(&mut OsRng).signer,
+ signature: random_signed(&mut OsRng).signature,
+ });
+ }
}
diff --git a/coordinator/src/tributary/handle.rs b/coordinator/src/tributary/handle.rs
index 3c223867..1fd60b05 100644
--- a/coordinator/src/tributary/handle.rs
+++ b/coordinator/src/tributary/handle.rs
@@ -585,8 +585,8 @@ pub async fn handle_application_tx<
None => {}
}
}
- Transaction::SignCompleted(id, tx, signed) => {
- // TODO: Confirm this is a valid ID
+ Transaction::SignCompleted { plan, tx_hash, .. } => {
+ // TODO: Confirm this is a valid plan ID
// TODO: Confirm this signer hasn't prior published a completion
let Some(key_pair) = TributaryDb::<D>::key_pair(txn, spec.set()) else { todo!() };
processors
@@ -594,8 +594,8 @@ pub async fn handle_application_tx<
spec.set().network,
CoordinatorMessage::Sign(sign::CoordinatorMessage::Completed {
key: key_pair.1.to_vec(),
- id,
- tx,
+ id: plan,
+ tx: tx_hash,
}),
)
.await;
diff --git a/coordinator/src/tributary/mod.rs b/coordinator/src/tributary/mod.rs
index f7858ff0..ae7a6f7b 100644
--- a/coordinator/src/tributary/mod.rs
+++ b/coordinator/src/tributary/mod.rs
@@ -232,6 +232,10 @@ pub enum Transaction {
DkgConfirmed(u32, [u8; 32], Signed),
// When we have synchrony on a batch, we can allow signing it
+ // TODO (never?): This is less efficient compared to an ExternalBlock provided transaction,
+ // which would be binding over the block hash and automatically achieve synchrony on all
+ // relevant batches. ExternalBlock was removed for this due to complexity around the pipeline
+ // with the current processor, yet it would still be an improvement.
Batch([u8; 32], [u8; 32]),
// When a Serai block is finalized, with the contained batches, we can allow the associated plan
// IDs
@@ -242,10 +246,18 @@ pub enum Transaction {
SignPreprocess(SignData),
SignShare(SignData),
- // TODO: We can't make this an Unsigned as we need to prevent spam, which requires a max of 1
- // claim per sender
- // Can we de-duplicate across senders though, if they claim the same hash completes?
- SignCompleted([u8; 32], Vec<u8>, Signed),
+ // This is defined as an Unsigned transaction in order to de-duplicate SignCompleted amongst
+ // reporters (who should all report the same thing)
+ // We do still track the signer in order to prevent a single signer from publishing arbitrarily
+ // many TXs without penalty
+ // Here, they're denoted as the first_signer, as only the signer of the first TX to be included
+ // with this pairing will be remembered on-chain
+ SignCompleted {
+ plan: [u8; 32],
+ tx_hash: Vec<u8>,
+ first_signer: <Ristretto as Ciphersuite>::G,
+ signature: SchnorrSignature<Ristretto>,
+ },
}
impl ReadWrite for Transaction {
@@ -354,13 +366,15 @@ impl ReadWrite for Transaction {
let mut plan = [0; 32];
reader.read_exact(&mut plan)?;
- let mut tx_len = [0];
- reader.read_exact(&mut tx_len)?;
- let mut tx = vec![0; usize::from(tx_len[0])];
- reader.read_exact(&mut tx)?;
+ let mut tx_hash_len = [0];
+ reader.read_exact(&mut tx_hash_len)?;
+ let mut tx_hash = vec![0; usize::from(tx_hash_len[0])];
+ reader.read_exact(&mut tx_hash)?;
- let signed = Signed::read(reader)?;
- Ok(Transaction::SignCompleted(plan, tx, signed))
+ let first_signer = Ristretto::read_G(reader)?;
+ let signature = SchnorrSignature::<Ristretto>::read(reader)?;
+
+ Ok(Transaction::SignCompleted { plan, tx_hash, first_signer, signature })
}
_ => Err(io::Error::new(io::ErrorKind::Other, "invalid transaction type")),
@@ -460,12 +474,14 @@ impl ReadWrite for Transaction {
writer.write_all(&[8])?;
data.write(writer)
}
- Transaction::SignCompleted(plan, tx, signed) => {
+ Transaction::SignCompleted { plan, tx_hash, first_signer, signature } => {
writer.write_all(&[9])?;
writer.write_all(plan)?;
- writer.write_all(&[u8::try_from(tx.len()).expect("tx hash length exceed 255 bytes")])?;
- writer.write_all(tx)?;
- signed.write(writer)
+ writer
+ .write_all(&[u8::try_from(tx_hash.len()).expect("tx hash length exceed 255 bytes")])?;
+ writer.write_all(tx_hash)?;
+ writer.write_all(&first_signer.to_bytes())?;
+ signature.write(writer)
}
}
}
@@ -486,7 +502,7 @@ impl TransactionTrait for Transaction {
Transaction::SignPreprocess(data) => TransactionKind::Signed(&data.signed),
Transaction::SignShare(data) => TransactionKind::Signed(&data.signed),
- Transaction::SignCompleted(_, _, signed) => TransactionKind::Signed(signed),
+ Transaction::SignCompleted { .. } => TransactionKind::Unsigned,
}
}
@@ -506,6 +522,12 @@ impl TransactionTrait for Transaction {
}
}
+ if let Transaction::SignCompleted { plan, tx_hash, first_signer, signature } = self {
+ if !signature.verify(*first_signer, self.sign_completed_challenge()) {
+ Err(TransactionError::InvalidContent)?;
+ }
+ }
+
Ok(())
}
}
@@ -545,7 +567,7 @@ impl Transaction {
Transaction::SignPreprocess(ref mut data) => &mut data.signed,
Transaction::SignShare(ref mut data) => &mut data.signed,
- Transaction::SignCompleted(_, _, ref mut signed) => signed,
+ Transaction::SignCompleted { .. } => panic!("signing SignCompleted"),
}
}
@@ -558,4 +580,18 @@ impl Transaction {
let sig_hash = self.sig_hash(genesis);
signed(self).signature = SchnorrSignature::<Ristretto>::sign(key, sig_nonce, sig_hash);
}
+
+ pub fn sign_completed_challenge(&self) -> <Ristretto as Ciphersuite>::F {
+ if let Transaction::SignCompleted { plan, tx_hash, first_signer, signature } = self {
+ let mut transcript =
+ RecommendedTranscript::new(b"Coordinator Tributary Transaction SignCompleted");
+ transcript.append_message(b"plan", plan);
+ transcript.append_message(b"tx_hash", tx_hash);
+ transcript.append_message(b"signer", first_signer.to_bytes());
+ transcript.append_message(b"nonce", signature.R.to_bytes());
+ Ristretto::hash_to_F(b"SignCompleted signature", &transcript.challenge(b"challenge"))
+ } else {
+ panic!("sign_completed_challenge called on transaction which wasn't SignCompleted")
+ }
+ }
}
diff --git a/coordinator/src/tributary/scanner.rs b/coordinator/src/tributary/scanner.rs
index a8e9588e..05df7395 100644
--- a/coordinator/src/tributary/scanner.rs
+++ b/coordinator/src/tributary/scanner.rs
@@ -76,7 +76,7 @@ async fn handle_block<
// mark the node as fatally slashed
TributaryDb::<D>::set_fatally_slashed(&mut txn, genesis, msgs.0.msg.sender);
- // TODO: disconnect the node from network/ban from further participation in Tributary
+ // TODO2: disconnect the node from network/ban from further participation in Tributary
}
TributaryTransaction::Application(tx) => {
handle_application_tx::<D, _, _, _, _, _>(
@@ -99,7 +99,7 @@ async fn handle_block<
event_id += 1;
}
- // TODO: Trigger any necessary re-attempts
+ // TODO2: Trigger any necessary re-attempts
}
pub async fn handle_new_blocks<