Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Cosmos SDK v0.46.0 #2675

Merged
merged 12 commits into from
Jul 29, 2022
Merged

feat!: Cosmos SDK v0.46.0 #2675

merged 12 commits into from
Jul 29, 2022

Conversation

aljo242
Copy link
Contributor

@aljo242 aljo242 commented Jul 28, 2022

This PR does the following:

  • updates Cosmos SDK to v0.46.0
  • updates spn to latest
  • updates ibc-go to v5 temporary branch

This PR does NOT

  • update application templates
  • update proto compilation

@ilgooz ilgooz linked an issue Jul 28, 2022 that may be closed by this pull request
8 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2022

Visit the preview URL for this PR (updated for commit 638abc8):

https://ignite-go-docs--pr2675-feat-sdk-v0-46-0-0zoge0jm.web.app

(expires Thu, 04 Aug 2022 17:51:19 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@aljo242 aljo242 marked this pull request as ready for review July 28, 2022 18:43
@aljo242
Copy link
Contributor Author

aljo242 commented Jul 28, 2022

This commit was made to a mock file which might be undesirable. We need to implement this interface, but I wasn't sure how to generate the mocking code.

@ilgooz ilgooz changed the base branch from develop to feat/sdk-ibc-upgrade July 29, 2022 07:53
@jeronimoalbi
Copy link
Member

jeronimoalbi commented Jul 29, 2022

This commit was made to a mock file which might be undesirable. We need to implement this interface, but I wasn't sure how to generate the mocking code.

I think you can do it with something like:

mockery --srcpkg github.com/cosmos/cosmos-sdk/x/bank/types --name QueryClient --structname BankClient --case underscore --print

What I'm not sure is where to add the generator comment, maybe somewhere in the ignite/services/network/network.go. If so the comment should look like:

//go:generate mockery --srcpkg github.com/cosmos/cosmos-sdk/x/bank/types --name QueryClient --structname BankClient --case underscore

Or maybe to the Makefile, because it's generating the mock for an interface defined in a dependency.

Also another alternative would be that we define our BankClient interface and generate the mock from it. It would embed the QueryClient.

By the way, I just noticed that the StakingClient in the ignite/services/network/mocks/staking_client.go file should also be added in a similar way.

@tbruyelle
Copy link
Contributor

tbruyelle commented Jul 29, 2022

What I'm not sure is where to add the generator comment, maybe somewhere in the ignite/services/network/network.go. If so the comment should look like:

//go:generate mockery --srcpkg github.com/cosmos/cosmos-sdk/x/bank/types --name QueryClient --structname BankClient --case underscore

@jeronimoalbi Indeed there's already some go:generate instructions in network.go/network.go, but also in network/testutil/mocks.go. If I get the logic the external interfaces are in testutil, so maybe BankClient should be added there.

Something like

diff --git a/ignite/services/network/testutil/mocks.go b/ignite/services/network/testutil/mocks.go
index 54fd02f8..28244037 100644
--- a/ignite/services/network/testutil/mocks.go
+++ b/ignite/services/network/testutil/mocks.go
@@ -2,6 +2,7 @@ package testutil

 import (
        "github.com/cosmos/cosmos-sdk/crypto/keyring"
+       banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
        campaigntypes "github.com/tendermint/spn/x/campaign/types"
        launchtypes "github.com/tendermint/spn/x/launch/types"
        profiletypes "github.com/tendermint/spn/x/profile/types"
@@ -32,3 +33,8 @@ type RewardClient interface {
 type AccountInfo interface {
        keyring.Info
 }
+
+//go:generate mockery --name BankClient --case underscore --output ../mocks
+type BankClient interface {
+       banktypes.QueryClient
+}

@fadeev
Copy link
Contributor

fadeev commented Jul 29, 2022

While upgrading an IBC-enabled module (in a test project) I've noticed that having a dependency on the cosmosibckeeper makes it difficult to upgrade the module without also proposing changes to Ignite.

wdyt about proposing to have a package like this in cosmos/ibc-go somewhere, so that we can have: less code in the template and an easier path for version upgrades?

@aljo242
Copy link
Contributor Author

aljo242 commented Jul 29, 2022

Added panic on the GetPubKey calls. We can resolve this later with the following issue

@aljo242
Copy link
Contributor Author

aljo242 commented Jul 29, 2022

While upgrading an IBC-enabled module (in a test project) I've noticed that having a dependency on the cosmosibckeeper makes it difficult to upgrade the module without also proposing changes to Ignite.

wdyt about proposing to have a package like this in cosmos/ibc-go somewhere, so that we can have: less code in the template and an easier path for version upgrades?

I'm always for adding less code in the templates to make upgrades easier. I think a goal for our new releases should be to decouple new chains from importing our core packages. There must be a way to provide lots of utility functions to consumers without having them import everything.

If we had multiple go modules within this repo, wouldn't this be possible?

@aljo242
Copy link
Contributor Author

aljo242 commented Jul 29, 2022

Good to merge?

@tbruyelle
Copy link
Contributor

Good to merge?

I wanted to test all the tutorials with the new binary, but I ran out of time. Has anyone done this yet ?

@ilgooz
Copy link
Member

ilgooz commented Jul 29, 2022

Good to merge?

I wanted to test all the tutorials with the new binary, but I ran out of time. Has anyone done this yet ?

Since these changes will be merged to another branch (feat/sdk-ibc-upgrade) and not develop directly, I think we will have a chance to run tutorials before merging it to develop?

We will also need to merge other PRs that will change our templates. So probably we can cut from time if we just test tutorials after all these are merged.

I will add this as an item to the issue description.

@ilgooz
Copy link
Member

ilgooz commented Jul 29, 2022

image

https://github.com//issues/2673

@aljo242 aljo242 merged commit d2bec46 into feat/sdk-ibc-upgrade Jul 29, 2022
@aljo242 aljo242 deleted the feat/sdk-v0.46.0 branch July 29, 2022 15:05
@tbruyelle
Copy link
Contributor

image

https://github.com/[/issues/2673](https://github.com/ignite/cli/issues/2673)

Yep I have this issue in mind, but also didn't have the time to work on it!

aljo242 pushed a commit that referenced this pull request Aug 17, 2022
* docs(changelog): add unreleased section

* feat!: Cosmos SDK v0.46.0 (#2675)

* update go mod

* update ibc path

* fix merge errors

* add temp client mock

* fix: deps (#2676)

* revert packages to old ibc-go

* fix remaining ibc paths

* fix template go.sum

* panic keyring errors

* add mockery generate instructions

* regenerate mocks

Co-authored-by: İlker G. Öztürk <[email protected]>

* fix merge

* update spn

* upgrade spn

* remove ibc-go fork

* template updates

* fix ibc and oracle templates

* update templates

* remaining tempalte updates

* fix template

* fix faucet test

* use beta ibc release

* update deps in template

* use ibcmodule idiom (#2692)

* Update ignite/templates/module/create/ibc/x/{{moduleName}}/module_ibc.go.plush

Co-authored-by: Lucas Btd <[email protected]>

* fix(`pkg/cosmosaccount`): register cryptocodec interfaces (#2702)

* register crypto codec interfaces

* improve error messages

* fix(template): removed unused methods and fix comments (#2708)

* fix(`cosmosclient`): use protobuf Any for TxMsgData  (#2714)

* fix comment

* use proto Any

* Update ignite/pkg/cosmosclient/cosmosclient.go

Co-authored-by: Lucas Btd <[email protected]>

Co-authored-by: Lucas Btd <[email protected]>

* chore(0.46): upgrade spn version (#2730)

* feat(`app`): add interchain accounts for `v0.46.0` (#2703)

* update template for ica

* Update ignite/templates/app/stargate/app/app.go.plush

Co-authored-by: Lucas Btd <[email protected]>

Co-authored-by: Lucas Btd <[email protected]>

* fix: change relayer call to use secp256k1 private keys in call (#2729)

This chage uses secp256k1 private keys instead of ASCII armored ones as
gRPC arguments when calling the TS relayer.

* docs: update tutorials (#2737)

- Change `ibc-go` import to use "v5"
- Add package missing definition to code examples that can be copy/pasted
- Change file references to paths, for consistency with other tutorials
- Fix interchange tutorial to use ports that don't overlap
- Remove unused imports
- Fix documentation issues

* feat: upgrade `ts-relayer` dependencies (#2722)

* chore: upgrade `@confio/relayer` to the latest version 0.5.1
* chore: update `@cosmjs` to version 0.28.9
* chore: remove `@cosmjs/launchpad` from dependencies
* chore: add new dependency `@cosmjs/encoding` for relayer
* chore: add `sinon` dependency required to build the relayer
* fix: remove "gasLimit" option from relayer
* fix: update `@cosmjs` to version 0.28.11

* fix(cosmosclient): fix account prefix config (#2743)

* fix(cosmosclient): account prefix config not correct when broadcasting a tx from a newly created default account

* add issue

Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Lucas Btd <[email protected]>
Co-authored-by: Denis Fadeev <[email protected]>
Co-authored-by: Jerónimo Albi <[email protected]>
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* docs(changelog): add unreleased section

* feat!: Cosmos SDK v0.46.0 (ignite#2675)

* update go mod

* update ibc path

* fix merge errors

* add temp client mock

* fix: deps (ignite#2676)

* revert packages to old ibc-go

* fix remaining ibc paths

* fix template go.sum

* panic keyring errors

* add mockery generate instructions

* regenerate mocks

Co-authored-by: İlker G. Öztürk <[email protected]>

* fix merge

* update spn

* upgrade spn

* remove ibc-go fork

* template updates

* fix ibc and oracle templates

* update templates

* remaining tempalte updates

* fix template

* fix faucet test

* use beta ibc release

* update deps in template

* use ibcmodule idiom (ignite#2692)

* Update ignite/templates/module/create/ibc/x/{{moduleName}}/module_ibc.go.plush

Co-authored-by: Lucas Btd <[email protected]>

* fix(`pkg/cosmosaccount`): register cryptocodec interfaces (ignite#2702)

* register crypto codec interfaces

* improve error messages

* fix(template): removed unused methods and fix comments (ignite#2708)

* fix(`cosmosclient`): use protobuf Any for TxMsgData  (ignite#2714)

* fix comment

* use proto Any

* Update ignite/pkg/cosmosclient/cosmosclient.go

Co-authored-by: Lucas Btd <[email protected]>

Co-authored-by: Lucas Btd <[email protected]>

* chore(0.46): upgrade spn version (ignite#2730)

* feat(`app`): add interchain accounts for `v0.46.0` (ignite#2703)

* update template for ica

* Update ignite/templates/app/stargate/app/app.go.plush

Co-authored-by: Lucas Btd <[email protected]>

Co-authored-by: Lucas Btd <[email protected]>

* fix: change relayer call to use secp256k1 private keys in call (ignite#2729)

This chage uses secp256k1 private keys instead of ASCII armored ones as
gRPC arguments when calling the TS relayer.

* docs: update tutorials (ignite#2737)

- Change `ibc-go` import to use "v5"
- Add package missing definition to code examples that can be copy/pasted
- Change file references to paths, for consistency with other tutorials
- Fix interchange tutorial to use ports that don't overlap
- Remove unused imports
- Fix documentation issues

* feat: upgrade `ts-relayer` dependencies (ignite#2722)

* chore: upgrade `@confio/relayer` to the latest version 0.5.1
* chore: update `@cosmjs` to version 0.28.9
* chore: remove `@cosmjs/launchpad` from dependencies
* chore: add new dependency `@cosmjs/encoding` for relayer
* chore: add `sinon` dependency required to build the relayer
* fix: remove "gasLimit" option from relayer
* fix: update `@cosmjs` to version 0.28.11

* fix(cosmosclient): fix account prefix config (ignite#2743)

* fix(cosmosclient): account prefix config not correct when broadcasting a tx from a newly created default account

* add issue

Co-authored-by: Alex Johnson <[email protected]>
Co-authored-by: Lucas Btd <[email protected]>
Co-authored-by: Denis Fadeev <[email protected]>
Co-authored-by: Jerónimo Albi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants