Skip to content

Commit

Permalink
fix pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
akildemir committed Jan 23, 2024
1 parent 2d408df commit b13c12a
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 119 deletions.
25 changes: 20 additions & 5 deletions coordinator/tributary/src/tendermint/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,26 @@ pub const LATENCY_TIME: u32 = 1667;
pub const TARGET_BLOCK_TIME: u32 = BLOCK_PROCESSING_TIME + (3 * LATENCY_TIME);

#[test]
fn assert_target_block_time_no_overflow() {
// make sure target block time didn't overflow.
let _ = BLOCK_PROCESSING_TIME.checked_add(LATENCY_TIME.checked_mul(3).unwrap()).unwrap();
fn assert_target_block_time() {
use serai_db::MemDb;

#[derive(Clone, Debug)]
pub struct DummyP2p;

#[async_trait::async_trait]
impl P2p for DummyP2p {
async fn broadcast(&self, _: [u8; 32], _: Vec<u8>) {
unimplemented!()
}
}

// Type paremeters don't matter here since we only need to call the block_time()
// and it only relies on the constants of the trait implementation. block_time() is in seconds,
// TARGET_BLOCK_TIME is in milliseconds.
assert_eq!(
<TendermintNetwork<MemDb, TendermintTx, DummyP2p> as Network>::block_time(),
TARGET_BLOCK_TIME / 1000
)
}

#[async_trait]
Expand Down Expand Up @@ -347,8 +364,6 @@ impl<D: Db, T: TransactionTrait, P: P2p> Network for TendermintNetwork<D, T, P>
};

// add tx to blockchain and broadcast to peers
// TODO: Make a function out of this following block
// Why? Seems like a single caller.
let mut to_broadcast = vec![TRANSACTION_MESSAGE];
tx.write(&mut to_broadcast).unwrap();
if self.blockchain.write().await.add_transaction::<Self>(
Expand Down
117 changes: 5 additions & 112 deletions coordinator/tributary/src/tendermint/tx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@ use crate::{
};

use tendermint::{
SignedMessageFor, Data,
round::RoundData,
time::CanonicalInstant,
commit_msg,
ext::{Network, Commit, RoundNumber, SignatureScheme},
verify_tendermint_evience,
ext::{Network, Commit},
};

pub use tendermint::Evidence;
pub use tendermint::{Evidence, decode_signed_message};

#[allow(clippy::large_enum_variant)]
#[derive(Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -63,27 +60,6 @@ impl Transaction for TendermintTx {
}
}

pub fn decode_signed_message<N: Network>(
mut data: &[u8],
) -> Result<SignedMessageFor<N>, TransactionError> {
SignedMessageFor::<N>::decode(&mut data).map_err(|_| TransactionError::InvalidContent)
}

fn decode_and_verify_signed_message<N: Network>(
data: &[u8],
schema: &N::SignatureScheme,
) -> Result<SignedMessageFor<N>, TransactionError> {
let msg = decode_signed_message::<N>(data)?;

// verify that evidence messages are signed correctly
if !msg.verify_signature(schema) {
Err(TransactionError::InvalidSignature)?
}
Ok(msg)
}

// TODO: Move this into tendermint-machine
// This function takes a TendermintTx, which can't be imported to tendermint create.
pub(crate) fn verify_tendermint_tx<N: Network>(
tx: &TendermintTx,
schema: &N::SignatureScheme,
Expand All @@ -92,91 +68,8 @@ pub(crate) fn verify_tendermint_tx<N: Network>(
tx.verify()?;

match tx {
TendermintTx::SlashEvidence(ev) => {
match ev {
Evidence::ConflictingMessages(first, second) => {
let first = decode_and_verify_signed_message::<N>(first, schema)?.msg;
let second = decode_and_verify_signed_message::<N>(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)?;
}

// 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::<N>(first, schema)?.msg;
let second = decode_and_verify_signed_message::<N>(second, schema)?.msg;

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)?;
}
return Ok(());
}
}

// No fault identified
Err(TransactionError::InvalidContent)?
}
Evidence::InvalidPrecommit(msg) => {
let msg = decode_and_verify_signed_message::<N>(msg, schema)?.msg;

let Data::Precommit(Some((id, sig))) = &msg.data else {
Err(TransactionError::InvalidContent)?
};
// TODO: We need to be passed in the genesis time to handle this edge case
if msg.block.0 == 0 {
todo!("invalid precommit signature on first block")
}

// get the last commit
let prior_commit = match commit(msg.block.0 - 1) {
Some(c) => c,
// If we have yet to sync the block in question, we will return InvalidContent based
// on our own temporal ambiguity
// This will also cause an InvalidContent for anything using a non-existent block,
// yet that's valid behavior
// TODO: Double check the ramifications of this
_ => Err(TransactionError::InvalidContent)?,
};

// calculate the end time till the msg round
let mut last_end_time = CanonicalInstant::new(prior_commit.end_time);
for r in 0 ..= msg.round.0 {
last_end_time = RoundData::<N>::new(RoundNumber(r), last_end_time).end_time();
}

// verify that the commit was actually invalid
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::<N>(msg, schema)?.msg;

let Data::Proposal(Some(vr), _) = &msg.data else {
Err(TransactionError::InvalidContent)?
};
if vr.0 < msg.round.0 {
Err(TransactionError::InvalidContent)?
}
}
}
}
TendermintTx::SlashEvidence(ev) => verify_tendermint_evience::<N>(ev, schema, commit)
.map_err(|_| TransactionError::InvalidContent)?,
}

Ok(())
Expand Down
117 changes: 115 additions & 2 deletions coordinator/tributary/tendermint/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod time;
use time::{sys_time, CanonicalInstant};

pub mod round;
use round::RoundData;

mod block;
use block::BlockData;
Expand Down Expand Up @@ -103,10 +104,11 @@ impl<V: ValidatorId, B: Block, S: Signature> SignedMessage<V, B, S> {
}

#[derive(Clone, PartialEq, Eq, Debug)]
enum TendermintError<N: Network> {
pub enum TendermintError<N: Network> {
Malicious(N::ValidatorId, Option<Evidence>),
Temporal,
AlreadyHandled,
InvalidEvidence,
}

// Type aliases to abstract over generic hell
Expand Down Expand Up @@ -139,6 +141,113 @@ pub enum Evidence {
InvalidValidRound(Vec<u8>),
}

pub fn decode_signed_message<N: Network>(mut data: &[u8]) -> Option<SignedMessageFor<N>> {
SignedMessageFor::<N>::decode(&mut data).ok()
}

fn decode_and_verify_signed_message<N: Network>(
data: &[u8],
schema: &N::SignatureScheme,
) -> Result<SignedMessageFor<N>, TendermintError<N>> {
let msg = decode_signed_message::<N>(data).ok_or(TendermintError::InvalidEvidence)?;

// verify that evidence messages are signed correctly
if !msg.verify_signature(schema) {
Err(TendermintError::InvalidEvidence)?;
}

Ok(msg)
}

pub fn verify_tendermint_evience<N: Network>(
evidence: &Evidence,
schema: &N::SignatureScheme,
commit: impl Fn(u64) -> Option<Commit<N::SignatureScheme>>,
) -> Result<(), TendermintError<N>> {
match evidence {
Evidence::ConflictingMessages(first, second) => {
let first = decode_and_verify_signed_message::<N>(first, schema)?.msg;
let second = decode_and_verify_signed_message::<N>(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(TendermintError::InvalidEvidence)?;
}

// Distinct messages within the same step
if !((first.round == second.round) && (first.data.step() == second.data.step())) {
Err(TendermintError::InvalidEvidence)?;
}
}
Evidence::ConflictingPrecommit(first, second) => {
let first = decode_and_verify_signed_message::<N>(first, schema)?.msg;
let second = decode_and_verify_signed_message::<N>(second, schema)?.msg;

if (first.sender != second.sender) || (first.block != second.block) {
Err(TendermintError::InvalidEvidence)?;
}

// 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(TendermintError::InvalidEvidence)?;
}
return Ok(());
}
}

// No fault identified
Err(TendermintError::InvalidEvidence)?;
}
Evidence::InvalidPrecommit(msg) => {
let msg = decode_and_verify_signed_message::<N>(msg, schema)?.msg;

let Data::Precommit(Some((id, sig))) = &msg.data else {
Err(TendermintError::InvalidEvidence)?
};
// TODO: We need to be passed in the genesis time to handle this edge case
if msg.block.0 == 0 {
todo!("invalid precommit signature on first block")
}

// get the last commit
let prior_commit = match commit(msg.block.0 - 1) {
Some(c) => c,
// If we have yet to sync the block in question, we will return InvalidContent based
// on our own temporal ambiguity
// This will also cause an InvalidContent for anything using a non-existent block,
// yet that's valid behavior
// TODO: Double check the ramifications of this
_ => Err(TendermintError::InvalidEvidence)?,
};

// calculate the end time till the msg round
let mut last_end_time = CanonicalInstant::new(prior_commit.end_time);
for r in 0 ..= msg.round.0 {
last_end_time = RoundData::<N>::new(RoundNumber(r), last_end_time).end_time();
}

// verify that the commit was actually invalid
if schema.verify(msg.sender, &commit_msg(last_end_time.canonical(), id.as_ref()), sig) {
Err(TendermintError::InvalidEvidence)?
}
}
Evidence::InvalidValidRound(msg) => {
let msg = decode_and_verify_signed_message::<N>(msg, schema)?.msg;

let Data::Proposal(Some(vr), _) = &msg.data else { Err(TendermintError::InvalidEvidence)? };
if vr.0 < msg.round.0 {
Err(TendermintError::InvalidEvidence)?
}
}
}
Ok(())
}

#[derive(Clone, PartialEq, Eq, Debug)]
pub enum SlashEvent {
Id(SlashReason, u64, u32),
Expand Down Expand Up @@ -543,7 +652,11 @@ impl<N: Network + 'static> TendermintMachine<N> {

self.slash(sender, slash).await
}
Err(TendermintError::Temporal | TendermintError::AlreadyHandled) => (),
Err(
TendermintError::Temporal |
TendermintError::AlreadyHandled |
TendermintError::InvalidEvidence,
) => (),
}
}
}
Expand Down

0 comments on commit b13c12a

Please sign in to comment.