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

chore: extract improvements from #21497 #21506

Merged
merged 3 commits into from
Sep 4, 2024
Merged

chore: extract improvements from #21497 #21506

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d72bde6
chore :extract improvements from #21497
julienrbrt Sep 2, 2024
4022ac6
except auth (ref #21507)
julienrbrt Sep 3, 2024
b246ea0
Apply suggestions from code review
julienrbrt Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 8 additions & 42 deletions UPGRADING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ For non depinject users, simply call `RegisterLegacyAminoCodec` and `RegisterInt

Additionally, thanks to the genesis simplification, as explained in [the genesis interface update](#genesis-interface), the module manager `InitGenesis` and `ExportGenesis` methods do not require the codec anymore.

##### GRPC-WEB
##### GRPC WEB

Grpc-web embedded client has been removed from the server. If you would like to use grpc-web, you can use the [envoy proxy](https://www.envoyproxy.io/docs/envoy/latest/start/start). Here's how to set it up:

Expand Down Expand Up @@ -347,6 +347,8 @@ Also, any usages of the interfaces `AnyUnpacker` and `UnpackInterfacesMessage` m

#### `**all**`

All modules were spun out into their own `go.mod`. Replace imports by `cosmossdk.io/x/{moduleName}`.

##### Core API

Core API has been introduced for modules since v0.47. With the deprecation of `sdk.Context`, we strongly recommend to use the `cosmossdk.io/core/appmodule` interfaces for the modules. This will allow the modules to work out of the box with server/v2 and baseapp, as well as limit their dependencies on the SDK.
Expand Down Expand Up @@ -399,7 +401,7 @@ All modules using dependency injection must update their imports.

##### Params

Previous module migrations have been removed. It is required to migrate to v0.50 prior to upgrading to v0.51 for not missing any module migrations.
Previous module migrations have been removed. It is required to migrate to v0.50 prior to upgrading to v0.52 for not missing any module migrations.

##### Genesis Interface

Expand Down Expand Up @@ -436,60 +438,24 @@ if err != nil {
}
```

#### `x/auth`

Auth was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/auth`

#### `x/authz`

Authz was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/authz`

#### `x/bank`

Bank was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/bank`

### `x/crsis`
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

The Crisis module was removed due to it not being supported or functional any longer.
The `x/crisis` module was removed due to it not being supported or functional any longer.
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Action Required: Remove x/crisis module usage

The x/crisis module has been removed as it is no longer supported. If your application is using this module, make sure to remove it before upgrading to this version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The `x/crisis` module was removed due to it not being supported or functional any longer.
The `x/crisis` module was removed due to it's not being supported or functional any longer.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akhilkumarpilli, the original sentence is grammatically correct. The phrase "it's" is a contraction of "it is" or "it has," which doesn't fit the context here. The correct possessive form is "its," so the original sentence should remain as is.


#### `x/distribution`

Distribution was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/distribution`

The existing chains using x/distribution module needs to add the new x/protocolpool module.

#### `x/group`

Group was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/group`
Existing chains using `x/distribution` module must add the new `x/protocolpool` module.
Copy link
Contributor

Choose a reason for hiding this comment

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

Required: Add x/protocolpool module

If your chain is using the x/distribution module, you must add the new x/protocolpool module when upgrading to this version.


#### `x/gov`

Gov was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/gov`

Gov v1beta1 proposal handler has been changed to take in a `context.Context` instead of `sdk.Context`.
This change was made to allow legacy proposals to be compatible with server/v2.
If you wish to migrate to server/v2, you should update your proposal handler to take in a `context.Context` and use services.
On the other hand, if you wish to keep using baseapp, simply unwrap the sdk context in your proposal handler.

#### `x/mint`

Mint was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/mint`

#### `x/slashing`

Slashing was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/slashing`

#### `x/staking`

Staking was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/staking`

#### `x/params`

A standalone Go module was created and it is accessible at "cosmossdk.io/x/params".

#### `x/protocolpool`

Introducing a new `x/protocolpool` module to handle community pool funds. Its store must be added while upgrading to v0.51.x.
Introducing a new `x/protocolpool` module to handle community pool funds. Its store must be added while upgrading to v0.52.x.
Copy link
Contributor

Choose a reason for hiding this comment

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

Required: Add x/protocolpool store during upgrade

When upgrading to v0.52.x, make sure to add the x/protocolpool store as part of the upgrade process.


Example:

Expand All @@ -506,7 +472,7 @@ func (app SimApp) RegisterUpgradeHandlers() {
}
```

Add `x/protocolpool` store while upgrading to v0.51.x:
Add `x/protocolpool` store while upgrading to v0.52.x:

```go
storetypes.StoreUpgrades{
Expand Down
2 changes: 1 addition & 1 deletion docs/learn/advanced/00-baseapp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ newly committed state and `finalizeBlockState` is set to `nil` to be reset on `F
During `InitChain`, the `RequestInitChain` provides `ConsensusParams` which contains parameters
related to block execution such as maximum gas and size in addition to evidence parameters. If these
parameters are non-nil, they are set in the BaseApp's `ParamStore`. Behind the scenes, the `ParamStore`
is managed by an `x/consensus_params` module. This allows the parameters to be tweaked via
is managed by an `x/consensus` module. This allows the parameters to be tweaked via
on-chain governance.

## Service Routers
Expand Down
2 changes: 1 addition & 1 deletion server/v2/cometbft/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type Consensus[T transaction.Tx] struct {
func NewConsensus[T transaction.Tx](
logger log.Logger,
appName string,
consensusAuthority string,
consensusAuthority string, // TODO remove
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure all references to consensusAuthority are removed when removing the parameter.

The TODO comment indicates the intention to remove the consensusAuthority parameter in the future. When doing so, please make sure to:

  • Update the NewConsensus function body to remove any references to consensusAuthority.
  • Update any code that invokes NewConsensus to remove the consensusAuthority argument.

Consider creating a GitHub issue to track the removal of the consensusAuthority parameter and link it to the TODO comment.

app *appmanager.AppManager[T],
mp mempool.Mempool[T],
indexedEvents map[string]struct{},
Expand Down
2 changes: 1 addition & 1 deletion server/v2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type AppI[T transaction.Tx] interface {
Name() string
InterfaceRegistry() server.InterfaceRegistry
GetAppManager() *appmanager.AppManager[T]
GetConsensusAuthority() string
GetConsensusAuthority() string // TODO remove
Copy link
Contributor

Choose a reason for hiding this comment

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

Acknowledge the TODO comment for removing the GetConsensusAuthority method.

The comment // TODO remove indicates a future intention to remove the GetConsensusAuthority method from the AppI interface. This change suggests a potential shift in the design or functionality of the interface, indicating that the method may no longer be necessary or relevant in future iterations of the codebase.

Consider creating a GitHub issue to track the removal of the GetConsensusAuthority method. This will help ensure that the removal is properly planned and executed in a future update. Do you want me to open a GitHub issue for this?

GetGPRCMethodsToMessageMap() map[string]func() gogoproto.Message
GetStore() any
}
6 changes: 5 additions & 1 deletion simapp/v2/app_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ var (
ModuleName: authtypes.ModuleName,
KvStoreKey: "acc",
},
{
ModuleName: accounts.ModuleName,
KvStoreKey: accounts.StoreKey,
},
},
// NOTE: The genutils module must occur after staking so that pools are
// properly initialized with tokens from genesis accounts.
Expand Down Expand Up @@ -260,7 +264,7 @@ var (
{
Name: consensustypes.ModuleName,
Config: appconfig.WrapAny(&consensusmodulev1.Module{
Authority: "consensus",
Authority: "consensus", // TODO remove.
}),
},
{
Expand Down
1 change: 0 additions & 1 deletion x/distribution/keeper/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

// BeginBlocker sets the proposer for determining distribution during endblock
// and distribute rewards for the previous block.
// TODO: use context.Context after including the comet service
func (k Keeper) BeginBlocker(ctx context.Context) error {
defer telemetry.ModuleMeasureSince(types.ModuleName, telemetry.Now(), telemetry.MetricKeyBeginBlocker)

Expand Down
Loading