Skip to content

Commit

Permalink
fix v2 behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt committed Feb 6, 2025
1 parent f54e59a commit fedaa70
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 27 deletions.
21 changes: 10 additions & 11 deletions server/v2/cometbft/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ import (
cometerrors "cosmossdk.io/server/v2/cometbft/types/errors"
"cosmossdk.io/server/v2/streaming"
"cosmossdk.io/store/v2/snapshots"
consensustypes "cosmossdk.io/x/consensus/types"
)

// InitialAppVersion returned by Info at height 0.
// Afterwards, the consensus params are queried from the app.
var InitialAppVersion uint64 = 0

const (
QueryPathApp = "app"
QueryPathP2P = "p2p"
Expand Down Expand Up @@ -139,7 +142,7 @@ func (c *consensus[T]) Info(ctx context.Context, _ *abciproto.InfoRequest) (*abc
}

// if height is 0, we dont know the consensus params
var appVersion uint64 = 0
var appVersion = InitialAppVersion
if version > 0 {
cp, err := GetConsensusParams(ctx, c.app)
// if the consensus params are not found, we set the app version to 0
Expand Down Expand Up @@ -281,21 +284,17 @@ func (c *consensus[T]) InitChain(ctx context.Context, req *abciproto.InitChainRe
// store chainID to be used later on in execution
c.chainID = req.ChainId

// TODO: check if we need to load the config from genesis.json or config.toml
// note the app version is not read from genesis
// user can update InitialAppVersion to that value if needed
// from height 1, we will query the app for the version

c.initialHeight = uint64(req.InitialHeight)
if c.initialHeight == 0 { // If initial height is 0, set it to 1
c.initialHeight = 1
}

if req.ConsensusParams != nil {
ctx = context.WithValue(ctx, corecontext.CometParamsInitInfoKey, &consensustypes.MsgUpdateParams{
Block: req.ConsensusParams.Block,
Evidence: req.ConsensusParams.Evidence,
Validator: req.ConsensusParams.Validator,
Abci: req.ConsensusParams.Abci,
Synchrony: req.ConsensusParams.Synchrony,
Feature: req.ConsensusParams.Feature,
})
ctx = context.WithValue(ctx, corecontext.CometParamsInitInfoKey, req.ConsensusParams)
}

ci, err := c.store.LastCommitID()
Expand Down
4 changes: 2 additions & 2 deletions simapp/v2/sim_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
v1 "github.com/cometbft/cometbft/api/cometbft/types/v1"
cmttypes "github.com/cometbft/cometbft/types"
"github.com/spf13/viper"
"github.com/stretchr/testify/require"
Expand All @@ -32,7 +33,6 @@ import (
"cosmossdk.io/server/v2/cometbft"
"cosmossdk.io/server/v2/streaming"
storev2 "cosmossdk.io/store/v2"
consensustypes "cosmossdk.io/x/consensus/types"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -372,7 +372,7 @@ func doChainInitWithGenesis[T Tx](
IsGenesis: true,
}

initialConsensusParams := &consensustypes.MsgUpdateParams{
initialConsensusParams := &v1.ConsensusParams{
Block: &cmtproto.BlockParams{
MaxBytes: 200000,
MaxGas: 100_000_000,
Expand Down
5 changes: 2 additions & 3 deletions tests/integration/v2/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
v1 "github.com/cometbft/cometbft/api/cometbft/types/v1"
cmtjson "github.com/cometbft/cometbft/libs/json"
cmttypes "github.com/cometbft/cometbft/types"
"github.com/stretchr/testify/require"
Expand All @@ -33,7 +34,6 @@ import (
"cosmossdk.io/store/v2/root"
bankkeeper "cosmossdk.io/x/bank/keeper"
banktypes "cosmossdk.io/x/bank/types"
consensustypes "cosmossdk.io/x/consensus/types"
txsigning "cosmossdk.io/x/tx/signing"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -276,8 +276,7 @@ func NewApp(
ctx := context.WithValue(
context.Background(),
corecontext.CometParamsInitInfoKey,
&consensustypes.MsgUpdateParams{
Authority: "consensus",
&v1.ConsensusParams{
Block: DefaultConsensusParams.Block,
Evidence: DefaultConsensusParams.Evidence,
Validator: DefaultConsensusParams.Validator,
Expand Down
7 changes: 1 addition & 6 deletions x/consensus/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,7 @@ func (v versionModifier) SetAppVersion(ctx context.Context, version uint64) erro
}

func (v versionModifier) AppVersion(ctx context.Context) (uint64, error) {
params, err := v.Keeper.Params(ctx, nil)
if err != nil {
return 0, err
}

return params.Params.Version.GetApp(), nil
return v.Keeper.AppVersion(ctx)
}

func ProvideAppVersionModifier(k keeper.Keeper) server.VersionModifier {
Expand Down
22 changes: 18 additions & 4 deletions x/consensus/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
v1 "github.com/cometbft/cometbft/api/cometbft/types/v1"
cmttypes "github.com/cometbft/cometbft/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -57,18 +58,26 @@ func (k *Keeper) GetAuthority() string {

// InitGenesis initializes the initial state of the module
func (k *Keeper) InitGenesis(ctx context.Context) error {
value, ok := ctx.Value(corecontext.CometParamsInitInfoKey).(*types.MsgUpdateParams)
value, ok := ctx.Value(corecontext.CometParamsInitInfoKey).(*v1.ConsensusParams)
if !ok || value == nil {
// no error for appv1 and appv2
return nil
}

consensusParams, err := value.ToProtoConsensusParams()
if err != nil {
// validate the consensus params
// this avoids to duplicate the validation in here as well
if _, err := (&types.MsgUpdateParams{
Block: value.Block,
Evidence: value.Evidence,
Validator: value.Validator,
Abci: value.Abci,
Synchrony: value.Synchrony,
Feature: value.Feature,
}).ToProtoConsensusParams(); err != nil {
return err
}

nextParams, err := k.paramCheck(ctx, consensusParams)
nextParams, err := k.paramCheck(ctx, *value)
if err != nil {
return err
}
Expand Down Expand Up @@ -143,6 +152,11 @@ func (k Keeper) paramCheck(ctx context.Context, consensusParams cmtproto.Consens

nextParams := params.Update(&consensusParams)

// do not override the version params
// the consensusParams.Version always has DefaultConsensusParams version
// as app version is updated by x/upgrade during migrations
nextParams.Version = params.Version

if err := nextParams.ValidateBasic(); err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion x/consensus/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (msg MsgUpdateParams) ToProtoConsensusParams() (cmtproto.ConsensusParams, e
Validator: &cmtproto.ValidatorParams{
PubKeyTypes: msg.Validator.PubKeyTypes,
},
Version: cmttypes.DefaultConsensusParams().ToProto().Version, // Version is stored in x/upgrade
Version: cmttypes.DefaultConsensusParams().ToProto().Version, // Version is updated during migration by x/upgrade
Feature: msg.Feature,
Synchrony: msg.Synchrony,
}
Expand Down

0 comments on commit fedaa70

Please sign in to comment.