Upgrade to cosmos sdk v0.46.0-rc1, against main #1443

faddat · 2022-05-26T14:12:53Z

Description

This is a version of #1413 that changes only 69 files, by targeting main. Along the way to making this, I frequently merged in upstream changes like:

git merge upstream/main because that way the scope of the review is limited, and it is more usable. If it didn't reflect changes in the main branch, it really wouldn't be highly suited to use, for example on the cosmos hub.

I reduced by a few more files by checking out the main branch's ci folder.
git checkout upstream/main -- .github
I added a few more files by fully removing RegisterRESTRoutes

There are two routes to merging this, one is outlined here, and one is in these PR's, which will give the same result, if they are merged into @crodriguezvega's branch in the following sequence.

#1439 - fumpts @crodriguezvega's v0.46 branch so it will more closely match #1412, reducing the scope of a review.
#1412 - bring @crodriguezvega's v0.46 branch current with main, save for changes made in the past day or two.
#1413 - identical to this PR. The other two above are to make reviewing this one easier.

In both cases, this unblocks work across cosmos using cosmos sdk v0.46.0-rc1.

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

Bumps [actions/setup-go](https://github.com/actions/setup-go) from 2.1.5 to 2.2.0. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v2.1.5...v2.2.0) --- updated-dependencies: - dependency-name: actions/setup-go dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

…ctions/setup-go-2.2.0 build(deps): bump actions/setup-go from 2.1.5 to 2.2.0

* deps: upgrade of SDK to 0.46 and tendermint to 0.35 * some changes from review comments * some review comments * refactor: simplify IBC redundant relay check in given restructure of SDK v0.46 (cosmos#1288) * refactor: simplify ibc redundancy check used as sdk middleware Instead of having the function checkRedundancy check if the call is on CheckTx or SimulateTx, only call the function on CheckTx or SimulateTx * add godoc * more review comments. * another review comment * remove tests since legacy REST endpoints have been removed in SDK 0.46 * chore: update sdk to v0.46.0-beta2 * refactor: ics27 indicator tests of deterministic error codes and message responses (cosmos#1349) * begin refactoring ack_test * fix test due to delayed block execution * refactor: switch ics27 response creation to use new SDK version, update tests * fix import * review comment Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * review comment. * pass nil context Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * review comment Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * remove unused import * remove unused import * fix for race condition in tests * remove replace directive to make it build in pre-monterrey mac OS X Co-authored-by: Carlos Rodriguez <crodveg@gmail.com> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: billy rennekamp <billy.rennekamp@gmail.com>

….go from core

…into sdk-master

codecov-commenter · 2022-05-26T14:23:30Z

Codecov Report

Merging #1443 (6fc548f) into main (b9a9502) will decrease coverage by 0.34%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main    #1443      +/-   ##
==========================================
- Coverage   80.36%   80.02%   -0.35%     
==========================================
  Files         166      165       -1     
  Lines       12105    12064      -41     
==========================================
- Hits         9728     9654      -74     
- Misses       1920     1952      +32     
- Partials      457      458       +1

Impacted Files	Coverage Δ
...27-interchain-accounts/controller/keeper/keeper.go	`94.73% <ø> (ø)`
.../apps/27-interchain-accounts/host/keeper/keeper.go	`83.33% <ø> (ø)`
modules/apps/27-interchain-accounts/module.go	`58.44% <ø> (+0.74%)`	⬆️
modules/apps/29-fee/keeper/keeper.go	`92.39% <ø> (ø)`
modules/apps/29-fee/module.go	`56.60% <ø> (+1.04%)`	⬆️
modules/apps/transfer/keeper/keeper.go	`94.11% <ø> (ø)`
modules/apps/transfer/module.go	`59.64% <ø> (+1.02%)`	⬆️
modules/core/02-client/proposal_handler.go	`77.77% <ø> (ø)`
modules/core/02-client/types/codec.go	`77.77% <ø> (ø)`
modules/core/02-client/types/proposal.go	`86.48% <ø> (ø)`
... and 14 more

faddat · 2022-05-26T14:27:11Z

testing/simapp/simd/cmd/genaccounts_test.go

@@ -63,7 +63,7 @@ func TestAddGenesisAccountCmd(t *testing.T) {
 			require.NoError(t, err)

 			serverCtx := server.NewContext(viper.New(), cfg, logger)
-			clientCtx := client.Context{}.WithJSONCodec(appCodec).WithHomeDir(home)
+			clientCtx := client.Context{}.WithCodec(appCodec).WithHomeDir(home)


I did this consistently because the WithJSONCodec is deprecated.

faddat · 2022-05-26T14:27:52Z

modules/core/simulation/genesis_test.go

@@ -25,13 +26,15 @@ func TestRandomizedGenState(t *testing.T) {
 	s := rand.NewSource(1)
 	r := rand.New(s)

+	initialStake := math.NewInt(1000)


this was done because of the new cosmos sdk math library

faddat · 2022-05-26T14:28:45Z

modules/core/02-client/types/proposal.go

@@ -5,7 +5,7 @@ import (

 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"


this is a teeny bit hacky:

The proper way could be to fully refactor all use of governance, but it is my belief that this works.

faddat · 2022-05-26T14:30:04Z

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -367,7 +367,7 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenConfirm() {

 	// use chainA (controller) for ChanOpenConfirm
 	msg := channeltypes.NewMsgChannelOpenConfirm(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, proofAck, proofHeight, icatypes.ModuleName)
-	handler := suite.chainA.GetSimApp().MsgServiceRouter().Handler(msg)
+	handler := suite.chainA.GetSimApp().BaseApp.MsgServiceRouter().Handler(msg)


Post-sdk-middleware changes + unchanges, this seems to be the correct way to summon the MsgServiceRouter.

faddat · 2022-05-26T14:30:41Z

modules/apps/27-interchain-accounts/controller/keeper/keeper.go

@@ -4,8 +4,9 @@ import (
 	"fmt"
 	"strings"

-	baseapp "github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/baseapp"


didn't name baseapp import, because it's already named baseapp by default.

faddat · 2022-05-26T14:32:19Z

modules/apps/27-interchain-accounts/host/ibc_module_test.go

-	differentResponses := tmprotostate.ABCIResponses{
-		DeliverTxs: []*abcitypes.ResponseDeliverTx{
-			&differentDeliverTx,
+// The safety of including ABCI error codes in the acknowledgement rests


I am concerned that I have complicated this test versus what is in main.

faddat · 2022-05-26T16:17:18Z

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -130,7 +150,7 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenInit() {

 	// use chainB (host) for ChanOpenInit
 	msg := channeltypes.NewMsgChannelOpenInit(path.EndpointB.ChannelConfig.PortID, icatypes.Version, channeltypes.ORDERED, []string{path.EndpointB.ConnectionID}, path.EndpointA.ChannelConfig.PortID, icatypes.ModuleName)
-	handler := suite.chainB.GetSimApp().MsgServiceRouter().Handler(msg)
+	handler := suite.chainB.GetSimApp().MsgServiceRouter.Handler(msg)
 	_, err := handler(suite.chainB.GetContext(), msg)


This differs from one of my earlier tests and looks cleaner. I will try and modify.

I do not know why this doesn't work the same as:

https://github.com/cosmos/ibc-go/pull/1443/files#diff-2114e4419debdc1aea40268325e1ad68ea7a5186f26d34fd1fe310f87a183ea7

faddat · 2022-05-26T16:36:06Z

testing/sdk_test.go

 		NumValidators:     4,
 		BondDenom:         sdk.DefaultBondDenom,
 		MinGasPrices:      fmt.Sprintf("0.000006%s", sdk.DefaultBondDenom),
 		AccountTokens:     sdk.TokensFromConsensusPower(1000, sdk.DefaultPowerReduction),
 		StakingTokens:     sdk.TokensFromConsensusPower(500, sdk.DefaultPowerReduction),
 		BondedTokens:      sdk.TokensFromConsensusPower(100, sdk.DefaultPowerReduction),
-		PruningStrategy:   storetypes.PruningOptionNothing,
+		PruningStrategy:   "nothing",


this wanted a string, so I gave it a string.

faddat · 2022-05-26T16:36:29Z

testing/sdk_test.go

@@ -126,134 +130,12 @@ func (s *IntegrationTestSuite) TearDownSuite() {
 	s.network.Cleanup()
 }

-// TestLegacyRestErrMessages creates two IBC txs, one that fails, one that


we can remove this because we no longer need to accommodate legacy rest

faddat · 2022-05-26T16:36:52Z

testing/sdk_test.go

 // - succeed using gRPC
 // In practice, we call this function on IBC txs.
 func (s *IntegrationTestSuite) testQueryIBCTx(txRes sdk.TxResponse, cmd *cobra.Command, args []string) {
 	val := s.network.Validators[0]

-	errMsg := "this transaction cannot be displayed via legacy REST endpoints, because it does not support" +


Same with this, there's no longer a need to support legacy rest.

faddat · 2022-05-26T16:38:21Z

testing/simapp/simd/cmd/root.go

+
+// initTendermintConfig helps to override default Tendermint Config values.
+// return tmcfg.DefaultConfig if no custom configuration is required for the application.
+func initTendermintConfig() *tmcfg.Config {


I think this wasn't needed.

This reverts commit d423a86.

faddat · 2022-05-26T17:45:48Z

testing/sdk_test.go

+	s.Require().NoError(err)
+
+	acc2pubkey, err := account2.GetPubKey()
+	s.Require().NoError(err)


I added these because the getpubkey() method now returns error

crodriguezvega · 2022-05-26T20:26:29Z

Hi @faddat. Thanks a lot for opening all these PRs!

I wanted to give you an overview of our planning and timelines to work on the upgrade to SDK 0.46 and its release.

We are in the final stages of fee middleware (ICS-29) and we have decided to release this feature first as ibc-go v.4.0.0 with SDK 0.45. After several discussions we have been recommended to make a release of fee middleware that is compatible with SDK 0.45.x. Therefore the official release of ibc-go with SDK 0.46 will come afterwards (most likely as v5.0.0). In the meantime we will work on the upgrade in this branch and if we haven't cut v5.0.0 by the time Cosmos Hub is going to release rho, then we have agreed with the Cosmos Hub team that they will use the upgrade branch. It's a bit hacky, but it's the only solution we found to be able to satisfy all parties. For that reason, for now we don't want to bring all changes from main into the upgrade branch, because we want to keep it strictly with only the changes required for the upgrade to 0.46, nothing else. Once we cut v4.0.0, then we can start thinking about bringing main to the upgrade branch in preparation for v5.0.0, but for now maybe we should just hold our horses a bit. :)

I expect that we cut v4.0.0 next month, so if we continue doing work on the upgrade to SDK 0.46 in parallel in the meantime, then v5.0.0 could follow soon after. Next week we are going to open a new PR to upgrade this branch to 0.46-rc1. And you PRs we will again be extremely useful.

I hope this sheds a bit of light about our plans and I hope that you don't mind if, after reviewing your PRs, we end up deciding closing some if we think that's the best approach for our workflow and timeline. Again, thanks a lot for your work; it is very much appreciated.

faddat · 2022-05-26T21:26:09Z

Thanks IBC-go team.

faddat · 2022-07-05T12:37:37Z

closing because tm 35 -> 34

dependabot bot and others added 29 commits February 10, 2022 03:04

Merge pull request #14 from notional-labs/dependabot/github_actions/a…

3359732

…ctions/setup-go-2.2.0 build(deps): bump actions/setup-go from 2.1.5 to 2.2.0

Merge branch 'carlos/upgrade-sdk-0.46-tendermint-0.35': update to beta-2

454011f

app.go

3eaaa11

mod and sum

d498467

remove sdk middleware from simpapp and ica controller

86de0f8

remove sdk middleware from ica host

b15a3bc

remove sdk middleware test from core module

ec33293

remove middleware from host and controller, and remove sdk_middleware…

1183101

….go from core

majority of tests now pass (cleanup of math)

5c769d3

Continue updating math

575d254

go mod update

7dbe24c

continue cleanup

1547fcb

escrow_test.go fee.total() -> fee.Total()...

4b36697

fix tests

efedf1a

bug from cosmos math

752207f

go 1.18

26959ba

Merge branch 'sdk-master' of https://github.com/notional-labs/ibc-go …

1d828c8

…into sdk-master

bump sdk

20c9d88

don't change mergify

3a5be21

fumpt

3c0edca

Merge remote-tracking branch 'origin/main' into sdk-master

d235150

Update test.yml

03dc0a2

further clean snapshots

32f78e3

antehandler

e4d62c1

Merge branch 'sdk-master' of https://github.com/notional-labs/ibc-go …

eb7892e

…into sdk-master

antehandler

f221d2d

Fix TestUpdateClientEventEmission

7d35705

faddat requested a review from AdityaSripal as a code owner May 26, 2022 14:12

Merge branch 'sdk-master' of https://github.com/notional-labs/ibc-go …

bb390c8

…into sdk-master

faddat added 2 commits May 26, 2022 21:25

make this branch match main as closely as possilbe

f94c3f2

merge scripts folder from upstream

4503d7c

faddat commented May 26, 2022

View reviewed changes

fix the sdk test

d4f1f0a

faddat commented May 26, 2022

View reviewed changes

faddat added 2 commits May 26, 2022 23:27

tidy

1ad1d84

remove RegisterRESTRoutes, it is no longer needed

46628e8

faddat changed the title ~~Sdk master~~ Upgrade to cosmos sdk v0.46.0-rc1, against main May 26, 2022

faddat commented May 26, 2022

View reviewed changes

faddat added 2 commits May 26, 2022 23:47

minimize changes to test_helpers.go

d423a86

Revert "minimize changes to test_helpers.go"

2314e47

This reverts commit d423a86.

faddat commented May 26, 2022

View reviewed changes

jackzampolin mentioned this pull request May 30, 2022

deps: upgrade to SDK 0.46-rc1 #1455

Merged

9 tasks

Merge remote-tracking branch 'upstream/main' into sdk-master

6fc548f

faddat mentioned this pull request Jun 11, 2022

Integration testing between major cosmos repositories cosmos/cosmos-sdk#12228

Closed

faddat closed this Jul 5, 2022

faddat mentioned this pull request Aug 4, 2022

Tracking issue for V10 CosmosContracts/juno#235

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to cosmos sdk v0.46.0-rc1, against main #1443

Upgrade to cosmos sdk v0.46.0-rc1, against main #1443

faddat commented May 26, 2022 •

edited

Loading

codecov-commenter commented May 26, 2022 •

edited

Loading

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022 •

edited

Loading

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022 •

edited

Loading

faddat May 26, 2022

faddat May 26, 2022

faddat May 26, 2022

crodriguezvega commented May 26, 2022 •

edited

Loading

faddat commented May 26, 2022 •

edited

Loading

faddat commented Jul 5, 2022

Upgrade to cosmos sdk v0.46.0-rc1, against main #1443

Upgrade to cosmos sdk v0.46.0-rc1, against main #1443

Conversation

faddat commented May 26, 2022 • edited Loading

Description

codecov-commenter commented May 26, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faddat May 26, 2022 • edited Loading

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

faddat May 26, 2022 • edited Loading

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

crodriguezvega commented May 26, 2022 • edited Loading

faddat commented May 26, 2022 • edited Loading

faddat commented Jul 5, 2022

faddat commented May 26, 2022 •

edited

Loading

codecov-commenter commented May 26, 2022 •

edited

Loading

faddat May 26, 2022 •

edited

Loading

faddat May 26, 2022 •

edited

Loading

crodriguezvega commented May 26, 2022 •

edited

Loading

faddat commented May 26, 2022 •

edited

Loading