feat: remove channel upgradability #8008

gjermundgaraba · 2025-02-25T13:27:28Z

Description

closes: #7999

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.

AdityaSripal · 2025-02-26T09:45:11Z

modules/core/04-channel/keeper/handshake_test.go

-// TestChanCloseConfirm tests the confirming closing channel ends by calling ChanCloseConfirm
-// on chainB. Both chains will use message passing to setup OPEN channels. ChanCloseInit is
-// bypassed on chainA by setting the channel state in the ChannelKeeper.
-func (suite *KeeperTestSuite) TestChanCloseConfirm() {


Why was this test removed. Only upgrade case should be removed

Huh, good point. Will check that!

AdityaSripal · 2025-02-26T09:49:05Z

modules/core/04-channel/keeper/migrations.go

+func (m *Migrator) Migrate7To8(ctx sdk.Context) error {
+	return v10.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc, m.keeper)


Why are you doing v10 migrate store in this function?

This is to have a migrator for channel, but keep the migration logic out of the keeper itself. We have the same process in 02-client:

func (m Migrator) Migrate2to3(ctx sdk.Context) error { return v7.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc, m.keeper) }

Yes but wouldn't this be called Migrate8To10?

That number refers to the consensus version of the store. It's a bit of a weird mix. Not sure what the best solution is. But client also mixes it the same way.

AdityaSripal · 2025-02-26T09:54:20Z

modules/core/04-channel/migrations/v10/store.go

+		var channel Channel
+		cdc.MustUnmarshal(iterator.Value(), &channel)
+
+		if channel.State == FLUSHING || channel.State == FLUSHCOMPLETE {


This will mess with the channel upgrade procedure. If we have a FLUSHCOMPLETE on one side, we are expecting the upgrade to occur successfully.

So if you downgrade from this point, you risk the other side upgrading successfully using your FLUSHCOMPLETE state and having the channel OPEN and invalid.

I would suggest that we just panic migrating this if any channel is in FLUSHING or FLUSHCOMPLETE.

Since these are gated by governance ultimately, we can do the upgrade to v10 when we have no active upgrades in-flight.

Creates much less headache for us thinking through these cases

Yeah, that makes sense. I was considering that, but figured it might just work out if both sides migrated. I guess panic might be safer in this case. Will fix.

It will error instead now

…pgradability

sonarqubecloud · 2025-02-26T14:28:09Z

Quality Gate passed for 'ibc-go'

Issues
64 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

AdityaSripal

LGTM module one nit

AdityaSripal · 2025-02-26T16:43:55Z

modules/core/04-channel/keeper/migrations.go

+func (m *Migrator) Migrate7To8(ctx sdk.Context) error {
+	return v10.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc, m.keeper)


Yes but wouldn't this be called Migrate8To10?

* feat: remove channel upgradability * lint * basic migration setup * feat: remove channel upgradeability * fix docs * register v10 migration in app module * error on upgrading channels + code review fixes (cherry picked from commit f7b24e1)

* feat: remove channel upgradability * lint * basic migration setup * feat: remove channel upgradeability * fix docs * register v10 migration in app module * error on upgrading channels + code review fixes (cherry picked from commit f7b24e1) Co-authored-by: Gjermund Garaba <gjermund@garaba.net>

* MsgTransfer For Eureka (#7957) (#8001) * initial progress * progress * initial build and existing tests pass * testing * documentation for tests * refactor * fix regression * Update modules/apps/transfer/types/packet.go Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * remove clientKeeperv2 necessity in code * remove clientkeeperv2 in transfer keeper --------- Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit 9f677ba) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * Remove 29-fee (#8000) (#8002) * rm 29-fee * rm docs * lint * add back test * doc link fix (cherry picked from commit 972afc2) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * feat: add cli commands for client v2 counterparty (#7997) (#8007) * feat: add client counterparty cli commands * chore: make wasm simapp possible to run more than once * add query to cli * lint (cherry picked from commit eeb8b8a) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: add deprecated functions for backwards compat (#8005) (#8009) * chore: add back some deprecated functions * Added DenomPathFromHash as deprecated function * fix docstring (cherry picked from commit 7bd0c6d) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: add backport mergify tasks (#7989) (#8015) * chore: add backport mergify tasks * add wasm ibc v10 to mergify backports (cherry picked from commit 11488cf) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: use ibc v2 name consistently (#8006) (#8017) (cherry picked from commit a71577c) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * callbacks: Error on RecvPacket callback execution (#8014) (#8020) * change behavior of recvPacket * fix tests * error on malformed callback data * CHANGELOG --------- Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit c5a36e4) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 (#7920) (#8021) * chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 Bumps google.golang.org/protobuf from 1.36.4 to 1.36.5. --- updated-dependencies: - dependency-name: google.golang.org/protobuf dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit af5b171) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump golangci/golangci-lint-action from 6.2.0 to 6.4.1 (#7969) (#8022) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.2.0 to 6.4.1. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v6.2.0...v6.4.1) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> (cherry picked from commit 1f11107) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: remove remnants of ics20-2 (#8016) (#8045) * chore: remove remnants of ics20-2 * rename FungibleTokenPacketDataV2 to make it clear that its an internal type * fix broken link (cherry picked from commit 40f3240) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore(deps): bump JamesIves/github-pages-deploy-action (#7991) (#8048) Bumps [JamesIves/github-pages-deploy-action](https://github.com/jamesives/github-pages-deploy-action) from 4.7.2 to 4.7.3. - [Release notes](https://github.com/jamesives/github-pages-deploy-action/releases) - [Commits](JamesIves/github-pages-deploy-action@v4.7.2...v4.7.3) --- updated-dependencies: - dependency-name: JamesIves/github-pages-deploy-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 205e6af) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump golangci/golangci-lint-action from 6.4.1 to 6.5.0 (#7983) (#8049) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.4.1 to 6.5.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v6.4.1...v6.5.0) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit 272c09b) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: remove channel upgradability (#8008) (#8053) * feat: remove channel upgradability * lint * basic migration setup * feat: remove channel upgradeability * fix docs * register v10 migration in app module * error on upgrading channels + code review fixes (cherry picked from commit f7b24e1) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: update sonarqube gh action (#7985) (#7988) (cherry picked from commit fcb6809) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * test: fix ica compatibility tests (#7976) (#7994) * test: fix ica compatibility tests * lint * fix event search * lint (cherry picked from commit 22efa6c) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * docs: document ICS23 MerklePath (#7993) (#7995) * proto docs * more go docs (cherry picked from commit 0e8a6c9) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * MsgTransfer For Eureka (#7957) (#8001) * initial progress * progress * initial build and existing tests pass * testing * documentation for tests * refactor * fix regression * Update modules/apps/transfer/types/packet.go Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * remove clientKeeperv2 necessity in code * remove clientkeeperv2 in transfer keeper --------- Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit 9f677ba) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * Remove 29-fee (#8000) (#8002) * rm 29-fee * rm docs * lint * add back test * doc link fix (cherry picked from commit 972afc2) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * feat: add cli commands for client v2 counterparty (#7997) (#8007) * feat: add client counterparty cli commands * chore: make wasm simapp possible to run more than once * add query to cli * lint (cherry picked from commit eeb8b8a) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: add deprecated functions for backwards compat (#8005) (#8009) * chore: add back some deprecated functions * Added DenomPathFromHash as deprecated function * fix docstring (cherry picked from commit 7bd0c6d) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: add backport mergify tasks (#7989) (#8015) * chore: add backport mergify tasks * add wasm ibc v10 to mergify backports (cherry picked from commit 11488cf) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: use ibc v2 name consistently (#8006) (#8017) (cherry picked from commit a71577c) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * callbacks: Error on RecvPacket callback execution (#8014) (#8020) * change behavior of recvPacket * fix tests * error on malformed callback data * CHANGELOG --------- Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit c5a36e4) Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 (#7920) (#8021) * chore(deps): bump google.golang.org/protobuf from 1.36.4 to 1.36.5 Bumps google.golang.org/protobuf from 1.36.4 to 1.36.5. --- updated-dependencies: - dependency-name: google.golang.org/protobuf dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * tidy --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit af5b171) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump golangci/golangci-lint-action from 6.2.0 to 6.4.1 (#7969) (#8022) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.2.0 to 6.4.1. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v6.2.0...v6.4.1) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> (cherry picked from commit 1f11107) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore: remove remnants of ics20-2 (#8016) (#8045) * chore: remove remnants of ics20-2 * rename FungibleTokenPacketDataV2 to make it clear that its an internal type * fix broken link (cherry picked from commit 40f3240) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> * chore(deps): bump JamesIves/github-pages-deploy-action (#7991) (#8048) Bumps [JamesIves/github-pages-deploy-action](https://github.com/jamesives/github-pages-deploy-action) from 4.7.2 to 4.7.3. - [Release notes](https://github.com/jamesives/github-pages-deploy-action/releases) - [Commits](JamesIves/github-pages-deploy-action@v4.7.2...v4.7.3) --- updated-dependencies: - dependency-name: JamesIves/github-pages-deploy-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> (cherry picked from commit 205e6af) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump golangci/golangci-lint-action from 6.4.1 to 6.5.0 (#7983) (#8049) Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 6.4.1 to 6.5.0. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](golangci/golangci-lint-action@v6.4.1...v6.5.0) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gjermund Garaba <gjermund@garaba.net> (cherry picked from commit 272c09b) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * feat: remove channel upgradability (#8008) (#8053) * feat: remove channel upgradability * lint * basic migration setup * feat: remove channel upgradeability * fix docs * register v10 migration in app module * error on upgrading channels + code review fixes (cherry picked from commit f7b24e1) Co-authored-by: Gjermund Garaba <gjermund@garaba.net> --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

gjermundgaraba added 6 commits February 23, 2025 13:41

feat: remove channel upgradability

241574f

lint

a78c2b8

basic migration setup

d6291b9

feat: remove channel upgradeability

c116a2c

Merge branch 'main' into feat/remove-channel-upgradability

722a263

fix docs

03749e8

gjermundgaraba marked this pull request as ready for review February 25, 2025 14:07

gjermundgaraba requested review from AdityaSripal and srdtrk as code owners February 25, 2025 14:07

register v10 migration in app module

2cd2024

AdityaSripal reviewed Feb 26, 2025

View reviewed changes

gjermundgaraba added 2 commits February 26, 2025 12:07

error on upgrading channels + code review fixes

7582a7e

Merge branch 'main' into feat/remove-channel-upgradability

b15e2f9

gjermundgaraba requested a review from AdityaSripal February 26, 2025 11:08

gjermundgaraba added 2 commits February 26, 2025 14:13

Merge remote-tracking branch 'origin/main' into feat/remove-channel-u…

cc8b31a

…pgradability

Merge branch 'main' into feat/remove-channel-upgradability

aa697e6

AdityaSripal approved these changes Feb 26, 2025

View reviewed changes

gjermundgaraba added the backport-to-v10.0.x label Feb 26, 2025

gjermundgaraba added this pull request to the merge queue Feb 26, 2025

Merged via the queue into main with commit f7b24e1 Feb 26, 2025
60 of 63 checks passed

gjermundgaraba deleted the feat/remove-channel-upgradability branch February 26, 2025 16:55

mergify bot mentioned this pull request Feb 26, 2025

feat: remove channel upgradability (backport #8008) #8053

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove channel upgradability #8008

feat: remove channel upgradability #8008

gjermundgaraba commented Feb 25, 2025

AdityaSripal Feb 26, 2025

gjermundgaraba Feb 26, 2025

gjermundgaraba Feb 26, 2025

AdityaSripal Feb 26, 2025

gjermundgaraba Feb 26, 2025

AdityaSripal Feb 26, 2025

gjermundgaraba Feb 26, 2025

AdityaSripal Feb 26, 2025 •

edited

Loading

gjermundgaraba Feb 26, 2025

gjermundgaraba Feb 26, 2025

sonarqubecloud bot commented Feb 26, 2025

AdityaSripal left a comment

AdityaSripal Feb 26, 2025

		func (m *Migrator) Migrate7To8(ctx sdk.Context) error {
		return v10.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc, m.keeper)

feat: remove channel upgradability #8008

feat: remove channel upgradability #8008

Conversation

gjermundgaraba commented Feb 25, 2025

Description

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

AdityaSripal Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 26, 2025

Quality Gate passed for 'ibc-go'

AdityaSripal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdityaSripal Feb 26, 2025 •

edited

Loading