Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Commit

Permalink
Merge pull request #348 from tendermint/double-sign-detection-and-log…
Browse files Browse the repository at this point in the history 
…ging-improvements

Double signing detection and logging improvements
  • Loading branch information
@tarcieri
tarcieri authored Aug 13, 2019
2 parents c36cb99 + dc8a112 commit 3b3aabf
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 58 deletions.
38 changes: 22 additions & 16 deletions src/chain/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl State {
&mut self,
new_state: consensus::State,
) -> Result<(), StateError> {
// TODO(tarcieri): rewrite this using `Ord` impl on `consensus::State`
// TODO(tarcieri): rewrite this using `PartialOrd` impl on `consensus::State`
if new_state.height < self.consensus_state.height {
fail!(
StateErrorKind::HeightRegression,
Expand Down Expand Up @@ -99,21 +99,21 @@ impl State {
)
}

if self.consensus_state.block_id != new_state.block_id &&
// If we incremented rounds allow locking on a new block
new_state.round == self.consensus_state.round &&
// if we precommitting after prevoting nil allow locking on new block
!(new_state.step == 3 && self.consensus_state.step == 2 && self.consensus_state.block_id.is_none())
if new_state.block_id != self.consensus_state.block_id &&
// disallow voting for two different block IDs during different steps
((new_state.block_id.is_some() && self.consensus_state.block_id.is_some()) ||
// disallow voting `<nil>` and for a block ID on the same step
(new_state.step == self.consensus_state.step))
{
fail!(
StateErrorKind::DoubleSign,
"Attempting to sign a second proposal at height:{} round:{} step:{} old block id:{} new block {}",
new_state.height,
new_state.round,
new_state.step,
self.consensus_state.block_id_prefix(),
new_state.block_id_prefix()
)
StateErrorKind::DoubleSign,
"Attempting to sign a second proposal at height:{} round:{} step:{} old block id:{} new block {}",
new_state.height,
new_state.round,
new_state.step,
self.consensus_state.block_id_prefix(),
new_state.block_id_prefix()
);
}
}
}
Expand Down Expand Up @@ -246,8 +246,8 @@ mod tests {

successful_update_test!(
step_update_with_nil_to_some_block_id_success,
state!(1, 1, 2, None),
state!(1, 1, 3, block_id!(EXAMPLE_BLOCK_ID))
state!(1, 1, 1, None),
state!(1, 1, 2, block_id!(EXAMPLE_BLOCK_ID))
);

successful_update_test!(
Expand All @@ -262,6 +262,12 @@ mod tests {
state!(2, 0, 0, None)
);

successful_update_test!(
step_update_with_block_id_and_nil_success,
state!(1, 0, 0, block_id!(EXAMPLE_BLOCK_ID)),
state!(1, 0, 1, None)
);

double_sign_test!(
step_update_with_different_block_id_double_sign,
state!(1, 1, 0, block_id!(EXAMPLE_BLOCK_ID)),
Expand Down
106 changes: 64 additions & 42 deletions src/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ use crate::{
};
use std::{fmt::Debug, os::unix::net::UnixStream, time::Instant};
use tendermint::{
amino_types::{PingRequest, PingResponse, PubKeyRequest, PubKeyResponse, RemoteError},
consensus::state::NIL_PLACEHOLDER,
net,
amino_types::{
PingRequest, PingResponse, PubKeyRequest, PubKeyResponse, RemoteError, SignedMsgType,
},
consensus, net,
};

/// Encrypted session with a validator node
Expand Down Expand Up @@ -119,25 +120,26 @@ impl Session {
}

/// Perform a digital signature operation
fn sign<T: TendermintRequest + Debug>(&mut self, mut request: T) -> Result<Response, Error> {
fn sign<R>(&mut self, mut request: R) -> Result<Response, Error>
where
R: TendermintRequest + Debug,
{
request.validate()?;

let registry = chain::REGISTRY.get();
let chain = registry.get_chain(&self.config.chain_id).unwrap();

if let Some(request_state) = &request.consensus_state() {
let mut chain_state = chain.state.lock().unwrap();

if let Err(e) = chain_state.update_consensus_state(request_state.clone()) {
// Report double signing error back to the validator
if e.kind() == StateErrorKind::DoubleSign {
return self.handle_double_signing(
request,
&chain_state.consensus_state().block_id_prefix(),
);
} else {
return Err(e.into());
}
let (_, request_state) = parse_request(&request)?;
let mut chain_state = chain.state.lock().unwrap();

if let Err(e) = chain_state.update_consensus_state(request_state) {
// Report double signing error back to the validator
if e.kind() == StateErrorKind::DoubleSign {
return self.handle_double_signing(
request,
&chain_state.consensus_state().block_id_prefix(),
);
} else {
return Err(e.into());
}
}

Expand All @@ -161,7 +163,7 @@ impl Session {
let started_at = Instant::now();
let signature = chain.keyring.sign_ed25519(None, &to_sign)?;

self.log_signing_request(&request, started_at);
self.log_signing_request(&request, started_at).unwrap();
request.set_signature(&signature);

Ok(request.build_response(None))
Expand All @@ -184,43 +186,38 @@ impl Session {
}

/// Write an INFO logline about a signing request
fn log_signing_request<T: TendermintRequest + Debug>(&self, request: &T, started_at: Instant) {
let (consensus_state, block_id) = request
.consensus_state()
.map(|state| (state.to_string(), state.block_id_prefix()))
.unwrap_or_else(|| (NIL_PLACEHOLDER.to_owned(), NIL_PLACEHOLDER.to_owned()));

let msg_type = request
.msg_type()
.map(|t| format!("{:?}", t))
.unwrap_or_else(|| "Unknown".to_owned());
fn log_signing_request<R>(&self, request: &R, started_at: Instant) -> Result<(), Error>
where
R: TendermintRequest + Debug,
{
let (msg_type, request_state) = parse_request(request)?;

info!(
"[{}@{}] signed {}:{} at h/r/s {} ({} ms)",
"[{}@{}] signed {:?}:{} at h/r/s {} ({} ms)",
&self.config.chain_id,
&self.config.addr,
msg_type,
block_id,
consensus_state,
request_state.block_id_prefix(),
request_state,
started_at.elapsed().as_millis(),
);

Ok(())
}

/// Handle attempted double signing
fn handle_double_signing<T: TendermintRequest + Debug>(
fn handle_double_signing<R>(
&self,
request: T,
request: R,
original_block_id: &str,
) -> Result<Response, Error> {
let msg_type = request
.msg_type()
.map(|t| format!("{:?}", t))
.unwrap_or_else(|| "Unknown".to_owned());

let request_state = request.consensus_state().unwrap();
) -> Result<Response, Error>
where
R: TendermintRequest + Debug,
{
let (msg_type, request_state) = parse_request(&request)?;

error!(
"[{}:{}] attempted double sign for {} at h/r/s: {} ({} != {})",
"[{}:{}] attempted double sign {:?} at h/r/s: {} ({} != {})",
&self.config.chain_id,
&self.config.addr,
msg_type,
Expand All @@ -233,3 +230,28 @@ impl Session {
Ok(request.build_response(Some(remote_err)))
}
}

/// Parse the consensus state from an incoming request
// TODO(tarcieri): fix the upstream Amino parser to do this correctly for us
fn parse_request<R>(request: &R) -> Result<(SignedMsgType, consensus::State), Error>
where
R: TendermintRequest + Debug,
{
let msg_type = match request.msg_type() {
Some(ty) => ty,
None => Err(err!(ProtocolError, "no message type for this request"))?,
};

let mut consensus_state = match request.consensus_state() {
Some(state) => state,
None => Err(err!(ProtocolError, "no consensus state in request"))?,
};

consensus_state.step = match msg_type {
SignedMsgType::Proposal => 0,
SignedMsgType::PreVote => 1,
SignedMsgType::PreCommit => 2,
};

Ok((msg_type, consensus_state))
}

0 comments on commit 3b3aabf

Please sign in to comment.