IBC Upgrade docs #8298

AdityaSripal · 2021-01-11T17:56:26Z

Description

Add docs for IBC Upgrades. Includes docs for users who want to upgrade their chain and counterparty clients, as well as a guide for developers who want to create IBC clients with upgrade functionality.

closes: #7877

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

AdityaSripal · 2021-01-11T17:59:10Z

docs/ibc/upgrades/quick-guide.md

+
+Note: Since upgrades are only implemented for Tendermint clients, this doc only discusses upgrades on Tendermint chains that would break counterparty IBC Tendermint Clients.
+
+1. Changing the Chain-ID: **Supported**


Future improvements may turn this list to a flowchart

colin-axner

LGTM, just left comments for clarification

docs/ibc/upgrades/developer-guide.md

docs/ibc/upgrades/quick-guide.md

colin-axner · 2021-01-12T13:54:54Z

docs/ibc/upgrades/quick-guide.md

+4. Changing the ProofSpecs: **Supported**, this should be changed if the proof structure needed to verify IBC proofs is changed across the upgrade. Ex: Switching from an IAVL store, to a SimpleTree Store
+5. Changing the UpgradePath: **Supported**, this might involve changing the key under which upgraded clients and consensus states are stored in the upgrade store, or even migrating the upgrade store itself.
+6. Migrating the IBC store: **Unsupported**, as the IBC store location is negotiated by the connection.
+7. Upgrading to a IBC path-breaking version of IBC: **Unsupported**, as IBC version is negotiated on connection handshake.


Isn't it supported so long as the old versions use the old path? Adding support for a new version with new path should be supported

Consider chainA and chainB are connected by an IBC channel. chainA decides to upgrade to a different version of IBC such that the paths in 24-host have changed. The client on chainB needs to upgrade its Verification methods to use the new paths.

This is theoretically possible by using the upgraded client state and then "converting" it into a new type that implements the different Verification method.

However, the IBC version that is used is negotiated in the connection. So that field needs to change as well which isn't supported with client upgrades.

Upgrading and breaking IBC paths is theoretically possible, it just isn't implemented yet and client upgrades alone will not upgrade the connection.

Technically the new IBC version could also store information in the old paths as well, and thus create "backwards-compatibility" for old connections, but this wouldn't be path breaking.

cc: @cwgoes

I'd prefer to avoid duplicating storage, though it is possible. Generally upgrading the version of IBC is a completely separate concern from upgrading a particular light client algorithm, and it should be treated as such - for example, the IBC version needs to be agreed on by both ends, while the light client versions are independent.

(sorry, I may have misunderstood the original comment - the light client upgrades should support changing paths used by the light client algorithm, but not IBC protocol paths in general, I think)

Maybe we could add this for clarity. It may not be immediately clear what a IBC path-breaking version of IBC is. But it's really just a non-backwards compatible version of IBC (IBC 2.0) ie we cannot upgrade IBC connection or channel versions with this upgrade mechanism

docs/ibc/upgrades/quick-guide.md

fedekunze · 2021-01-12T14:06:47Z

docs/ibc/upgrades/developer-guide.md

@@ -0,0 +1,40 @@
+# IBC Client Developer Guide to Upgrades


Suggested change

# IBC Client Developer Guide to Upgrades

# IBC Client Upgrade Guide

This document is specifically for developers who are trying to implement an IBC client. It is not relevant for end users. Is there a better title you had in mind that makes it clear this is for developers?

no, it's just that client developer sounds like wallets, exchanges, etc. Maybe light client devs?

docs/ibc/upgrades/developer-guide.md

fedekunze · 2021-01-12T14:09:15Z

docs/ibc/upgrades/developer-guide.md

+) (upgradedClient ClientState, upgradedConsensus ConsensusState, err error)
+```
+
+Note that the clients should have prior knowledge of the merkle path that the upgraded client and upgraded consensus states will use. The height at which the upgrade has occurred should also be encoded in the proof. In the Tendermint client implementation, the upgrade height is encoded in the proof key.


Note that the clients should have prior knowledge of the merkle path that the upgraded client and upgraded consensus states will use

where do clients know this info? I'd be nice to add that

fedekunze · 2021-01-12T14:15:07Z

docs/ibc/upgrades/overview.md

@@ -0,0 +1,8 @@
+### Upgrading IBC Chains Overview


fedekunze · 2021-01-12T14:15:52Z

docs/ibc/upgrades/quick-guide.md

@@ -0,0 +1,44 @@
+# How to Upgrade IBC Chains and their Clients


I don't understand the difference with the first doc from this title

This doc is intended for the end users of the upgrade process. Relayers and stake holders of the upgrading chain.

fedekunze · 2021-01-12T14:18:07Z

docs/ibc/upgrades/quick-guide.md

+## Upgrading Chains
+
+
+### IBC Client Breaking Upgrades


I think we should outline the types of upgrades that clients could encounter (planned, unplanned) and with breaking an non-breaking state machine changes. This doc should also document the recommended steps for each of the 4 combinations

Yup good point. Currently unplanned upgrades are not supported. All of the cases given only apply if they are planned.

Will write this point in.

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

docs/ibc/upgrades/developer-guide.md

docs/ibc/upgrades/quick-guide.md

cwgoes

ACK

cwgoes · 2021-01-20T02:39:35Z

docs/ibc/upgrades/quick-guide.md

+4. Changing the ProofSpecs: **Supported**, this should be changed if the proof structure needed to verify IBC proofs is changed across the upgrade. Ex: Switching from an IAVL store, to a SimpleTree Store
+5. Changing the UpgradePath: **Supported**, this might involve changing the key under which upgraded clients and consensus states are stored in the upgrade store, or even migrating the upgrade store itself.
+6. Migrating the IBC store: **Unsupported**, as the IBC store location is negotiated by the connection.
+7. Upgrading to a IBC path-breaking version of IBC: **Unsupported**, as IBC version is negotiated on connection handshake.


I'd prefer to avoid duplicating storage, though it is possible. Generally upgrading the version of IBC is a completely separate concern from upgrading a particular light client algorithm, and it should be treated as such - for example, the IBC version needs to be agreed on by both ends, while the light client versions are independent.

cwgoes · 2021-01-20T02:43:02Z

docs/ibc/upgrades/quick-guide.md

+4. Changing the ProofSpecs: **Supported**, this should be changed if the proof structure needed to verify IBC proofs is changed across the upgrade. Ex: Switching from an IAVL store, to a SimpleTree Store
+5. Changing the UpgradePath: **Supported**, this might involve changing the key under which upgraded clients and consensus states are stored in the upgrade store, or even migrating the upgrade store itself.
+6. Migrating the IBC store: **Unsupported**, as the IBC store location is negotiated by the connection.
+7. Upgrading to a IBC path-breaking version of IBC: **Unsupported**, as IBC version is negotiated on connection handshake.


(sorry, I may have misunderstood the original comment - the light client upgrades should support changing paths used by the light client algorithm, but not IBC protocol paths in general, I think)

colin-axner

ACK but see comment

fedekunze

LGTM

…to aditya/upgrade-docs

AdityaSripal added 2 commits January 11, 2021 14:32

start upgrade guide and some organization

ae80d21

add client developer guide

8cbe60a

AdityaSripal requested review from aaronc, alessio and alexanderbez as code owners January 11, 2021 17:56

AdityaSripal requested review from colin-axner, fedekunze and cwgoes January 11, 2021 17:56

remove top-level upgrade doc

ae3b1af

AdityaSripal commented Jan 11, 2021

View reviewed changes

Merge branch 'master' into aditya/upgrade-docs

9b0492a

colin-axner reviewed Jan 12, 2021

View reviewed changes

fedekunze reviewed Jan 12, 2021

View reviewed changes

Apply suggestions from code review

37e0686

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

cwgoes reviewed Jan 14, 2021

View reviewed changes

docs/ibc/upgrades/developer-guide.md Outdated Show resolved Hide resolved

docs/ibc/upgrades/quick-guide.md Show resolved Hide resolved

AdityaSripal added 3 commits January 19, 2021 12:45

address @colin-axner suggestions

6e25e9f

add order and synopsis

74d5412

address @fedekunze review

fce5bc7

AdityaSripal requested review from fedekunze, colin-axner and cwgoes January 19, 2021 17:58

Merge branch 'master' into aditya/upgrade-docs

2a6f315

cwgoes approved these changes Jan 20, 2021

View reviewed changes

colin-axner approved these changes Jan 20, 2021

View reviewed changes

fedekunze approved these changes Jan 20, 2021

View reviewed changes

AdityaSripal added 2 commits January 20, 2021 09:21

address final @colin-axner comments

9edd8db

Merge branch 'aditya/upgrade-docs' of github.com:cosmos/cosmos-sdk in…

00e0a7f

…to aditya/upgrade-docs

AdityaSripal added the A:automerge Automatically merge PR once all prerequisites pass. label Jan 20, 2021

Merge branch 'master' into aditya/upgrade-docs

ee1669e

mergify bot merged commit d4e63ff into master Jan 20, 2021

mergify bot deleted the aditya/upgrade-docs branch January 20, 2021 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBC Upgrade docs #8298

IBC Upgrade docs #8298

AdityaSripal commented Jan 11, 2021 •

edited

Loading

AdityaSripal Jan 11, 2021

colin-axner left a comment

colin-axner Jan 12, 2021

AdityaSripal Jan 19, 2021

cwgoes Jan 20, 2021

cwgoes Jan 20, 2021

colin-axner Jan 20, 2021

fedekunze Jan 12, 2021

AdityaSripal Jan 19, 2021

fedekunze Jan 20, 2021

fedekunze Jan 12, 2021

fedekunze Jan 12, 2021

fedekunze Jan 12, 2021

AdityaSripal Jan 19, 2021

fedekunze Jan 12, 2021

AdityaSripal Jan 19, 2021

cwgoes left a comment

cwgoes Jan 20, 2021

cwgoes Jan 20, 2021

colin-axner left a comment

fedekunze left a comment


		Note: Since upgrades are only implemented for Tendermint clients, this doc only discusses upgrades on Tendermint chains that would break counterparty IBC Tendermint Clients.

		1. Changing the Chain-ID: Supported

	# IBC Client Developer Guide to Upgrades
	# IBC Client Upgrade Guide

		@@ -0,0 +1,44 @@
		# How to Upgrade IBC Chains and their Clients

IBC Upgrade docs #8298

IBC Upgrade docs #8298

Conversation

AdityaSripal commented Jan 11, 2021 • edited Loading

Description

Choose a reason for hiding this comment

colin-axner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwgoes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colin-axner left a comment

Choose a reason for hiding this comment

fedekunze left a comment

Choose a reason for hiding this comment

AdityaSripal commented Jan 11, 2021 •

edited

Loading