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.

Sign up for GitHub

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

Jump to bottom

feat: integrate with v0.52.x (1/n) #4289

Merged
merged 27 commits into from
Oct 15, 2024
Merged

feat: integrate with v0.52.x (1/n) #4289

merged 27 commits into from
Oct 15, 2024

Conversation

julienrbrt
Copy link
Member

@julienrbrt julienrbrt commented Aug 6, 2024 •
edited
Loading

v052 is getting audited, this is time for us to update modules and simapp to the latest version.
This PR focuses on:

  • Updating Ignite to use latest SDK
  • Update module wiring to core v1 (follow-up, made only to pass build, follow up will clean it up and wire environment)
  • Update simapp to v0.52
  • Delete IBC (unsupported for now)

@julienrbrt julienrbrt mentioned this pull request Aug 6, 2024
Merged
@julienrbrt julienrbrt mentioned this pull request Aug 15, 2024
Merged
@github-actions github-actions bot added the type:services Service-related issues. label Aug 15, 2024
@julienrbrt julienrbrt changed the title feat: integrate with v0.52.x feat: integrate with v0.52.x (1/n) Aug 15, 2024
@faddat
Copy link
Contributor

faddat commented Sep 3, 2024

Is it possible to keep this in a branch other than main until IBC works with v0.52.x?

Not sure that shipping without IBC is such a great idea.

@julienrbrt
Copy link
Member Author

Is it possible to keep this in a branch other than main until IBC works with v0.52.x?

Not sure that shipping without IBC is such a great idea.

It won't be shipped as final, but we will have an alpha without IBC yes.
Timeline of IBC integrating with 0.52 is undetermined.

@julienrbrt julienrbrt marked this pull request as ready for review September 11, 2024 22:20
@julienrbrt julienrbrt mentioned this pull request Sep 19, 2024
Merged
@julienrbrt julienrbrt changed the base branch from main to feature/v0.52.x October 1, 2024 14:07
@julienrbrt
Copy link
Member Author

I'll update to https://github.com/cosmos/cosmos-sdk/releases/tag/v0.52.0-beta.2 before we should merge this.

@robert-zaremba
Copy link

is there anything else blocking this branch?

@julienrbrt
Copy link
Member Author

julienrbrt commented Oct 9, 2024 •
edited
Loading

is there anything else blocking this branch?

reviews :D and the message above

@julienrbrt julienrbrt changed the base branch from feature/v0.52.x to main October 14, 2024 08:50
@julienrbrt
Copy link
Member Author

moved back the target branch to main, as most tests are passing.

Pantani
Pantani reviewed Oct 14, 2024
View reviewed changes
ignite/pkg/cosmosclient/bank.go Show resolved Hide resolved
ignite/pkg/cosmosclient/cosmosclient.go Show resolved Hide resolved
ignite/pkg/cosmosclient/cosmosclient.go
"cosmossdk.io/core/transaction"
banktypes "cosmossdk.io/x/bank/types"
staking "cosmossdk.io/x/staking/types"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

remove and run make format

ignite/services/plugin/plugin_test.go Show resolved Hide resolved
ignite/templates/app/files-minimal/app/app.go.plush
// supply custom module basics
map[string]module.AppModuleBasic{
genutiltypes.ModuleName: genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator),
// this line is used by starport scaffolding # stargate/appConfig/moduleBasic
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove this placeholder from the rest of the code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like it wasn't existing, so nothing to here actually.

ignite/templates/app/files/cmd/{{binaryNamePrefix}}d/cmd/testnet.go.plush Show resolved Hide resolved
ignite/templates/app/files/cmd/{{binaryNamePrefix}}d/cmd/testnet.go.plush Show resolved Hide resolved
ignite/templates/app/files/cmd/{{binaryNamePrefix}}d/cmd/root.go.plush Show resolved Hide resolved
ignite/templates/typed/map/map.go Show resolved Hide resolved
integration/ibc/cmd_ibc_test.go Show resolved Hide resolved
Pantani
Pantani reviewed Oct 14, 2024
View reviewed changes
Copy link
Collaborator

@Pantani Pantani left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are still using the MemStoreKey const into the x/module/types/keys.go? If not, can we remove it?

ignite/templates/app/files/app/export.go.plush
@@ -69,15 +76,10 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
allowedAddrsMap[addr] = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 74 uses a log.Fatal instead panic

@julienrbrt
Copy link
Member Author

We are still using the MemStoreKey const into the x/module/types/keys.go? If not, can we remove it?

Yeah, the modules will be improved in a follow up. This is just so it builds and work (see check boxes from issue description)

@julienrbrt julienrbrt mentioned this pull request Oct 14, 2024
Open
@julienrbrt
Copy link
Member Author

I'll do the linting in a follow-up.
Merging now!

@mergify mergify bot merged commit 01c94ac into main Oct 15, 2024
40 of 47 checks passed
@mergify mergify bot deleted the julien/052 branch October 15, 2024 12:27
@julienrbrt julienrbrt mentioned this pull request Oct 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toschdev toschdev toschdev approved these changes

@Pantani Pantani Pantani approved these changes

@ilgooz ilgooz Awaiting requested review from ilgooz ilgooz is a code owner

@salmad3 salmad3 Awaiting requested review from salmad3 salmad3 is a code owner

@jeronimoalbi jeronimoalbi Awaiting requested review from jeronimoalbi jeronimoalbi is a code owner

Assignees

@julienrbrt julienrbrt

Labels
component:ci CI/CD workflow and automated jobs. component:cmd component:configs component:packages component:templates type:internal type:services Service-related issues.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@julienrbrt @faddat @robert-zaremba @Pantani @toschdev