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

Auto-updating #3505

Merged
merged 120 commits into from
Dec 16, 2016
Merged

Auto-updating #3505

merged 120 commits into from
Dec 16, 2016

Conversation

gavofyork
Copy link
Contributor

@gavofyork gavofyork commented Nov 18, 2016

Parity can automatically stop itself from getting on an old fork and can update itself to stay in line with the current protocol.

Apologies for the size; there's a lot of license boilerplate, a large auto-generated file and cargo stuff, so the amount of code to review is not quite as big as the number suggests.

Parity checks an on-chain contract (address determined from the registry) to derive information about the protocol's current forks, about the releases of Parity and about the binary builds for those releases. Using this information, it can download updates and install them into an updates folder ($DATA(parity-hypervisor) at present). Parity's startup procedure is altered so it can recognise later releases and (re)spawn instances of those rather than continue with its own logic. When a new update is installed, Parity automatically stops and restarts with the new version seamlessly.

Auto-updating is not for everyone, so several configuration options are provided to limit or disable this functionality:

  • --auto-update determines what updates are installed, if any, and can be configured to all (all releases in the current release track---stable or beta), critical (same as all, except releases that are not security or consensus-critical are not automatically installed), and none (no releases are automatically installed).
  • --no-download: normally the latest releases are downloaded ready for installation (a UI component exists to facilitate this). This prevents that activity.
  • --no-consensus: normally if it is detected that this client would fork out of consensus, further syncing & block-importing would be disabled. This prevents that and allows the client to continue on the non-canonical fork.
  • --force-direct: normally Parity will run the latest installed version; this option disables that and ensures that the originally installed version of Parity runs.

This code detects when Parity is being run during a normal development process and disables the latest-installed-respawning. This functionality is also disabled when the executable running is anything other than parity or parity.exe. (e.g. Linking or renaming /usr/bin/parity to /usr/bin/parity.orig will allow you to always run the original parity via /usr/bin/parity.orig.)

To disable all auto-update functionality, run Parity with --auto-update=none --no-download --no-consensus --force-direct. To enable all auto-update functionality run with --auto-update=all. By default, Parity will run with --auto-update=critical and will update itself only when security and/or consensus-critical releases are made.

The contract governing all information (including binary hashes of new releases) can be found in our contracts repo. It may be used by all clients and can track the status of the entire protocol, including forks, EIP-voting, releases and binaries. Though only a single account is in control of each client (and therefore in issuing new binaries), Parity plans on using an m-of-n multisig wallet with several core developers requiring sign-off prior to deployment to avoid any potential insecurities and coercion problems.

To get effect an update a number of steps are needed. Firstly, the GithubHint contract should be updated with the permanent web location for the parity binary for each platform. Then, a new release must be made for the client ("parity") and tagged with a track (stable/beta/nightly) together with a hash which should be equal to the Git commit at which the release is taken (right-aligned, since it's a bytes32). Finally, for each platform that we support, the Keccak-256 hash of the parity binary executable for that platform should be noted. The platform string under which the binary is mapped should correspond to the platform that the Rust compiler gives.

Five JSON RPCs have been added to support this functionality:

  • parity_consensusCapability returns an object or string detailing the state of our capability of maintaining consensus. Either "capable", {"capableUntil":N}, {"incapableSince":N} or "unknown" (N is a block number).
  • parity_versionInfo returns a VersionInfo object describing our current version. Looks like: {"hash":H,"track":T,"version":{"major":N,"minor":N,"patch":N}} (H is a 160-bit Git commit hash, T is a ReleaseTrack, either "stable", "beta", "nightly" or "unknown" and N is a version number).
  • parity_releasesInfo returns a ReleasesInfo object describing the current status of releases. Looks like: {"fork":N,"minor":null,"this_fork":MN,"track":R} (N is a block number representing the latest known fork of this chain which may be in the future, MN is a block number representing the latest known fork that the currently running binary can sync past or null if not known, R is a ReleaseInfo object describing the latest release in this binary's release track).
  • parity_upgradeReady returns a ReleaseInfo object describing the release which is available for upgrade or null if none is available. It looks like: {"binary":H,"fork":15100,"is_critical":true,"version":V} where H is the Keccak-256 checksum of the release's parity binary and V is a VersionInfo object describing the release.
  • parity_executeUpgrade returns true if the upgrade to the release specified in parity_upgradeReady was successfully executed, false if not.

Gitlab configuration is updated to track nightly builds and call a server oracle process which updates the Operations contract. A separate repo (http://github.com/ethcore/push-release) exists for the server-side code.

Yet todo:

  • Use file-contents rather than a symlink to support Windows;
  • Ensure restarts only happen in cradle;
  • Ensure logging & other one-time-only inits don't get initialised a second time;
  • Integrate with client so that when out of consensus we pause syncing;
  • Integrate with RPCs;
  • UI to display if an update is pending and allow installation if so;
  • Place info! under updater target;
  • Remove testing-cradle code;
  • Ensure that updating doesn't happen while warp sync has not yet verified the entire chain.

For another PR:

  • Support for the "patch" upgrade track;
  • Persist preferences in UserDefaults and provide UI.

Also refactored Informant to get rid of the superfluous IoServiceHandler and added some nicer registrar interfaces to Client.

- Add auto-gen'ed Operations and Registry ABIs.
- Add Updater for managing updates.
- Add fields in Client to enable update checking and registry.
@gavofyork gavofyork added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Nov 18, 2016
@rphmeier
Copy link
Contributor

Seems strange to put this in Client, maybe the Service or somewhere in the parity module.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling bf9ed2d on check-updates into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling bf9ed2d on check-updates into ** on master**.

@@ -908,8 +930,16 @@ impl BlockChainClient for Client {
r
}

fn disable(&self) {
self.set_mode(IpcMode::Off);
self.enabled.store(false, AtomicOrdering::Relaxed);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps should also clear the block queue as it won't be processed anyway.

/// Returns engine-related extra info for `BlockId`.
fn block_extra_info(&self, id: BlockId) -> Option<BTreeMap<String, String>>;

/// Returns engine-related extra info for `UncleId`.
/// Returns engine-related extra info for `UncleID`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct?

@@ -29,6 +29,12 @@ pub use self::denominations::*;

use super::spec::*;

/// Most recent fork block that we support on Mainnet.
pub const FORK_SUPPORTED_FRONTIER: u64 = 2675000;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at present it's used by the CI only.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How? It is not mentioned anywhere else in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh - it's part of the server-side stuff; there's a link in the PR's comment.

pub const FORK_SUPPORTED_FRONTIER: u64 = 2675000;

/// Most recent fork block that we support on Ropsten.
pub const FORK_SUPPORTED_ROPSTEN: u64 = 10;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at present it's used by the CI only.

@@ -339,6 +360,9 @@ Legacy Options:
--extradata STRING Equivalent to --extra-data STRING.
--cache MB Equivalent to --cache-size MB.

Internal Options:
--can-restart Executable will auto-restart if exiting with 125.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

125 or 69?

return;
fn println_trace_main(s: String) {
if env::var("RUST_LOG").ok().and_then(|s| s.find("main=trace")).is_some() {
println!("{}", s);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it need to go into stdout? Might interfere with commands that write result to stdout, e.g. parity export blocks

Copy link
Contributor Author

@gavofyork gavofyork Dec 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is used for the normative output then using parity as a command. e.g. users expect parity tools hash myfile to place the hash on stdout like any other unix tool.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is it will also produce tracing output in stdout besides normative output. And if normative output is json or binary it will get broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what you mean here. if it is being used for anything other than information which should go onto stdout, then it is being used incorrectly.

if an execution option requires a message to be placed on stderr or in a log, it should do that explicitly, not pass it back to the dispatch context.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currentlyparity export blocks -l trace produces trace messages in stdout (line 271 below) as well as the json output

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right; i'll sort this.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 85.853% when pulling a6a8e43 on check-updates into 5fb3457 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 85.861% when pulling a6a8e43 on check-updates into 5fb3457 on master.

fn collect_latest(&self) -> Result<OperationsInfo, String> {
if let Some(ref operations) = *self.operations.lock() {
let hh: H256 = self.this.hash.into();
info!(target: "updater", "Looking up this_fork for our release: {}/{:?}", CLIENT_ID, hh);
Copy link
Collaborator

@arkpar arkpar Dec 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional info!?

info!(target: "updater", "Looking up this_fork for our release: {}/{:?}", CLIENT_ID, hh);
let this_fork = operations.release(CLIENT_ID, &self.this.hash.into()).ok()
.and_then(|(fork, track, _, _)| {
info!(target: "updater", "Operations returned fork={}, track={}", fork as u64, track);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meant to be displayed?

impl ChainNotify for Updater {
fn new_blocks(&self, _imported: Vec<H256>, _invalid: Vec<H256>, _enacted: Vec<H256>, _retracted: Vec<H256>, _sealed: Vec<H256>, _proposed: Vec<Bytes>, _duration: u64) {
// TODO: something like this
// if !self.client.upgrade().map_or(true, |c| c.is_major_syncing()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This notification want be sent while the client is major syncing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll fix this up

@arkpar arkpar added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 16, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 85.818% when pulling b437265 on check-updates into 5fb3457 on master.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Dec 16, 2016
@gavofyork gavofyork merged commit 6e5a583 into master Dec 16, 2016
@jacogr jacogr deleted the check-updates branch December 22, 2016 14:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants