From 22cb5753d0450ac14594f4f174a581eeb10ca696 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sun, 11 Dec 2016 16:52:41 +0100 Subject: [PATCH] Improve capability information and disable old clients. --- ethcore/src/client/client.rs | 17 +++++++++ ethcore/src/client/test_client.rs | 2 + ethcore/src/client/traits.rs | 6 ++- parity/configuration.rs | 13 +++---- parity/run.rs | 1 - parity/updater.rs | 62 ++++++++++++++++++++++++++----- 6 files changed, 83 insertions(+), 18 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 7fa714e8f..f07f7328d 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -127,6 +127,7 @@ impl SleepState { /// Blockchain database client backed by a persistent database. Owns and manages a blockchain and a block queue. /// Call `import_block()` to import a block asynchronously; `flush_queue()` flushes the queue. pub struct Client { + enabled: AtomicBool, mode: Mutex, chain: RwLock>, tracedb: RwLock>, @@ -164,6 +165,7 @@ impl Client { message_channel: IoChannel, db_config: &DatabaseConfig, ) -> Result, ClientError> { + let path = path.to_path_buf(); let gb = spec.genesis_block(); @@ -226,6 +228,7 @@ impl Client { }; let client = Arc::new(Client { + enabled: AtomicBool::new(true), sleep_state: Mutex::new(SleepState::new(awake)), liveness: AtomicBool::new(awake), mode: Mutex::new(config.mode.clone()), @@ -411,6 +414,12 @@ impl Client { /// This is triggered by a message coming from a block queue when the block is ready for insertion pub fn import_verified_blocks(&self) -> usize { + + // Shortcut out if we know we're incapable of syncing the chain. + if !self.enabled.load(AtomicOrdering::Relaxed) { + return 0; + } + let max_blocks_to_import = 4; let (imported_blocks, import_results, invalid_blocks, imported, duration, is_empty) = { let mut imported_blocks = Vec::with_capacity(max_blocks_to_import); @@ -909,8 +918,16 @@ impl BlockChainClient for Client { r } + fn disable(&self) { + self.set_mode(IpcMode::Off); + self.enabled.store(false, AtomicOrdering::Relaxed); + } + fn set_mode(&self, new_mode: IpcMode) { trace!(target: "mode", "Client::set_mode({:?})", new_mode); + if !self.enabled.load(AtomicOrdering::Relaxed) { + return; + } { let mut mode = self.mode.lock(); *mode = new_mode.clone().into(); diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index dd37a3c02..ba2b0f11b 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -677,6 +677,8 @@ impl BlockChainClient for TestBlockChainClient { fn set_mode(&self, _: Mode) { unimplemented!(); } + fn disable(&self) { unimplemented!(); } + fn pruning_info(&self) -> PruningInfo { PruningInfo { earliest_chain: 1, diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 8f5d4c77a..8c36ac382 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -54,7 +54,7 @@ pub trait BlockChainClient : Sync + Send { /// Look up the block number for the given block ID. fn block_number(&self, id: BlockId) -> Option; - + /// Get raw block body data by block id. /// Block body is an RLP list of two items: uncles and transactions. fn block_body(&self, id: BlockId) -> Option; @@ -252,6 +252,10 @@ pub trait BlockChainClient : Sync + Send { /// Set the mode. fn set_mode(&self, mode: Mode); + /// Disable the client from importing blocks. This cannot be undone in this session and indicates + /// that a subsystem has reason to believe this executable incapable of syncing the chain. + fn disable(&self); + /// Returns engine-related extra info for `BlockId`. fn block_extra_info(&self, id: BlockId) -> Option>; diff --git a/parity/configuration.rs b/parity/configuration.rs index 74a936fb5..e690437b9 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -282,7 +282,6 @@ impl Configuration { no_periodic_snapshot: self.args.flag_no_periodic_snapshot, check_seal: !self.args.flag_no_seal_check, download_old_blocks: !self.args.flag_no_ancient_blocks, - require_consensus: !self.args.flag_no_consensus, serve_light: self.args.flag_serve_light, verifier_settings: verifier_settings, }; @@ -628,6 +627,7 @@ impl Configuration { fn update_policy(&self) -> Result { Ok(UpdatePolicy { enable_downloading: !self.args.flag_no_download, + require_consensus: !self.args.flag_no_consensus, filter: match self.args.flag_auto_update.as_ref() { "none" => UpdateFilter::None, "critical" => UpdateFilter::Critical, @@ -943,7 +943,7 @@ mod tests { acc_conf: Default::default(), gas_pricer: Default::default(), miner_extras: Default::default(), - update_policy: UpdatePolicy { enable_downloading: true, filter: UpdateFilter::Critical }, + update_policy: UpdatePolicy { enable_downloading: true, require_consensus: true, filter: UpdateFilter::Critical }, mode: Default::default(), tracing: Default::default(), compaction: Default::default(), @@ -961,7 +961,6 @@ mod tests { no_periodic_snapshot: false, check_seal: true, download_old_blocks: true, - require_consensus: true, serve_light: false, verifier_settings: Default::default(), })); @@ -992,14 +991,14 @@ mod tests { fn should_parse_updater_options() { // when let conf0 = parse(&["parity"]); - let conf1 = parse(&["parity", "--auto-update", "all"]); + let conf1 = parse(&["parity", "--auto-update", "all", "--no-consensus"]); let conf2 = parse(&["parity", "--no-download", "--auto-update=all"]); let conf3 = parse(&["parity", "--auto-update=xxx"]); // then - assert_eq!(conf0.update_policy().unwrap(), UpdatePolicy{enable_downloading: true, filter: UpdateFilter::Critical}); - assert_eq!(conf1.update_policy().unwrap(), UpdatePolicy{enable_downloading: true, filter: UpdateFilter::All}); - assert_eq!(conf2.update_policy().unwrap(), UpdatePolicy{enable_downloading: false, filter: UpdateFilter::All}); + assert_eq!(conf0.update_policy().unwrap(), UpdatePolicy{enable_downloading: true, require_consensus: true, filter: UpdateFilter::Critical}); + assert_eq!(conf1.update_policy().unwrap(), UpdatePolicy{enable_downloading: true, require_consensus: false, filter: UpdateFilter::All}); + assert_eq!(conf2.update_policy().unwrap(), UpdatePolicy{enable_downloading: false, require_consensus: true, filter: UpdateFilter::All}); assert!(conf3.update_policy().is_err()); } diff --git a/parity/run.rs b/parity/run.rs index 04085cd58..7a583a39d 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -94,7 +94,6 @@ pub struct RunCmd { pub no_periodic_snapshot: bool, pub check_seal: bool, pub download_old_blocks: bool, - pub require_consensus: bool, pub serve_light: bool, pub verifier_settings: VerifierSettings, } diff --git a/parity/updater.rs b/parity/updater.rs index 79abdbe1a..a13ae32ff 100644 --- a/parity/updater.rs +++ b/parity/updater.rs @@ -40,6 +40,8 @@ pub enum UpdateFilter { pub struct UpdatePolicy { /// Download potential updates. pub enable_downloading: bool, + /// Disable client if we know we're incapable of syncing. + pub require_consensus: bool, /// Which of those downloaded should be automatically installed. pub filter: UpdateFilter, } @@ -48,6 +50,7 @@ impl Default for UpdatePolicy { fn default() -> Self { UpdatePolicy { enable_downloading: false, + require_consensus: true, filter: UpdateFilter::None, } } @@ -81,6 +84,23 @@ pub struct OperationsInfo { pub minor: Option, } +/// Information on the current version's consensus capabililty. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum CapState { + /// Unknown. + Unknown, + /// Capable of consensus indefinitely. + Capable, + /// Capable of consensus up until a definite block. + CapableUntil(u64), + /// Incapable of consensus since a particular block. + IncapableSince(u64), +} + +impl Default for CapState { + fn default() -> Self { CapState::Unknown } +} + #[derive(Debug, Default)] struct UpdaterState { latest: Option, @@ -88,6 +108,8 @@ struct UpdaterState { fetching: Option, ready: Option, installed: Option, + + capability: CapState, } /// Service for checking for updates and determining whether we can achieve consensus. @@ -135,18 +157,16 @@ impl Updater { let r = Arc::new(u); *r.fetcher.lock() = Some(fetch::Client::new(r.clone())); *r.weak_self.lock() = Arc::downgrade(&r); + + r.poll(); + r } /// Is the currently running client capable of supporting the current chain? - /// `Some` answer or `None` if information on the running client is not available. - pub fn is_capable(&self) -> Option { - self.state.lock().latest.as_ref().and_then(|latest| { - latest.this_fork.map(|this_fork| { - let current_number = self.client.upgrade().map_or(0, |c| c.block_number(BlockId::Latest).unwrap_or(0)); - this_fork >= latest.fork || current_number < latest.fork - }) - }) + /// We default to true if there's no clear information. + pub fn capability(&self) -> CapState { + self.state.lock().capability } /// The release which is ready to be upgraded to, if any. If this returns `Some`, then @@ -294,6 +314,7 @@ impl Updater { let current_number = self.client.upgrade().map_or(0, |c| c.block_number(BlockId::Latest).unwrap_or(0)); + let mut capability = CapState::Unknown; let latest = self.collect_latest().ok(); if let Some(ref latest) = latest { info!(target: "updater", "Latest release in our track is v{} it is {}critical ({} binary is {})", @@ -321,8 +342,31 @@ impl Updater { } } info!(target: "updater", "Fork: this/current/latest/latest-known: {}/#{}/#{}/#{}", match latest.this_fork { Some(f) => format!("#{}", f), None => "unknown".into(), }, current_number, latest.track.fork, latest.fork); + + if let Some(this_fork) = latest.this_fork { + if this_fork < latest.fork { + // We're behind the latest fork. Now is the time to be upgrading; perhaps we're too late... + if let Some(c) = self.client.upgrade() { + let current_number = c.block_number(BlockId::Latest).unwrap_or(0); + if current_number >= latest.fork - 1 { + // We're at (or past) the last block we can import. Disable the client. + if self.update_policy.require_consensus { + c.disable(); + } + capability = CapState::IncapableSince(latest.fork); + } else { + capability = CapState::CapableUntil(latest.fork); + } + } + } else { + capability = CapState::Capable; + } + } } - (*self.state.lock()).latest = latest; + + let mut s = self.state.lock(); + s.latest = latest; + s.capability = capability; } }