First Joint: sdk 46 #39

faddat · 2022-09-02T10:17:19Z

I'll use this branch to bump bcna to v46, mainly as a test/exploration.

....and it's done now.

The next step, which will be in a separate PR, will be the liquid staking module.

opinion: ibc-go v5.0.0 will be out soon, and it likely makes the most sense, to upgrade to 46+lsm. BCNA would be the first to do this, and it would enable the full feature set of quicksilver, while improving performance and making the code here generally easier to work with.

Huge thanks to:

cosmos-sdk team
ignite cli team
ibc-go team
iqlusion

RaulBernal · 2022-09-02T11:11:17Z

Ok thanks by your support ;)

faddat · 2022-09-02T11:19:00Z

thanks for the state sync script that saved more hours of my life than I can even estimate.

faddat · 2022-09-02T11:20:18Z

This PR will remove starport's cmd folder shims, and leave the chain otherwise compatible with the nightly build of starport. Then we can do one that adds the lsm.

tbruyelle · 2022-09-02T16:58:47Z

Hi there, ignite-hq shouldn't be used any more, it's the previous org.

To get the latest ignite CLI, it's a little tricky for the moment : you can't go get github.com/ignite/cli@develop because of this v0.0.0-nighlty tag (this kind of tag is not compatible with go modules, it will be fixed later).

revert the changes ignite -> ignite-hq (in code and in go.mod)
run GOSUMDB=off go get -u github.com/ignite/[email protected] (for some reasons, this latest version is not present at sum.golang.org, so you need to disable that)
run go mod tidy, if there's no error, you're good.

faddat · 2022-09-02T17:40:08Z

thanks!

faddat · 2022-09-02T17:47:37Z

@tbruyelle what is

"GOSUMDB=off"

haven't seen this before.

tbruyelle · 2022-09-03T08:11:21Z

@tbruyelle what is

"GOSUMDB=off"

haven't seen this before.

That disables the checksum verification with sum.golang.org. For some reason, it doesn't work with our latest version (sum.golang.org returns a 404 not found). It might also be related to this v0.0.0-nightly tag.

CI is passing now 🎉

robert-zaremba · 2022-09-07T06:53:25Z

Where is the state sync script?

RaulBernal · 2022-09-07T06:55:58Z

Where is the state sync script?

Do you mean this?
https://github.com/BitCannaGlobal/cosmos-statesync_client

gjermundgaraba · 2022-09-08T16:51:34Z

testutil/network/network.go

 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/ignite/cli/ignite/pkg/cosmoscmd"
+	"github.com/jaekwon/testify/require"


Not sure if this was intended or not, but since this looks pretty abandoned I would perhaps suggest using github.com/stretchr/testify instead.

That came in with the updated ignite cli code.... let's see if the alternative works.

my first inclination was to remove ignite altogether.

we removed Ignite CLI - was causing problems in CI.

we removed Ignite CLI - was causing problems in CI.
Before remove it:

How to reproduce the problems?

Should be fixed in github.com/ignite/cli v0.24.0

I have opened a new branch here
main...SDK-v0.46.1-&-IBC/go-v5-on-the-way

ICA integration fixed

IBC production version 5

Please fell free to collaborate there 🙏

RaulBernal · 2022-09-08T17:01:55Z

@faddat we have just integrated the ICA in our main (production) branch, so this development should also contains it. Thanks 😊

gjermundgaraba · 2022-09-08T17:13:20Z

go.mod

 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
-	github.com/ignite/cli v0.23.0
+	github.com/ignite/cli v0.23.1-0.20220902151340-d0fd60f1d4ad
+	github.com/jaekwon/testify v1.6.1


Same here. I also see that github.com/stretchr/testify is already included, so probably not necessary with this.

thanks for the review!

I think that the ignite cli uses the jaekwon code. Both imports came in when updating the ignite cli, and I believe came from upstream.

faddat · 2022-09-12T07:13:53Z

@bjaanes thanks very much for the review! I've addressed your findings here, and that was quite helpful.

faddat · 2022-09-12T07:14:41Z

@robert-zaremba also on just about every cosmos chain of a certain age. Gaia def has it, I did significant refactors, but it is fundamentally from here.

RaulBernal · 2022-09-19T09:58:31Z

Your code run smooth as I tested in a new dev-chain :D
Thanks for collaborating 👍

RaulBernal · 2022-09-19T10:20:12Z

Question @faddat Should we bump Tendermint to v0.35.x? I think yes

RaulBernal · 2022-09-19T21:23:49Z

@faddat I have compiled your code and works flawless :)

Migrations to ibc-go v4 and v5 appears to be trivial; look:
https://github.com/cosmos/ibc-go/tree/main/docs/migrations

robert-zaremba · 2022-09-28T17:13:27Z

Question faddat Should we bump Tendermint to v0.35.x? I think yes

No, tendermint v0.35 is retracted.

robert-zaremba · 2022-09-28T17:14:18Z

Re IBC v5: important decision is if you want to enable ICA and IBC fees.

RaulBernal · 2022-09-29T09:57:36Z

Re IBC v5: important decision is if you want to enable ICA and IBC fees.

Full integration of ICA here. (and new stable production version IBC/go v5.0):
https://github.com/BitCannaGlobal/bcna/tree/SDK_046

also for you @faddat @bjaanes @tbruyelle
Controller code should be wired and disabled in a upgrade if not the it returns a panic when you query the controller params in example:

./gaiad-v7.0.3-linux-amd64 query interchain-accounts  controller params --node https://rpc-cosmoshub.goldenratiostaking.net:443
Error: rpc error: code = Unknown desc = panic message redacted to hide potentially sensitive system info: panic
Usage:
  gaiad query interchain-accounts controller params [flags]

Examples:
gaiad query interchain-accounts controller params

Or:

./bcnad query ica controller params --trace
Error: rpc error: code = Unknown desc = 
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Query.func1
	/Users/raul/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/abci.go:409
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:838
runtime.panicwrap
	/usr/local/go/src/runtime/error.go:328
github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/keeper.(*Keeper).Params
	<autogenerated>:1
github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types._Query_Params_Handler
	/Users/raul/go/pkg/mod/github.com/cosmos/ibc-go/[email protected]/modules/apps/27-interchain-accounts/controller/types/query.pb.go:204
github.com/cosmos/cosmos-sdk/baseapp.(*GRPCQueryRouter).RegisterService.func1
	/Users/raul/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/grpcrouter.go:85
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).handleQueryGRPC
	/Users/raul/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/abci.go:577
github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Query
	/Users/raul/go/pkg/mod/github.com/cosmos/[email protected]/baseapp/abci.go:421
github.com/tendermint/tendermint/abci/client.(*localClient).QuerySync
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/abci/client/local_client.go:256
github.com/tendermint/tendermint/proxy.(*appConnQuery).QuerySync
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/proxy/app_conn.go:159
github.com/tendermint/tendermint/rpc/core.ABCIQuery
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/rpc/core/abci.go:20
reflect.Value.call
	/usr/local/go/src/reflect/value.go:556
reflect.Value.Call
	/usr/local/go/src/reflect/value.go:339
github.com/tendermint/tendermint/rpc/jsonrpc/server.makeJSONRPCHandler.func1
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/rpc/jsonrpc/server/http_json_handler.go:96
github.com/tendermint/tendermint/rpc/jsonrpc/server.handleInvalidJSONRPCPaths.func1
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/rpc/jsonrpc/server/http_json_handler.go:122
net/http.HandlerFunc.ServeHTTP
	/usr/local/go/src/net/http/server.go:2084
net/http.(*ServeMux).ServeHTTP
	/usr/local/go/src/net/http/server.go:2462
github.com/tendermint/tendermint/rpc/jsonrpc/server.maxBytesHandler.ServeHTTP
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/rpc/jsonrpc/server/http_server.go:238
github.com/tendermint/tendermint/rpc/jsonrpc/server.RecoverAndLogHandler.func1
	/Users/raul/go/pkg/mod/github.com/tendermint/[email protected]/rpc/jsonrpc/server/http_server.go:211
net/http.HandlerFunc.ServeHTTP
	/usr/local/go/src/net/http/server.go:2084
net/http.serverHandler.ServeHTTP
	/usr/local/go/src/net/http/server.go:2916
net/http.(*conn).serve
	/usr/local/go/src/net/http/server.go:1966
value method github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/keeper.Keeper.Params called using nil *Keeper pointer: panic
Usage:
  bcnad query interchain-accounts controller params [flags]

For this and for other issue the IBC Team relased the security version.

RaulBernal · 2022-09-29T11:13:50Z

Definitely we decided to close this PR in favor to develop the branch https://github.com/BitCannaGlobal/bcna/tree/SDK_046
We hope to see your collaboration there.

tbruyelle · 2022-09-29T14:16:53Z

also for you @faddat @bjaanes @tbruyelle
Controller code should be wired and disabled in a upgrade if not the it returns a panic when you query the controller params in example:

Thanks for the report, I created an issue in CLI ignite/cli#2867.

Why do you say we need to disable it ?

RaulBernal · 2022-09-29T16:04:16Z

also for you @faddat @bjaanes @tbruyelle
Controller code should be wired and disabled in a upgrade if not the it returns a panic when you query the controller params in example:

Thanks for the report, I created an issue in CLI ignite/cli#2867.

Why do you say we need to disable it ?

I'm curious too to know @robert-zaremba and @faddat commented it before

faddat · 2022-11-02T22:37:54Z

whole thing needs a refactor, will do soon....

RaulBernal · 2022-11-03T07:20:03Z

whole thing needs a refactor, will do soon....

Certainly it needs because I refactored the branch v1.4.X & v1.5.X based in:

Your PRs
Paloma/Alek PRs
Ignite cosmoscmd removing doc
Gaiad & Cosmos SDK repos

I planed to do next week but maybe before if I can. Anyway you can work in https://github.com/BitCannaGlobal/bcna/tree/SDK_046

robert-zaremba · 2022-11-04T00:22:10Z

BTW, the ICA controller stack is going to be updated in IBC v6.
Enabling ICA controller is more complex because you need to implement account registration. I recommend to do it separately.

robert-zaremba · 2022-11-04T00:22:54Z

AS for Ignite CLI - we had problems with it in CI and eventually just removed it because nobody was using it.

RaulBernal · 2022-11-04T06:59:34Z

BTW, the ICA controller stack is going to be updated in IBC v6. Enabling ICA controller is more complex because you need to implement account registration. I recommend to do it separately.

In our next upgrade (next monday) we will include the ICA stack (with controller = disabled) in the v0.45 branch. By the moment is not our business enable it.

Next upgrade will be v0.46 and IBC/5 or 6

Many thanks by your advice 🙏

RaulBernal · 2022-11-04T07:02:34Z

AS for Ignite CLI - we had problems with it in CI and eventually just removed it because nobody was using it.

Attending your past comments and @faddat ones, we have removed cosmoscmd from our current branchs (v0.45) & (v0.46)

Thanks everyone by your support 👏 👏

v0.46 migration is at https://github.com/BitCannaGlobal/bcna/tree/SDK_046 branch :)

tbruyelle · 2022-11-04T09:30:10Z

@RaulBernal We removed cosmoscmd from Ignite CLI v0.25.0, here is the complete migration guide for existing chain : https://docs.ignite.com/migration/v0.25.0#removing-cosmoscmd

About the ICA controller bug you have raised, there's also a migration guide : https://docs.ignite.com/migration/v0.25.0#fix-ica-controller-keeper-wiring

AS for Ignite CLI - we had problems with it in CI and eventually just removed it because nobody was using it.

@robert-zaremba I'm pretty sure that you no longer have the problem with the latest version of CLI, we fixed a couple of dependencies issues. Anyway if you did the effort to remove the CLI dependency from your chain, there's no reason to re-add it :) I just try to clarify for readers.

RaulBernal · 2022-11-04T10:07:03Z

@RaulBernal We removed cosmoscmd from Ignite CLI v0.25.0, here is the complete migration guide for existing chain : https://docs.ignite.com/migration/v0.25.0#removing-cosmoscmd

About the ICA controller bug you have raised, there's also a migration guide : https://docs.ignite.com/migration/v0.25.0#fix-ica-controller-keeper-wiring

AS for Ignite CLI - we had problems with it in CI and eventually just removed it because nobody was using it.

@robert-zaremba I'm pretty sure that you no longer have the problem with the latest version of CLI, we fixed a couple of dependencies issues. Anyway if you did the effort to remove the CLI dependency from your chain, there's no reason to re-add it :) I just try to clarify for readers.

Yeah I've participated in that doc with my testing :) really a pleasure work with you (I only get from ignite/cli the openapi package)

In next days I'm going to refactor again with all guides. We make COSMOS big with guides. I think I'm going to share a doc integrating the NFT feature in SDK v046 (and Ignite CLI)

robert-zaremba · 2022-11-04T11:14:11Z

Sure. Thanks for the note @tbruyelle .

sdk-46

9d8ffaf

totally broken don't look at it

0f4eec1

faddat changed the title ~~sdk-46~~ lsm and sdk 46 Sep 2, 2022

faddat changed the title ~~lsm and sdk 46~~ sdk 46 Sep 2, 2022

faddat added 6 commits September 2, 2022 19:01

early build with more to fix

cad6e9b

modules

b32d3e4

fumpt

02b33ee

push for review

296c5cf

ci

a7149dd

Merge remote-tracking branch 'origin/main' into patch-1

fafb694

aljo242 mentioned this pull request Sep 2, 2022

chore: sdk 46 idioms (import nothing with "rest") ignite/cli#2807

Closed

faddat added 2 commits September 3, 2022 00:43

bump cli proper-like

2ad30e8

46

edfedaa

faddat added 2 commits September 3, 2022 00:49

46

60d4ca6

errcheck fixes

565501d

faddat changed the title ~~sdk 46~~ First Joint: sdk 46 Sep 2, 2022

tbruyelle mentioned this pull request Sep 6, 2022

Fix nightly tags ignite/cli#2811

Closed

gjermundgaraba reviewed Sep 8, 2022

View reviewed changes

Merge branch 'main' into patch-1

ffee11a

merge main, address testify import

9a8c9f0

RaulBernal closed this Nov 4, 2022

First Joint: sdk 46 #39

First Joint: sdk 46 #39

Conversation

faddat commented Sep 2, 2022 • edited Loading

RaulBernal commented Sep 2, 2022

faddat commented Sep 2, 2022

faddat commented Sep 2, 2022

tbruyelle commented Sep 2, 2022

faddat commented Sep 2, 2022

faddat commented Sep 2, 2022

tbruyelle commented Sep 3, 2022

robert-zaremba commented Sep 7, 2022

RaulBernal commented Sep 7, 2022

gjermundgaraba Sep 8, 2022

Choose a reason for hiding this comment

faddat Sep 12, 2022

Choose a reason for hiding this comment

faddat Sep 12, 2022

Choose a reason for hiding this comment

robert-zaremba Sep 28, 2022

Choose a reason for hiding this comment

RaulBernal Sep 29, 2022

Choose a reason for hiding this comment

RaulBernal commented Sep 8, 2022

gjermundgaraba Sep 8, 2022

Choose a reason for hiding this comment

faddat Sep 12, 2022

Choose a reason for hiding this comment

faddat commented Sep 12, 2022

faddat commented Sep 12, 2022

RaulBernal commented Sep 19, 2022

RaulBernal commented Sep 19, 2022

RaulBernal commented Sep 19, 2022

robert-zaremba commented Sep 28, 2022

robert-zaremba commented Sep 28, 2022

RaulBernal commented Sep 29, 2022 • edited Loading

RaulBernal commented Sep 29, 2022 • edited Loading

tbruyelle commented Sep 29, 2022

RaulBernal commented Sep 29, 2022

faddat commented Nov 2, 2022

RaulBernal commented Nov 3, 2022

robert-zaremba commented Nov 4, 2022

robert-zaremba commented Nov 4, 2022

RaulBernal commented Nov 4, 2022

RaulBernal commented Nov 4, 2022

tbruyelle commented Nov 4, 2022 • edited Loading

RaulBernal commented Nov 4, 2022

robert-zaremba commented Nov 4, 2022

faddat commented Sep 2, 2022 •

edited

Loading

RaulBernal commented Sep 29, 2022 •

edited

Loading

RaulBernal commented Sep 29, 2022 •

edited

Loading

tbruyelle commented Nov 4, 2022 •

edited

Loading