Skip to content

Commit

Permalink
refactor!: migrate to gov v1 via the additions of MsgRecoverClient
Browse files Browse the repository at this point in the history
…and `MsgIBCSoftwareUpgrade`. The legacy proposal types `ClientUpdateProposal` and `UpgradeClientProposal` have been removed. (#4620)

* Add proto message, implement sdk.Msg for Recover Client. (#4494)

* Add proto message for Recover client, implement sdk.Message interface.

* Update modules/core/02-client/types/msgs.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Charly <charly@interchain.io>

* Remove gogoproto false for cmp, lint, move ibctesting address inline.

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>

* add protos, msgs, keeper handler to upgrade clients using v1 governance proposals (#4436)

* add protos and keeper function placeholder

* add keeper functions/tests, msgs and tests

* update to 0.47.5 release branch

* Move signer as last proto field. (#4510)

* msg server function and tests for MsgScheduleIBCClientUpgrade (#4442)

* Add 02-client implementation for Recover client. (#4499)

* Add 02-client implementation for Recover client.

* Partially address feedback.

* Docu RecoverClient, add label, re-use error.

* Add implementation for recover client on message server. (#4503)

* Add message server handler for recovering a client

* Don't assign to deprecated attrs, clean up unused fields.

* Further clean-up, remove declaration of unmutated vars.

* Add cmd for submitting a recover client prop. (#4522)

* Add cmd for submitting a recover client prop.

* Bump cosmossdk in e2e.

* Use govtypes.ModuleName, rename old govtypes to govv1beta1

* Update modules/core/02-client/client/cli/tx.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* Add auth flag.

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* rename command

* docs: fixed broken links (#4571)

* Add e2e test for recovering a client. (#4543)

Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>

* feat: add unpacket inerfaces message assertion (#4588)

* add cli for MsgIBCSoftwareUpgrade (#4558)

* docs: use MsgRecoverClient in docs (#4580)

* docs: recover client update

* Update docs/ibc/proposals.md

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

* refactor: remove legacy client update proposal (#4581)

* refactor: remove legacy client update proposal

* e2e: swap from ClientUpdateProposal e2e to RecoverClient

* refactor: remove unused events

* feat: add proposal simulator interface function and tests (#4466)

Co-authored-by: colin axner <25233464+colin-axner@users.noreply.github.com>

* refactor!: remove UpgradeProposal type (#4602)

* create separate event emission for ibc software upgrades vs an upgraded client (#4594)

* add new event type

* update event name

* fix build

* remove: legacy event emissions

* remove: unnecessary assignments, apply suggestion from code review

* e2e: schedule IBC software upgrade (#4585)

* wip e2e test

* query proposal

* update upgrade height in plan

* rm unnecessary wait/authority

* rm test artifact from merge

* add checks for scheduled plan

* hook up upgrade query client

* plan height

* pr fixes

* update test

* import space

* update newchainID value

* update clientID upgrade

* linter

* gci

* rm unnecessary event

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Sishir Giri <sis1001@berkeley.edu>
Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>
Co-authored-by: Cian Hatton <cian@interchain.io>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: sontrinh16 <trinhleson2000@gmail.com>
Co-authored-by: catShaark <gundamaster5@gmail.com>
Co-authored-by: khanh-notional <50263489+catShaark@users.noreply.github.com>

* chore: docs for MsgIBCSoftwareUpgrade (#4601)

* chore: update docs for UpgradeProposal -> MsgIBCSoftwareUpgrade

* chore: anticipate link change

* fix event docs

* refactor: s.Assert -> s.Require

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* make proto-all

* chore: update compiler assertion

* refactor: ordering, order as follows: create, update, upgrade, misbheaviour, recover, ibcsoftwareupgrade, update params

* refactor: simplify ibc software upgrade emitted event

* lint lint lint

* Apply suggestions from code review

Co-authored-by: Charly <charly@interchain.io>

* review of feat/govv1

* pr nits

* fix tests for error wrapping

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: Charly <charly@interchain.io>
Co-authored-by: Charly <charly@interchain.berlin>
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sishir Giri <sis1001@berkeley.edu>
Co-authored-by: Cian Hatton <cian@interchain.io>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: sontrinh16 <trinhleson2000@gmail.com>
Co-authored-by: catShaark <gundamaster5@gmail.com>
Co-authored-by: khanh-notional <50263489+catShaark@users.noreply.github.com>
  • Loading branch information
14 people authored Sep 13, 2023
1 parent 70a0f76 commit 26f7fe3
Show file tree
Hide file tree
Showing 58 changed files with 2,637 additions and 1,927 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
2 changes: 1 addition & 1 deletion .github/compatibility-test-matrices/unreleased/client.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
9 changes: 7 additions & 2 deletions docs/architecture/adr-026-ibc-client-recovery-mechanisms.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- 2021/05/20: Revision to simplify consensus state copying, remove initial height
- 2022/04/08: Revision to deprecate AllowUpdateAfterExpiry and AllowUpdateAfterMisbehaviour
- 2022/07/15: Revision to allow updating of TrustingPeriod
- 2023/09/05: Revision to migrate from gov v1beta1 to gov v1

## Status

Expand Down Expand Up @@ -44,16 +45,19 @@ We elect not to deal with chains which have actually halted, which is necessaril
1. `allow_update_after_misbehaviour` (boolean, default true). Note that this flag has been deprecated, it remains to signal intent but checks against this value will not be enforced.
1. Require Tendermint light clients (ICS 07) to expose the following additional state mutation functions
1. `Unfreeze()`, which unfreezes a light client after misbehaviour and clears any frozen height previously set
1. Add a new governance proposal type, `ClientUpdateProposal`, in the `x/ibc` module
1. Extend the base `Proposal` with two client identifiers (`string`).
1. Add a new governance proposal with `MsgRecoverClient`.
1. Create a new Msg with two client identifiers (`string`) and a signer.
1. The first client identifier is the proposed client to be updated. This client must be either frozen or expired.
1. The second client is a substitute client. It carries all the state for the client which may be updated. It must have identitical client and chain parameters to the client which may be updated (except for latest height, frozen height, and chain-id). It should be continually updated during the voting period.
1. If this governance proposal passes, the client on trial will be updated to the latest state of the substitute.
1. The signer must be the authority set for the ibc module.

Previously, `AllowUpdateAfterExpiry` and `AllowUpdateAfterMisbehaviour` were used to signal the recovery options for an expired or frozen client, and governance proposals were not allowed to overwrite the client if these parameters were set to false. However, this has now been deprecated because a code migration can overwrite the client and consensus states regardless of the value of these parameters. If governance would vote to overwrite a client or consensus state, it is likely that governance would also be willing to perform a code migration to do the same.

In addition, `TrustingPeriod` was initally not allowed to be updated by a client upgrade proposal. However, due to the number of situations experienced in production where the `TrustingPeriod` of a client should be allowed to be updated because of ie: initial misconfiguration for a canonical channel, governance should be allowed to update this client parameter.

In versions older than ibc-go v8, `MsgRecoverClient` was a governance proposal type `ClientUpdateProposal`. It has been removed and replaced by `MsgRecoverClient` in the migration from governance v1beta1 to governance v1.

Note that this should NOT be lightly updated, as there may be a gap in time between when misbehaviour has occured and when the evidence of misbehaviour is submitted. For example, if the `UnbondingPeriod` is 2 weeks and the `TrustingPeriod` has also been set to two weeks, a validator could wait until right before `UnbondingPeriod` finishes, submit false information, then unbond and exit without being slashed for misbehaviour. Therefore, we recommend that the trusting period for the 07-tendermint client be set to 2/3 of the `UnbondingPeriod`.

Note that clients frozen due to misbehaviour must wait for the evidence to expire to avoid becoming refrozen.
Expand Down Expand Up @@ -83,3 +87,4 @@ No neutral consequences.
- [Prior discussion](https://github.com/cosmos/ics/issues/421)
- [Epoch number discussion](https://github.com/cosmos/ics/issues/439)
- [Upgrade plan discussion](https://github.com/cosmos/ics/issues/445)
- [Migration from gov v1beta1 to gov v1](https://github.com/cosmos/ibc-go/issues/3672)
10 changes: 5 additions & 5 deletions docs/ibc/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ callbacks to IBC applications.
| update_client_proposal | client_type | {clientType} |
| update_client_proposal | consensus_height | {consensusHeight} |

### UpgradeProposal
### IBCSoftwareUpgrade

| Type | Attribute Key | Attribute Value |
|-------------------------|-----------------|-------------------|
| upgrade_client_proposal | title | {title} |
| upgrade_client_proposal | height | {height} |
| Type | Attribute Key | Attribute Value |
|-------------------------------|---------------------|---------------------------------|
| schedule_ibc_software_upgrade | title | {title} |
| schedule_ibc_software_upgrade | upgrade_plan_height | {plan.height} |

## ICS 03 - Connection

Expand Down
34 changes: 10 additions & 24 deletions docs/ibc/proposals.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ See also the relevant documentation: [ADR-026, IBC client recovery mechanisms](.
## Preconditions

- The chain is updated with ibc-go >= v1.1.0.
- There exists an active client (with a known client identifier) for the same counterparty chain as the expired client.
- The governance deposit.

Expand All @@ -76,11 +75,10 @@ The client is attached to the expected Akash `chain-id`. Note that although the

### Step 2

If the chain has been updated to ibc-go >= v1.1.0, anyone can submit the governance proposal to recover the client by executing this via CLI.
Anyone can submit the governance proposal to recover the client by executing the following via CLI.
If the chain is on an ibc-go version older than v8, please see the [relevant documentation](https://ibc.cosmos.network/v7.3.0/ibc/proposals.html).

> Note that the Cosmos SDK has updated how governance proposals are submitted in SDK v0.46, now requiring to pass a .json proposal file
- From SDK v0.46.x onwards
- From ibc-go v8 onwards

```shell
<binary> tx gov submit-proposal [path-to-proposal-json]
Expand All @@ -92,30 +90,20 @@ If the chain has been updated to ibc-go >= v1.1.0, anyone can submit the governa
{
"messages": [
{
"@type": "/ibc.core.client.v1.ClientUpdateProposal",
"title": "title_string",
"description": "description_string",
"@type": "/ibc.core.client.v1.MsgRecoverClient",
"subject_client_id": "expired_client_id_string",
"substitute_client_id": "active_client_id_string"
"substitute_client_id": "active_client_id_string",
"signer": "<gov-address>"
}
],
"metadata": "<metadata>",
"deposit": "10stake"
"title": "My proposal",
"summary": "A short summary of my proposal",
"expedited": false
}
```

Alternatively there's a legacy command (that is no longer recommended though):
```shell
<binary> tx gov submit-legacy-proposal update-client <expired-client-id> <active-client-id>
```
- Until SDK v0.45.x
```shell
<binary> tx gov submit-proposal update-client <expired-client-id> <active-client-id>
```
The `<expired-client-id>` identifier is the proposed client to be updated. This client must be either frozen or expired.

The `<active-client-id>` represents a substitute client. It carries all the state for the client which may be updated. It must have identical client and chain parameters to the client which may be updated (except for latest height, frozen height, and chain ID). It should be continually updated during the voting period.
Expand All @@ -124,6 +112,4 @@ After this, all that remains is deciding who funds the governance deposit and en

## Important considerations

Please note that from v1.0.0 of ibc-go it will not be allowed for transactions to go to expired clients anymore, so please update to at least this version to prevent similar issues in the future.
Please also note that if the client on the other end of the transaction is also expired, that client will also need to update. This process updates only one client.
Please note that if the counterparty client is also expired, that client will also need to update. This process updates only one client.
8 changes: 4 additions & 4 deletions docs/ibc/upgrades/genesis-restart.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ Genesis restarts still require the usage of an IBC upgrade proposal in order to

If the IBC-connected chain is conducting an upgrade that will break counterparty clients, it must ensure that the upgrade is first supported by IBC using the [IBC Client Breaking Upgrade List](https://github.com/cosmos/ibc-go/blob/main/docs/ibc/upgrades/quick-guide.md#ibc-client-breaking-upgrades) and then execute the upgrade process described below in order to prevent counterparty clients from breaking.

1. Create a 02-client [`UpgradeProposal`](https://github.com/cosmos/ibc-go/blob/main/docs/ibc/proto-docs.md#upgradeproposal) with an `UpgradePlan` and a new IBC ClientState in the `UpgradedClientState` field. Note that the `UpgradePlan` must specify an upgrade height **only** (no upgrade time), and the `ClientState` should only include the fields common to all valid clients and zero out any client-customizable fields (such as TrustingPeriod).
2. Vote on and pass the `UpgradeProposal`.
1. Create a governance proposal with the [`MsgIBCSoftwareUpgrade`](https://buf.build/cosmos/ibc/docs/main:ibc.core.client.v1#ibc.core.client.v1.MsgIBCSoftwareUpgrade) which contains an `UpgradePlan` and a new IBC `ClientState` in the `UpgradedClientState` field. Note that the `UpgradePlan` must specify an upgrade height **only** (no upgrade time), and the `ClientState` should only include the fields common to all valid clients and zero out any client-customizable fields (such as `TrustingPeriod`).
2. Vote on and pass the governance proposal.
3. Halt the node after successful upgrade.
4. Export the genesis file.
5. Swap to the new binary.
6. Run migrations on the genesis file.
7. Remove the `UpgradeProposal` plan from the genesis file. This may be done by migrations.
7. Remove the upgrade plan set by the governance proposal from the genesis file. This may be done by migrations.
8. Change desired chain-specific fields (chain id, unbonding period, etc). This may be done by migrations.
8. Reset the node's data.
9. Start the chain.

Upon the `UpgradeProposal` passing, the upgrade module will commit the UpgradedClient under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedClient`. On the block right before the upgrade height, the upgrade module will also commit an initial consensus state for the next chain under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedConsState`.
Upon passing the governance proposal, the upgrade module will commit the `UpgradedClient` under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedClient`. On the block right before the upgrade height, the upgrade module will also commit an initial consensus state for the next chain under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedConsState`.

Once the chain reaches the upgrade height and halts, a relayer can upgrade the counterparty clients to the last block of the old chain. They can then submit the proofs of the `UpgradedClient` and `UpgradedConsensusState` against this last block and upgrade the counterparty client.

Expand Down
6 changes: 3 additions & 3 deletions docs/ibc/upgrades/quick-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ Note: Since upgrades are only implemented for Tendermint clients, this doc only

If the IBC-connected chain is conducting an upgrade that will break counterparty clients, it must ensure that the upgrade is first supported by IBC using the list above and then execute the upgrade process described below in order to prevent counterparty clients from breaking.

1. Create a 02-client [`UpgradeProposal`](https://github.com/cosmos/ibc-go/blob/main/docs/ibc/proto-docs.md#upgradeproposal) with an `UpgradePlan` and a new IBC ClientState in the `UpgradedClientState` field. Note that the `UpgradePlan` must specify an upgrade height **only** (no upgrade time), and the `ClientState` should only include the fields common to all valid clients and zero out any client-customizable fields (such as TrustingPeriod).
2. Vote on and pass the `UpgradeProposal`.
1. Create a governance proposal with the [`MsgIBCSoftwareUpgrade`](https://buf.build/cosmos/ibc/docs/main:ibc.core.client.v1#ibc.core.client.v1.MsgIBCSoftwareUpgrade) message which contains an `UpgradePlan` and a new IBC `ClientState` in the `UpgradedClientState` field. Note that the `UpgradePlan` must specify an upgrade height **only** (no upgrade time), and the `ClientState` should only include the fields common to all valid clients (chain-specified parameters) and zero out any client-customizable fields (such as `TrustingPeriod`).
2. Vote on and pass the governance proposal.

Upon the `UpgradeProposal` passing, the upgrade module will commit the UpgradedClient under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedClient`. On the block right before the upgrade height, the upgrade module will also commit an initial consensus state for the next chain under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedConsState`.
Upon passing the governance proposal, the upgrade module will commit the `UpgradedClient` under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedClient`. On the block right before the upgrade height, the upgrade module will also commit an initial consensus state for the next chain under the key: `upgrade/UpgradedIBCState/{upgradeHeight}/upgradedConsState`.

Once the chain reaches the upgrade height and halts, a relayer can upgrade the counterparty clients to the last block of the old chain. They can then submit the proofs of the `UpgradedClient` and `UpgradedConsensusState` against this last block and upgrade the counterparty client.

Expand Down
Loading

0 comments on commit 26f7fe3

Please sign in to comment.