Skip to content

Commit

Permalink
refactor(gov)!: use collections for constitution and params state (#1…
Browse files Browse the repository at this point in the history
…6118)

Co-authored-by: unknown unknown <unknown@unknown>
  • Loading branch information
testinginprod and unknown unknown authored May 12, 2023
1 parent cd45ab2 commit f7418c6
Show file tree
Hide file tree
Showing 20 changed files with 69 additions and 101 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/genutil) [#15999](https://github.com/cosmos/cosmos-sdk/pull/15999) Genutil now takes the `GenesisTxHanlder` interface instead of deliverTx. The interface is implemented on baseapp
* (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16040) Remove unused aliases in math.go
* (x/gov) [#16106](https://github.com/cosmos/cosmos-sdk/pull/16106) Remove gRPC query methods from Keeper
* (x/gov) [#16118](https://github.com/cosmos/cosmos-sdk/pull/16118/) Use collections for constituion and params state management.

### Client Breaking Changes

* (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) `HistoricalInfoKey` now has a binary format.
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ func TestImportExportQueues(t *testing.T) {
assert.NilError(t, err)
proposalID2 := proposal2.Id

params, _ := s1.GovKeeper.GetParams(ctx)
params, err := s1.GovKeeper.Params.Get(ctx)
assert.NilError(t, err)
votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID2, addrs[0], params.MinDeposit)
assert.NilError(t, err)
assert.Assert(t, votingStarted)
Expand Down Expand Up @@ -145,7 +146,7 @@ func TestImportExportQueues(t *testing.T) {

ctx2 := s2.app.BaseApp.NewContext(false, cmtproto.Header{})

params, err = s2.GovKeeper.GetParams(ctx2)
params, err = s2.GovKeeper.Params.Get(ctx2)
assert.NilError(t, err)
// Jump the time forward past the DepositPeriod and VotingPeriod
ctx2 = ctx2.WithBlockTime(ctx2.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod))
Expand Down
4 changes: 2 additions & 2 deletions x/gov/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
return err
}

params, err := keeper.GetParams(ctx)
params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}
Expand Down Expand Up @@ -152,7 +152,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
// once the regular voting period expires again, the tally is repeated
// according to the regular proposal rules.
proposal.Expedited = false
params, err := keeper.GetParams(ctx)
params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions x/gov/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
require.False(t, inactiveQueue.Valid())
inactiveQueue.Close()

params, _ := suite.GovKeeper.GetParams(ctx)
params, _ := suite.GovKeeper.Params.Get(ctx)
newHeader = ctx.BlockHeader()
newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod)
ctx = ctx.WithBlockHeader(newHeader)
Expand Down Expand Up @@ -137,7 +137,7 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
require.NotNil(t, res)

newHeader = ctx.BlockHeader()
params, _ := suite.GovKeeper.GetParams(ctx)
params, _ := suite.GovKeeper.Params.Get(ctx)
newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(-1) * time.Second)
ctx = ctx.WithBlockHeader(newHeader)

Expand Down Expand Up @@ -280,7 +280,7 @@ func TestTickPassedVotingPeriod(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, res1)

params, _ := suite.GovKeeper.GetParams(ctx)
params, _ := suite.GovKeeper.Params.Get(ctx)
votingPeriod := params.VotingPeriod
if tc.expedited {
votingPeriod = params.ExpeditedVotingPeriod
Expand Down Expand Up @@ -389,7 +389,7 @@ func TestProposalPassedEndblocker(t *testing.T) {
require.NoError(t, err)

newHeader := ctx.BlockHeader()
params, _ := suite.GovKeeper.GetParams(ctx)
params, _ := suite.GovKeeper.Params.Get(ctx)
newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod)
ctx = ctx.WithBlockHeader(newHeader)

Expand Down Expand Up @@ -435,7 +435,7 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) {
err = suite.GovKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")
require.NoError(t, err)

params, _ := suite.GovKeeper.GetParams(ctx)
params, _ := suite.GovKeeper.Params.Get(ctx)
newHeader := ctx.BlockHeader()
newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod)
ctx = ctx.WithBlockHeader(newHeader)
Expand Down Expand Up @@ -485,7 +485,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
ctx := app.BaseApp.NewContext(false, cmtproto.Header{})
depositMultiplier := getDepositMultiplier(true)
addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 3, valTokens.Mul(math.NewInt(depositMultiplier)))
params, err := suite.GovKeeper.GetParams(ctx)
params, err := suite.GovKeeper.Params.Get(ctx)
require.NoError(t, err)

SortAddresses(addrs)
Expand Down
8 changes: 4 additions & 4 deletions x/gov/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k
panic(err)
}

err = k.SetParams(ctx, *data.Params)
err = k.Params.Set(ctx, *data.Params)
if err != nil {
panic(err)
}

err = k.SetConstitution(ctx, data.Constitution)
err = k.Constitution.Set(ctx, data.Constitution)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -76,12 +76,12 @@ func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) (*v1.GenesisState, error)
return nil, err
}

constitution, err := k.GetConstitution(ctx)
constitution, err := k.Constitution.Get(ctx)
if err != nil {
return nil, err
}

params, err := k.GetParams(ctx)
params, err := k.Params.Get(ctx)
if err != nil {
return nil, err
}
Expand Down
5 changes: 4 additions & 1 deletion x/gov/keeper/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/require"

"cosmossdk.io/math"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
cmttime "github.com/cometbft/cometbft/types/time"
Expand Down Expand Up @@ -102,7 +104,8 @@ func setupGovKeeper(t *testing.T) (
govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too.
govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler)
govKeeper.SetLegacyRouter(govRouter)
govKeeper.SetParams(ctx, v1.DefaultParams())
err := govKeeper.Params.Set(ctx, v1.DefaultParams())
require.NoError(t, err)

// Register all handlers for the MegServiceRouter.
msr.SetInterfaceRegistry(encCfg.InterfaceRegistry)
Expand Down
21 changes: 0 additions & 21 deletions x/gov/keeper/constitution.go

This file was deleted.

4 changes: 2 additions & 2 deletions x/gov/keeper/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito

// Check if deposit has provided sufficient total funds to transition the proposal into the voting period
activatedVotingPeriod := false
params, err := keeper.GetParams(ctx)
params, err := keeper.Params.Get(ctx)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -344,7 +344,7 @@ func (keeper Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uin
// required at the time of proposal submission. This threshold amount is determined by
// the deposit parameters. Returns nil on success, error otherwise.
func (keeper Keeper) validateInitialDeposit(ctx context.Context, initialDeposit sdk.Coins, expedited bool) error {
params, err := keeper.GetParams(ctx)
params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions x/gov/keeper/deposit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func TestValidateInitialDeposit(t *testing.T) {
}
params.MinInitialDepositRatio = sdkmath.LegacyNewDec(tc.minInitialDepositPercent).Quo(sdkmath.LegacyNewDec(100)).String()

govKeeper.SetParams(ctx, params)
govKeeper.Params.Set(ctx, params)

err := govKeeper.ValidateInitialDeposit(ctx, tc.initialDeposit, tc.expedited)

Expand Down Expand Up @@ -325,7 +325,7 @@ func TestChargeDeposit(t *testing.T) {
params.ProposalCancelDest = authtypes.NewModuleAddress(disttypes.ModuleName).String()
}

err := govKeeper.SetParams(ctx, params)
err := govKeeper.Params.Set(ctx, params)
require.NoError(t, err)

tp := TestProposal
Expand Down
4 changes: 2 additions & 2 deletions x/gov/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewQueryServer(k Keeper) v1.QueryServer {
}

func (q queryServer) Constitution(ctx context.Context, _ *v1.QueryConstitutionRequest) (*v1.QueryConstitutionResponse, error) {
constitution, err := q.k.GetConstitution(ctx)
constitution, err := q.k.Constitution.Get(ctx)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func (q queryServer) Params(ctx context.Context, req *v1.QueryParamsRequest) (*v
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

params, err := q.k.GetParams(ctx)
params, err := q.k.Params.Get(ctx)
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestHooks(t *testing.T) {
require.NoError(t, err)
require.True(t, govHooksReceiver.AfterProposalSubmissionValid)

params, _ := govKeeper.GetParams(ctx)
params, _ := govKeeper.Params.Get(ctx)
newHeader := ctx.BlockHeader()
newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(1) * time.Second)
ctx = ctx.WithBlockHeader(newHeader)
Expand Down
17 changes: 16 additions & 1 deletion x/gov/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"time"

"cosmossdk.io/collections"

corestoretypes "cosmossdk.io/core/store"
"cosmossdk.io/errors"
"cosmossdk.io/log"
Expand Down Expand Up @@ -47,6 +49,10 @@ type Keeper struct {
// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
authority string

Schema collections.Schema
Constitution collections.Item[string]
Params collections.Item[v1.Params]
}

// GetAuthority returns the x/gov module's authority.
Expand Down Expand Up @@ -80,7 +86,8 @@ func NewKeeper(
config.MaxMetadataLen = types.DefaultConfig().MaxMetadataLen
}

return &Keeper{
sb := collections.NewSchemaBuilder(storeService)
k := &Keeper{
storeService: storeService,
authKeeper: authKeeper,
bankKeeper: bankKeeper,
Expand All @@ -90,7 +97,15 @@ func NewKeeper(
router: router,
config: config,
authority: authority,
Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue),
Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)),
}
schema, err := sb.Build()
if err != nil {
panic(err)
}
k.Schema = schema
return k
}

// Hooks gets the hooks for governance *Keeper {
Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (k msgServer) UpdateParams(goCtx context.Context, msg *v1.MsgUpdateParams)
}

ctx := sdk.UnwrapSDKContext(goCtx)
if err := k.SetParams(ctx, msg.Params); err != nil {
if err := k.Params.Set(ctx, msg.Params); err != nil {
return nil, err
}

Expand Down
18 changes: 9 additions & 9 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() {

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
initialDeposit := coins
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -319,7 +319,7 @@ func (suite *KeeperTestSuite) TestVoteReq() {
proposer := addrs[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -465,7 +465,7 @@ func (suite *KeeperTestSuite) TestVoteWeightedReq() {
proposer := simtestutil.AddTestAddrsIncremental(suite.bankKeeper, suite.stakingKeeper, suite.ctx, 1, sdkmath.NewInt(50000000))[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -711,7 +711,7 @@ func (suite *KeeperTestSuite) TestDepositReq() {
proposer := addrs[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -793,7 +793,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgSubmitProposal() {
proposer := simtestutil.AddTestAddrsIncremental(suite.bankKeeper, suite.stakingKeeper, suite.ctx, 1, sdkmath.NewInt(50000000))[0]
coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
initialDeposit := coins
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit

suite.acctKeeper.EXPECT().StringToBytes("").Return(nil, errors.New(emptyAddressError))
Expand Down Expand Up @@ -907,7 +907,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgVote() {
proposer := addrs[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -1043,7 +1043,7 @@ func (suite *KeeperTestSuite) TestLegacyVoteWeighted() {
proposer := addrs[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -1297,7 +1297,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() {
proposer := addrs[0]

coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(100)))
params, _ := suite.govKeeper.GetParams(suite.ctx)
params, _ := suite.govKeeper.Params.Get(suite.ctx)
minDeposit := params.MinDeposit
bankMsg := &banktypes.MsgSend{
FromAddress: govAcct.String(),
Expand Down Expand Up @@ -1699,7 +1699,7 @@ func (suite *KeeperTestSuite) TestSubmitProposal_InitialDeposit() {
params := v1.DefaultParams()
params.MinDeposit = tc.minDeposit
params.MinInitialDepositRatio = tc.minInitialDepositRatio.String()
govKeeper.SetParams(ctx, params)
govKeeper.Params.Set(ctx, params)

msg, err := v1.NewMsgSubmitProposal(TestProposal, tc.initialDeposit, address.String(), "test", "Proposal", "description of proposal", false)
suite.Require().NoError(err)
Expand Down
Loading

0 comments on commit f7418c6

Please sign in to comment.