From 31eaad17b3c74954d9431b697a88de707afc0744 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Sat, 9 Mar 2019 17:43:01 -0800 Subject: [PATCH] Track chain state in registry (fixes #60) This commit moves the last sign state tracking for chains into the global chain registry. It also allows configurable locations for the chain state tracking files, which should probably make it easier to ensure they don't clobber each other in tests. This doesn't fully implement the double signing plan in #60 but at this point I think we're close enough. The remaining item is (optionally) running a user-specified command on startup to query the current block height. --- .gitignore | 3 +- src/chain/mod.rs | 26 +++-- src/{last_sign_state.rs => chain/state.rs} | 113 +++++++++++++++------ src/commands/start.rs | 9 +- src/config/chain.rs | 4 + src/error.rs | 7 +- src/lib.rs | 1 - src/session.rs | 29 +----- 8 files changed, 124 insertions(+), 68 deletions(-) rename src/{last_sign_state.rs => chain/state.rs} (66%) diff --git a/.gitignore b/.gitignore index f7b95ee..e134f36 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /target -**/*.rs.bk tmkms.toml +**/*.rs.bk +**/*priv_validator_state.json # Ignore VIM swap files *.swp diff --git a/src/chain/mod.rs b/src/chain/mod.rs index f767519..e08b7cd 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -3,9 +3,11 @@ mod guard; pub mod key; mod registry; +pub mod state; -pub use self::{guard::Guard, registry::REGISTRY}; -use crate::config::chain::ChainConfig; +pub use self::{guard::Guard, registry::REGISTRY, state::LastSignState}; +use crate::{config::chain::ChainConfig, error::KmsError}; +use std::{path::PathBuf, sync::Mutex}; pub use tendermint::chain::Id; /// Information about a particular Tendermint blockchain network @@ -15,13 +17,25 @@ pub struct Chain { /// Key format configuration pub key_format: key::Format, + + /// State from the last block signed for this chain + pub state: Mutex, } -impl<'a> From<&ChainConfig> for Chain { - fn from(config: &ChainConfig) -> Chain { - Self { +impl Chain { + /// Attempt to create a `Chain` state from the given configuration + pub fn from_config(config: &ChainConfig) -> Result { + let state_file = match config.state_file { + Some(ref path) => path.to_owned(), + None => PathBuf::from(&format!("{}_priv_validator_state.json", config.id)), + }; + + let last_sign_state = LastSignState::load_state(state_file)?; + + Ok(Self { id: config.id, key_format: config.key_format.clone(), - } + state: Mutex::new(last_sign_state), + }) } } diff --git a/src/last_sign_state.rs b/src/chain/state.rs similarity index 66% rename from src/last_sign_state.rs rename to src/chain/state.rs index 82bad20..787a583 100644 --- a/src/last_sign_state.rs +++ b/src/chain/state.rs @@ -1,3 +1,4 @@ +use crate::{chain, error::KmsError}; use abscissa::Error; use atomicwrites::{AtomicFile, OverwriteBehavior}; use serde_json; @@ -7,8 +8,35 @@ use std::{ io::{self, prelude::*}, path::{Path, PathBuf}, }; -use tendermint::{block, chain}; +use tendermint::block; + +/// Check and update the chain position for the given `chain::Id` +pub fn check_and_update_hrs( + chain_id: chain::Id, + height: i64, + round: i64, + step: i8, + block_id: Option, +) -> Result<(), KmsError> { + let registry = chain::REGISTRY.get(); + let chain = registry + .chain(chain_id) + .unwrap_or_else(|| panic!("can't update state for unregistered chain: {}", chain_id)); + + // TODO(tarcieri): better handle `PoisonErrore`? + let mut last_sign_state = chain.state.lock().unwrap(); + + last_sign_state + .check_and_update_hrs(height, round, step, block_id) + .map_err(|e| { + warn!("double sign event: {}", e); + e + })?; + + Ok(()) +} +/// Position of the chain the last time we attempted to sign #[derive(Clone, Debug, Default, Serialize, Deserialize)] struct LastSignData { pub height: i64, @@ -17,26 +45,38 @@ struct LastSignData { pub block_id: Option, } +/// State tracking for double signing prevention pub struct LastSignState { data: LastSignData, path: PathBuf, - _chain_id: chain::Id, } /// Error type #[derive(Debug)] pub struct LastSignError(Error); +/// Kinds of errors #[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)] pub enum LastSignErrorKind { + /// Height regressed #[fail(display = "height regression")] HeightRegression, + + /// Step regressed #[fail(display = "step regression")] StepRegression, + + /// Round regressed #[fail(display = "round regression")] RoundRegression, - #[fail(display = "invalid block id")] + + /// Double sign detected + #[fail(display = "double sign detected")] DoubleSign, + + /// Error syncing state to disk + #[fail(display = "error syncing state to disk")] + SyncError, } impl From> for LastSignError { @@ -52,11 +92,14 @@ impl Display for LastSignError { } impl LastSignState { - pub fn load_state(path: &Path, chain_id: chain::Id) -> std::io::Result { + /// Load the state from the given path + pub fn load_state

(path: P) -> std::io::Result + where + P: AsRef, + { let mut lst = LastSignState { data: LastSignData::default(), - path: path.to_owned(), - _chain_id: chain_id, + path: path.as_ref().to_owned(), }; match fs::read_to_string(path) { @@ -75,15 +118,7 @@ impl LastSignState { } } - pub fn sync_to_disk(&mut self) -> std::io::Result<()> { - let json = serde_json::to_string(&self.data)?; - - AtomicFile::new(&self.path, OverwriteBehavior::AllowOverwrite) - .write(|f| f.write_all(json.as_bytes()))?; - - Ok(()) - } - + /// Check and update the chain's height, round, and step pub fn check_and_update_hrs( &mut self, height: i64, @@ -98,8 +133,7 @@ impl LastSignState { self.data.height, height ); - } - if height == self.data.height { + } else if height == self.data.height { if round < self.data.round { fail!( LastSignErrorKind::RoundRegression, @@ -108,8 +142,7 @@ impl LastSignState { self.data.round, round ) - } - if round == self.data.round { + } else if round == self.data.round { if step < self.data.step { fail!( LastSignErrorKind::StepRegression, @@ -121,24 +154,46 @@ impl LastSignState { ) } - if block_id != None && self.data.block_id != None && self.data.block_id != block_id + if block_id.is_some() + && self.data.block_id.is_some() + && self.data.block_id != block_id { fail!( - LastSignErrorKind::DoubleSign, - "Attempting to sign a second proposal at height:{} round:{} step:{} old block id:{} new block {}", - height, - round, - step, - self.data.block_id.clone().unwrap(), - block_id.unwrap() + LastSignErrorKind::DoubleSign, + "Attempting to sign a second proposal at height:{} round:{} step:{} old block id:{} new block {}", + height, + round, + step, + self.data.block_id.clone().unwrap(), + block_id.unwrap() ) } } } + self.data.height = height; self.data.round = round; self.data.step = step; self.data.block_id = block_id; + + self.sync_to_disk().map_err(|e| { + err!( + LastSignErrorKind::SyncError, + "error writing state to {}: {}", + self.path.display(), + e + ) + })?; + Ok(()) + } + + /// Sync the current state to disk + fn sync_to_disk(&mut self) -> std::io::Result<()> { + let json = serde_json::to_string(&self.data)?; + + AtomicFile::new(&self.path, OverwriteBehavior::AllowOverwrite) + .write(|f| f.write_all(json.as_bytes()))?; + Ok(()) } } @@ -147,7 +202,7 @@ impl LastSignState { mod tests { use super::*; use std::str::FromStr; - use tendermint::{block, chain}; + use tendermint::block; const EXAMPLE_BLOCK_ID: &str = "26C0A41F3243C6BCD7AD2DFF8A8D83A71D29D307B5326C227F734A1A512FE47D"; @@ -167,7 +222,6 @@ mod tests { block_id: None, }, path: EXAMPLE_PATH.into(), - _chain_id: "example-chain".parse::().unwrap(), }; assert_eq!( last_sign_state.check_and_update_hrs(2, 0, 0, None).unwrap(), @@ -185,7 +239,6 @@ mod tests { block_id: Some(block::Id::from_str(EXAMPLE_BLOCK_ID).unwrap()), }, path: EXAMPLE_PATH.into(), - _chain_id: "example-chain".parse::().unwrap(), }; let double_sign_block = block::Id::from_str(EXAMPLE_DOUBLE_SIGN_BLOCK_ID).unwrap(); let err = last_sign_state.check_and_update_hrs(1, 1, 1, Some(double_sign_block)); diff --git a/src/commands/start.rs b/src/commands/start.rs index f02353c..2642d5e 100644 --- a/src/commands/start.rs +++ b/src/commands/start.rs @@ -1,5 +1,5 @@ use crate::{ - chain, + chain::{self, Chain}, client::Client, config::{KmsConfig, ValidatorConfig}, keyring::KeyRing, @@ -47,7 +47,12 @@ impl Callable for StartCommand { let config = KmsConfig::get_global(); for chain_config in &config.chain { - chain::REGISTRY.register(chain_config.into()).unwrap(); + chain::REGISTRY + .register(Chain::from_config(&chain_config).unwrap_or_else(|e| { + status_err!("error initializing chain '{}': {}", chain_config.id, e); + process::exit(1); + })) + .unwrap(); } KeyRing::load_from_config(&config.providers).unwrap_or_else(|e| { diff --git a/src/config/chain.rs b/src/config/chain.rs index 5323db4..a94d696 100644 --- a/src/config/chain.rs +++ b/src/config/chain.rs @@ -1,6 +1,7 @@ //! Chain configuration use crate::chain; +use std::path::PathBuf; /// Chain configuration #[derive(Clone, Deserialize, Debug)] @@ -10,4 +11,7 @@ pub struct ChainConfig { /// Key format configuration pub key_format: chain::key::Format, + + /// Path to chain-specific `priv_validator_state.json` file + pub state_file: Option, } diff --git a/src/error.rs b/src/error.rs index a2cb553..1433ef8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,6 @@ // Error types -use crate::last_sign_state; -use crate::prost; +use crate::{chain, prost}; use abscissa::Error; use signatory; use std::{ @@ -151,8 +150,8 @@ impl From for KmsError { } } -impl From for KmsError { - fn from(other: last_sign_state::LastSignError) -> Self { +impl From for KmsError { + fn from(other: chain::state::LastSignError) -> Self { err!(KmsErrorKind::DoubleSign, other).into() } } diff --git a/src/lib.rs b/src/lib.rs index d4a4fb7..63c1f12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,7 +35,6 @@ mod client; mod commands; mod config; mod keyring; -mod last_sign_state; mod rpc; mod session; mod unix_connection; diff --git a/src/session.rs b/src/session.rs index fa407fc..e656835 100644 --- a/src/session.rs +++ b/src/session.rs @@ -14,15 +14,14 @@ use std::{ }; use tendermint::{ amino_types::{PingRequest, PingResponse, PubKeyRequest}, - chain, public_keys::SecretConnectionKey, SecretConnection, }; use crate::{ + chain, error::KmsError, keyring::KeyRing, - last_sign_state::LastSignState, prost::Message, rpc::{Request, Response, TendermintRequest}, unix_connection::UnixConnection, @@ -35,9 +34,6 @@ pub struct Session { /// TCP connection to a validator node connection: Connection, - - /// Stateful double sign defense that is synced to disk - last_sign_state: LastSignState, } impl Session> { @@ -54,15 +50,10 @@ impl Session> { let signer = Ed25519Signer::from(secret_connection_key); let public_key = SecretConnectionKey::from(ed25519::public_key(&signer)?); let connection = SecretConnection::new(socket, &public_key, &signer)?; - let last_sign_state = LastSignState::load_state( - Path::new(&(chain_id.to_string() + "_priv_validator_state.json")), - chain_id, - )?; Ok(Self { chain_id, connection, - last_sign_state, }) } } @@ -77,15 +68,10 @@ impl Session> { let socket = UnixStream::connect(socket_path)?; let connection = UnixConnection::new(socket); - let last_sign_state = LastSignState::load_state( - Path::new(&(chain_id.as_ref().to_owned() + "_priv_validator_state.json")), - chain_id, - )?; Ok(Self { chain_id, connection, - last_sign_state, }) } } @@ -135,20 +121,15 @@ where request.validate()?; if let Some(cs) = request.consensus_state() { - match self.last_sign_state.check_and_update_hrs( + chain::state::check_and_update_hrs( + self.chain_id, cs.height, cs.round, cs.step, cs.block_id, - ) { - Ok(()) => (), - Err(e) => { - debug! {"Double sign event: {}",e} - return Err(KmsError::from(e)); - } - } + )?; } - self.last_sign_state.sync_to_disk()?; + let mut to_sign = vec![]; request.sign_bytes(self.chain_id, &mut to_sign)?;