From a9d25bf71dd171e9c22fe1df2b2e045a806cb586 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 23 Nov 2021 17:26:53 -0700 Subject: [PATCH 01/14] ADR-047: Initial Draft. --- .../adr-047-extend-upgrade-plan.md | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/architecture/adr-047-extend-upgrade-plan.md diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md new file mode 100644 index 000000000000..0df21a3ab7f6 --- /dev/null +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -0,0 +1,188 @@ +# ADR 047: Extend Upgrade Plan + +## Changelog + +- Nov, 23, 2021: Initial Draft + +## Status + +DRAFT Not Implemented + +## Abstract + +This ADR expands the existing upgrade `Plan` proto message to include new fields for defining pre-run and post-run processes. +It also defines a structure for providing downloadable assets involved in an upgrade. + +## Context + +The `upgrade` module in conjunction with Cosmovisor are designed to facilitate and automate a blockchain's transition from one version to another. + +Users submit a software upgrade governance proposal containing an upgrade `Plan`. +The `Plan` currently contains the following fields: +- `name`: A short string identifying the new version. +- `height`: The chain height at which the upgrade is to be performed. +- `info`: A string containing information about the upgrade. It can either be a json object (with a specific structure defined through documentation), or it can be a URL that will return such json. + +The json object from the `info` field identifies URLs used to download the new blockchain executable for different platforms (OS and Architecture, e.g. "linux/amd64"). +Such a URL can either return the executable file directly or can return an archive containing the executable and possibly other assets. +If an archive contains a `bin/pre-upgrade` file, Cosmovisor executes it (and waits for it to finish) before attempting to restart the chain. + +Currently, there is no mechanism that makes Cosmovisor run a command after the upgraded chain has been restarted. + +## Decision + +### Protobuf Updates + +We will define a new message for providing upgrade instructions and add it as a new field in the `Plan` message. +The upgrade instructions will contain a list of assets available for each platform. +It will also allow for the definition of a pre-run and post-run command. + +```protobuf +message Plan { + // ... (existing fields) + + UpgradeInstructions upgrade = 6; +} +``` + +The new `UpgradeInstructions upgrade` field MUST be optional. + +```protobuf +message UpgradeInstructions { + string pre_run = 1; + string post_run = 2; + repeated Asset assets = 3; + string description = 4; +} +``` + +All fields in the `UpgradeInstructions` SHOULD be optional. +- `pre_run` is a command to run prior to the upgraded chain restarting. + If defined, this command SHOULD behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/master/docs/migrations/pre-upgrade.md) command. + The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. +- `post_run` is a command to run after the upgraded chain has been started. If defined, this command SHOULD be only executed once. + The output and exit code SHOULD be logged but SHOULD NOT affect the running of the upgraded chain. + The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. +- `assets` SHOULD allow any number of entries. + It SHOULD have only one entry per platform. +- `description` contains additional information about the upgrade and might contain references to external resources. + It SHOULD NOT be used for structured processing information. + +```protobuf +message Asset { + string platform = 1; + string url = 2; + string checksum = 3; +} +``` + +- `platform` is a required string that SHOULD be in the format `{OS}/{CPU}`, e.g. `"linux/amd64"`. + The string `"any"` SHOULD also be allowed. + An `Asset` with a `platform` of `"any"` SHOULD be used as a fallback when a specific `{OS}/{CPU}` entry is not found. + That is, if an `Asset` exists with a `platform` that matches the system's OS and CPU, that should be used; + otherwise, if an `Asset` exists with a `platform` of `any`, that should be used; + otherwise no asset should be downloaded. +- `url` is a required string that MUST conform to [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt). +- `checksum` SHOULD be provided, but is not required. + It SHOULD be a hex encoded checksum of the expected result of a request to the `url`. + Implementations utilizing these `UpgradeInstructions` SHOULD fail if the checksum of the result of the `url` is not equal to this provided `checksum`. + SHA-256 SHOULD be the default hashing algorithm. + This `checksum` field SHOULD allow other hashing algorithms by allowing the checksum to be prefixed by the algorithm, then a colon. + E.g. A `checksum` of `"sha256:6e4003b3104a5936d8142191352ba6b7af530fcb883578946b538a816a9571f9"` SHOULD be equal to `"6e4003b3104a5936d8142191352ba6b7af530fcb883578946b538a816a9571f9"`, + and a `checksum` of `"sha512:2cafa43204f0f51229c01c1bdd3b29b6a446bfdc313a474a7091a87fe122d3defaee0de7aa7534496385c8dad02a8d76a8c39634c742677d20c9fdc2da59448e"` SHOULD be allowed. + If the `url` contains a `checksum` query parameter, this `checksum` field SHOULD still be populated and if populated MUST equal the query parameter `checksum` value. + +### Upgrade module updates. + +If an upgrade `Plan` does not use the new `UpgradeInstructions` field, existing functionality will be maintained. + +We will update the creation of the `upgrade-info.json` file to include the `UpgradeInstructions`. + +We will update the optional validation available via CLI to account for the new `Plan` structure. +We will add the following validation: +1. If `UpgradeInstructions` are provided: + 1. There MUST be at least one entry in `assets`. + 1. All of the `assets` MUST have a unique `platform`. +1. The following validation is currently done using the `info` field. We will apply similar validation to the `UpgradeInstructions`. + For each `Asset`: + 1. The `platform` MUST have the format `{OS}/{CPU}` or be `"any"`. + 1. The `url` field MUST NOT be empty. + 1. The `url` field MUST be a proper URL. + 1. A `checksum` MUST be provided either in the `checksum` field or as a query parameter in the `url`. + 1. If the `checksum` field has a value and the `url` also has a `checksum` query parameter, the two values MUST be equal. + 1. The `url` MUST return either a file or an archive containing eitehr `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. + 1. If a `checksum` is provided (in the field or as a query param), the checksum of the result of the `url` MUST equal the provided checksum. + +### Cosmovisor updates + +If the `upgrade-info.json` file does not contain any `UpgradeInstructions`, existing functionality will be maintained. + +We will update Cosmovisor to look for and handle the new `UpgradeInstructions` in `upgrade-info.json`. +If the `UpgradeInstructions` are provided, we will do the following: +1. The `info` field will be ignored. +1. The `assets` field will be used to identify the asset to download based on the `platform` that Cosmovisor is running in. +1. If the downloaded asset is an archive that contains a `bin/pre-upgrade` file, that file will be ignored. +1. If a `checksum` is provided (either in the field or as a query param in the `url`), and the downloaded asset has a different checksum, the upgrade process will be interrupted and Cosmovisor will exit with an error. +1. If a `pre_run` command is defined, it will be executed at the same point in the process where the `bin/pre-upgrade` file would have been executed. + It will be executed using the same environment as other commands run by Cosmovisor. +1. If a `post_run` command is defined, it will be executed after executing the command that restarts the chain. + It will be executed in a background process using the same environment as the other commands. + Any output generated by the command will be logged. + Once complete, the exit code will be logged. + +## Consequences + +### Backwards Compatibility + +Since the only change to existing definitions is the addition of the `upgrade` field to the `Plan` message, and that field MUST be optional, there are no backwards incompatibilities with respects to the proto messages. +Additionally, current behavior will be maintained when no `UpgradeInstructions` are provided, so there are no backwards incompatibilities with respects to either the upgrade module or Cosmovisor. + +### Forwards Compatibility + +In order to utilize the `UpgradeInstructions` as part of a software upgrade, both of the following must be true: +1. The chain must already be using a sufficiently advanced version of the Cosmos SDK. +1. The chain's nodes must be using a sufficiently advanced version of Cosmovisor. + +### Positive + +1. The structure for defining downloadable assets is clearer since it is now defined in the proto instead of in documentation. +1. Availability of a pre-run command becomes more obvious. +1. A post-run command becomes possible. + +### Negative + +1. The `Plan` message becomes larger. +1. There is no option for providing a URL that will return the `UpgradeInstructions`. + Existing functionality of the `info` field will remain, but will not allow for the newly defined structure. +1. There is no built-in mechanism for preventing the `assets` from having two entries with the same `platform`. + Behavior of this situation is undefined. +1. All `assets` are assumed to either be the executable or be an archive that contains the executable. + There is no way to define more than one downloadable asset for a platform. +1. A chain software upgrade is required before this new functionality can be used. + I.e. The next upgrade must use the old definitions, but the one after that can use the new structure. + +### Neutral + +1. Existing functionality is maintained when the `UpgradeInstructions` aren't provided. + +## Further Discussions + +1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r698708349): + Consider different names for `UpgradeInstructions upgrade` (either the message type or field name). +1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r754655072): + 1. Consider putting the `string platform` field inside `UpgradeInstructions` and make `UpgradeInstructions` a repeated field in `Plan`. + 1. Consider using a `oneof` field in the `Plan` which could either be `UpgradeInstructions` or else a URL that should return the `UpgradeInstructions`. + 1. Consider allowing `info` to either be a JSON serialized version of `UpgradeInstructions` or else a URL that returns that. +1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r755462876): + Consider not including the `UpgradeInstructions.description` field, using the `info` field for that purpose instead. +1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r754643691): + Consider allowing multiple assets to be downloaded for any given `platform` by adding a `name` field to the `Asset` message. + +## References + +- [Current upgrade.proto](https://github.com/cosmos/cosmos-sdk/blob/master/proto/cosmos/upgrade/v1beta1/upgrade.proto) +- [Upgrade Module README](https://github.com/cosmos/cosmos-sdk/blob/master/x/upgrade/spec/README.md) +- [Cosmovisor README](https://github.com/cosmos/cosmos-sdk/blob/master/cosmovisor/README.md) +- [Pre-upgrade README](https://github.com/cosmos/cosmos-sdk/blob/master/docs/migrations/pre-upgrade.md) +- [Draft/POC PR #10032](https://github.com/cosmos/cosmos-sdk/pull/10032) +- [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt) From 07d7c1f4219f85cd87ce891b7484e12469b0e8b3 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 23 Nov 2021 17:30:00 -0700 Subject: [PATCH 02/14] Update ADR main README to include ADR 047: Extend Upgrade Plan. --- docs/architecture/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 03b22eb33f21..d1f9a3ba39f2 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -82,3 +82,4 @@ When writing ADRs, follow the same best practices for writing RFCs. When writing ### Draft - [ADR 044: Guidelines for Updating Protobuf Definitions](./adr-044-protobuf-updates-guidelines.md) +- [ADR 047: Extend Upgrade Plan](./adr-047-extend-upgrade-plan.md) From 81f41ef0bb91826b3ec6f5e2615e864cdfc19174 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Fri, 3 Dec 2021 15:50:31 -0700 Subject: [PATCH 03/14] Upgrade ADR-047 with comments from the PR. --- .../adr-047-extend-upgrade-plan.md | 75 +++++++++---------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 0df21a3ab7f6..06d73bcee39c 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -10,20 +10,23 @@ DRAFT Not Implemented ## Abstract -This ADR expands the existing upgrade `Plan` proto message to include new fields for defining pre-run and post-run processes. -It also defines a structure for providing downloadable assets involved in an upgrade. +This ADR expands the existing x/upgrade `Plan` proto message to include new fields for defining pre-run and post-run processes. +It also defines a structure for providing downloadable artifacts involved in an upgrade. ## Context The `upgrade` module in conjunction with Cosmovisor are designed to facilitate and automate a blockchain's transition from one version to another. Users submit a software upgrade governance proposal containing an upgrade `Plan`. -The `Plan` currently contains the following fields: +The [Plan](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/proto/cosmos/upgrade/v1beta1/upgrade.proto#L12) currently contains the following fields: - `name`: A short string identifying the new version. - `height`: The chain height at which the upgrade is to be performed. -- `info`: A string containing information about the upgrade. It can either be a json object (with a specific structure defined through documentation), or it can be a URL that will return such json. +- `info`: A string containing information about the upgrade. -The json object from the `info` field identifies URLs used to download the new blockchain executable for different platforms (OS and Architecture, e.g. "linux/amd64"). +The `info` string can be anything. +However, Cosmovisor will try to use the `info` field to automatically download a new version of the blockchain executable. +For the auto-download to work, Cosmovisor expects it to be either a stringified JSON object (with a specific structure defined through documentation), or a URL that will return such JSON. +The JSON object identifies URLs used to download the new blockchain executable for different platforms (OS and Architecture, e.g. "linux/amd64"). Such a URL can either return the executable file directly or can return an archive containing the executable and possibly other assets. If an archive contains a `bin/pre-upgrade` file, Cosmovisor executes it (and waits for it to finish) before attempting to restart the chain. @@ -34,7 +37,7 @@ Currently, there is no mechanism that makes Cosmovisor run a command after the u ### Protobuf Updates We will define a new message for providing upgrade instructions and add it as a new field in the `Plan` message. -The upgrade instructions will contain a list of assets available for each platform. +The upgrade instructions will contain a list of artifacts available for each platform. It will also allow for the definition of a pre-run and post-run command. ```protobuf @@ -49,39 +52,39 @@ The new `UpgradeInstructions upgrade` field MUST be optional. ```protobuf message UpgradeInstructions { - string pre_run = 1; - string post_run = 2; - repeated Asset assets = 3; - string description = 4; + string pre_run = 1; + string post_run = 2; + repeated Artifact artifacts = 3; + string description = 4; } ``` All fields in the `UpgradeInstructions` SHOULD be optional. - `pre_run` is a command to run prior to the upgraded chain restarting. - If defined, this command SHOULD behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/master/docs/migrations/pre-upgrade.md) command. + If defined, this command SHOULD behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) command. The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. - `post_run` is a command to run after the upgraded chain has been started. If defined, this command SHOULD be only executed once. The output and exit code SHOULD be logged but SHOULD NOT affect the running of the upgraded chain. The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. -- `assets` SHOULD allow any number of entries. +- `artifacts` SHOULD allow any number of entries. It SHOULD have only one entry per platform. - `description` contains additional information about the upgrade and might contain references to external resources. It SHOULD NOT be used for structured processing information. ```protobuf -message Asset { +message Artifact { string platform = 1; - string url = 2; + string url = 2; string checksum = 3; } ``` - `platform` is a required string that SHOULD be in the format `{OS}/{CPU}`, e.g. `"linux/amd64"`. The string `"any"` SHOULD also be allowed. - An `Asset` with a `platform` of `"any"` SHOULD be used as a fallback when a specific `{OS}/{CPU}` entry is not found. - That is, if an `Asset` exists with a `platform` that matches the system's OS and CPU, that should be used; - otherwise, if an `Asset` exists with a `platform` of `any`, that should be used; - otherwise no asset should be downloaded. + An `Artifact` with a `platform` of `"any"` SHOULD be used as a fallback when a specific `{OS}/{CPU}` entry is not found. + That is, if an `Artifact` exists with a `platform` that matches the system's OS and CPU, that should be used; + otherwise, if an `Artifact` exists with a `platform` of `any`, that should be used; + otherwise no artifact should be downloaded. - `url` is a required string that MUST conform to [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt). - `checksum` SHOULD be provided, but is not required. It SHOULD be a hex encoded checksum of the expected result of a request to the `url`. @@ -101,16 +104,16 @@ We will update the creation of the `upgrade-info.json` file to include the `Upgr We will update the optional validation available via CLI to account for the new `Plan` structure. We will add the following validation: 1. If `UpgradeInstructions` are provided: - 1. There MUST be at least one entry in `assets`. - 1. All of the `assets` MUST have a unique `platform`. + 1. There MUST be at least one entry in `artifacts`. + 1. All of the `artifacts` MUST have a unique `platform`. 1. The following validation is currently done using the `info` field. We will apply similar validation to the `UpgradeInstructions`. - For each `Asset`: + For each `Artifact`: 1. The `platform` MUST have the format `{OS}/{CPU}` or be `"any"`. 1. The `url` field MUST NOT be empty. 1. The `url` field MUST be a proper URL. 1. A `checksum` MUST be provided either in the `checksum` field or as a query parameter in the `url`. 1. If the `checksum` field has a value and the `url` also has a `checksum` query parameter, the two values MUST be equal. - 1. The `url` MUST return either a file or an archive containing eitehr `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. + 1. The `url` MUST return either a file or an archive containing either `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. 1. If a `checksum` is provided (in the field or as a query param), the checksum of the result of the `url` MUST equal the provided checksum. ### Cosmovisor updates @@ -120,9 +123,9 @@ If the `upgrade-info.json` file does not contain any `UpgradeInstructions`, exis We will update Cosmovisor to look for and handle the new `UpgradeInstructions` in `upgrade-info.json`. If the `UpgradeInstructions` are provided, we will do the following: 1. The `info` field will be ignored. -1. The `assets` field will be used to identify the asset to download based on the `platform` that Cosmovisor is running in. -1. If the downloaded asset is an archive that contains a `bin/pre-upgrade` file, that file will be ignored. -1. If a `checksum` is provided (either in the field or as a query param in the `url`), and the downloaded asset has a different checksum, the upgrade process will be interrupted and Cosmovisor will exit with an error. +1. The `artifacts` field will be used to identify the artifact to download based on the `platform` that Cosmovisor is running in. +1. If the artifact is an archive that contains a `bin/pre-upgrade` file, that file will be ignored. +1. If a `checksum` is provided (either in the field or as a query param in the `url`), and the downloaded artifact has a different checksum, the upgrade process will be interrupted and Cosmovisor will exit with an error. 1. If a `pre_run` command is defined, it will be executed at the same point in the process where the `bin/pre-upgrade` file would have been executed. It will be executed using the same environment as other commands run by Cosmovisor. 1. If a `post_run` command is defined, it will be executed after executing the command that restarts the chain. @@ -134,7 +137,7 @@ If the `UpgradeInstructions` are provided, we will do the following: ### Backwards Compatibility -Since the only change to existing definitions is the addition of the `upgrade` field to the `Plan` message, and that field MUST be optional, there are no backwards incompatibilities with respects to the proto messages. +Since the only change to existing definitions is the addition of the `upgrade` field to the `Plan` message, and that field is optional, there are no backwards incompatibilities with respects to the proto messages. Additionally, current behavior will be maintained when no `UpgradeInstructions` are provided, so there are no backwards incompatibilities with respects to either the upgrade module or Cosmovisor. ### Forwards Compatibility @@ -145,7 +148,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot ### Positive -1. The structure for defining downloadable assets is clearer since it is now defined in the proto instead of in documentation. +1. The structure for defining artifacts is clearer since it is now defined in the proto instead of in documentation. 1. Availability of a pre-run command becomes more obvious. 1. A post-run command becomes possible. @@ -153,17 +156,13 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot 1. The `Plan` message becomes larger. 1. There is no option for providing a URL that will return the `UpgradeInstructions`. - Existing functionality of the `info` field will remain, but will not allow for the newly defined structure. -1. There is no built-in mechanism for preventing the `assets` from having two entries with the same `platform`. - Behavior of this situation is undefined. -1. All `assets` are assumed to either be the executable or be an archive that contains the executable. - There is no way to define more than one downloadable asset for a platform. +1. The only way to provide multiple assets for a platform is to use an archive as the platform's artifact. 1. A chain software upgrade is required before this new functionality can be used. I.e. The next upgrade must use the old definitions, but the one after that can use the new structure. ### Neutral -1. Existing functionality is maintained when the `UpgradeInstructions` aren't provided. +1. Existing functionality of the `info` field is maintained when the `UpgradeInstructions` aren't provided. ## Further Discussions @@ -176,13 +175,13 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot 1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r755462876): Consider not including the `UpgradeInstructions.description` field, using the `info` field for that purpose instead. 1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r754643691): - Consider allowing multiple assets to be downloaded for any given `platform` by adding a `name` field to the `Asset` message. + Consider allowing multiple artifacts to be downloaded for any given `platform` by adding a `name` field to the `Artifact` message. ## References -- [Current upgrade.proto](https://github.com/cosmos/cosmos-sdk/blob/master/proto/cosmos/upgrade/v1beta1/upgrade.proto) -- [Upgrade Module README](https://github.com/cosmos/cosmos-sdk/blob/master/x/upgrade/spec/README.md) -- [Cosmovisor README](https://github.com/cosmos/cosmos-sdk/blob/master/cosmovisor/README.md) -- [Pre-upgrade README](https://github.com/cosmos/cosmos-sdk/blob/master/docs/migrations/pre-upgrade.md) +- [Current upgrade.proto](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/proto/cosmos/upgrade/v1beta1/upgrade.proto) +- [Upgrade Module README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/x/upgrade/spec/README.md) +- [Cosmovisor README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/cosmovisor/README.md) +- [Pre-upgrade README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) - [Draft/POC PR #10032](https://github.com/cosmos/cosmos-sdk/pull/10032) - [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt) From 6a06647a8ebbd29ca0af0753049453544c3d15cf Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Fri, 3 Dec 2021 16:01:55 -0700 Subject: [PATCH 04/14] Fix references to bin/pre-upgrade. --- docs/architecture/adr-047-extend-upgrade-plan.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 06d73bcee39c..00ebab6c8ac9 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -28,7 +28,6 @@ However, Cosmovisor will try to use the `info` field to automatically download a For the auto-download to work, Cosmovisor expects it to be either a stringified JSON object (with a specific structure defined through documentation), or a URL that will return such JSON. The JSON object identifies URLs used to download the new blockchain executable for different platforms (OS and Architecture, e.g. "linux/amd64"). Such a URL can either return the executable file directly or can return an archive containing the executable and possibly other assets. -If an archive contains a `bin/pre-upgrade` file, Cosmovisor executes it (and waits for it to finish) before attempting to restart the chain. Currently, there is no mechanism that makes Cosmovisor run a command after the upgraded chain has been restarted. @@ -124,9 +123,8 @@ We will update Cosmovisor to look for and handle the new `UpgradeInstructions` i If the `UpgradeInstructions` are provided, we will do the following: 1. The `info` field will be ignored. 1. The `artifacts` field will be used to identify the artifact to download based on the `platform` that Cosmovisor is running in. -1. If the artifact is an archive that contains a `bin/pre-upgrade` file, that file will be ignored. 1. If a `checksum` is provided (either in the field or as a query param in the `url`), and the downloaded artifact has a different checksum, the upgrade process will be interrupted and Cosmovisor will exit with an error. -1. If a `pre_run` command is defined, it will be executed at the same point in the process where the `bin/pre-upgrade` file would have been executed. +1. If a `pre_run` command is defined, it will be executed at the same point in the process where the `app pre-upgrade` command would have been executed. It will be executed using the same environment as other commands run by Cosmovisor. 1. If a `post_run` command is defined, it will be executed after executing the command that restarts the chain. It will be executed in a background process using the same environment as the other commands. From 268d4c4dac056597ff16a4c0979d635b0c7e73ed Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Mon, 6 Dec 2021 10:00:37 -0700 Subject: [PATCH 05/14] adr-047: Add to the Cosmovisor updates section. Further overload the info field to allow for the new structure. Remove the negative entry about that. --- docs/architecture/adr-047-extend-upgrade-plan.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 00ebab6c8ac9..579faa8b4e3d 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -131,6 +131,10 @@ If the `UpgradeInstructions` are provided, we will do the following: Any output generated by the command will be logged. Once complete, the exit code will be logged. +We will update Cosmovisor to allow JSON of the new `UpgradeInstructions` to be allowed in the existing `info` field. +We will also allow a url in the `info` field to return such JSON. +When parsing the `info` field, Cosmovisor will first look for the new `UpgradeInstructions` structure; if not found, Cosmovisor will fallback to existing functionality. + ## Consequences ### Backwards Compatibility @@ -154,9 +158,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot 1. The `Plan` message becomes larger. 1. There is no option for providing a URL that will return the `UpgradeInstructions`. -1. The only way to provide multiple assets for a platform is to use an archive as the platform's artifact. -1. A chain software upgrade is required before this new functionality can be used. - I.e. The next upgrade must use the old definitions, but the one after that can use the new structure. +1. The only way to provide multiple assets (executables and other files) for a platform is to use an archive as the platform's artifact. ### Neutral From fba53530e6b051f38a8ac9ea9cf432d682f33c38 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Mon, 6 Dec 2021 10:01:50 -0700 Subject: [PATCH 06/14] adr-047: Fix a couple headings to be more standard. --- docs/architecture/adr-047-extend-upgrade-plan.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 579faa8b4e3d..cbfaf8176e98 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -94,7 +94,7 @@ message Artifact { and a `checksum` of `"sha512:2cafa43204f0f51229c01c1bdd3b29b6a446bfdc313a474a7091a87fe122d3defaee0de7aa7534496385c8dad02a8d76a8c39634c742677d20c9fdc2da59448e"` SHOULD be allowed. If the `url` contains a `checksum` query parameter, this `checksum` field SHOULD still be populated and if populated MUST equal the query parameter `checksum` value. -### Upgrade module updates. +### Upgrade Module Updates If an upgrade `Plan` does not use the new `UpgradeInstructions` field, existing functionality will be maintained. @@ -115,7 +115,7 @@ We will add the following validation: 1. The `url` MUST return either a file or an archive containing either `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. 1. If a `checksum` is provided (in the field or as a query param), the checksum of the result of the `url` MUST equal the provided checksum. -### Cosmovisor updates +### Cosmovisor Updates If the `upgrade-info.json` file does not contain any `UpgradeInstructions`, existing functionality will be maintained. From c7849a893d1431d68f4a3c784dbfa8eeb369abce Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Fri, 10 Dec 2021 13:36:12 -0700 Subject: [PATCH 07/14] ADR-047: Some more updates from comments. --- .../adr-047-extend-upgrade-plan.md | 59 ++++++++++++------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index cbfaf8176e98..34aeee309774 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -29,6 +29,12 @@ For the auto-download to work, Cosmovisor expects it to be either a stringified The JSON object identifies URLs used to download the new blockchain executable for different platforms (OS and Architecture, e.g. "linux/amd64"). Such a URL can either return the executable file directly or can return an archive containing the executable and possibly other assets. +If the URL returns an archive, it is decompressed into `{DAEMON_HOME}/cosmovisor/{upgrade name}`. +Then, if `{DAEMON_HOME}/cosmovisor/{upgrade name}/bin/{DAEMON_NAME}` does not exist, but `{DAEMON_HOME}/cosmovisor/{upgrade name}/{DAEMON_NAME}` does, the latter is copied to the former. +If the URL returns something other than an archive, it is downloaded to `{DAEMON_HOME}/cosmovisor/{upgrade name}/bin/{DAEMON_NAME}`. + +Both `DAEMON_HOME` and `DAEMON_NAME` are [environment variables used to configure Cosmovisor](https://github.com/cosmos/cosmos-sdk/blob/cosmovisor/v1.0.0/cosmovisor/README.md#command-line-arguments-and-environment-variables). + Currently, there is no mechanism that makes Cosmovisor run a command after the upgraded chain has been restarted. ## Decision @@ -58,23 +64,25 @@ message UpgradeInstructions { } ``` -All fields in the `UpgradeInstructions` SHOULD be optional. +All fields in the `UpgradeInstructions` are optional. - `pre_run` is a command to run prior to the upgraded chain restarting. - If defined, this command SHOULD behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) command. - The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. -- `post_run` is a command to run after the upgraded chain has been started. If defined, this command SHOULD be only executed once. + If defined, then the app supervisors (e.g. Cosmovisor) MUST NOT run `app pre-run`. + If defined, this command MUST behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) command. + The working directory this command runs from MUST be `{DAEMON_HOME}/cosmovisor/{upgrade name}`. +- `post_run` is a command to run after the upgraded chain has been started. If defined, this command MUST be only executed once. The output and exit code SHOULD be logged but SHOULD NOT affect the running of the upgraded chain. - The working directory this command runs from SHOULD be `{DAEMON_HOME}/{upgrade name}`. -- `artifacts` SHOULD allow any number of entries. + The working directory this command runs from MUST be `{DAEMON_HOME}/cosmovisor/{upgrade name}`. +- `artifacts` define items to be downloaded. It SHOULD have only one entry per platform. -- `description` contains additional information about the upgrade and might contain references to external resources. +- `description` contains human-readable information about the upgrade and might contain references to external resources. It SHOULD NOT be used for structured processing information. ```protobuf message Artifact { - string platform = 1; - string url = 2; - string checksum = 3; + string platform = 1; + string url = 2; + string checksum = 3; + string checksum_algo = 4; } ``` @@ -84,15 +92,20 @@ message Artifact { That is, if an `Artifact` exists with a `platform` that matches the system's OS and CPU, that should be used; otherwise, if an `Artifact` exists with a `platform` of `any`, that should be used; otherwise no artifact should be downloaded. -- `url` is a required string that MUST conform to [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt). -- `checksum` SHOULD be provided, but is not required. - It SHOULD be a hex encoded checksum of the expected result of a request to the `url`. - Implementations utilizing these `UpgradeInstructions` SHOULD fail if the checksum of the result of the `url` is not equal to this provided `checksum`. - SHA-256 SHOULD be the default hashing algorithm. - This `checksum` field SHOULD allow other hashing algorithms by allowing the checksum to be prefixed by the algorithm, then a colon. - E.g. A `checksum` of `"sha256:6e4003b3104a5936d8142191352ba6b7af530fcb883578946b538a816a9571f9"` SHOULD be equal to `"6e4003b3104a5936d8142191352ba6b7af530fcb883578946b538a816a9571f9"`, - and a `checksum` of `"sha512:2cafa43204f0f51229c01c1bdd3b29b6a446bfdc313a474a7091a87fe122d3defaee0de7aa7534496385c8dad02a8d76a8c39634c742677d20c9fdc2da59448e"` SHOULD be allowed. - If the `url` contains a `checksum` query parameter, this `checksum` field SHOULD still be populated and if populated MUST equal the query parameter `checksum` value. +- `url` is a required URL string that MUST conform to [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt). + A request to this `url` MUST return either an executable file or an archive containing either `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. +- `checksum` is a checksum of the expected result of a request to the `url`. + It is not required, but is recommended. + If provided, it MUST be a hex encoded checksum string. + Tools utilizing these `UpgradeInstructions` MUST fail if a `checksum` is provided but is different from the checksum of the result returned by the `url`. +- `checksum_algo` is a string identify the algorithm used to generate the `checksum`. + Recommended algorithms: `sha256`, `sha512`. + Algorithms also supported (but not recommended): `sha1`, `md5`. + If a `checksum` is provided, a `checksum_algo` MUST also be provided. + +A `url` is not required to contain a `checksum` query parameter. +If the `url` does contain a `checksum` query parameter, the `checksum` and `checksum_algo` fields MUST also be populated, and their values MUST match the value of the query parameter. +For example, if the `url` is `"https://example.com?checksum=md5:d41d8cd98f00b204e9800998ecf8427e"`, then the `checksum` field must be `"d41d8cd98f00b204e9800998ecf8427e"` and the `checksum_algo` field must be `"md5"`. ### Upgrade Module Updates @@ -105,6 +118,10 @@ We will add the following validation: 1. If `UpgradeInstructions` are provided: 1. There MUST be at least one entry in `artifacts`. 1. All of the `artifacts` MUST have a unique `platform`. + 1. For each `Artifact`, if the `url` contains a `checksum` query parameter: + 1. The `checksum` query parameter value MUST be in the format of `{checksum_algo}:{checksum}`. + 1. The `{checksum}` from the query parameter MUST equal the `checksum` provided in the `Artifact`. + 1. The `{checksum_algo}` from the query parameter MUST equal the `checksum_algo` provided in the `Artifact`. 1. The following validation is currently done using the `info` field. We will apply similar validation to the `UpgradeInstructions`. For each `Artifact`: 1. The `platform` MUST have the format `{OS}/{CPU}` or be `"any"`. @@ -115,6 +132,8 @@ We will add the following validation: 1. The `url` MUST return either a file or an archive containing either `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. 1. If a `checksum` is provided (in the field or as a query param), the checksum of the result of the `url` MUST equal the provided checksum. +Downloading of an `Artifact` will happen the same way that URLs from `info` are currently downloaded. + ### Cosmovisor Updates If the `upgrade-info.json` file does not contain any `UpgradeInstructions`, existing functionality will be maintained. @@ -181,7 +200,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot - [Current upgrade.proto](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/proto/cosmos/upgrade/v1beta1/upgrade.proto) - [Upgrade Module README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/x/upgrade/spec/README.md) -- [Cosmovisor README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/cosmovisor/README.md) +- [Cosmovisor README](https://github.com/cosmos/cosmos-sdk/blob/cosmovisor/v1.0.0/cosmovisor/README.md) - [Pre-upgrade README](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) - [Draft/POC PR #10032](https://github.com/cosmos/cosmos-sdk/pull/10032) - [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt) From 99179118a116be7bde6dae03b9414c904db094fe Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 11 Jan 2022 11:29:58 -0700 Subject: [PATCH 08/14] ADR-47: Updates from commeents. Just clarification stuff. --- .../adr-047-extend-upgrade-plan.md | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 34aeee309774..1e4542f5829d 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -6,7 +6,7 @@ ## Status -DRAFT Not Implemented +PROPOSED Not Implemented ## Abstract @@ -41,19 +41,19 @@ Currently, there is no mechanism that makes Cosmovisor run a command after the u ### Protobuf Updates -We will define a new message for providing upgrade instructions and add it as a new field in the `Plan` message. +We will update the `x/upgrade.Plan` message for providing upgrade instructions. The upgrade instructions will contain a list of artifacts available for each platform. -It will also allow for the definition of a pre-run and post-run command. +It allows for the definition of a pre-run and post-run commands. ```protobuf message Plan { // ... (existing fields) - UpgradeInstructions upgrade = 6; + UpgradeInstructions instructions = 6; } ``` -The new `UpgradeInstructions upgrade` field MUST be optional. +The new `UpgradeInstructions instructions` field MUST be optional. ```protobuf message UpgradeInstructions { @@ -66,10 +66,19 @@ message UpgradeInstructions { All fields in the `UpgradeInstructions` are optional. - `pre_run` is a command to run prior to the upgraded chain restarting. - If defined, then the app supervisors (e.g. Cosmovisor) MUST NOT run `app pre-run`. - If defined, this command MUST behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) command. + If defined, it will be executed after halting and downloading the new artifact but before restarting the upgraded chain. The working directory this command runs from MUST be `{DAEMON_HOME}/cosmovisor/{upgrade name}`. -- `post_run` is a command to run after the upgraded chain has been started. If defined, this command MUST be only executed once. + This command MUST behave the same as the current [pre-upgrade](https://github.com/cosmos/cosmos-sdk/blob/v0.44.5/docs/migrations/pre-upgrade.md) command. + It does not take in any command-line arguments and is expected to terminate with the following exit codes: + + | Exit status code | How it is handled in Cosmosvisor | + |------------------|---------------------------------------------------------------------------------------------------------------------| + | `0` | Assumes `pre-upgrade` command executed successfully and continues the upgrade. | + | `1` | Default exit code when `pre-upgrade` command has not been implemented. | + | `30` | `pre-upgrade` command was executed but failed. This fails the entire upgrade. | + | `31` | `pre-upgrade` command was executed but failed. But the command is retried until exit code `1` or `30` are returned. | + If defined, then the app supervisors (e.g. Cosmovisor) MUST NOT run `app pre-run`. +- `post_run` is a command to run after the upgraded chain has been started. If defined, this command MUST be only executed at most once by an upgrading node. The output and exit code SHOULD be logged but SHOULD NOT affect the running of the upgraded chain. The working directory this command runs from MUST be `{DAEMON_HOME}/cosmovisor/{upgrade name}`. - `artifacts` define items to be downloaded. @@ -77,6 +86,8 @@ All fields in the `UpgradeInstructions` are optional. - `description` contains human-readable information about the upgrade and might contain references to external resources. It SHOULD NOT be used for structured processing information. + + ```protobuf message Artifact { string platform = 1; @@ -94,6 +105,7 @@ message Artifact { otherwise no artifact should be downloaded. - `url` is a required URL string that MUST conform to [RFC 1738: Uniform Resource Locators](https://www.ietf.org/rfc/rfc1738.txt). A request to this `url` MUST return either an executable file or an archive containing either `bin/{DAEMON_NAME}` or `{DAEMON_NAME}`. + The URL should not contain checksum - it should be specified by the `checksum` attribute. - `checksum` is a checksum of the expected result of a request to the `url`. It is not required, but is recommended. If provided, it MUST be a hex encoded checksum string. @@ -158,7 +170,7 @@ When parsing the `info` field, Cosmovisor will first look for the new `UpgradeIn ### Backwards Compatibility -Since the only change to existing definitions is the addition of the `upgrade` field to the `Plan` message, and that field is optional, there are no backwards incompatibilities with respects to the proto messages. +Since the only change to existing definitions is the addition of the `instructions` field to the `Plan` message, and that field is optional, there are no backwards incompatibilities with respects to the proto messages. Additionally, current behavior will be maintained when no `UpgradeInstructions` are provided, so there are no backwards incompatibilities with respects to either the upgrade module or Cosmovisor. ### Forwards Compatibility @@ -186,7 +198,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot ## Further Discussions 1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r698708349): - Consider different names for `UpgradeInstructions upgrade` (either the message type or field name). + Consider different names for `UpgradeInstructions instructions` (either the message type or field name). 1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r754655072): 1. Consider putting the `string platform` field inside `UpgradeInstructions` and make `UpgradeInstructions` a repeated field in `Plan`. 1. Consider using a `oneof` field in the `Plan` which could either be `UpgradeInstructions` or else a URL that should return the `UpgradeInstructions`. From 538aa17969f7482bd51f325b993c4c0b646bda70 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 11 Jan 2022 11:41:26 -0700 Subject: [PATCH 09/14] ADR-47: Some more clarification tweaks. --- docs/architecture/adr-047-extend-upgrade-plan.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 1e4542f5829d..3efe764ee1aa 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -10,7 +10,7 @@ PROPOSED Not Implemented ## Abstract -This ADR expands the existing x/upgrade `Plan` proto message to include new fields for defining pre-run and post-run processes. +This ADR expands the existing x/upgrade `Plan` proto message to include new fields for defining pre-run and post-run processes within upgrade tooling. It also defines a structure for providing downloadable artifacts involved in an upgrade. ## Context @@ -44,6 +44,7 @@ Currently, there is no mechanism that makes Cosmovisor run a command after the u We will update the `x/upgrade.Plan` message for providing upgrade instructions. The upgrade instructions will contain a list of artifacts available for each platform. It allows for the definition of a pre-run and post-run commands. +These commands are not consensus guaranteed; they will be executed by cosmosvisor (or other) during its upgrade handling. ```protobuf message Plan { From fdf039daaa18f7f5d59f3e9c71e663437905b3a0 Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 11 Jan 2022 12:07:43 -0700 Subject: [PATCH 10/14] ADR-047: Allow a URL to the new JSON, but not the new JSON. --- docs/architecture/adr-047-extend-upgrade-plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 3efe764ee1aa..7b4754f9e3ec 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -163,7 +163,7 @@ If the `UpgradeInstructions` are provided, we will do the following: Any output generated by the command will be logged. Once complete, the exit code will be logged. -We will update Cosmovisor to allow JSON of the new `UpgradeInstructions` to be allowed in the existing `info` field. +We will update Cosmovisor to allow the `info` field to be a URL to JSON of the new `UpgradeInstructions` structure. We will also allow a url in the `info` field to return such JSON. When parsing the `info` field, Cosmovisor will first look for the new `UpgradeInstructions` structure; if not found, Cosmovisor will fallback to existing functionality. From 2e717070272e128e1addf5e25774b1b913bbb9af Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Tue, 11 Jan 2022 12:35:47 -0700 Subject: [PATCH 11/14] ADR-047: Add sentence about Cosmovisor stopping without a new version available. --- docs/architecture/adr-047-extend-upgrade-plan.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 7b4754f9e3ec..ff1e5d3dc8ad 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -33,6 +33,8 @@ If the URL returns an archive, it is decompressed into `{DAEMON_HOME}/cosmovisor Then, if `{DAEMON_HOME}/cosmovisor/{upgrade name}/bin/{DAEMON_NAME}` does not exist, but `{DAEMON_HOME}/cosmovisor/{upgrade name}/{DAEMON_NAME}` does, the latter is copied to the former. If the URL returns something other than an archive, it is downloaded to `{DAEMON_HOME}/cosmovisor/{upgrade name}/bin/{DAEMON_NAME}`. +If an upgrade height is reached and the new version of the executable version isn't available, Cosmovisor will stop running. + Both `DAEMON_HOME` and `DAEMON_NAME` are [environment variables used to configure Cosmovisor](https://github.com/cosmos/cosmos-sdk/blob/cosmovisor/v1.0.0/cosmovisor/README.md#command-line-arguments-and-environment-variables). Currently, there is no mechanism that makes Cosmovisor run a command after the upgraded chain has been restarted. From f06ade31a5589264782805fb1555c5c801d78c0a Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 13 Jan 2022 15:06:25 -0700 Subject: [PATCH 12/14] ADR-047: Add a couple discussion links. --- docs/architecture/adr-047-extend-upgrade-plan.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index ff1e5d3dc8ad..0dbe6d6572b5 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -210,6 +210,10 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot Consider not including the `UpgradeInstructions.description` field, using the `info` field for that purpose instead. 1. [Draft PR #10032 Comment](https://github.com/cosmos/cosmos-sdk/pull/10032/files?authenticity_token=pLtzpnXJJB%2Fif2UWiTp9Td3MvRrBF04DvjSuEjf1azoWdLF%2BSNymVYw9Ic7VkqHgNLhNj6iq9bHQYnVLzMXd4g%3D%3D&file-filters%5B%5D=.go&file-filters%5B%5D=.proto#r754643691): Consider allowing multiple artifacts to be downloaded for any given `platform` by adding a `name` field to the `Artifact` message. +1. [PR #10502 Comment](https://github.com/cosmos/cosmos-sdk/pull/10602#discussion_r781438288) + Allow the new `UpgradeInstructions` to be provided via URL. +1. [PR #10502 Comment](https://github.com/cosmos/cosmos-sdk/pull/10602#discussion_r781438288) + Allow definition of a `signer` for assets (as an alternative to using a `checksum`). ## References From 238a06e41ee9cde8ef010d5030d764f2a2f26a5b Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 20 Jan 2022 11:47:24 -0700 Subject: [PATCH 13/14] ADR-047: Add a bit to the negative about increased message size. --- docs/architecture/adr-047-extend-upgrade-plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index 0dbe6d6572b5..f81ab9ee3bfd 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -190,7 +190,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot ### Negative -1. The `Plan` message becomes larger. +1. The `Plan` message becomes larger. This is negligable because A) the `x/upgrades` module only stores at most one upgrade plan, and B) upgrades are rare enough that the increased gas cost isn't a concern. 1. There is no option for providing a URL that will return the `UpgradeInstructions`. 1. The only way to provide multiple assets (executables and other files) for a platform is to use an archive as the platform's artifact. From ff60d43329f718bba3e9eecac6f731efb4d2f43a Mon Sep 17 00:00:00 2001 From: Daniel Wedul Date: Thu, 20 Jan 2022 12:25:09 -0700 Subject: [PATCH 14/14] ADR-047: Update to reflect decisions made in discussions. Add timelines. Indicate that existing info field functionality is being deprecated. --- .../adr-047-extend-upgrade-plan.md | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/docs/architecture/adr-047-extend-upgrade-plan.md b/docs/architecture/adr-047-extend-upgrade-plan.md index f81ab9ee3bfd..0df03006201b 100644 --- a/docs/architecture/adr-047-extend-upgrade-plan.md +++ b/docs/architecture/adr-047-extend-upgrade-plan.md @@ -39,6 +39,16 @@ Both `DAEMON_HOME` and `DAEMON_NAME` are [environment variables used to configur Currently, there is no mechanism that makes Cosmovisor run a command after the upgraded chain has been restarted. +The current upgrade process has this timeline: +1. An upgrade governance proposal is submitted and approved. +1. The upgrade height is reached. +1. The `x/upgrade` module writes the `upgrade_info.json` file. +1. The chain halts. +1. Cosmovisor backs up the data directory (if set up to do so). +1. Cosmovisor downloads the new executable (if not already in place). +1. Cosmovisor executes the `${DAEMON_NAME} pre-upgrade`. +1. Cosmovisor restarts the app using the new version and same args originally provided. + ## Decision ### Protobuf Updates @@ -46,7 +56,7 @@ Currently, there is no mechanism that makes Cosmovisor run a command after the u We will update the `x/upgrade.Plan` message for providing upgrade instructions. The upgrade instructions will contain a list of artifacts available for each platform. It allows for the definition of a pre-run and post-run commands. -These commands are not consensus guaranteed; they will be executed by cosmosvisor (or other) during its upgrade handling. +These commands are not consensus guaranteed; they will be executed by Cosmosvisor (or other) during its upgrade handling. ```protobuf message Plan { @@ -89,8 +99,6 @@ All fields in the `UpgradeInstructions` are optional. - `description` contains human-readable information about the upgrade and might contain references to external resources. It SHOULD NOT be used for structured processing information. - - ```protobuf message Artifact { string platform = 1; @@ -125,6 +133,8 @@ For example, if the `url` is `"https://example.com?checksum=md5:d41d8cd98f00b204 ### Upgrade Module Updates If an upgrade `Plan` does not use the new `UpgradeInstructions` field, existing functionality will be maintained. +The parsing of the `info` field as either a URL or `binaries` JSON will be deprecated. +During validation, if the `info` field is used as such, a warning will be issued, but not an error. We will update the creation of the `upgrade-info.json` file to include the `UpgradeInstructions`. @@ -165,9 +175,19 @@ If the `UpgradeInstructions` are provided, we will do the following: Any output generated by the command will be logged. Once complete, the exit code will be logged. -We will update Cosmovisor to allow the `info` field to be a URL to JSON of the new `UpgradeInstructions` structure. -We will also allow a url in the `info` field to return such JSON. -When parsing the `info` field, Cosmovisor will first look for the new `UpgradeInstructions` structure; if not found, Cosmovisor will fallback to existing functionality. +We will deprecate the use of the `info` field for anything other than human readable information. +A warning will be logged if the `info` field is used to define the assets (either by URL or JSON). + +The new upgrade timeline is very similar to the current one. Changes are in bold: +1. An upgrade governance proposal is submitted and approved. +1. The upgrade height is reached. +1. The `x/upgrade` module writes the `upgrade_info.json` file **(now possibly with `UpgradeInstructions`)**. +1. The chain halts. +1. Cosmovisor backs up the data directory (if set up to do so). +1. Cosmovisor downloads the new executable (if not already in place). +1. Cosmovisor executes **the `pre_run` command if provided**, or else the `${DAEMON_NAME} pre-upgrade` command. +1. Cosmovisor restarts the app using the new version and same args originally provided. +1. **Cosmovisor immediately runs the `post_run` command in a detached process.** ## Consequences @@ -190,7 +210,7 @@ In order to utilize the `UpgradeInstructions` as part of a software upgrade, bot ### Negative -1. The `Plan` message becomes larger. This is negligable because A) the `x/upgrades` module only stores at most one upgrade plan, and B) upgrades are rare enough that the increased gas cost isn't a concern. +1. The `Plan` message becomes larger. This is negligible because A) the `x/upgrades` module only stores at most one upgrade plan, and B) upgrades are rare enough that the increased gas cost isn't a concern. 1. There is no option for providing a URL that will return the `UpgradeInstructions`. 1. The only way to provide multiple assets (executables and other files) for a platform is to use an archive as the platform's artifact.