wasmd 30 + ibc v4 + IBCFees

[Reecepbcups]

Pending ibc-go ICS23 v0.9.0 bump 2868 PR

wasmd 0.30.0
ibc go v4.2.0

New IBCFeeKeeper by following official guide

• https://ibc.cosmos.network/main/middleware/ics29-fee/integration.html

ibcRouter updated wasmStack with ibc fee keeper middleware. Same for transfer & icaHost

Added ibc v3->4 upgrade comment from official guide

• https://github.com/cosmos/ibc-go/blob/v5.1.0/docs/migrations/v3-to-v4.md#migration-to-fix-support-for-base-denoms-with-slashes

[faddat]

dude I love this!

I'm going to approve from the notional side.

We'll need approvals from others, though.

Sorry, after review, I gotta ask if you've got the migrations in here:

https://github.com/cosmos/ibc-go/blob/v5.1.0/docs/migrations/v3-to-v4.md

[faddat]

I think this requires one or more upgrade handlers:

https://github.com/cosmos/ibc-go/blob/v5.1.0/docs/migrations/v3-to-v4.md

I think that also to avoid the problem plaguing stride, we need to initialize the ibcfee keeper specifically.

[vuong177]

@Reecepbcups should we hold it until v13? IBCFee is a new feature and I'm not sure we need it in v12.

[Reecepbcups]

@vuong177 IBC v4 & ibcfees is required for wasmd 0.30.0 - so if we want latest wasmd has to be in v12

[Reecepbcups]

@faddat I looked over this document and seems you only need the upgrade handler if your chain has factory/address/name denoms.
Which we don't have until v12 launch with the tokenfactory

It also only says migration is needed for 3.1->4. We are 3.4->4.

But I have still added

[faddat]

So I suspect that those are outdated documents. But I don't know. I'm going to reach out to the IBC team to make sure.

[faddat]

I would also like to wait to merge this until these changes land upstream:

cosmos/ibc-go#2868

And then the other thing that we are lacking here, so we add the IBC fee module, but are we initializing its store key explicitly in an upgrade handler?

[Reecepbcups]

@faddat oops yea I had forgot to do this. It is now, done in above commit thx

[faddat]

Ok, so per the dev chat, we're going to wait on an alternative impl that doesn't include the ibcfee module.

Personally, prefer this route.

[faddat]

let's do it, but wait on upgrading uni till ibc-go changes for ics23 v0.9.0 are in

[vuong177]

failed to load latest version: failed to load store: initial version set to 30, but found earlier version 1

I got that err when running a testnet. @faddat @Reecepbcups.

[Reecepbcups]

failed to load latest version: failed to load store: initial version set to 30, but found earlier version 1

I got that err when running a testnet. @faddat @Reecepbcups.

my test_node.sh ran just fine.
Was this a mainnet state -> this branch upgrade that failed?

I don't see any wasmKeeper Params can be changed that relate to version
and it shouldn't need storeUpgrade keys should it? hmm

[vuong177]

Hmm I got it when running the upgrade binary through gov.

[faddat]

requesting changes due to reported issue

[vuong177]

The e2e upgrade test still fails, we need it to pass all before merging. @Reecepbcups

[faddat]

To me @vuong177 it looked to me like it could be the test itself as an issue. I just kicked off CI again to check.

[JakeHartnell]

LGTM. utACK.

[the-frey]

Looks like this needs a resolve against base branch but agree we should get this one in if poss.

[giansalex]

Can we enable StargateQueries as part of wasmd 0.30? CosmWasm/wasmd#1069

[Reecepbcups]

Can we enable StargateQueries as part of wasmd 0.30? CosmWasm/wasmd#1069

@giansalex Yes, I will commit this in the morning

[the-frey]

Which queries do you want to see enabled @giansalex ?

[giansalex]

Which queries do you want to see enabled @giansalex ?

I am looking for the routes needed to implement ICQ in a contract.

• /ibc.core.connection.v1.Query/Connection
• /ibc.core.client.v1.Query/ClientState
• /ibc.core.client.v1.Query/ConsensusState

[the-frey]

I think this is ready to ship - QQ should we have stargate queries in another PR? That feels like the sketchiest thing that we might have to back out of after testing & might be good to have it atomically within a single PR