diff --git a/coordinator/src/tributary/scanner.rs b/coordinator/src/tributary/scanner.rs index ed8949283..78e719208 100644 --- a/coordinator/src/tributary/scanner.rs +++ b/coordinator/src/tributary/scanner.rs @@ -13,7 +13,7 @@ use serai_client::{validator_sets::primitives::ValidatorSet, subxt::utils::Encod use tributary::{ TransactionKind, Transaction as TributaryTransaction, Block, TributaryReader, tendermint::{ - tx::{TendermintTx, decode_evidence}, + tx::{TendermintTx, Evidence, decode_signed_message}, TendermintNetwork, }, }; @@ -80,7 +80,23 @@ async fn handle_block< TributaryTransaction::Tendermint(TendermintTx::SlashEvidence(ev)) => { // Since the evidence is on the chain, it should already have been validated // We can just punish the signer - let msgs = decode_evidence::>(&ev).unwrap(); + let data = match ev { + Evidence::ConflictingMessages(first, second) => (first, Some(second)), + Evidence::ConflictingPrecommit(first, second) => (first, Some(second)), + Evidence::InvalidPrecommit(first) => (first, None), + Evidence::InvalidValidRound(first) => (first, None), + }; + let msgs = ( + decode_signed_message::>(&data.0).unwrap(), + if data.1.is_some() { + Some( + decode_signed_message::>(&data.1.unwrap()) + .unwrap(), + ) + } else { + None + }, + ); // Since anything with evidence is fundamentally faulty behavior, not just temporal errors, // mark the node as fatally slashed diff --git a/coordinator/tributary/src/lib.rs b/coordinator/tributary/src/lib.rs index 8f3ccffd5..126233730 100644 --- a/coordinator/tributary/src/lib.rs +++ b/coordinator/tributary/src/lib.rs @@ -15,6 +15,8 @@ use ::tendermint::{ TendermintMachine, TendermintHandle, }; +pub use ::tendermint::Evidence; + use serai_db::Db; use tokio::sync::RwLock; diff --git a/coordinator/tributary/src/tendermint/mod.rs b/coordinator/tributary/src/tendermint/mod.rs index 67e0021b3..828327798 100644 --- a/coordinator/tributary/src/tendermint/mod.rs +++ b/coordinator/tributary/src/tendermint/mod.rs @@ -319,19 +319,19 @@ impl Network for TendermintNetwork self.p2p.broadcast(self.genesis, to_broadcast).await } - async fn slash(&mut self, validator: Self::ValidatorId, slash_event: SlashEvent) { + async fn slash(&mut self, validator: Self::ValidatorId, slash_event: SlashEvent) { log::error!( "validator {} triggered a slash event on tributary {} (with evidence: {})", hex::encode(validator), hex::encode(self.genesis), - matches!(slash_event, SlashEvent::WithEvidence(_, _)), + matches!(slash_event, SlashEvent::WithEvidence(_)), ); let signer = self.signer(); let Some(tx) = (match slash_event { - SlashEvent::WithEvidence(m1, m2) => { + SlashEvent::WithEvidence(evidence) => { // create an unsigned evidence tx - Some(TendermintTx::SlashEvidence((m1, m2).encode())) + Some(TendermintTx::SlashEvidence(evidence)) } SlashEvent::Id(_reason, _block, _round) => { // TODO: Increase locally observed slash points diff --git a/coordinator/tributary/src/tendermint/tx.rs b/coordinator/tributary/src/tendermint/tx.rs index d68183823..99d6015d2 100644 --- a/coordinator/tributary/src/tendermint/tx.rs +++ b/coordinator/tributary/src/tendermint/tx.rs @@ -1,6 +1,6 @@ use std::io; -use scale::Decode; +use scale::{Encode, Decode, IoReader}; use blake2::{Digest, Blake2s256}; @@ -19,53 +19,24 @@ use tendermint::{ ext::{Network, Commit, RoundNumber, SignatureScheme}, }; +pub use tendermint::Evidence; + #[allow(clippy::large_enum_variant)] #[derive(Clone, PartialEq, Eq, Debug)] pub enum TendermintTx { - SlashEvidence(Vec), + SlashEvidence(Evidence), } impl ReadWrite for TendermintTx { fn read(reader: &mut R) -> io::Result { - let mut kind = [0]; - reader.read_exact(&mut kind)?; - match kind[0] { - 0 => { - let mut len = [0; 4]; - reader.read_exact(&mut len)?; - let mut len = - usize::try_from(u32::from_le_bytes(len)).expect("running on a 16-bit system?"); - - let mut data = vec![]; - - // Read chunk-by-chunk so a claimed 4 GB length doesn't cause a 4 GB allocation - // While we could check the length is sane, that'd require we know what a sane length is - // We'd also have to maintain that length's sanity even as other parts of the codebase, - // and even entire crates, change - // This is fine as it'll eventually hit the P2P message size limit, yet doesn't require - // knowing it nor does it make any assumptions - const CHUNK_LEN: usize = 1024; - let mut chunk = [0; CHUNK_LEN]; - while len > 0 { - let to_read = len.min(CHUNK_LEN); - data.reserve(to_read); - reader.read_exact(&mut chunk[.. to_read])?; - data.extend(&chunk[.. to_read]); - len -= to_read; - } - Ok(TendermintTx::SlashEvidence(data)) - } - _ => Err(io::Error::new(io::ErrorKind::Other, "invalid transaction type")), - } + Evidence::decode(&mut IoReader(reader)) + .map(TendermintTx::SlashEvidence) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "invalid evidence format")) } fn write(&self, writer: &mut W) -> io::Result<()> { match self { - TendermintTx::SlashEvidence(ev) => { - writer.write_all(&[0])?; - writer.write_all(&u32::try_from(ev.len()).unwrap().to_le_bytes())?; - writer.write_all(ev) - } + TendermintTx::SlashEvidence(ev) => writer.write_all(&ev.encode()), } } } @@ -92,13 +63,23 @@ impl Transaction for TendermintTx { } } -pub fn decode_evidence( - mut ev: &[u8], -) -> Result<(SignedMessageFor, Option>), TransactionError> { - <(SignedMessageFor, Option>)>::decode(&mut ev).map_err(|_| { - dbg!("failed to decode"); - TransactionError::InvalidContent - }) +pub fn decode_signed_message( + mut data: &[u8], +) -> Result, TransactionError> { + SignedMessageFor::::decode(&mut data).map_err(|_| TransactionError::InvalidContent) +} + +fn decode_and_verify_signed_message( + data: &[u8], + schema: &N::SignatureScheme, +) -> Result, TransactionError> { + let msg = decode_signed_message::(data)?; + + // verify that evidence messages are signed correctly + if !msg.verify_signature(schema) { + Err(TransactionError::InvalidSignature)? + } + Ok(msg) } // TODO: Move this into tendermint-machine @@ -115,70 +96,59 @@ pub(crate) fn verify_tendermint_tx( match tx { // TODO: Only allow one evidence per validator, since evidence is fatal TendermintTx::SlashEvidence(ev) => { - let (first, second) = decode_evidence::(ev)?; - - // verify that evidence messages are signed correctly - if !first.verify_signature(&schema) { - Err(TransactionError::InvalidSignature)? - } - let first = first.msg; - - if let Some(second) = second { - if !second.verify_signature(&schema) { - Err(TransactionError::InvalidSignature)? - } - let second = second.msg; - - // 2 types of evidence here - // 1- multiple distinct messages for the same block + round + step - // 2- precommitted to multiple blocks + match ev { + Evidence::ConflictingMessages(first, second) => { + let first = decode_and_verify_signed_message::(first, &schema)?.msg; + let second = decode_and_verify_signed_message::(second, &schema)?.msg; + + // Make sure they're distinct messages, from the same sender, within the same block + if (first == second) || (first.sender != second.sender) || (first.block != second.block) { + Err(TransactionError::InvalidContent)?; + } - // Make sure they're distinct messages, from the same sender, within the same block - if (first == second) || (first.sender != second.sender) || (first.block != second.block) { - Err(TransactionError::InvalidContent)?; + // Distinct messages within the same step + if !((first.round == second.round) && (first.data.step() == second.data.step())) { + Err(TransactionError::InvalidContent)?; + } } + Evidence::ConflictingPrecommit(first, second) => { + let first = decode_and_verify_signed_message::(first, &schema)?.msg; + let second = decode_and_verify_signed_message::(second, &schema)?.msg; - // Distinct messages within the same step - if (first.round == second.round) && (first.data.step() == second.data.step()) { - return Ok(()); - } + if (first.sender != second.sender) || (first.block != second.block) { + Err(TransactionError::InvalidContent)?; + } - // check whether messages are precommits to different blocks - // The inner signatures don't need to be verified since the outer signatures were - // While the inner signatures may be invalid, that would've yielded a invalid precommit - // signature slash instead of distinct precommit slash - if let Data::Precommit(Some((h1, _))) = first.data { - if let Data::Precommit(Some((h2, _))) = second.data { - if h1 == h2 { - Err(TransactionError::InvalidContent)?; + // check whether messages are precommits to different blocks + // The inner signatures don't need to be verified since the outer signatures were + // While the inner signatures may be invalid, that would've yielded a invalid precommit + // signature slash instead of distinct precommit slash + if let Data::Precommit(Some((h1, _))) = first.data { + if let Data::Precommit(Some((h2, _))) = second.data { + if h1 == h2 { + Err(TransactionError::InvalidContent)?; + } + return Ok(()); } - return Ok(()); } - } - // No fault identified - Err(TransactionError::InvalidContent)? - } + // No fault identified + Err(TransactionError::InvalidContent)? + } + Evidence::InvalidPrecommit(msg) => { + let msg = decode_and_verify_signed_message::(msg, &schema)?.msg; - // 2 types of evidence can be here - // 1- invalid commit signature - // 2- vr number that was greater than or equal to the current round - match &first.data { - Data::Proposal(vr, _) => { - // check the vr - if vr.is_none() || vr.unwrap().0 < first.round.0 { + let Data::Precommit(Some((id, sig))) = &msg.data else { Err(TransactionError::InvalidContent)? - } - } - Data::Precommit(Some((id, sig))) => { + }; // TODO: We need to be passed in the genesis time to handle this edge case - if first.block.0 == 0 { + if msg.block.0 == 0 { todo!("invalid precommit signature on first block") } // get the last commit // TODO: Why do we use u32 when Tendermint uses u64? - let prior_commit = match u32::try_from(first.block.0 - 1) { + let prior_commit = match u32::try_from(msg.block.0 - 1) { Ok(n) => match commit(n) { Some(c) => c, // If we have yet to sync the block in question, we will return InvalidContent based @@ -193,16 +163,25 @@ pub(crate) fn verify_tendermint_tx( // calculate the end time till the msg round let mut last_end_time = CanonicalInstant::new(prior_commit.end_time); - for r in 0 ..= first.round.0 { + for r in 0 ..= msg.round.0 { last_end_time = RoundData::::new(RoundNumber(r), last_end_time).end_time(); } // verify that the commit was actually invalid - if schema.verify(first.sender, &commit_msg(last_end_time.canonical(), id.as_ref()), sig) { + if schema.verify(msg.sender, &commit_msg(last_end_time.canonical(), id.as_ref()), sig) { + Err(TransactionError::InvalidContent)? + } + } + Evidence::InvalidValidRound(msg) => { + let msg = decode_and_verify_signed_message::(msg, &schema)?.msg; + + let Data::Proposal(Some(vr), _) = &msg.data else { + Err(TransactionError::InvalidContent)? + }; + if vr.0 < msg.round.0 { Err(TransactionError::InvalidContent)? } } - _ => Err(TransactionError::InvalidContent)?, } } } diff --git a/coordinator/tributary/src/tests/transaction/mod.rs b/coordinator/tributary/src/tests/transaction/mod.rs index 266a11428..e6389efc7 100644 --- a/coordinator/tributary/src/tests/transaction/mod.rs +++ b/coordinator/tributary/src/tests/transaction/mod.rs @@ -16,7 +16,7 @@ use scale::Encode; use ::tendermint::{ ext::{Network, Signer as SignerTrait, SignatureScheme, BlockNumber, RoundNumber}, - SignedMessageFor, DataFor, Message, SignedMessage, Data, + SignedMessageFor, DataFor, Message, SignedMessage, Data, Evidence, }; use crate::{ @@ -212,5 +212,5 @@ pub async fn random_evidence_tx( let data = Data::Proposal(Some(RoundNumber(0)), b); let signer_id = signer.validator_id().await.unwrap(); let signed = signed_from_data::(signer, signer_id, 0, 0, data).await; - TendermintTx::SlashEvidence((signed, None::>).encode()) + TendermintTx::SlashEvidence(Evidence::InvalidValidRound(signed.encode())) } diff --git a/coordinator/tributary/src/tests/transaction/tendermint.rs b/coordinator/tributary/src/tests/transaction/tendermint.rs index 108121b56..aba077676 100644 --- a/coordinator/tributary/src/tests/transaction/tendermint.rs +++ b/coordinator/tributary/src/tests/transaction/tendermint.rs @@ -10,7 +10,7 @@ use scale::Encode; use tendermint::{ time::CanonicalInstant, round::RoundData, - Data, SignedMessageFor, commit_msg, + Data, commit_msg, Evidence, ext::{RoundNumber, Commit, Signer as SignerTrait}, }; @@ -51,7 +51,7 @@ async fn invalid_valid_round() { async move { let data = Data::Proposal(valid_round, TendermintBlock(vec![])); let signed = signed_from_data::(signer.clone().into(), signer_id, 0, 0, data).await; - (signed.clone(), TendermintTx::SlashEvidence((signed, None::>).encode())) + (signed.clone(), TendermintTx::SlashEvidence(Evidence::InvalidValidRound(signed.encode()))) } }; @@ -69,7 +69,7 @@ async fn invalid_valid_round() { let mut random_sig = [0u8; 64]; OsRng.fill_bytes(&mut random_sig); signed.sig = random_sig; - let tx = TendermintTx::SlashEvidence((signed.clone(), None::>).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::InvalidValidRound(signed.encode())); // should fail assert!(verify_tendermint_tx::(&tx, validators, commit).is_err()); @@ -89,7 +89,7 @@ async fn invalid_precommit_signature() { let signed = signed_from_data::(signer.clone().into(), signer_id, 1, 0, Data::Precommit(precommit)) .await; - (signed.clone(), TendermintTx::SlashEvidence((signed, None::>).encode())) + (signed.clone(), TendermintTx::SlashEvidence(Evidence::InvalidPrecommit(signed.encode()))) } }; @@ -119,7 +119,7 @@ async fn invalid_precommit_signature() { let mut random_sig = [0u8; 64]; OsRng.fill_bytes(&mut random_sig); signed.sig = random_sig; - let tx = TendermintTx::SlashEvidence((signed.clone(), None::>).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::InvalidPrecommit(signed.encode())); assert!(verify_tendermint_tx::(&tx, validators, commit).is_err()); } } @@ -134,19 +134,45 @@ async fn evidence_with_prevote() { let prevote = |block_id| { let signer = signer.clone(); async move { - TendermintTx::SlashEvidence( - ( - signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) - .await, - None::>, - ) + // it should fail for all reasons. + let mut txs = vec![]; + txs.push(TendermintTx::SlashEvidence(Evidence::InvalidPrecommit( + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await .encode(), - ) + ))); + txs.push(TendermintTx::SlashEvidence(Evidence::InvalidValidRound( + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await + .encode(), + ))); + // Since these require a second message, provide this one again + // ConflictingMessages can be fired for actually conflicting Prevotes however + txs.push(TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await + .encode(), + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await + .encode(), + ))); + txs.push(TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit( + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await + .encode(), + signed_from_data::(signer.clone().into(), signer_id, 0, 0, Data::Prevote(block_id)) + .await + .encode(), + ))); + txs } }; - // No prevote message should be valid as slash evidence at this time - for prevote in [prevote(None).await, prevote(Some([0x22u8; 32])).await] { + // No prevote message alone should be valid as slash evidence at this time + for prevote in prevote(None).await { + assert!(verify_tendermint_tx::(&prevote, validators.clone(), commit).is_err()); + } + for prevote in prevote(Some([0x22u8; 32])).await { assert!(verify_tendermint_tx::(&prevote, validators.clone(), commit).is_err()); } } @@ -169,23 +195,35 @@ async fn conflicting_msgs_evidence_tx() { { // non-conflicting data should fail let signed_1 = signed_for_b_r(0, 0, Data::Proposal(None, TendermintBlock(vec![0x11]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(&signed_1)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_1.encode(), + )); assert!(verify_tendermint_tx::(&tx, validators.clone(), commit).is_err()); // conflicting data should pass let signed_2 = signed_for_b_r(0, 0, Data::Proposal(None, TendermintBlock(vec![0x22]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap(); // Except if it has a distinct round number, as we don't check cross-round conflicts // (except for Precommit) let signed_2 = signed_for_b_r(0, 1, Data::Proposal(None, TendermintBlock(vec![0x22]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap_err(); // Proposals for different block numbers should also fail as evidence let signed_2 = signed_for_b_r(1, 0, Data::Proposal(None, TendermintBlock(vec![0x22]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap_err(); } @@ -193,23 +231,35 @@ async fn conflicting_msgs_evidence_tx() { { // non-conflicting data should fail let signed_1 = signed_for_b_r(0, 0, Data::Prevote(Some([0x11; 32]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(&signed_1)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_1.encode(), + )); assert!(verify_tendermint_tx::(&tx, validators.clone(), commit).is_err()); // conflicting data should pass let signed_2 = signed_for_b_r(0, 0, Data::Prevote(Some([0x22; 32]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap(); // Except if it has a distinct round number, as we don't check cross-round conflicts // (except for Precommit) let signed_2 = signed_for_b_r(0, 1, Data::Prevote(Some([0x22; 32]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap_err(); // Proposals for different block numbers should also fail as evidence let signed_2 = signed_for_b_r(1, 0, Data::Prevote(Some([0x22; 32]))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap_err(); } @@ -218,17 +268,26 @@ async fn conflicting_msgs_evidence_tx() { let sig = signer.sign(&[]).await; // the inner signature doesn't matter let signed_1 = signed_for_b_r(0, 0, Data::Precommit(Some(([0x11; 32], sig)))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(&signed_1)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit( + signed_1.encode(), + signed_1.encode(), + )); assert!(verify_tendermint_tx::(&tx, validators.clone(), commit).is_err()); // For precommit, the round number is ignored let signed_2 = signed_for_b_r(0, 1, Data::Precommit(Some(([0x22; 32], sig)))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit( + signed_1.encode(), + signed_2.encode(), + )); verify_tendermint_tx::(&tx, validators.clone(), commit).unwrap(); // Yet the block number isn't let signed_2 = signed_for_b_r(1, 0, Data::Precommit(Some(([0x22; 32], sig)))).await; - let tx = TendermintTx::SlashEvidence((&signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingPrecommit( + signed_1.encode(), + signed_2.encode(), + )); assert!(verify_tendermint_tx::(&tx, validators.clone(), commit).is_err()); } @@ -248,7 +307,10 @@ async fn conflicting_msgs_evidence_tx() { ) .await; - let tx = TendermintTx::SlashEvidence((signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); // update schema so that we don't fail due to invalid signature let signer_pub = @@ -265,7 +327,10 @@ async fn conflicting_msgs_evidence_tx() { { let signed_1 = signed_for_b_r(0, 0, Data::Proposal(None, TendermintBlock(vec![]))).await; let signed_2 = signed_for_b_r(0, 0, Data::Prevote(None)).await; - let tx = TendermintTx::SlashEvidence((signed_1, Some(signed_2)).encode()); + let tx = TendermintTx::SlashEvidence(Evidence::ConflictingMessages( + signed_1.encode(), + signed_2.encode(), + )); assert!(verify_tendermint_tx::(&tx, validators.clone(), commit).is_err()); } } diff --git a/coordinator/tributary/tendermint/src/ext.rs b/coordinator/tributary/tendermint/src/ext.rs index 1f95362d1..3d13a3b3e 100644 --- a/coordinator/tributary/tendermint/src/ext.rs +++ b/coordinator/tributary/tendermint/src/ext.rs @@ -282,7 +282,7 @@ pub trait Network: Sized + Send + Sync { /// Trigger a slash for the validator in question who was definitively malicious. /// /// The exact process of triggering a slash is undefined and left to the network as a whole. - async fn slash(&mut self, validator: Self::ValidatorId, slash_event: SlashEvent); + async fn slash(&mut self, validator: Self::ValidatorId, slash_event: SlashEvent); /// Validate a block. async fn validate(&mut self, block: &Self::Block) -> Result<(), BlockError>; diff --git a/coordinator/tributary/tendermint/src/lib.rs b/coordinator/tributary/tendermint/src/lib.rs index 744162831..22a9972e5 100644 --- a/coordinator/tributary/tendermint/src/lib.rs +++ b/coordinator/tributary/tendermint/src/lib.rs @@ -104,7 +104,7 @@ impl SignedMessage { #[derive(Clone, PartialEq, Eq, Debug)] enum TendermintError { - Malicious(N::ValidatorId, Option>), + Malicious(N::ValidatorId, Option), Temporal, AlreadyHandled, } @@ -131,13 +131,18 @@ pub enum SlashReason { InvalidMessage, } -// TODO: Move WithEvidence to a proper Evidence enum, denoting the explicit reason its faulty -// This greatly simplifies the checking process and prevents new-reasons added here not being -// handled elsewhere +#[derive(Clone, PartialEq, Eq, Debug, Encode, Decode)] +pub enum Evidence { + ConflictingMessages(Vec, Vec), + ConflictingPrecommit(Vec, Vec), + InvalidPrecommit(Vec), + InvalidValidRound(Vec), +} + #[derive(Clone, PartialEq, Eq, Debug)] -pub enum SlashEvent { +pub enum SlashEvent { Id(SlashReason, u64, u32), - WithEvidence(SignedMessageFor, Option>), + WithEvidence(Evidence), } /// A machine executing the Tendermint protocol. @@ -266,7 +271,7 @@ impl TendermintMachine { self.reset(round, proposal).await; } - async fn slash(&mut self, validator: N::ValidatorId, slash_event: SlashEvent) { + async fn slash(&mut self, validator: N::ValidatorId, slash_event: SlashEvent) { // TODO: If the new slash event has evidence, emit to prevent a low-importance slash from // cancelling emission of high-importance slashes if !self.block.slashes.contains(&validator) { @@ -500,12 +505,16 @@ impl TendermintMachine { log::trace!("added block {} (produced by machine)", hex::encode(id.as_ref())); self.reset(msg.round, proposal).await; } - Err(TendermintError::Malicious(sender, evidence_msg)) => { + Err(TendermintError::Malicious(sender, evidence)) => { let current_msg = SignedMessage { msg: msg.clone(), sig: sig.clone() }; - let slash = if let Some(old_msg) = evidence_msg { + let slash = if let Some(ev) = evidence { // if the malicious message contains a block, only vote to slash // TODO: Should this decision be made at a higher level? + // A higher-level system may be able to verify if the contained block is fatally + // invalid + // A higher-level system may accept the bandwidth size of this, even if the issue is + // just the valid round field if let Data::Proposal(_, _) = ¤t_msg.msg.data { SlashEvent::Id( SlashReason::InvalidBlock, @@ -513,11 +522,8 @@ impl TendermintMachine { self.block.round().number.0, ) } else { - // if old msg and new msg is not the same, use both as evidence. - SlashEvent::WithEvidence( - old_msg.clone(), - if old_msg != current_msg { Some(current_msg.clone()) } else { None }, - ) + // slash with evidence otherwise + SlashEvent::WithEvidence(ev) } } else { // we don't have evidence. Slash with vote. @@ -563,7 +569,10 @@ impl TendermintMachine { if !self.validators.verify(msg.sender, &commit_msg(end_time.canonical(), id.as_ref()), sig) { log::warn!(target: "tendermint", "Validator produced an invalid commit signature"); - Err(TendermintError::Malicious(msg.sender, Some(signed.clone())))?; + Err(TendermintError::Malicious( + msg.sender, + Some(Evidence::InvalidPrecommit(signed.encode())), + ))?; } return Ok(true); } @@ -676,7 +685,12 @@ impl TendermintMachine { .unwrap(); // Slash the validator for publishing an invalid commit signature - self.slash(*validator, SlashEvent::WithEvidence(msg, None)).await; + self + .slash( + *validator, + SlashEvent::WithEvidence(Evidence::InvalidPrecommit(msg.encode())), + ) + .await; } } } @@ -760,7 +774,10 @@ impl TendermintMachine { // Malformed message if vr.0 >= self.block.round().number.0 { log::warn!(target: "tendermint", "Validator claimed a round from the future was valid"); - Err(TendermintError::Malicious(msg.sender, Some(signed.clone())))?; + Err(TendermintError::Malicious( + msg.sender, + Some(Evidence::InvalidValidRound(signed.encode())), + ))?; } if self.block.log.has_consensus(*vr, Data::Prevote(Some(block.id()))) { diff --git a/coordinator/tributary/tendermint/src/message_log.rs b/coordinator/tributary/tendermint/src/message_log.rs index 8af7f0ec2..4af1fd1cd 100644 --- a/coordinator/tributary/tendermint/src/message_log.rs +++ b/coordinator/tributary/tendermint/src/message_log.rs @@ -1,8 +1,9 @@ use std::{sync::Arc, collections::HashMap}; use log::debug; +use parity_scale_codec::Encode; -use crate::{ext::*, RoundNumber, Step, Data, DataFor, TendermintError, SignedMessageFor}; +use crate::{ext::*, RoundNumber, Step, Data, DataFor, TendermintError, SignedMessageFor, Evidence}; type RoundLog = HashMap<::ValidatorId, HashMap>>; pub(crate) struct MessageLog { @@ -33,7 +34,10 @@ impl MessageLog { target: "tendermint", "Validator sent multiple messages for the same block + round + step" ); - Err(TendermintError::Malicious(msg.sender, Some(existing.clone())))?; + Err(TendermintError::Malicious( + msg.sender, + Some(Evidence::ConflictingMessages(existing.encode(), signed.encode())), + ))?; } return Ok(false); } @@ -44,7 +48,10 @@ impl MessageLog { if let Data::Precommit(Some((prev_hash, _))) = prev.msg.data { if hash != prev_hash { debug!(target: "tendermint", "Validator precommitted to multiple blocks"); - Err(TendermintError::Malicious(msg.sender, Some(prev.clone())))?; + Err(TendermintError::Malicious( + msg.sender, + Some(Evidence::ConflictingPrecommit(prev.encode(), signed.encode())), + ))?; } } else { panic!("message in precommitted wasn't Precommit"); diff --git a/coordinator/tributary/tendermint/tests/ext.rs b/coordinator/tributary/tendermint/tests/ext.rs index 8fd6242d3..928efbbc8 100644 --- a/coordinator/tributary/tendermint/tests/ext.rs +++ b/coordinator/tributary/tendermint/tests/ext.rs @@ -137,7 +137,7 @@ impl Network for TestNetwork { } } - async fn slash(&mut self, _: TestValidatorId, _: SlashEvent) { + async fn slash(&mut self, _: TestValidatorId, _: SlashEvent) { dbg!("Slash"); todo!() }