From 7e2b31e5da559d5a064635014b2045a283109fed Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 31 Dec 2024 12:14:32 -0500 Subject: [PATCH] Clean the transaction definitions in the coordinator Moves to borsh for serialization. No longer includes nonces anywhere in the TX. --- Cargo.lock | 1 - coordinator/LICENSE | 2 +- coordinator/README.md | 20 +- coordinator/cosign/Cargo.toml | 4 +- coordinator/src/tributary/transaction.rs | 632 +++++------------- coordinator/substrate/Cargo.toml | 10 +- coordinator/substrate/src/lib.rs | 3 + coordinator/tributary/src/block.rs | 2 +- coordinator/tributary/src/blockchain.rs | 2 +- coordinator/tributary/src/lib.rs | 2 +- coordinator/tributary/src/mempool.rs | 26 +- coordinator/tributary/src/tendermint/tx.rs | 2 +- coordinator/tributary/src/tests/block.rs | 4 +- coordinator/tributary/src/tests/blockchain.rs | 2 +- .../tributary/src/tests/transaction/mod.rs | 6 +- coordinator/tributary/src/transaction.rs | 12 +- 16 files changed, 220 insertions(+), 510 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40f0e276..7e26d78a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8344,7 +8344,6 @@ dependencies = [ name = "serai-coordinator-substrate" version = "0.1.0" dependencies = [ - "blake2", "borsh", "futures", "log", diff --git a/coordinator/LICENSE b/coordinator/LICENSE index f684d027..26d57cbb 100644 --- a/coordinator/LICENSE +++ b/coordinator/LICENSE @@ -1,6 +1,6 @@ AGPL-3.0-only license -Copyright (c) 2023 Luke Parker +Copyright (c) 2023-2024 Luke Parker This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General Public License Version 3 as diff --git a/coordinator/README.md b/coordinator/README.md index ed41ef71..27552a5a 100644 --- a/coordinator/README.md +++ b/coordinator/README.md @@ -1,7 +1,19 @@ # Coordinator -The Serai coordinator communicates with other coordinators to prepare batches -for Serai and sign transactions. +- [`tendermint`](/tributary/tendermint) is an implementation of the Tendermint BFT algorithm. -In order to achieve consensus over gossip, and order certain events, a -micro-blockchain is instantiated. +- [`tributary`](./tributary) is a micro-blockchain framework. Instead of a producing a blockchain + daemon like the Polkadot SDK or Cosmos SDK intend to, `tributary` is solely intended to be an + embedded asynchronous task within an application. + + The Serai coordinator spawns a tributary for each validator set it's coordinating. This allows + the participating validators to communicate in a byzantine-fault-tolerant manner (relying on + Tendermint for consensus). + +- [`cosign`](./cosign) contains a library to decide which Substrate blocks should be cosigned and + to evaluate cosigns. + +- [`substrate`](./substrate) contains a library to index the Substrate blockchain and handle its + events. + +- [`src`](./src) contains the source code for the Coordinator binary itself. diff --git a/coordinator/cosign/Cargo.toml b/coordinator/cosign/Cargo.toml index fa5bd8ee..bf111f85 100644 --- a/coordinator/cosign/Cargo.toml +++ b/coordinator/cosign/Cargo.toml @@ -29,5 +29,5 @@ log = { version = "0.4", default-features = false, features = ["std"] } tokio = { version = "1", default-features = false } -serai-db = { version = "0.1.1", path = "../../common/db" } -serai-task = { version = "0.1", path = "../../common/task" } +serai-db = { path = "../../common/db", version = "0.1.1" } +serai-task = { path = "../../common/task", version = "0.1" } diff --git a/coordinator/src/tributary/transaction.rs b/coordinator/src/tributary/transaction.rs index 860dbd0f..fd8126ce 100644 --- a/coordinator/src/tributary/transaction.rs +++ b/coordinator/src/tributary/transaction.rs @@ -4,9 +4,7 @@ use std::io; use zeroize::Zeroizing; use rand_core::{RngCore, CryptoRng}; -use blake2::{Digest, Blake2s256}; -use transcript::{Transcript, RecommendedTranscript}; - +use blake2::{digest::typenum::U32, Digest, Blake2b}; use ciphersuite::{ group::{ff::Field, GroupEncoding}, Ciphersuite, Ristretto, @@ -14,22 +12,30 @@ use ciphersuite::{ use schnorr::SchnorrSignature; use scale::{Encode, Decode}; -use processor_messages::coordinator::SubstrateSignableId; +use borsh::{BorshSerialize, BorshDeserialize}; + +use serai_client::primitives::PublicKey; + +use processor_messages::sign::VariantSignId; use tributary::{ - TRANSACTION_SIZE_LIMIT, ReadWrite, - transaction::{Signed, TransactionError, TransactionKind, Transaction as TransactionTrait}, + ReadWrite, + transaction::{ + Signed as TributarySigned, TransactionError, TransactionKind, Transaction as TransactionTrait, + }, }; -#[derive(Clone, Copy, PartialEq, Eq, Debug, Encode)] +/// The label for data from a signing protocol. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Encode, BorshSerialize, BorshDeserialize)] pub enum Label { + /// A preprocess. Preprocess, + /// A signature share. Share, } impl Label { - // TODO: Should nonces be u8 thanks to our use of topics? - pub fn nonce(&self) -> u32 { + fn nonce(&self) -> u32 { match self { Label::Preprocess => 0, Label::Share => 1, @@ -37,474 +43,202 @@ impl Label { } } -#[derive(Clone, PartialEq, Eq)] -pub struct SignData { - pub plan: Id, - pub attempt: u32, - pub label: Label, - - pub data: Vec>, - - pub signed: Signed, +fn borsh_serialize_public( + public: &PublicKey, + writer: &mut W, +) -> Result<(), io::Error> { + // This doesn't use `encode_to` as `encode_to` panics if the writer returns an error + writer.write_all(&public.encode()) +} +fn borsh_deserialize_public(reader: &mut R) -> Result { + Decode::decode(&mut scale::IoReader(reader)).map_err(io::Error::other) } -impl Debug for SignData { - fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { - fmt - .debug_struct("SignData") - .field("id", &hex::encode(self.plan.encode())) - .field("attempt", &self.attempt) - .field("label", &self.label) - .field("signer", &hex::encode(self.signed.signer.to_bytes())) - .finish_non_exhaustive() +/// `tributary::Signed` without the nonce. +/// +/// All of our nonces are deterministic to the type of transaction and fields within. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Signed { + pub signer: ::G, + pub signature: SchnorrSignature, +} + +impl BorshSerialize for Signed { + fn serialize(&self, writer: &mut W) -> Result<(), io::Error> { + writer.write_all(self.signer.to_bytes().as_ref())?; + self.signature.write(writer) + } +} +impl BorshDeserialize for Signed { + fn deserialize_reader(reader: &mut R) -> Result { + let signer = Ristretto::read_G(reader)?; + let signature = SchnorrSignature::read(reader)?; + Ok(Self { signer, signature }) } } -impl SignData { - pub(crate) fn read(reader: &mut R) -> io::Result { - let plan = Id::decode(&mut scale::IoReader(&mut *reader)) - .map_err(|_| io::Error::other("invalid plan in SignData"))?; - - let mut attempt = [0; 4]; - reader.read_exact(&mut attempt)?; - let attempt = u32::from_le_bytes(attempt); - - let mut label = [0; 1]; - reader.read_exact(&mut label)?; - let label = match label[0] { - 0 => Label::Preprocess, - 1 => Label::Share, - _ => Err(io::Error::other("invalid label in SignData"))?, - }; - - let data = { - let mut data_pieces = [0]; - reader.read_exact(&mut data_pieces)?; - if data_pieces[0] == 0 { - Err(io::Error::other("zero pieces of data in SignData"))?; - } - let mut all_data = vec![]; - for _ in 0 .. data_pieces[0] { - let mut data_len = [0; 2]; - reader.read_exact(&mut data_len)?; - let mut data = vec![0; usize::from(u16::from_le_bytes(data_len))]; - reader.read_exact(&mut data)?; - all_data.push(data); - } - all_data - }; - - let signed = Signed::read_without_nonce(reader, label.nonce())?; - - Ok(SignData { plan, attempt, label, data, signed }) - } - - pub(crate) fn write(&self, writer: &mut W) -> io::Result<()> { - writer.write_all(&self.plan.encode())?; - writer.write_all(&self.attempt.to_le_bytes())?; - writer.write_all(&[match self.label { - Label::Preprocess => 0, - Label::Share => 1, - }])?; - - writer.write_all(&[u8::try_from(self.data.len()).unwrap()])?; - for data in &self.data { - if data.len() > u16::MAX.into() { - // Currently, the largest individual preprocess is a Monero transaction - // It provides 4 commitments per input (128 bytes), a 64-byte proof for them, along with a - // key image and proof (96 bytes) - // Even with all of that, we could support 227 inputs in a single TX - // Monero is limited to ~120 inputs per TX - // - // Bitcoin has a much higher input count of 520, yet it only uses 64 bytes per preprocess - Err(io::Error::other("signing data exceeded 65535 bytes"))?; - } - writer.write_all(&u16::try_from(data.len()).unwrap().to_le_bytes())?; - writer.write_all(data)?; - } - - self.signed.write_without_nonce(writer) +impl Signed { + /// Provide a nonce to convert a `Signed` into a `tributary::Signed`. + fn nonce(&self, nonce: u32) -> TributarySigned { + TributarySigned { signer: self.signer, nonce, signature: self.signature } } } -#[derive(Clone, PartialEq, Eq)] +/// The Tributary transaction definition used by Serai +#[derive(Clone, PartialEq, Eq, Debug, BorshSerialize, BorshDeserialize)] pub enum Transaction { + /// A vote to remove a participant for invalid behavior RemoveParticipant { - participant: ::G, + /// The participant to remove + #[borsh( + serialize_with = "borsh_serialize_public", + deserialize_with = "borsh_deserialize_public" + )] + participant: PublicKey, + /// The transaction's signer and signature signed: Signed, }, + /// A participation in the DKG DkgParticipation { participation: Vec, + /// The transaction's signer and signature signed: Signed, }, - DkgConfirmationNonces { - // The confirmation attempt + /// The preprocess to confirm the DKG results on-chain + DkgConfirmationPreprocess { + /// The attempt number of this signing protocol attempt: u32, - // The nonces for DKG confirmation attempt #attempt - confirmation_nonces: [u8; 64], + // The preprocess + preprocess: [u8; 64], + /// The transaction's signer and signature signed: Signed, }, + /// The signature share to confirm the DKG results on-chain DkgConfirmationShare { - // The confirmation attempt + /// The attempt number of this signing protocol attempt: u32, - // The share for DKG confirmation attempt #attempt + // The signature share confirmation_share: [u8; 32], + /// The transaction's signer and signature signed: Signed, }, - // Co-sign a Substrate block. - CosignSubstrateBlock([u8; 32]), + /// Intend to co-sign a finalized Substrate block + /// + /// When the time comes to start a new co-signing protocol, the most recent Substrate block will + /// be the one selected to be cosigned. + CosignSubstrateBlock { + /// THe hash of the Substrate block to sign + hash: [u8; 32], + }, - // 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. + /// Acknowledge a Substrate block + /// + /// This is provided after the block has been cosigned. + /// + /// With the acknowledgement of a Substrate block, we can whitelist all the `VariantSignId`s + /// resulting from its handling. + SubstrateBlock { + /// The hash of the Substrate block + hash: [u8; 32], + }, + + /// Acknowledge a Batch + /// + /// Once everyone has acknowledged the Batch, we can begin signing it. Batch { - block: [u8; 32], - batch: u32, - }, - // When a Serai block is finalized, with the contained batches, we can allow the associated plan - // IDs - SubstrateBlock(u64), - - SubstrateSign(SignData), - Sign(SignData<[u8; 32]>), - // 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, - first_signer: ::G, - signature: SchnorrSignature, + /// The hash of the Batch's serialization. + /// + /// Generally, we refer to a Batch by its ID/the hash of its instructions. Here, we want to + /// ensure consensus on the Batch, and achieving consensus on its hash is the most effective + /// way to do that. + hash: [u8; 32], }, - SlashReport(Vec, Signed), -} + /// The local view of slashes observed by the transaction's sender + SlashReport { + /// The slash points accrued by each validator + slash_points: Vec, + /// The transaction's signer and signature + signed: Signed, + }, -impl Debug for Transaction { - fn fmt(&self, fmt: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { - match self { - Transaction::RemoveParticipant { participant, signed } => fmt - .debug_struct("Transaction::RemoveParticipant") - .field("participant", &hex::encode(participant.to_bytes())) - .field("signer", &hex::encode(signed.signer.to_bytes())) - .finish_non_exhaustive(), - Transaction::DkgParticipation { signed, .. } => fmt - .debug_struct("Transaction::DkgParticipation") - .field("signer", &hex::encode(signed.signer.to_bytes())) - .finish_non_exhaustive(), - Transaction::DkgConfirmationNonces { attempt, signed, .. } => fmt - .debug_struct("Transaction::DkgConfirmationNonces") - .field("attempt", attempt) - .field("signer", &hex::encode(signed.signer.to_bytes())) - .finish_non_exhaustive(), - Transaction::DkgConfirmationShare { attempt, signed, .. } => fmt - .debug_struct("Transaction::DkgConfirmationShare") - .field("attempt", attempt) - .field("signer", &hex::encode(signed.signer.to_bytes())) - .finish_non_exhaustive(), - Transaction::CosignSubstrateBlock(block) => fmt - .debug_struct("Transaction::CosignSubstrateBlock") - .field("block", &hex::encode(block)) - .finish(), - Transaction::Batch { block, batch } => fmt - .debug_struct("Transaction::Batch") - .field("block", &hex::encode(block)) - .field("batch", &batch) - .finish(), - Transaction::SubstrateBlock(block) => { - fmt.debug_struct("Transaction::SubstrateBlock").field("block", block).finish() - } - Transaction::SubstrateSign(sign_data) => { - fmt.debug_struct("Transaction::SubstrateSign").field("sign_data", sign_data).finish() - } - Transaction::Sign(sign_data) => { - fmt.debug_struct("Transaction::Sign").field("sign_data", sign_data).finish() - } - Transaction::SignCompleted { plan, tx_hash, .. } => fmt - .debug_struct("Transaction::SignCompleted") - .field("plan", &hex::encode(plan)) - .field("tx_hash", &hex::encode(tx_hash)) - .finish_non_exhaustive(), - Transaction::SlashReport(points, signed) => fmt - .debug_struct("Transaction::SignCompleted") - .field("points", points) - .field("signed", signed) - .finish(), - } - } + Sign { + /// The ID of the object being signed + id: VariantSignId, + /// The attempt number of this signing protocol + attempt: u32, + /// The label for this data within the signing protocol + label: Label, + /// The data itself + /// + /// There will be `n` blobs of data where `n` is the amount of key shares the validator sending + /// this transaction has. + data: Vec>, + /// The transaction's signer and signature + signed: Signed, + }, } impl ReadWrite for Transaction { fn read(reader: &mut R) -> io::Result { - let mut kind = [0]; - reader.read_exact(&mut kind)?; - - match kind[0] { - 0 => Ok(Transaction::RemoveParticipant { - participant: Ristretto::read_G(reader)?, - signed: Signed::read_without_nonce(reader, 0)?, - }), - - 1 => { - let participation = { - let mut participation_len = [0; 4]; - reader.read_exact(&mut participation_len)?; - let participation_len = u32::from_le_bytes(participation_len); - - if participation_len > u32::try_from(TRANSACTION_SIZE_LIMIT).unwrap() { - Err(io::Error::other( - "participation present in transaction exceeded transaction size limit", - ))?; - } - let participation_len = usize::try_from(participation_len).unwrap(); - - let mut participation = vec![0; participation_len]; - reader.read_exact(&mut participation)?; - participation - }; - - let signed = Signed::read_without_nonce(reader, 0)?; - - Ok(Transaction::DkgParticipation { participation, signed }) - } - - 2 => { - let mut attempt = [0; 4]; - reader.read_exact(&mut attempt)?; - let attempt = u32::from_le_bytes(attempt); - - let mut confirmation_nonces = [0; 64]; - reader.read_exact(&mut confirmation_nonces)?; - - let signed = Signed::read_without_nonce(reader, 0)?; - - Ok(Transaction::DkgConfirmationNonces { attempt, confirmation_nonces, signed }) - } - - 3 => { - let mut attempt = [0; 4]; - reader.read_exact(&mut attempt)?; - let attempt = u32::from_le_bytes(attempt); - - let mut confirmation_share = [0; 32]; - reader.read_exact(&mut confirmation_share)?; - - let signed = Signed::read_without_nonce(reader, 1)?; - - Ok(Transaction::DkgConfirmationShare { attempt, confirmation_share, signed }) - } - - 4 => { - let mut block = [0; 32]; - reader.read_exact(&mut block)?; - Ok(Transaction::CosignSubstrateBlock(block)) - } - - 5 => { - let mut block = [0; 32]; - reader.read_exact(&mut block)?; - let mut batch = [0; 4]; - reader.read_exact(&mut batch)?; - Ok(Transaction::Batch { block, batch: u32::from_le_bytes(batch) }) - } - - 6 => { - let mut block = [0; 8]; - reader.read_exact(&mut block)?; - Ok(Transaction::SubstrateBlock(u64::from_le_bytes(block))) - } - - 7 => SignData::read(reader).map(Transaction::SubstrateSign), - 8 => SignData::read(reader).map(Transaction::Sign), - - 9 => { - let mut plan = [0; 32]; - reader.read_exact(&mut plan)?; - - 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 first_signer = Ristretto::read_G(reader)?; - let signature = SchnorrSignature::::read(reader)?; - - Ok(Transaction::SignCompleted { plan, tx_hash, first_signer, signature }) - } - - 10 => { - let mut len = [0]; - reader.read_exact(&mut len)?; - let len = len[0]; - // If the set has as many validators as MAX_KEY_SHARES_PER_SET, then the amount of distinct - // validators (the amount of validators reported on) will be at most - // `MAX_KEY_SHARES_PER_SET - 1` - if u32::from(len) > (serai_client::validator_sets::primitives::MAX_KEY_SHARES_PER_SET - 1) { - Err(io::Error::other("more points reported than allowed validator"))?; - } - let mut points = vec![0u32; len.into()]; - for points in &mut points { - let mut these_points = [0; 4]; - reader.read_exact(&mut these_points)?; - *points = u32::from_le_bytes(these_points); - } - Ok(Transaction::SlashReport(points, Signed::read_without_nonce(reader, 0)?)) - } - - _ => Err(io::Error::other("invalid transaction type")), - } + borsh::from_reader(reader) } fn write(&self, writer: &mut W) -> io::Result<()> { - match self { - Transaction::RemoveParticipant { participant, signed } => { - writer.write_all(&[0])?; - writer.write_all(&participant.to_bytes())?; - signed.write_without_nonce(writer) - } - - Transaction::DkgParticipation { participation, signed } => { - writer.write_all(&[1])?; - writer.write_all(&u32::try_from(participation.len()).unwrap().to_le_bytes())?; - writer.write_all(participation)?; - signed.write_without_nonce(writer) - } - - Transaction::DkgConfirmationNonces { attempt, confirmation_nonces, signed } => { - writer.write_all(&[2])?; - writer.write_all(&attempt.to_le_bytes())?; - writer.write_all(confirmation_nonces)?; - signed.write_without_nonce(writer) - } - - Transaction::DkgConfirmationShare { attempt, confirmation_share, signed } => { - writer.write_all(&[3])?; - writer.write_all(&attempt.to_le_bytes())?; - writer.write_all(confirmation_share)?; - signed.write_without_nonce(writer) - } - - Transaction::CosignSubstrateBlock(block) => { - writer.write_all(&[4])?; - writer.write_all(block) - } - - Transaction::Batch { block, batch } => { - writer.write_all(&[5])?; - writer.write_all(block)?; - writer.write_all(&batch.to_le_bytes()) - } - - Transaction::SubstrateBlock(block) => { - writer.write_all(&[6])?; - writer.write_all(&block.to_le_bytes()) - } - - Transaction::SubstrateSign(data) => { - writer.write_all(&[7])?; - data.write(writer) - } - Transaction::Sign(data) => { - writer.write_all(&[8])?; - data.write(writer) - } - Transaction::SignCompleted { plan, tx_hash, first_signer, signature } => { - writer.write_all(&[9])?; - writer.write_all(plan)?; - 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) - } - Transaction::SlashReport(points, signed) => { - writer.write_all(&[10])?; - writer.write_all(&[u8::try_from(points.len()).unwrap()])?; - for points in points { - writer.write_all(&points.to_le_bytes())?; - } - signed.write_without_nonce(writer) - } - } + borsh::to_writer(writer, self) } } impl TransactionTrait for Transaction { - fn kind(&self) -> TransactionKind<'_> { + fn kind(&self) -> TransactionKind { match self { Transaction::RemoveParticipant { participant, signed } => { - TransactionKind::Signed((b"remove", participant.to_bytes()).encode(), signed) + TransactionKind::Signed((b"RemoveParticipant", participant).encode(), signed.nonce(0)) } Transaction::DkgParticipation { signed, .. } => { - TransactionKind::Signed(b"dkg".to_vec(), signed) + TransactionKind::Signed(b"DkgParticipation".encode(), signed.nonce(0)) + } + Transaction::DkgConfirmationPreprocess { attempt, signed, .. } => { + TransactionKind::Signed((b"DkgConfirmation", attempt).encode(), signed.nonce(0)) } - Transaction::DkgConfirmationNonces { attempt, signed, .. } | Transaction::DkgConfirmationShare { attempt, signed, .. } => { - TransactionKind::Signed((b"dkg_confirmation", attempt).encode(), signed) + TransactionKind::Signed((b"DkgConfirmation", attempt).encode(), signed.nonce(1)) } - Transaction::CosignSubstrateBlock(_) => TransactionKind::Provided("cosign"), + Transaction::CosignSubstrateBlock { .. } => TransactionKind::Provided("CosignSubstrateBlock"), + Transaction::SubstrateBlock { .. } => TransactionKind::Provided("SubstrateBlock"), + Transaction::Batch { .. } => TransactionKind::Provided("Batch"), - Transaction::Batch { .. } => TransactionKind::Provided("batch"), - Transaction::SubstrateBlock(_) => TransactionKind::Provided("serai"), - - Transaction::SubstrateSign(data) => { - TransactionKind::Signed((b"substrate", data.plan, data.attempt).encode(), &data.signed) + Transaction::Sign { id, attempt, label, signed, .. } => { + TransactionKind::Signed((b"Sign", id, attempt).encode(), signed.nonce(label.nonce())) } - Transaction::Sign(data) => { - TransactionKind::Signed((b"sign", data.plan, data.attempt).encode(), &data.signed) - } - Transaction::SignCompleted { .. } => TransactionKind::Unsigned, - Transaction::SlashReport(_, signed) => { - TransactionKind::Signed(b"slash_report".to_vec(), signed) + Transaction::SlashReport { signed, .. } => { + TransactionKind::Signed(b"SlashReport".encode(), signed.nonce(0)) } } } fn hash(&self) -> [u8; 32] { - let mut tx = self.serialize(); + let mut tx = ReadWrite::serialize(self); 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::>(), signed.signature.serialize()); } - Blake2s256::digest([b"Coordinator Tributary Transaction".as_slice(), &tx].concat()).into() + Blake2b::::digest(&tx).into() } + // We don't have any verification logic embedded into the transaction. We just slash anyone who + // publishes an invalid transaction. fn verify(&self) -> Result<(), TransactionError> { - // TODO: Check SubstrateSign's lengths here - - if let Transaction::SignCompleted { first_signer, signature, .. } = self { - if !signature.verify(*first_signer, self.sign_completed_challenge()) { - Err(TransactionError::InvalidContent)?; - } - } - Ok(()) } } impl Transaction { - // Used to initially construct transactions so we can then get sig hashes and perform signing - pub fn empty_signed() -> Signed { - Signed { - signer: Ristretto::generator(), - nonce: 0, - signature: SchnorrSignature:: { - R: Ristretto::generator(), - s: ::F::ZERO, - }, - } - } - // Sign a transaction pub fn sign( &mut self, @@ -512,76 +246,38 @@ impl Transaction { genesis: [u8; 32], key: &Zeroizing<::F>, ) { - fn signed(tx: &mut Transaction) -> (u32, &mut Signed) { - #[allow(clippy::match_same_arms)] // Doesn't make semantic sense here - let nonce = match tx { - Transaction::RemoveParticipant { .. } => 0, - - Transaction::DkgParticipation { .. } => 0, - // Uses a nonce of 0 as it has an internal attempt counter we distinguish by - Transaction::DkgConfirmationNonces { .. } => 0, - // Uses a nonce of 1 due to internal attempt counter and due to following - // DkgConfirmationNonces - Transaction::DkgConfirmationShare { .. } => 1, - - Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"), + fn signed(tx: &mut Transaction) -> &mut Signed { + #[allow(clippy::match_same_arms)] // This doesn't make semantic sense here + match tx { + Transaction::RemoveParticipant { ref mut signed, .. } | + Transaction::DkgParticipation { ref mut signed, .. } | + Transaction::DkgConfirmationPreprocess { ref mut signed, .. } => signed, + Transaction::DkgConfirmationShare { ref mut signed, .. } => signed, + Transaction::CosignSubstrateBlock { .. } => panic!("signing CosignSubstrateBlock"), + Transaction::SubstrateBlock { .. } => panic!("signing SubstrateBlock"), Transaction::Batch { .. } => panic!("signing Batch"), - Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"), - Transaction::SubstrateSign(data) => data.label.nonce(), - Transaction::Sign(data) => data.label.nonce(), + Transaction::Sign { ref mut signed, .. } => signed, - Transaction::SignCompleted { .. } => panic!("signing SignCompleted"), - - Transaction::SlashReport(_, _) => 0, - }; - - ( - nonce, - #[allow(clippy::match_same_arms)] - match tx { - Transaction::RemoveParticipant { ref mut signed, .. } | - Transaction::DkgParticipation { ref mut signed, .. } | - Transaction::DkgConfirmationNonces { ref mut signed, .. } => signed, - Transaction::DkgConfirmationShare { ref mut signed, .. } => signed, - - Transaction::CosignSubstrateBlock(_) => panic!("signing CosignSubstrateBlock"), - - Transaction::Batch { .. } => panic!("signing Batch"), - Transaction::SubstrateBlock(_) => panic!("signing SubstrateBlock"), - - Transaction::SubstrateSign(ref mut data) => &mut data.signed, - Transaction::Sign(ref mut data) => &mut data.signed, - - Transaction::SignCompleted { .. } => panic!("signing SignCompleted"), - - Transaction::SlashReport(_, ref mut signed) => signed, - }, - ) + Transaction::SlashReport { ref mut signed, .. } => signed, + } } - let (nonce, signed_ref) = signed(self); - signed_ref.signer = Ristretto::generator() * key.deref(); - signed_ref.nonce = nonce; - + // Decide the nonce to sign with let sig_nonce = Zeroizing::new(::F::random(rng)); - signed(self).1.signature.R = ::generator() * sig_nonce.deref(); - let sig_hash = self.sig_hash(genesis); - signed(self).1.signature = SchnorrSignature::::sign(key, sig_nonce, sig_hash); - } - pub fn sign_completed_challenge(&self) -> ::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") + { + // Set the signer and the nonce + let signed = signed(self); + signed.signer = Ristretto::generator() * key.deref(); + signed.signature.R = ::generator() * sig_nonce.deref(); } + + // Get the signature hash (which now includes `R || A` making it valid as the challenge) + let sig_hash = self.sig_hash(genesis); + + // Sign the signature + signed(self).signature = SchnorrSignature::::sign(key, sig_nonce, sig_hash); } } diff --git a/coordinator/substrate/Cargo.toml b/coordinator/substrate/Cargo.toml index 4d66c05e..21577d62 100644 --- a/coordinator/substrate/Cargo.toml +++ b/coordinator/substrate/Cargo.toml @@ -20,16 +20,16 @@ workspace = true [dependencies] scale = { package = "parity-scale-codec", version = "3", default-features = false, features = ["std", "derive"] } borsh = { version = "1", default-features = false, features = ["std", "derive", "de_strict_order"] } -serai-client = { path = "../../substrate/client", default-features = false, features = ["serai", "borsh"] } +serai-client = { path = "../../substrate/client", version = "0.1", default-features = false, features = ["serai", "borsh"] } log = { version = "0.4", default-features = false, features = ["std"] } futures = { version = "0.3", default-features = false, features = ["std"] } tokio = { version = "1", default-features = false } -serai-db = { version = "0.1.1", path = "../../common/db" } -serai-task = { version = "0.1", path = "../../common/task" } +serai-db = { path = "../../common/db", version = "0.1.1" } +serai-task = { path = "../../common/task", version = "0.1" } -serai-cosign = { path = "../cosign" } +serai-cosign = { path = "../cosign", version = "0.1" } -messages = { package = "serai-processor-messages", path = "../../processor/messages" } +messages = { package = "serai-processor-messages", version = "0.1", path = "../../processor/messages" } diff --git a/coordinator/substrate/src/lib.rs b/coordinator/substrate/src/lib.rs index 9c3c8863..41378508 100644 --- a/coordinator/substrate/src/lib.rs +++ b/coordinator/substrate/src/lib.rs @@ -96,6 +96,9 @@ impl NewSet { } /// The channel for notifications to sign a slash report, as emitted by an ephemeral event stream. +/// +/// These notifications MAY be for irrelevant validator sets. The only guarantee is the +/// notifications for all relevant validator sets will be included. pub struct SignSlashReport; impl SignSlashReport { pub(crate) fn send(txn: &mut impl DbTxn, set: &ValidatorSet) { diff --git a/coordinator/tributary/src/block.rs b/coordinator/tributary/src/block.rs index 6f3374bd..d632ce57 100644 --- a/coordinator/tributary/src/block.rs +++ b/coordinator/tributary/src/block.rs @@ -135,7 +135,7 @@ impl Block { // Check TXs are sorted by nonce. let nonce = |tx: &Transaction| { if let TransactionKind::Signed(_, Signed { nonce, .. }) = tx.kind() { - *nonce + nonce } else { 0 } diff --git a/coordinator/tributary/src/blockchain.rs b/coordinator/tributary/src/blockchain.rs index 1664860b..0eee391b 100644 --- a/coordinator/tributary/src/blockchain.rs +++ b/coordinator/tributary/src/blockchain.rs @@ -323,7 +323,7 @@ impl Blockchain { } TransactionKind::Signed(order, Signed { signer, nonce, .. }) => { let next_nonce = nonce + 1; - txn.put(Self::next_nonce_key(&self.genesis, signer, &order), next_nonce.to_le_bytes()); + txn.put(Self::next_nonce_key(&self.genesis, &signer, &order), next_nonce.to_le_bytes()); self.mempool.remove(&tx.hash()); } } diff --git a/coordinator/tributary/src/lib.rs b/coordinator/tributary/src/lib.rs index 9b23dc6c..476dbf93 100644 --- a/coordinator/tributary/src/lib.rs +++ b/coordinator/tributary/src/lib.rs @@ -110,7 +110,7 @@ impl Transaction { } } - pub fn kind(&self) -> TransactionKind<'_> { + pub fn kind(&self) -> TransactionKind { match self { Transaction::Tendermint(tx) => tx.kind(), Transaction::Application(tx) => tx.kind(), diff --git a/coordinator/tributary/src/mempool.rs b/coordinator/tributary/src/mempool.rs index 7558bae0..e83c3acb 100644 --- a/coordinator/tributary/src/mempool.rs +++ b/coordinator/tributary/src/mempool.rs @@ -81,11 +81,11 @@ impl Mempool { } Transaction::Application(tx) => match tx.kind() { TransactionKind::Signed(order, Signed { signer, nonce, .. }) => { - let amount = *res.txs_per_signer.get(signer).unwrap_or(&0) + 1; - res.txs_per_signer.insert(*signer, amount); + let amount = *res.txs_per_signer.get(&signer).unwrap_or(&0) + 1; + res.txs_per_signer.insert(signer, amount); if let Some(prior_nonce) = - res.last_nonce_in_mempool.insert((*signer, order.clone()), *nonce) + res.last_nonce_in_mempool.insert((signer, order.clone()), nonce) { assert_eq!(prior_nonce, nonce - 1); } @@ -133,14 +133,14 @@ impl Mempool { match app_tx.kind() { TransactionKind::Signed(order, Signed { signer, .. }) => { // Get the nonce from the blockchain - let Some(blockchain_next_nonce) = blockchain_next_nonce(*signer, order.clone()) else { + let Some(blockchain_next_nonce) = blockchain_next_nonce(signer, order.clone()) else { // Not a participant Err(TransactionError::InvalidSigner)? }; let mut next_nonce = blockchain_next_nonce; if let Some(mempool_last_nonce) = - self.last_nonce_in_mempool.get(&(*signer, order.clone())) + self.last_nonce_in_mempool.get(&(signer, order.clone())) { assert!(*mempool_last_nonce >= blockchain_next_nonce); next_nonce = *mempool_last_nonce + 1; @@ -148,14 +148,14 @@ impl Mempool { // If we have too many transactions from this sender, don't add this yet UNLESS we are // this sender - let amount_in_pool = *self.txs_per_signer.get(signer).unwrap_or(&0) + 1; + let amount_in_pool = *self.txs_per_signer.get(&signer).unwrap_or(&0) + 1; if !internal && (amount_in_pool > ACCOUNT_MEMPOOL_LIMIT) { Err(TransactionError::TooManyInMempool)?; } verify_transaction(app_tx, self.genesis, &mut |_, _| Some(next_nonce))?; - self.last_nonce_in_mempool.insert((*signer, order.clone()), next_nonce); - self.txs_per_signer.insert(*signer, amount_in_pool); + self.last_nonce_in_mempool.insert((signer, order.clone()), next_nonce); + self.txs_per_signer.insert(signer, amount_in_pool); } TransactionKind::Unsigned => { // check we have the tx in the pool/chain @@ -205,7 +205,7 @@ impl Mempool { // Sort signed by nonce let nonce = |tx: &Transaction| { if let TransactionKind::Signed(_, Signed { nonce, .. }) = tx.kind() { - *nonce + nonce } else { unreachable!() } @@ -242,11 +242,11 @@ impl Mempool { if let Some(tx) = self.txs.remove(tx) { if let TransactionKind::Signed(order, Signed { signer, nonce, .. }) = tx.kind() { - let amount = *self.txs_per_signer.get(signer).unwrap() - 1; - self.txs_per_signer.insert(*signer, amount); + let amount = *self.txs_per_signer.get(&signer).unwrap() - 1; + self.txs_per_signer.insert(signer, amount); - if self.last_nonce_in_mempool.get(&(*signer, order.clone())) == Some(nonce) { - self.last_nonce_in_mempool.remove(&(*signer, order)); + if self.last_nonce_in_mempool.get(&(signer, order.clone())) == Some(&nonce) { + self.last_nonce_in_mempool.remove(&(signer, order)); } } } diff --git a/coordinator/tributary/src/tendermint/tx.rs b/coordinator/tributary/src/tendermint/tx.rs index 8af40708..ea2a7256 100644 --- a/coordinator/tributary/src/tendermint/tx.rs +++ b/coordinator/tributary/src/tendermint/tx.rs @@ -39,7 +39,7 @@ impl ReadWrite for TendermintTx { } impl Transaction for TendermintTx { - fn kind(&self) -> TransactionKind<'_> { + fn kind(&self) -> TransactionKind { // There's an assert elsewhere in the codebase expecting this behavior // If we do want to add Provided/Signed TendermintTxs, review the implications carefully TransactionKind::Unsigned diff --git a/coordinator/tributary/src/tests/block.rs b/coordinator/tributary/src/tests/block.rs index c5bf19c6..03493e21 100644 --- a/coordinator/tributary/src/tests/block.rs +++ b/coordinator/tributary/src/tests/block.rs @@ -60,8 +60,8 @@ impl ReadWrite for NonceTransaction { } impl TransactionTrait for NonceTransaction { - fn kind(&self) -> TransactionKind<'_> { - TransactionKind::Signed(vec![], &self.2) + fn kind(&self) -> TransactionKind { + TransactionKind::Signed(vec![], self.2.clone()) } fn hash(&self) -> [u8; 32] { diff --git a/coordinator/tributary/src/tests/blockchain.rs b/coordinator/tributary/src/tests/blockchain.rs index 6103a62f..3c4df327 100644 --- a/coordinator/tributary/src/tests/blockchain.rs +++ b/coordinator/tributary/src/tests/blockchain.rs @@ -425,7 +425,7 @@ async fn block_tx_ordering() { } impl TransactionTrait for SignedTx { - fn kind(&self) -> TransactionKind<'_> { + fn kind(&self) -> TransactionKind { match self { SignedTx::Signed(signed) => signed.kind(), SignedTx::Provided(pro) => pro.kind(), diff --git a/coordinator/tributary/src/tests/transaction/mod.rs b/coordinator/tributary/src/tests/transaction/mod.rs index 1f85947a..eeaa0acb 100644 --- a/coordinator/tributary/src/tests/transaction/mod.rs +++ b/coordinator/tributary/src/tests/transaction/mod.rs @@ -67,7 +67,7 @@ impl ReadWrite for ProvidedTransaction { } impl Transaction for ProvidedTransaction { - fn kind(&self) -> TransactionKind<'_> { + fn kind(&self) -> TransactionKind { match self.0[0] { 1 => TransactionKind::Provided("order1"), 2 => TransactionKind::Provided("order2"), @@ -119,8 +119,8 @@ impl ReadWrite for SignedTransaction { } impl Transaction for SignedTransaction { - fn kind(&self) -> TransactionKind<'_> { - TransactionKind::Signed(vec![], &self.1) + fn kind(&self) -> TransactionKind { + TransactionKind::Signed(vec![], self.1.clone()) } fn hash(&self) -> [u8; 32] { diff --git a/coordinator/tributary/src/transaction.rs b/coordinator/tributary/src/transaction.rs index 8e9342d7..d7ff4092 100644 --- a/coordinator/tributary/src/transaction.rs +++ b/coordinator/tributary/src/transaction.rs @@ -109,7 +109,7 @@ impl Signed { #[allow(clippy::large_enum_variant)] #[derive(Clone, PartialEq, Eq, Debug)] -pub enum TransactionKind<'a> { +pub enum TransactionKind { /// This transaction should be provided by every validator, in an exact order. /// /// The contained static string names the orderer to use. This allows two distinct provided @@ -137,14 +137,14 @@ pub enum TransactionKind<'a> { Unsigned, /// A signed transaction. - Signed(Vec, &'a Signed), + Signed(Vec, Signed), } // TODO: Should this be renamed TransactionTrait now that a literal Transaction exists? // Or should the literal Transaction be renamed to Event? pub trait Transaction: 'static + Send + Sync + Clone + Eq + Debug + ReadWrite { /// Return what type of transaction this is. - fn kind(&self) -> TransactionKind<'_>; + fn kind(&self) -> TransactionKind; /// Return the hash of this transaction. /// @@ -198,8 +198,8 @@ pub(crate) fn verify_transaction( match tx.kind() { TransactionKind::Provided(_) | TransactionKind::Unsigned => {} TransactionKind::Signed(order, Signed { signer, nonce, signature }) => { - if let Some(next_nonce) = get_and_increment_nonce(signer, &order) { - if *nonce != next_nonce { + if let Some(next_nonce) = get_and_increment_nonce(&signer, &order) { + if nonce != next_nonce { Err(TransactionError::InvalidNonce)?; } } else { @@ -208,7 +208,7 @@ pub(crate) fn verify_transaction( } // TODO: Use a batch verification here - if !signature.verify(*signer, tx.sig_hash(genesis)) { + if !signature.verify(signer, tx.sig_hash(genesis)) { Err(TransactionError::InvalidSignature)?; } }