feat: Upgrade SDK v0.47 to v0.50

[deepan95dev]

Related Github tickets

• Upgrading SDK v0.47.4 to v0.50.2

Background

This PR contains all the changes that has to be done for the upgrade, including integration tests/unit tests. This also contains latest master branch commits upto this

Testing completed

• [x] keeper_integration_tests are moved to tests/integration/{module} with local fixture setup, leaving the dependency on app/app_setup.go (so the file app_setup.go is deleted)
• [x] New test cases added are
• _util/keeper/addresstest.go which covers util/keeper/address.go
• _/x/gravity/migrations/v1/migratetest.go which covers /x/gravity/migrations/v1/migrate.go
• [x] Most of the test coverage exists as it is with upgrade related changes, only few got updated as listed below
• _tests/integration/evm/keeper/keeper_integrationtest.go had putMessageInQueue in the case there is another upload valset already in only but later we needed to add for two more functions, TestFirstSnapshot_OnSnapshotBuilt() function and TestOldPublishedSnapshot_OnSnapshotBuilt function
• In x/paloma/ante.go after a series of discussion we arrived at the conclusion to use libmeta.getSigner but the test cases were failing so I mentioned here with comment, you may interpret this change to adapt with the test case.
• [ ] tested in a private testnet
• [x] Upgrade testing is done

Breaking changes

• [ ] I have checked my code for breaking changes
• [ ] If there are breaking changes, there is a supporting migration.

[deepan95dev]

Blocking this until I find some time to step through all the changes. Other than code review, there's a few things missing here:

• [ ] Rebase on latest version of master
• [ ] PR description including short description & already performed tests
• [ ] before these changes are added to master, we need full E2E testing including compatibility with Paloma's side loading app Pigeon. We can compile a list of manual testing steps required to pass successfully before we consider going ahead.

How shall we proceed with E2E testing with Paloma's Pigeon App?

[byte-bandit]

How shall we proceed with E2E testing with Paloma's Pigeon App?

@taariq @verabehr @aleem1314

I suggest Volume compiles a list of manual testing steps and required proof of verification to share with the vitwit team. Happy to drive this on Notion until we settle on a run book.

[deepan95dev]

@byte-bandit we have implemented CustomGetSigners and moved the InterfaceRegistry to app/interface_registry.go which comprises all MetaData signers info.

[byte-bandit]

@byte-bandit we have implemented CustomGetSigners and moved the InterfaceRegistry to app/interface_registry.go which comprises all MetaData signers info.

Great. Let's keep this in the history.

I think it's actually worth trying to utilise the cosmos.msg.v1.signer option instead of the custom getter. I thought that the libmeta implementation did more than just return the []signers field, but it doesn't. The combinatory logic only happens in the ante handler.

Apologies for the confusion. Remove the custom getter (though keep it in git history please) and re-introduce the cosmos.msg.v1.signer options pointing to the metadata field on all messages please.

[vishal-kanna]

Okay

[vishal-kanna]

There's a lot of noise coming from the change in context. Since almost the entire code base is reliant on the sdk.Context implementation anyway, we'd be able to save a lot of touches by using func(goCtx context.Context) {ctx := sdk.UnwrapContext(goCtx)} instead of going the other way.

That would also render the remaining clode base more legible and idiomatic, and prevent bugs introduced by mass "search/replace" tools where ctx is overriden with a scoped version.

Though going forward, it's probably better to stay in line with the Cosmos style of sdkCtx. @byte-bandit Is it adding a function and using it inplace of the other way

[byte-bandit]

There's a lot of noise coming from the change in context. Since almost the entire code base is reliant on the sdk.Context implementation anyway, we'd be able to save a lot of touches by using func(goCtx context.Context) {ctx := sdk.UnwrapContext(goCtx)} instead of going the other way. That would also render the remaining clode base more legible and idiomatic, and prevent bugs introduced by mass "search/replace" tools where ctx is overriden with a scoped version. Though going forward, it's probably better to stay in line with the Cosmos style of sdkCtx. @byte-bandit Is it adding a function and using it inplace of the other way

That's not what I meant. There's absolutely nothing we gain from simply replacingsdkCtx := sdk.UnwrapSDKContext with with sdkCtx := common.SdkContext.

I was talking about function signatures and how we are naming the parameters. But I'm not going to go into detail here now, as the SDK itself uses this naming paradigm, and I believe we should stick with it - even though I personally think it's a subpar solution. We just need to be very careful now, that we actually use sdkCtx everywhere after it was cast, and respect branching contexts as well.

So, please revert back to sdk.UnwrapSDKContext.

[taariq]

@aleem1314 we have just TWO unresolved convos on the PR. We will merge on your check and resolve.

[faddat]

Just wanted to say that I thought this is cool :-)

[taariq]

Just wanted to say that I thought this is cool :-)

That means so much to us @faddat! Thank you!