Upgrade CCV modules to SDK v0.47

[mpoke]

Problem

Currently, the entire ICS code requires SDK v0.45 and IBC v4 which will soon reach EoL.
We want to support SDK v47 and IBC v7 to allow upgrading provider chains and onboarding new consumer chains.

Closing criteria

• provider code upgraded to required dependencies and API changes supported
• provider code upgraded to required dependencies and API changes supported
• unittests support required dependencies and are passing
• integration, MBT tests support required dependencies and are passing
• e2e tests support required dependencies and are passing
• handle proto deps
• push protos to BSR
• rework and add tests AfterUnbondingInitiated on provider side to support new cosmos-sdk behaviour
• support CancelUnbond operation on the provider (cosmos-sdk v47 related)

Task list

Must have

Beta Give feedback

1. Remove SPM #753
   type: tech-debt +1
2. Upgrade proto files to SDK 0.47 #942
   scope: cosmos-sdk type: refactoring type: tech-debt +3
   
3. Upgrade consumer to SDK 0.47 #943
   scope: cosmos-sdk type: refactoring type: tech-debt +3
   
4. Update provider to SDK 0.47 #947
   scope: cosmos-sdk type: refactoring type: tech-debt +3
   
5. Upgrade integration & MBT tests to SDK 0.47 #944
   scope: cosmos-sdk scope: testing type: refactoring type: tech-debt +4
   
6. Support CancelUnbonding (sdk v47) operation in ICS #948
   scope: cosmos-sdk type: feature-request +2
   
7. Upgrade e2e tests to SDK 0.47 #946
   scope: testing type: tech-debt +2
   
8. Rework AfterUnbondingInitiated staking hook on ICS provider #945
   scope: cosmos-sdk type: refactoring +2
   
9. EPIC: v2.0.0 release #949
   4 of 4
   admin: epic +1
   
10. add migration code

Nice to have

Beta Give feedback

1. Refactor e2e testing framework to support testing ICS chains running different cosmos-sdk and IBC versions
2. Refactor or remove ibc_legacy_testing from ICS #1008
   scope: testing type: refactoring +2
   

[faddat]

• New branch sdk47 #817 addresses this issue fully, and does producer and consumer.

• Sdkv47 ics cosmos-sdk#15856

That SDK PR contains the changes that we think are needed to 47 for ics.

I also think that instead of splitting go.mod, we should instead just add a v2 to the module path such as:

module github.com/cosmos/interchain-security/v2

Then, v2 can consume v1 for test purposes, like testing 45 producer and 47 consumer scenarios.

[faddat]

https://github.com/cosmos/interchain-security/blob/main/CONTRIBUTING.md#development-procedure

so, it is not possible to follow the steps you outlined here or on gaia without breaking main.

Also, we are going to upgrade the whole repository. We feel that it doesn't make sense to do it partially.

[adizere]

Also, we are going to upgrade the whole repository. We feel that it doesn't make sense to do it partially.

An experience we had from other repos (this is not about the Hub) is that feature branches could come in handy to help split large work in smaller PRs. We used a feature branch to capture a multi-month work related to ABCI 2.0, for example.

The gist of using feature branches: Small PRs target the longer-lived feature branch, and the feature branch may intermittently fail to lint/make/CI. It's important to scope each PRs and review them as if it's landing on main with care. Eventually, you merge the whole feature branch into main when it's ready, and then it's still advised to do a sync review with 1-2 engineers. This is no silver bullet, and has some costs, but it has worked better than having big PRs.

[mpoke]

Also, we are going to upgrade the whole repository. We feel that it doesn't make sense to do it partially.

An experience we had from other repos (this is not about the Hub) is that feature branches could come in handy to help split large work in smaller PRs. We used a feature branch to capture a multi-month work related to ABCI 2.0, for example.

The gist of using feature branches: Small PRs target the longer-lived feature branch, and the feature branch may intermittently fail to lint/make/CI. It's important to scope each PRs and review them as if it's landing on main with care. Eventually, you merge the whole feature branch into main when it's ready, and then it's still advised to do a sync review with 1-2 engineers. This is no silver bullet, and has some costs, but it has worked better than having big PRs.

Thank you @adizere.

@faddat We are currently updating the guidelines for working on large contributions #868. Constructive feedback from the community is greatly appreciated.

[faddat]

• Notional's Pull Requests: upgrade to SDKv47 #874

[faddat]

This is the item that we most urgently need feedback on:

• fix: changes to sdk for interchain security cosmos-sdk#15917

the question is: does upgrading to sdk 47 require using a forked version of sdk 47?

thanks

[mpoke]

This is the item that we most urgently need feedback on:

• fix: changes to sdk for interchain security cosmos-sdk#15917

the question is: does upgrading to sdk 47 require using a forked version of sdk 47?

thanks

@faddat I believe that this comment addresses your concern.

[mpoke]

finally, i must say that it is simply not good to block work on simple stuff, like the questions we've been asking for a very long time now, including in this issue, which you've chosen not to answer

1. @vuong177 found that it is necessary to use a forked cosmos-sdk v0.47.x -- do you agree, or nah? in either case, what are your thoughts on the matter?
2. We also came up with the idea of adding some things to ibc-go, reducing the amount of boilerplate that we need to copy into this repo from ibc. Do you agree, or nah? In either case, what are your thoughts on the matter?

@faddat Thank you for sharing your feedback. If you have concerns or you see any issues regarding Interchain Security, please open a new issue (if one is not already opened) and participate in thoughtful discussion on that issue. We'll try to address your concerns as soon as possible.

Regarding the first point, you have received immediate feedback on both PRs you have opened on the SDK repo: cosmos/cosmos-sdk#15856 and cosmos/cosmos-sdk#15917.

Regarding the second point, I need more context? That's the reason we are working with issues with a clear problem description where we can have thorough discussions.