Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document IBC upgrade support for genesis dump/restart #21

Closed
4 tasks
colin-axner opened this issue Jan 19, 2021 · 4 comments · Fixed by #247
Closed
4 tasks

Document IBC upgrade support for genesis dump/restart #21

colin-axner opened this issue Jan 19, 2021 · 4 comments · Fixed by #247
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

Currently IBC only supports IBC breaking change upgrades via the x/upgrade module. We should add additional support via the genesis dump state/restart mechanism.

Problem Definition

If the x/upgrade has a bug that prevents it from being used, genesis dump/restart would be required. In this case, the chain-id would need to be incremented, effectively breaking all existing IBC channels. To avoid this, the chain would need to halt and wait for the core devs to push a fix which fixes the code at the halting height and then the chain would likely want to use the upgrade module to swap binaries to something more stable.

While this scenario is unlikely, if it did ever occur, I'm sure it would cause a lot of people headaches.

Proposal

Support genesis dump/restart. Revision numbers are already handled, so we just need to rework the current design to be a little more modular and not rely solely on the x/upgrade module. Ideally we could isolate the IBC specific code into it's own proposal. Then upgrade proposals which want to do an IBC upgrade, would include the IBC proposal as well (gov proposals can have multiple changes in one).

I'll defer to @AdityaSripal on the specific design, but I think it's worthwhile to ensure IBC upgrades aren't dependent on bug-free code (by limiting the amount of code the IBC upgrades rely on)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@colin-axner
Copy link
Contributor Author

We discussed this today. The current mechanism supports genesis dump state restart, but it must happen in place of the upgrade migration.

Pass an x/upgrade software upgrade proposal, let nodes panic, export genesis, change desired fields (chain-id), reset state, boot up nodes.

We should

  • test this
  • add documentation

@colin-axner colin-axner changed the title Add IBC upgrade support for genesis dump/restart Test and Document IBC upgrade support for genesis dump/restart Feb 24, 2021
@AdityaSripal AdityaSripal reopened this Feb 24, 2021
@colin-axner colin-axner transferred this issue from cosmos/cosmos-sdk Mar 4, 2021
@colin-axner colin-axner added this to the 1.0.0 milestone May 28, 2021
@colin-axner
Copy link
Contributor Author

Currently not supported:

Error: error validating genesis file /home/bartleby/.gm/network2/config/genesis.json: invalid client consensus state timestamp:<seconds:1623670799 nanos:385043798 > root:<> next_validators_hash:"Y\2631\031K\317\231.\336\300\233\204\265\264?(\335\326\003\323\331%\2432`\351\035\371\007\257\251\272"  clientID 07-tendermint-1 index 8: root cannot be empty: invalid consensus state [cosmos/ibc-go@v1.0.0-alpha2.0.20210615093027-ca070bd2c6aa/modules/light-clients/07-tendermint/types/consensus_state.go:46]

We need to loosen genesis validation since there may exist consensus states with empty roots (due to upgrades)

@AdityaSripal
Copy link
Member

I'd rather we just use a sentinel root value in upgrade consensus state and keep that check. Rather than remembering to put it wherever needed.

@colin-axner
Copy link
Contributor Author

I tested this and an upgrade + genesis restart worked fine. A genesis restart without an upgrade did not work with the relayers since the data gets reset. The trusted header could no longer be queried. When I get time I'll add documentation for this upgrade path

@colin-axner colin-axner changed the title Test and Document IBC upgrade support for genesis dump/restart Document IBC upgrade support for genesis dump/restart Jul 6, 2021
faddat referenced this issue in notional-labs/ibc-go Mar 1, 2022
* Add vitwitchain config

* Add domain

* Add vitwitchain-1 conf

* Update vitwitchain-1.json
faddat pushed a commit to faddat/ibc-go that referenced this issue Mar 29, 2023
…ency-from-core-ibc

remove wasm client keeper from IBC keeper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants