From 6816f8b489e6654ef8da403a597c20f244487ad5 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 10 Jul 2018 12:17:53 +0200 Subject: [PATCH] Updater verification (#8787) * getting started * refactor main * unwrap_or -> unwrap_or_else * force parity to lower version number to trigger update * Fix typos * formating * some minor refactoring * enable lints and fix some warnings * make it compile * minor tweaks to make it work * address review comments * Rename exe to exe_path and minor import changes * updater: unreleased -> unknown * Add `debug` configuration to force parity-updater * Introduce a new feature `test-updater` in order conditionally hardcode the version number in parity in order to force an update * This should only be used for debug/dev purposes * nits * Pulled latest submodule of `wasm-tests` --- Cargo.toml | 4 + ethcore/src/client/client.rs | 2 +- parity/configuration.rs | 2 +- parity/main.rs | 188 +++++++++++++++++++++--------- parity/run.rs | 4 +- updater/Cargo.toml | 6 + updater/src/lib.rs | 2 + updater/src/service.rs | 1 + updater/src/types/version_info.rs | 8 +- updater/src/updater.rs | 66 +++++++---- 10 files changed, 197 insertions(+), 86 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 04d0949bd..0a92f4e07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -106,6 +106,10 @@ deadlock_detection = ["parking_lot/deadlock_detection"] # `valgrind --tool=massif /path/to/parity ` # and `massif-visualizer` for visualization memory_profiling = [] +# hardcode version number 1.3.7 of parity to force an update +# in order to manually test that parity fall-over to the local version +# in case of invalid or deprecated command line arguments are entered +test-updater = ["parity-updater/test-updater"] [lib] path = "parity/lib.rs" diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index bdd00a273..3f0f31576 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -2380,7 +2380,7 @@ mod tests { let (new_hash, new_block) = get_good_dummy_block_hash(); let go = { - // Separate thread uncommited transaction + // Separate thread uncommitted transaction let go = Arc::new(AtomicBool::new(false)); let go_thread = go.clone(); let another_client = client.clone(); diff --git a/parity/configuration.rs b/parity/configuration.rs index ab9baac1f..7878a49e3 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -103,7 +103,7 @@ impl Configuration { /// # Example /// /// ``` - /// let _cfg = parity::Configuration::parse_cli(&["--light", "--chain", "koven"]).unwrap(); + /// let _cfg = parity::Configuration::parse_cli(&["--light", "--chain", "kovan"]).unwrap(); /// ``` pub fn parse_cli>(command: &[S]) -> Result { let config = Configuration { diff --git a/parity/main.rs b/parity/main.rs index c9d5d204d..4a02f632f 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -18,54 +18,90 @@ #![warn(missing_docs)] -extern crate parity; - extern crate ctrlc; extern crate dir; extern crate fdlimit; #[macro_use] extern crate log; extern crate panic_hook; +extern crate parity; extern crate parking_lot; #[cfg(windows)] extern crate winapi; -use std::{process, env}; -use std::sync::atomic::{AtomicBool, Ordering}; -use std::io::{self as stdio, Read, Write}; +use std::ffi::OsString; use std::fs::{remove_file, metadata, File, create_dir_all}; +use std::io::{self as stdio, Read, Write}; use std::path::PathBuf; use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::{process, env}; + use ctrlc::CtrlC; use dir::default_hypervisor_path; use fdlimit::raise_fd_limit; use parity::{start, ExecutionAction}; use parking_lot::{Condvar, Mutex}; -fn updates_path(name: &str) -> PathBuf { - let mut dest = PathBuf::from(default_hypervisor_path()); +const PLEASE_RESTART_EXIT_CODE: i32 = 69; +const PARITY_EXECUTABLE_NAME: &str = "parity"; + +#[derive(Debug)] +enum Error { + BinaryNotFound, + ExitCode(i32), + Restart, + Unknown +} + +fn update_path(name: &str) -> PathBuf { + let mut dest = default_hypervisor_path(); dest.push(name); dest } -fn latest_exe_path() -> Option { - File::open(updates_path("latest")).ok() - .and_then(|mut f| { let mut exe = String::new(); f.read_to_string(&mut exe).ok().map(|_| updates_path(&exe)) }) +fn latest_exe_path() -> Result { + File::open(update_path("latest")).and_then(|mut f| { + let mut exe_path = String::new(); + trace!(target: "updater", "latest binary path: {:?}", f); + f.read_to_string(&mut exe_path).map(|_| update_path(&exe_path)) + }) + .or(Err(Error::BinaryNotFound)) } -fn set_spec_name_override(spec_name: String) { +fn latest_binary_is_newer(current_binary: &Option, latest_binary: &Option) -> bool { + match ( + current_binary + .as_ref() + .and_then(|p| metadata(p.as_path()).ok()) + .and_then(|m| m.modified().ok()), + latest_binary + .as_ref() + .and_then(|p| metadata(p.as_path()).ok()) + .and_then(|m| m.modified().ok()) + ) { + (Some(latest_exe_time), Some(this_exe_time)) if latest_exe_time > this_exe_time => true, + _ => false, + } +} + +fn set_spec_name_override(spec_name: &str) { if let Err(e) = create_dir_all(default_hypervisor_path()) - .and_then(|_| File::create(updates_path("spec_name_override")) + .and_then(|_| File::create(update_path("spec_name_override")) .and_then(|mut f| f.write_all(spec_name.as_bytes()))) { - warn!("Couldn't override chain spec: {} at {:?}", e, updates_path("spec_name_override")); + warn!("Couldn't override chain spec: {} at {:?}", e, update_path("spec_name_override")); } } fn take_spec_name_override() -> Option { - let p = updates_path("spec_name_override"); - let r = File::open(p.clone()).ok() - .and_then(|mut f| { let mut spec_name = String::new(); f.read_to_string(&mut spec_name).ok().map(|_| spec_name) }); + let p = update_path("spec_name_override"); + let r = File::open(p.clone()) + .ok() + .and_then(|mut f| { + let mut spec_name = String::new(); + f.read_to_string(&mut spec_name).ok().map(|_| spec_name) + }); let _ = remove_file(p); r } @@ -96,23 +132,35 @@ fn global_init() { #[cfg(not(windows))] fn global_cleanup() {} -// Starts ~/.parity-updates/parity and returns the code it exits with. -fn run_parity() -> Option { +// Starts parity binary installed via `parity-updater` and returns the code it exits with. +fn run_parity() -> Result<(), Error> { global_init(); - use ::std::ffi::OsString; + let prefix = vec![OsString::from("--can-restart"), OsString::from("--force-direct")]; - let res = latest_exe_path().and_then(|exe| process::Command::new(exe) + + let res: Result<(), Error> = latest_exe_path() + .and_then(|exe| process::Command::new(exe) .args(&(env::args_os().skip(1).chain(prefix.into_iter()).collect::>())) .status() - .map(|es| es.code().unwrap_or(128)) .ok() - ); + .map_or(Err(Error::Unknown), |es| { + match es.code() { + // Process success + Some(0) => Ok(()), + // Please restart + Some(PLEASE_RESTART_EXIT_CODE) => Err(Error::Restart), + // Process error code `c` + Some(c) => Err(Error::ExitCode(c)), + // Unknown error, couldn't determine error code + _ => Err(Error::Unknown), + } + }) + ); + global_cleanup(); res } -const PLEASE_RESTART_EXIT_CODE: i32 = 69; - #[derive(Debug)] /// Status used to exit or restart the program. struct ExitStatus { @@ -126,7 +174,7 @@ struct ExitStatus { spec_name_override: Option, } -// Run our version of parity. +// Run `locally installed version` of parity (i.e, not installed via `parity-updater`) // Returns the exit error code. fn main_direct(force_can_restart: bool) -> i32 { global_init(); @@ -244,7 +292,7 @@ fn main_direct(force_can_restart: bool) -> i32 { if lock.should_restart { if let Some(ref spec_name) = lock.spec_name_override { - set_spec_name_override(spec_name.clone()); + set_spec_name_override(&spec_name.clone()); } PLEASE_RESTART_EXIT_CODE } else { @@ -281,46 +329,80 @@ macro_rules! trace_main { fn main() { panic_hook::set_abort(); - // assuming the user is not running with `--force-direct`, then: - // if argv[0] == "parity" and this executable != ~/.parity-updates/parity, run that instead. + // the user has specified to run its originally installed binary (not via `parity-updater`) let force_direct = std::env::args().any(|arg| arg == "--force-direct"); - let exe = std::env::current_exe().ok(); - let development = exe.as_ref().and_then(|p| p.parent().and_then(|p| p.parent()).and_then(|p| p.file_name()).map(|n| n == "target")).unwrap_or(false); - let same_name = exe.as_ref().map(|p| p.file_stem().map_or(false, |s| s == "parity") && p.extension().map_or(true, |x| x == "exe")).unwrap_or(false); - trace_main!("Starting up {} (force-direct: {}, development: {}, same-name: {})", std::env::current_exe().map(|x| format!("{}", x.display())).unwrap_or("".to_owned()), force_direct, development, same_name); + + // absolute path to the current `binary` + let exe_path = std::env::current_exe().ok(); + + // the binary is named `target/xx/yy` + let development = exe_path + .as_ref() + .and_then(|p| { + p.parent() + .and_then(|p| p.parent()) + .and_then(|p| p.file_name()) + .map(|n| n == "target") + }) + .unwrap_or(false); + + // the binary is named `parity` + let same_name = exe_path + .as_ref() + .map_or(false, |p| { + p.file_stem().map_or(false, |n| n == PARITY_EXECUTABLE_NAME) + }); + + trace_main!("Starting up {} (force-direct: {}, development: {}, same-name: {})", + std::env::current_exe().ok().map_or_else(|| "".into(), |x| format!("{}", x.display())), + force_direct, + development, + same_name); + if !force_direct && !development && same_name { - // looks like we're not running ~/.parity-updates/parity when the user is expecting otherwise. + // Try to run the latest installed version of `parity`, + // Upon failure it falls back to the locally installed version of `parity` // Everything run inside a loop, so we'll be able to restart from the child into a new version seamlessly. loop { - // If we fail to run the updated parity then fallback to local version. - let latest_exe = latest_exe_path(); + // `Path` to the latest downloaded binary + let latest_exe = latest_exe_path().ok(); + + // `Latest´ binary exist let have_update = latest_exe.as_ref().map_or(false, |p| p.exists()); - let is_non_updated_current = exe.as_ref().map_or(false, |exe| latest_exe.as_ref().map_or(false, |lexe| exe.canonicalize().ok() != lexe.canonicalize().ok())); - let update_is_newer = match ( - latest_exe.as_ref() - .and_then(|p| metadata(p.as_path()).ok()) - .and_then(|m| m.modified().ok()), - exe.as_ref() - .and_then(|p| metadata(p.as_path()).ok()) - .and_then(|m| m.modified().ok()) - ) { - (Some(latest_exe_time), Some(this_exe_time)) if latest_exe_time > this_exe_time => true, - _ => false, - }; - trace_main!("Starting... (have-update: {}, non-updated-current: {}, update-is-newer: {})", have_update, is_non_updated_current, update_is_newer); - let exit_code = if have_update && is_non_updated_current && update_is_newer { - trace_main!("Attempting to run latest update ({})...", latest_exe.as_ref().expect("guarded by have_update; latest_exe must exist for have_update; qed").display()); - run_parity().unwrap_or_else(|| { trace_main!("Falling back to local..."); main_direct(true) }) + + // Canonicalized path to the current binary is not the same as to latest binary + let canonicalized_path_not_same = exe_path + .as_ref() + .map_or(false, |exe| latest_exe.as_ref() + .map_or(false, |lexe| exe.canonicalize().ok() != lexe.canonicalize().ok())); + + // Downloaded `binary` is newer + let update_is_newer = latest_binary_is_newer(&latest_exe, &exe_path); + trace_main!("Starting... (have-update: {}, non-updated-current: {}, update-is-newer: {})", have_update, canonicalized_path_not_same, update_is_newer); + + let exit_code = if have_update && canonicalized_path_not_same && update_is_newer { + trace_main!("Attempting to run latest update ({})...", + latest_exe.as_ref().expect("guarded by have_update; latest_exe must exist for have_update; qed").display()); + match run_parity() { + Ok(_) => 0, + // Restart parity + Err(Error::Restart) => PLEASE_RESTART_EXIT_CODE, + // Fall back to local version + Err(e) => { + error!(target: "updater", "Updated binary could not be executed error: {:?}. Falling back to local version", e); + main_direct(true) + } + } } else { trace_main!("No latest update. Attempting to direct..."); main_direct(true) }; - trace_main!("Latest exited with {}", exit_code); + trace_main!("Latest binary exited with exit code: {}", exit_code); if exit_code != PLEASE_RESTART_EXIT_CODE { trace_main!("Quitting..."); process::exit(exit_code); } - trace_main!("Rerunning..."); + trace!(target: "updater", "Re-running updater loop"); } } else { trace_main!("Running direct"); diff --git a/parity/run.rs b/parity/run.rs index aa1ba9d71..2155e434e 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -717,8 +717,8 @@ fn execute_impl(cmd: RunCmd, logger: Arc, on_client_rq: // the updater service let updater_fetch = fetch.clone(); let updater = Updater::new( - Arc::downgrade(&(service.client() as Arc)), - Arc::downgrade(&sync_provider), + &Arc::downgrade(&(service.client() as Arc)), + &Arc::downgrade(&sync_provider), update_policy, hash_fetch::Client::with_fetch(contract_client.clone(), cpu_pool.clone(), updater_fetch, event_loop.remote()) ); diff --git a/updater/Cargo.toml b/updater/Cargo.toml index 1bd8736bd..8bf708b76 100644 --- a/updater/Cargo.toml +++ b/updater/Cargo.toml @@ -28,3 +28,9 @@ rand = "0.4" ethcore = { path = "../ethcore", features = ["test-helpers"] } tempdir = "0.3" matches = "0.1" + +[features] +# hardcode version number 1.3.7 of parity to force an update +# in order to manually test that parity fall-over to the local version +# in case of invalid or deprecated command line arguments are entered +test-updater = [] diff --git a/updater/src/lib.rs b/updater/src/lib.rs index f27d74e7d..75447e8d5 100644 --- a/updater/src/lib.rs +++ b/updater/src/lib.rs @@ -16,6 +16,8 @@ //! Updater for Parity executables +#![warn(missing_docs)] + extern crate ethabi; extern crate ethcore; extern crate ethcore_bytes as bytes; diff --git a/updater/src/service.rs b/updater/src/service.rs index 604c01ec7..1cf35d4ea 100644 --- a/updater/src/service.rs +++ b/updater/src/service.rs @@ -16,6 +16,7 @@ use types::{CapState, ReleaseInfo, OperationsInfo, VersionInfo}; +/// Parity updater service trait pub trait Service: Send + Sync { /// Is the currently running client capable of supporting the current chain? /// We default to true if there's no clear information. diff --git a/updater/src/types/version_info.rs b/updater/src/types/version_info.rs index 955be0566..df2026b28 100644 --- a/updater/src/types/version_info.rs +++ b/updater/src/types/version_info.rs @@ -55,14 +55,14 @@ impl VersionInfo { let t = track.into(); VersionInfo { version: Version { - major: (semver >> 16) as u64, - minor: ((semver >> 8) & 0xff) as u64, - patch: (semver & 0xff) as u64, + major: u64::from(semver >> 16), + minor: u64::from((semver >> 8) & 0xff), + patch: u64::from(semver & 0xff), build: vec![], pre: vec![], }, track: t, - hash: hash, + hash, } } } diff --git a/updater/src/updater.rs b/updater/src/updater.rs index 8e9efa0aa..a3ed413c4 100644 --- a/updater/src/updater.rs +++ b/updater/src/updater.rs @@ -27,15 +27,16 @@ use target_info::Target; use bytes::Bytes; use ethcore::BlockNumber; -use ethcore::filter::Filter; use ethcore::client::{BlockId, BlockChainClient, ChainNotify, ChainRoute}; +use ethcore::filter::Filter; use ethereum_types::H256; -use sync::{SyncProvider}; use hash_fetch::{self as fetch, HashFetch}; use path::restrict_permissions_owner; use service::Service; +use sync::{SyncProvider}; use types::{ReleaseInfo, OperationsInfo, CapState, VersionInfo, ReleaseTrack}; use version; +use semver::Version; use_contract!(operations_contract, "Operations", "res/operations.json"); @@ -155,7 +156,7 @@ pub struct Updater, } -const CLIENT_ID: &'static str = "parity"; +const CLIENT_ID: &str = "parity"; lazy_static! { static ref CLIENT_ID_HASH: H256 = CLIENT_ID.as_bytes().into(); @@ -189,7 +190,7 @@ pub trait OperationsClient: Send + Sync + 'static { fn release_block_number(&self, from: BlockNumber, release: &ReleaseInfo) -> Option; } -/// OperationsClient that delegates calls to the operations contract. +/// `OperationsClient` that delegates calls to the operations contract. pub struct OperationsContractClient { operations_contract: operations_contract::Operations, client: Weak, @@ -267,7 +268,7 @@ impl OperationsClient for OperationsContractClient { // get the release info for the latest version in track let in_track = self.release_info(latest_in_track, &do_call)?; let mut in_minor = Some(in_track.clone()); - const PROOF: &'static str = "in_minor initialised and assigned with Some; loop breaks if None assigned; qed"; + const PROOF: &str = "in_minor initialized and assigned with Some; loop breaks if None assigned; qed"; // if the minor version has changed, let's check the minor version on a different track while in_minor.as_ref().expect(PROOF).version.version.minor != this.version.minor { @@ -308,7 +309,7 @@ impl OperationsClient for OperationsContractClient { from_block: BlockId::Number(from), to_block: BlockId::Latest, address: Some(vec![address]), - topics: topics, + topics, limit: None, }; @@ -333,7 +334,7 @@ pub trait TimeProvider: Send + Sync + 'static { fn now(&self) -> Instant; } -/// TimeProvider implementation that delegates calls to std::time. +/// `TimeProvider` implementation that delegates calls to `std::time`. pub struct StdTimeProvider; impl TimeProvider for StdTimeProvider { @@ -349,7 +350,7 @@ pub trait GenRange: Send + Sync + 'static { fn gen_range(&self, low: u64, high: u64) -> u64; } -/// GenRange implementation that uses a rand::thread_rng for randomness. +/// `GenRange` implementation that uses a `rand::thread_rng` for randomness. pub struct ThreadRngGenRange; impl GenRange for ThreadRngGenRange { @@ -359,14 +360,15 @@ impl GenRange for ThreadRngGenRange { } impl Updater { + /// `Updater` constructor pub fn new( - client: Weak, - sync: Weak, + client: &Weak, + sync: &Weak, update_policy: UpdatePolicy, fetcher: fetch::Client, ) -> Arc { let r = Arc::new(Updater { - update_policy: update_policy, + update_policy, weak_self: Mutex::new(Default::default()), client: client.clone(), sync: Some(sync.clone()), @@ -375,12 +377,21 @@ impl Updater { operations_contract::Operations::default(), client.clone()), exit_handler: Mutex::new(None), - this: VersionInfo::this(), + this: if cfg!(feature = "test-updater") { + VersionInfo { + track: ReleaseTrack::Stable, + version: Version::new(1, 3, 7), + hash: 0.into(), + } + } else { + VersionInfo::this() + }, time_provider: StdTimeProvider, rng: ThreadRngGenRange, state: Mutex::new(Default::default()), }); *r.weak_self.lock() = Arc::downgrade(&r); + r.poll(); r } @@ -447,7 +458,7 @@ impl Updater { - let delay = 2usize.pow(retries) as u64; + let delay = 2_usize.pow(retries) as u64; // cap maximum backoff to 1 day let delay = cmp::min(delay, 24 * 60 * 60); let backoff = (retries, self.time_provider.now() + Duration::from_secs(delay)); @@ -599,14 +610,19 @@ impl Updater Updater