From dddd5294d304359f6287281e0191679c843c1b8d Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Thu, 11 May 2023 18:29:00 +0200 Subject: [PATCH 01/31] move constitution to collections --- x/gov/genesis.go | 4 ++-- x/gov/keeper/constitution.go | 21 --------------------- x/gov/keeper/grpc_query.go | 2 +- x/gov/keeper/keeper.go | 14 +++++++++++++- x/gov/types/keys.go | 3 ++- 5 files changed, 18 insertions(+), 26 deletions(-) delete mode 100644 x/gov/keeper/constitution.go diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 0c0c156f6910..29a74a81ac3a 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -21,7 +21,7 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k panic(err) } - err = k.SetConstitution(ctx, data.Constitution) + err = k.Constitution.Set(ctx, data.Constitution) if err != nil { panic(err) } @@ -76,7 +76,7 @@ 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 } diff --git a/x/gov/keeper/constitution.go b/x/gov/keeper/constitution.go deleted file mode 100644 index 0f791a6f1821..000000000000 --- a/x/gov/keeper/constitution.go +++ /dev/null @@ -1,21 +0,0 @@ -package keeper - -import ( - "context" - - "github.com/cosmos/cosmos-sdk/x/gov/types" -) - -// GetConstitution gets the chain's constitution. -func (keeper Keeper) GetConstitution(ctx context.Context) (string, error) { - store := keeper.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.KeyConstitution) - - return string(bz), err -} - -// GetConstitution sets the chain's constitution. -func (keeper Keeper) SetConstitution(ctx context.Context, constitution string) error { - store := keeper.storeService.OpenKVStore(ctx) - return store.Set(types.KeyConstitution, []byte(constitution)) -} diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 934acc2611e6..9d5e60cda6f5 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -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 } diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 26c018ee7e45..a1fd7fb66e56 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "cosmossdk.io/collections" "fmt" "time" @@ -47,6 +48,9 @@ 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] } // GetAuthority returns the x/gov module's authority. @@ -80,7 +84,8 @@ func NewKeeper( config.MaxMetadataLen = types.DefaultConfig().MaxMetadataLen } - return &Keeper{ + sb := collections.NewSchemaBuilder(storeService) + k := &Keeper{ storeService: storeService, authKeeper: authKeeper, bankKeeper: bankKeeper, @@ -90,7 +95,14 @@ func NewKeeper( router: router, config: config, authority: authority, + Constitution: collections.NewItem(sb, types.KeyConstitution, "constitution", collections.StringValue), + } + schema, err := sb.Build() + if err != nil { + panic(err) } + k.Schema = schema + return k } // Hooks gets the hooks for governance *Keeper { diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 4725685fbf1c..01e842e541f1 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -1,6 +1,7 @@ package types import ( + "cosmossdk.io/collections" "encoding/binary" "time" @@ -53,7 +54,7 @@ var ( ParamsKey = []byte{0x30} // KeyConstitution is the key string used to store the chain's constitution - KeyConstitution = []byte("constitution") + KeyConstitution = collections.NewPrefix("constitution") ) var lenTime = len(sdk.FormatTimeBytes(time.Now())) From 3d8306ba38f483c3d71144480c1702bd8ddb2793 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Thu, 11 May 2023 18:38:03 +0200 Subject: [PATCH 02/31] move params to collections --- x/gov/abci.go | 4 ++-- x/gov/abci_test.go | 12 +++++----- x/gov/genesis.go | 4 ++-- x/gov/keeper/common_test.go | 5 ++++- x/gov/keeper/deposit.go | 4 ++-- x/gov/keeper/deposit_test.go | 4 ++-- x/gov/keeper/grpc_query.go | 2 +- x/gov/keeper/hooks_test.go | 2 +- x/gov/keeper/keeper.go | 5 ++++- x/gov/keeper/msg_server.go | 2 +- x/gov/keeper/msg_server_test.go | 18 +++++++-------- x/gov/keeper/params.go | 34 ----------------------------- x/gov/keeper/proposal.go | 6 ++--- x/gov/keeper/tally.go | 2 +- x/gov/simulation/operations.go | 4 ++-- x/gov/simulation/operations_test.go | 8 +++---- x/gov/types/keys.go | 5 +++-- 17 files changed, 47 insertions(+), 74 deletions(-) delete mode 100644 x/gov/keeper/params.go diff --git a/x/gov/abci.go b/x/gov/abci.go index 9d4085d6933d..eb5e4e9c6779 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -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 } @@ -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 } diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 3c99ab77e9b3..095d12c34508 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -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) @@ -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) @@ -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 @@ -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) @@ -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) @@ -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) diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 29a74a81ac3a..f8cf816f8ba3 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -16,7 +16,7 @@ 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) } @@ -81,7 +81,7 @@ func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) (*v1.GenesisState, error) return nil, err } - params, err := k.GetParams(ctx) + params, err := k.Params.Get(ctx) if err != nil { return nil, err } diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index 242e459a74cb..c6738cc66ff5 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -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" @@ -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) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 7cfebba67efb..d523931fc966 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -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 } @@ -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 } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 3b9ce3cf5890..1ef0818f12f7 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -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) @@ -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 diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 9d5e60cda6f5..ae0b9e57ed61 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -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 } diff --git a/x/gov/keeper/hooks_test.go b/x/gov/keeper/hooks_test.go index 64290f5db3e6..29c5ccd29402 100644 --- a/x/gov/keeper/hooks_test.go +++ b/x/gov/keeper/hooks_test.go @@ -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) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index a1fd7fb66e56..ef4ae7328880 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -2,10 +2,11 @@ package keeper import ( "context" - "cosmossdk.io/collections" "fmt" "time" + "cosmossdk.io/collections" + corestoretypes "cosmossdk.io/core/store" "cosmossdk.io/errors" "cosmossdk.io/log" @@ -51,6 +52,7 @@ type Keeper struct { Schema collections.Schema Constitution collections.Item[string] + Params collections.Item[v1.Params] } // GetAuthority returns the x/gov module's authority. @@ -96,6 +98,7 @@ func NewKeeper( config: config, authority: authority, Constitution: collections.NewItem(sb, types.KeyConstitution, "constitution", collections.StringValue), + Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), } schema, err := sb.Build() if err != nil { diff --git a/x/gov/keeper/msg_server.go b/x/gov/keeper/msg_server.go index d37dbb5339d9..39ca9971e818 100644 --- a/x/gov/keeper/msg_server.go +++ b/x/gov/keeper/msg_server.go @@ -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 } diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index e44f4f60be57..9769636a660e 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -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(), @@ -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(), @@ -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(), @@ -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(), @@ -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)) @@ -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(), @@ -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(), @@ -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(), @@ -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) diff --git a/x/gov/keeper/params.go b/x/gov/keeper/params.go deleted file mode 100644 index 4e81c8ea58d3..000000000000 --- a/x/gov/keeper/params.go +++ /dev/null @@ -1,34 +0,0 @@ -package keeper - -import ( - "context" - - "github.com/cosmos/cosmos-sdk/x/gov/types" - v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" -) - -// SetParams sets the gov module's parameters. -// CONTRACT: This method performs no validation of the parameters. -func (k Keeper) SetParams(ctx context.Context, params v1.Params) error { - store := k.storeService.OpenKVStore(ctx) - bz, err := k.cdc.Marshal(¶ms) - if err != nil { - return err - } - return store.Set(types.ParamsKey, bz) -} - -// GetParams gets the gov module's parameters. -func (k Keeper) GetParams(ctx context.Context) (params v1.Params, err error) { - store := k.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.ParamsKey) - if err != nil { - return params, err - } - if bz == nil { - return params, nil - } - - err = k.cdc.Unmarshal(bz, ¶ms) - return params, err -} diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index c0163661e707..7548429d978e 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -89,7 +89,7 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met return v1.Proposal{}, err } - params, err := keeper.GetParams(ctx) + params, err := keeper.Params.Get(ctx) if err != nil { return v1.Proposal{}, err } @@ -151,7 +151,7 @@ func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, prop // burn the (deposits * proposal_cancel_rate) amount or sent to cancellation destination address. // and deposits * (1 - proposal_cancel_rate) will be sent to depositors. - params, err := keeper.GetParams(ctx) + params, err := keeper.Params.Get(ctx) if err != nil { return err } @@ -370,7 +370,7 @@ func (keeper Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Propo startTime := sdkCtx.BlockHeader().Time proposal.VotingStartTime = &startTime var votingPeriod *time.Duration - params, err := keeper.GetParams(ctx) + params, err := keeper.Params.Get(ctx) if err != nil { return err } diff --git a/x/gov/keeper/tally.go b/x/gov/keeper/tally.go index ed1084e7038b..e6910c4794e8 100644 --- a/x/gov/keeper/tally.go +++ b/x/gov/keeper/tally.go @@ -99,7 +99,7 @@ func (keeper Keeper) Tally(ctx context.Context, proposal v1.Proposal) (passes, b totalVotingPower = totalVotingPower.Add(votingPower) } - params, err := keeper.GetParams(ctx) + params, err := keeper.Params.Get(ctx) if err != nil { return false, false, tallyResults, err } diff --git a/x/gov/simulation/operations.go b/x/gov/simulation/operations.go index 43ffaa313b9f..33d7d454df13 100644 --- a/x/gov/simulation/operations.go +++ b/x/gov/simulation/operations.go @@ -291,7 +291,7 @@ func simulateMsgSubmitProposal( // didntVote := whoVotes[numVotes:] whoVotes = whoVotes[:numVotes] - params, _ := k.GetParams(ctx) + params, _ := k.Params.Get(ctx) votingPeriod := params.VotingPeriod fops := make([]simtypes.FutureOperation, numVotes+1) @@ -557,7 +557,7 @@ func randomDeposit( return nil, true, nil // skip } - params, _ := k.GetParams(ctx) + params, _ := k.Params.Get(ctx) minDeposit := params.MinDeposit if expedited { minDeposit = params.ExpeditedMinDeposit diff --git a/x/gov/simulation/operations_test.go b/x/gov/simulation/operations_test.go index c545c8be8c59..5aeacb5b8f17 100644 --- a/x/gov/simulation/operations_test.go +++ b/x/gov/simulation/operations_test.go @@ -212,7 +212,7 @@ func TestSimulateMsgCancelProposal(t *testing.T) { require.NoError(t, err) submitTime := ctx.BlockHeader().Time - params, _ := suite.GovKeeper.GetParams(ctx) + params, _ := suite.GovKeeper.Params.Get(ctx) depositPeriod := params.MaxDepositPeriod proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "title", "summary", proposer, false) @@ -257,7 +257,7 @@ func TestSimulateMsgDeposit(t *testing.T) { require.NoError(t, err) submitTime := ctx.BlockHeader().Time - params, _ := suite.GovKeeper.GetParams(ctx) + params, _ := suite.GovKeeper.Params.Get(ctx) depositPeriod := params.MaxDepositPeriod proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), false) @@ -304,7 +304,7 @@ func TestSimulateMsgVote(t *testing.T) { require.NoError(t, err) submitTime := ctx.BlockHeader().Time - params, _ := suite.GovKeeper.GetParams(ctx) + params, _ := suite.GovKeeper.Params.Get(ctx) depositPeriod := params.MaxDepositPeriod proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), false) @@ -348,7 +348,7 @@ func TestSimulateMsgVoteWeighted(t *testing.T) { contentMsg, err := v1.NewLegacyContent(v1beta1.NewTextProposal("Test", "description"), govAcc) require.NoError(t, err) submitTime := ctx.BlockHeader().Time - params, _ := suite.GovKeeper.GetParams(ctx) + params, _ := suite.GovKeeper.Params.Get(ctx) depositPeriod := params.MaxDepositPeriod proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "test", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), false) diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 01e842e541f1..799e2f481bd6 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -1,10 +1,11 @@ package types import ( - "cosmossdk.io/collections" "encoding/binary" "time" + "cosmossdk.io/collections" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" "github.com/cosmos/cosmos-sdk/types/kv" @@ -51,7 +52,7 @@ var ( VotesKeyPrefix = []byte{0x20} // ParamsKey is the key to query all gov params - ParamsKey = []byte{0x30} + ParamsKey = collections.NewPrefix(30) // KeyConstitution is the key string used to store the chain's constitution KeyConstitution = collections.NewPrefix("constitution") From e559d413e42becfc793f1ccfbeba19b24ff0ef0f Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Thu, 11 May 2023 18:40:04 +0200 Subject: [PATCH 03/31] chore: CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8db37146400..b4a5cdc57543 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -192,6 +192,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. From 41d8e73c9d8dbecf6cffd5ebee80cb50afa241db Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Thu, 11 May 2023 18:56:00 +0200 Subject: [PATCH 04/31] address PR reviews --- x/gov/keeper/keeper.go | 2 +- x/gov/types/keys.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index ef4ae7328880..60f67e642fb1 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -97,7 +97,7 @@ func NewKeeper( router: router, config: config, authority: authority, - Constitution: collections.NewItem(sb, types.KeyConstitution, "constitution", collections.StringValue), + 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() diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 799e2f481bd6..a1a3ab3c52c3 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -54,8 +54,8 @@ var ( // ParamsKey is the key to query all gov params ParamsKey = collections.NewPrefix(30) - // KeyConstitution is the key string used to store the chain's constitution - KeyConstitution = collections.NewPrefix("constitution") + // ConstitutionKey is the key string used to store the chain's constitution + ConstitutionKey = collections.NewPrefix("constitution") ) var lenTime = len(sdk.FormatTimeBytes(time.Now())) From 6bb70e6dadc1bf08e7285746d059bad00d617c3f Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 09:00:34 +0200 Subject: [PATCH 05/31] be consistent --- x/gov/types/keys.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index a1a3ab3c52c3..1aedd4ab209b 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -55,7 +55,7 @@ var ( ParamsKey = collections.NewPrefix(30) // ConstitutionKey is the key string used to store the chain's constitution - ConstitutionKey = collections.NewPrefix("constitution") + ConstitutionKey = collections.NewPrefix(31) ) var lenTime = len(sdk.FormatTimeBytes(time.Now())) From c503920000e014f07328e33948b8681a388122db Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 09:02:32 +0200 Subject: [PATCH 06/31] more test fixes --- tests/integration/gov/genesis_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index 65621bab3330..e27a22c6754b 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -2,6 +2,7 @@ package gov_test import ( "encoding/json" + "github.com/stretchr/testify/require" "testing" abci "github.com/cometbft/cometbft/abci/types" @@ -88,7 +89,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) + require.NoError(t, err) votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID2, addrs[0], params.MinDeposit) assert.NilError(t, err) assert.Assert(t, votingStarted) @@ -145,7 +147,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)) From 0cd34ed80a00d7c15682ac1c1fd7d7bdb35b5638 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 09:22:42 +0200 Subject: [PATCH 07/31] use assert and not require --- tests/integration/gov/genesis_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index e27a22c6754b..68821ba65f74 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -2,7 +2,6 @@ package gov_test import ( "encoding/json" - "github.com/stretchr/testify/require" "testing" abci "github.com/cometbft/cometbft/abci/types" @@ -90,7 +89,7 @@ func TestImportExportQueues(t *testing.T) { proposalID2 := proposal2.Id params, err := s1.GovKeeper.Params.Get(ctx) - require.NoError(t, err) + assert.NilError(t, err) votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID2, addrs[0], params.MinDeposit) assert.NilError(t, err) assert.Assert(t, votingStarted) From 915618b5ac9dd8cf57d0cfefa9e10dfbe38d0598 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 09:38:59 +0200 Subject: [PATCH 08/31] fix prefixes numbers --- x/gov/types/keys.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 1aedd4ab209b..239cad98deb6 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -52,10 +52,10 @@ var ( VotesKeyPrefix = []byte{0x20} // ParamsKey is the key to query all gov params - ParamsKey = collections.NewPrefix(30) + ParamsKey = collections.NewPrefix(48) // ConstitutionKey is the key string used to store the chain's constitution - ConstitutionKey = collections.NewPrefix(31) + ConstitutionKey = collections.NewPrefix(49) ) var lenTime = len(sdk.FormatTimeBytes(time.Now())) From d291e994538d079ce91407342f4ffb9bc3823835 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 10:22:38 +0200 Subject: [PATCH 09/31] move deposit to collections1 --- x/gov/keeper/deposit.go | 103 ++++++++++------------------------- x/gov/keeper/deposit_test.go | 8 ++- x/gov/keeper/keeper.go | 2 + x/gov/types/keys.go | 2 +- 4 files changed, 38 insertions(+), 77 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index d523931fc966..fe2adaff1220 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -2,6 +2,8 @@ package keeper import ( "context" + "cosmossdk.io/collections" + stderr "errors" "fmt" "cosmossdk.io/errors" @@ -37,35 +39,18 @@ func (keeper Keeper) GetDeposit(ctx context.Context, proposalID uint64, deposito // SetDeposit sets a Deposit to the gov store func (keeper Keeper) SetDeposit(ctx context.Context, deposit v1.Deposit) error { - store := keeper.storeService.OpenKVStore(ctx) - bz, err := keeper.cdc.Marshal(&deposit) - if err != nil { - return err - } - depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor) if err != nil { return err } - - return store.Set(types.DepositKey(deposit.ProposalId, depositor), bz) -} - -// GetAllDeposits returns all the deposits from the store -func (keeper Keeper) GetAllDeposits(ctx context.Context) (deposits v1.Deposits, err error) { - err = keeper.IterateAllDeposits(ctx, func(deposit v1.Deposit) error { - deposits = append(deposits, &deposit) - return nil - }) - - return + return keeper.Deposits.Set(ctx, collections.Join(deposit.ProposalId, sdk.AccAddress(depositor)), deposit) } // GetDeposits returns all the deposits of a proposal func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposits v1.Deposits, err error) { - err = keeper.IterateDeposits(ctx, proposalID, func(deposit v1.Deposit) error { + err = keeper.IterateDeposits(ctx, proposalID, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { deposits = append(deposits, &deposit) - return nil + return false }) return @@ -73,27 +58,17 @@ func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposi // DeleteAndBurnDeposits deletes and burns all the deposits on a specific proposal. func (keeper Keeper) DeleteAndBurnDeposits(ctx context.Context, proposalID uint64) error { - store := keeper.storeService.OpenKVStore(ctx) - - err := keeper.IterateDeposits(ctx, proposalID, func(deposit v1.Deposit) error { - err := keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, deposit.Amount) - if err != nil { - return err - } - - depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor) - if err != nil { - return err - } - - err = store.Delete(types.DepositKey(proposalID, depositor)) - if err != nil { - return err - } - return nil + coinsToBurn := sdk.NewCoins() + err := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + coinsToBurn = coinsToBurn.Add(deposit.Amount...) + _ = keeper.Deposits.Remove(ctx, key) // can't error, otherwise the iterator wouldn't report it + return false }) + if err != nil { + return err + } - return err + return keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, coinsToBurn) } // IterateAllDeposits iterates over all the stored deposits and performs a callback function. @@ -123,28 +98,12 @@ func (keeper Keeper) IterateAllDeposits(ctx context.Context, cb func(deposit v1. } // IterateDeposits iterates over all the proposals deposits and performs a callback function -func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(deposit v1.Deposit) error) error { - store := keeper.storeService.OpenKVStore(ctx) - iterator := storetypes.KVStorePrefixIterator(runtime.KVStoreAdapter(store), types.DepositsKey(proposalID)) - - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - var deposit v1.Deposit - err := keeper.cdc.Unmarshal(iterator.Value(), &deposit) - if err != nil { - return err - } - - err = cb(deposit) - // exit early without error if cb returns ErrStopIterating - if errors.IsOf(err, errors.ErrStopIterating) { - return nil - } else if err != nil { - return err - } +func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) bool) error { + pair := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID) + err := keeper.Deposits.Walk(ctx, pair, cb) + if err != nil && !stderr.Is(err, collections.ErrInvalidIterator) { + return err } - return nil } @@ -317,25 +276,19 @@ func (keeper Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destA // RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal. func (keeper Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uint64) error { - store := keeper.storeService.OpenKVStore(ctx) - - err := keeper.IterateDeposits(ctx, proposalID, func(deposit v1.Deposit) error { - depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor) - if err != nil { - return err - } - + var err error + iterErr := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + depositor := key.K2() err = keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount) if err != nil { - return err - } - - err = store.Delete(types.DepositKey(proposalID, depositor)) - if err != nil { - return err + return true } - return nil + _ = keeper.Deposits.Remove(ctx, key) // cannot error + return false }) + if iterErr != nil { + return iterErr + } return err } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 1ef0818f12f7..b890bd337efd 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "cosmossdk.io/collections" "fmt" "testing" @@ -120,7 +121,12 @@ func TestDeposits(t *testing.T) { // Test deposit iterator // NOTE order of deposits is determined by the addresses - deposits, _ := govKeeper.GetAllDeposits(ctx) + var deposits v1.Deposits + err = govKeeper.Deposits.Walk(ctx, nil, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + deposits = append(deposits, &deposit) + return false + }) + require.NoError(t, err) require.Len(t, deposits, 2) propDeposits, _ := govKeeper.GetDeposits(ctx, proposalID) require.Equal(t, deposits, propDeposits) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 60f67e642fb1..bfcb97505bc7 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -53,6 +53,7 @@ type Keeper struct { Schema collections.Schema Constitution collections.Item[string] Params collections.Item[v1.Params] + Deposits collections.Map[collections.Pair[uint64, sdk.AccAddress], v1.Deposit] } // GetAuthority returns the x/gov module's authority. @@ -99,6 +100,7 @@ func NewKeeper( authority: authority, Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), + Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), } schema, err := sb.Build() if err != nil { diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 239cad98deb6..c7ea67b66d8b 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -47,7 +47,7 @@ var ( ProposalIDKey = []byte{0x03} VotingPeriodProposalKeyPrefix = []byte{0x04} - DepositsKeyPrefix = []byte{0x10} + DepositsKeyPrefix = collections.NewPrefix(16) VotesKeyPrefix = []byte{0x20} From 69323234ea16d2e3c23da506c98b355515c17c9e Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 10:52:44 +0200 Subject: [PATCH 10/31] move another part of deposits to use collections --- x/gov/keeper/deposit.go | 31 +++---------------------------- x/gov/keeper/deposit_test.go | 17 ++++++++--------- x/gov/keeper/grpc_query.go | 22 +++++++--------------- x/gov/keeper/proposal.go | 3 ++- x/gov/migrations/v2/store_test.go | 10 +++++++++- x/gov/types/keys.go | 10 ---------- x/gov/types/keys_test.go | 11 ----------- 7 files changed, 29 insertions(+), 75 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index fe2adaff1220..da9663033817 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -17,26 +17,6 @@ import ( v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) -// GetDeposit gets the deposit of a specific depositor on a specific proposal -func (keeper Keeper) GetDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress) (deposit v1.Deposit, err error) { - store := keeper.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.DepositKey(proposalID, depositorAddr)) - if err != nil { - return deposit, err - } - - if bz == nil { - return deposit, types.ErrDepositNotFound - } - - err = keeper.cdc.Unmarshal(bz, &deposit) - if err != nil { - return deposit, err - } - - return deposit, nil -} - // SetDeposit sets a Deposit to the gov store func (keeper Keeper) SetDeposit(ctx context.Context, deposit v1.Deposit) error { depositor, err := keeper.authKeeper.StringToBytes(deposit.Depositor) @@ -155,12 +135,12 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito } // Add or update deposit object - deposit, err := keeper.GetDeposit(ctx, proposalID, depositorAddr) + deposit, err := keeper.Deposits.Get(ctx, collections.Join(proposalID, depositorAddr)) switch { case err == nil: // deposit exists deposit.Amount = sdk.NewCoins(deposit.Amount...).Add(depositAmount...) - case errors.IsOf(err, types.ErrDepositNotFound): + case errors.IsOf(err, collections.ErrNotFound): // deposit doesn't exist deposit = v1.NewDeposit(proposalID, depositorAddr, depositAmount) default: @@ -192,7 +172,6 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito // send to a destAddress if defined or burn otherwise. // Remaining funds are send back to the depositor. func (keeper Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destAddress, proposalCancelRate string) error { - store := keeper.storeService.OpenKVStore(ctx) rate := sdkmath.LegacyMustNewDecFromStr(proposalCancelRate) var cancellationCharges sdk.Coins @@ -234,11 +213,7 @@ func (keeper Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destA return err } } - - err = store.Delete(types.DepositKey(deposit.ProposalId, depositerAddress)) - if err != nil { - return err - } + return keeper.Deposits.Remove(ctx, collections.Join(deposit.ProposalId, sdk.AccAddress(depositerAddress))) } // burn the cancellation fee or sent the cancellation charges to destination address. diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index b890bd337efd..6e27a5607241 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -13,7 +13,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types" - "github.com/cosmos/cosmos-sdk/x/gov/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) @@ -69,8 +68,8 @@ func TestDeposits(t *testing.T) { require.True(t, sdk.NewCoins(proposal.TotalDeposit...).Equal(sdk.NewCoins())) // Check no deposits at beginning - _, err = govKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) - require.ErrorIs(t, err, types.ErrDepositNotFound) + _, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[1])) + require.ErrorIs(t, err, collections.ErrNotFound) proposal, err = govKeeper.GetProposal(ctx, proposalID) require.Nil(t, err) require.Nil(t, proposal.VotingStartTime) @@ -79,7 +78,7 @@ func TestDeposits(t *testing.T) { votingStarted, err := govKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fourStake) require.NoError(t, err) require.False(t, votingStarted) - deposit, err := govKeeper.GetDeposit(ctx, proposalID, TestAddrs[0]) + deposit, err := govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[0])) require.Nil(t, err) require.Equal(t, fourStake, sdk.NewCoins(deposit.Amount...)) require.Equal(t, TestAddrs[0].String(), deposit.Depositor) @@ -92,7 +91,7 @@ func TestDeposits(t *testing.T) { votingStarted, err = govKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fiveStake) require.NoError(t, err) require.False(t, votingStarted) - deposit, err = govKeeper.GetDeposit(ctx, proposalID, TestAddrs[0]) + deposit, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[0])) require.Nil(t, err) require.Equal(t, fourStake.Add(fiveStake...), sdk.NewCoins(deposit.Amount...)) require.Equal(t, TestAddrs[0].String(), deposit.Depositor) @@ -105,7 +104,7 @@ func TestDeposits(t *testing.T) { votingStarted, err = govKeeper.AddDeposit(ctx, proposalID, TestAddrs[1], fourStake) require.NoError(t, err) require.True(t, votingStarted) - deposit, err = govKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) + deposit, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[1])) require.Nil(t, err) require.Equal(t, TestAddrs[1].String(), deposit.Depositor) require.Equal(t, fourStake, sdk.NewCoins(deposit.Amount...)) @@ -136,12 +135,12 @@ func TestDeposits(t *testing.T) { require.Equal(t, fourStake, sdk.NewCoins(deposits[1].Amount...)) // Test Refund Deposits - deposit, err = govKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) + deposit, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[1])) require.Nil(t, err) require.Equal(t, fourStake, sdk.NewCoins(deposit.Amount...)) govKeeper.RefundAndDeleteDeposits(ctx, proposalID) - deposit, err = govKeeper.GetDeposit(ctx, proposalID, TestAddrs[1]) - require.ErrorIs(t, err, types.ErrDepositNotFound) + deposit, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[1])) + require.ErrorIs(t, err, collections.ErrNotFound) require.Equal(t, addr0Initial, bankKeeper.GetAllBalances(ctx, TestAddrs[0])) require.Equal(t, addr1Initial, bankKeeper.GetAllBalances(ctx, TestAddrs[1])) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index ae0b9e57ed61..d96e21b84f6d 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "cosmossdk.io/collections" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -91,9 +92,9 @@ func (q queryServer) Proposals(ctx context.Context, req *v1.QueryProposalsReques if err != nil { return nil, err } - _, err = q.k.GetDeposit(ctx, p.Id, depositor) + has, err := q.k.Deposits.Has(ctx, collections.Join(p.Id, sdk.AccAddress(depositor))) // if no error, deposit found, matchDepositor = true - matchDepositor = err == nil + matchDepositor = err == nil && has } if matchVoter && matchDepositor && matchStatus { @@ -225,7 +226,7 @@ func (q queryServer) Deposit(ctx context.Context, req *v1.QueryDepositRequest) ( if err != nil { return nil, err } - deposit, err := q.k.GetDeposit(ctx, req.ProposalId, depositor) + deposit, err := q.k.Deposits.Get(ctx, collections.Join(req.ProposalId, sdk.AccAddress(depositor))) if err != nil { if errors.IsOf(err, types.ErrDepositNotFound) { return nil, status.Errorf(codes.InvalidArgument, @@ -248,19 +249,10 @@ func (q queryServer) Deposits(ctx context.Context, req *v1.QueryDepositsRequest) } var deposits []*v1.Deposit - - store := q.k.storeService.OpenKVStore(ctx) - depositStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.DepositsKey(req.ProposalId)) - - pageRes, err := query.Paginate(depositStore, req.Pagination, func(key, value []byte) error { - var deposit v1.Deposit - if err := q.k.cdc.Unmarshal(value, &deposit); err != nil { - return err - } - + _, pageRes, err := query.CollectionFilteredPaginate(ctx, q.k.Deposits, req.Pagination, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) { deposits = append(deposits, &deposit) - return nil - }) + return false, nil // we don't include results as they're being appended to the slice above. + }, query.WithCollectionPaginationPairPrefix[uint64, sdk.AccAddress](req.ProposalId)) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 7548429d978e..8509a58ee6e3 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "cosmossdk.io/collections" "errors" "fmt" "time" @@ -323,7 +324,7 @@ func (keeper Keeper) GetProposalsFiltered(ctx context.Context, params v1.QueryPr // match depositor (if supplied) if len(params.Depositor) > 0 { - _, err = keeper.GetDeposit(ctx, p.Id, params.Depositor) + _, err = keeper.Deposits.Get(ctx, collections.Join(p.Id, params.Depositor)) // if no error, deposit found, matchDepositor = true matchDepositor = err == nil } diff --git a/x/gov/migrations/v2/store_test.go b/x/gov/migrations/v2/store_test.go index 24cc9b416d6f..d2db23fc3c35 100644 --- a/x/gov/migrations/v2/store_test.go +++ b/x/gov/migrations/v2/store_test.go @@ -2,6 +2,8 @@ package v2_test import ( "bytes" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" "testing" "time" @@ -64,7 +66,7 @@ func TestMigrateStore(t *testing.T) { { "DepositKey", v1.DepositKey(proposalID, addr1), dummyValue, - types.DepositKey(proposalID, addr1), dummyValue, + depositKey(proposalID, addr1), dummyValue, }, { "VotesKeyPrefix", @@ -94,3 +96,9 @@ func TestMigrateStore(t *testing.T) { }) } } + +// depositKey key of a specific deposit from the store. +// NOTE(tip): legacy, eventually remove me. +func depositKey(proposalID uint64, depositorAddr sdk.AccAddress) []byte { + return append(append(types.DepositsKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...), address.MustLengthPrefix(depositorAddr.Bytes())...) +} diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index c7ea67b66d8b..5095cd5fa60f 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -102,16 +102,6 @@ func InactiveProposalQueueKey(proposalID uint64, endTime time.Time) []byte { return append(InactiveProposalByTimeKey(endTime), GetProposalIDBytes(proposalID)...) } -// DepositsKey gets the first part of the deposits key based on the proposalID -func DepositsKey(proposalID uint64) []byte { - return append(DepositsKeyPrefix, GetProposalIDBytes(proposalID)...) -} - -// DepositKey key of a specific deposit from the store -func DepositKey(proposalID uint64, depositorAddr sdk.AccAddress) []byte { - return append(DepositsKey(proposalID), address.MustLengthPrefix(depositorAddr.Bytes())...) -} - // VotesKey gets the first part of the votes key based on the proposalID func VotesKey(proposalID uint64) []byte { return append(VotesKeyPrefix, GetProposalIDBytes(proposalID)...) diff --git a/x/gov/types/keys_test.go b/x/gov/types/keys_test.go index 351863e9bfe4..689fa7a22f0a 100644 --- a/x/gov/types/keys_test.go +++ b/x/gov/types/keys_test.go @@ -36,17 +36,6 @@ func TestProposalKeys(t *testing.T) { require.Panics(t, func() { SplitInactiveProposalQueueKey([]byte("test")) }) } -func TestDepositKeys(t *testing.T) { - key := DepositsKey(2) - proposalID := SplitProposalKey(key) - require.Equal(t, int(proposalID), 2) - - key = DepositKey(2, addr) - proposalID, depositorAddr := SplitKeyDeposit(key) - require.Equal(t, int(proposalID), 2) - require.Equal(t, addr, depositorAddr) -} - func TestVoteKeys(t *testing.T) { key := VotesKey(2) proposalID := SplitProposalKey(key) From a9cc7baa88bbe393b853187544ba01836b4f1b88 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 11:06:39 +0200 Subject: [PATCH 11/31] add panic in genesis --- x/gov/genesis.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x/gov/genesis.go b/x/gov/genesis.go index f8cf816f8ba3..cc141346ffe1 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -34,7 +34,10 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k var totalDeposits sdk.Coins for _, deposit := range data.Deposits { - k.SetDeposit(ctx, *deposit) + err := k.SetDeposit(ctx, *deposit) + if err != nil { + panic(err) + } totalDeposits = totalDeposits.Add(deposit.Amount...) } From f29f12ed644724a43da3f567cf662e36b3fbb85f Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 11:12:51 +0200 Subject: [PATCH 12/31] remove unused func --- x/gov/keeper/invariants.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index 540a100e474c..74bf6836947f 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -13,13 +13,6 @@ func RegisterInvariants(ir sdk.InvariantRegistry, keeper *Keeper, bk types.BankK ir.RegisterRoute(types.ModuleName, "module-account", ModuleAccountInvariant(keeper, bk)) } -// AllInvariants runs all invariants of the governance module -func AllInvariants(keeper *Keeper, bk types.BankKeeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - return ModuleAccountInvariant(keeper, bk)(ctx) - } -} - // ModuleAccountInvariant checks that the module account coins reflects the sum of // deposit amounts held on store. func ModuleAccountInvariant(keeper *Keeper, bk types.BankKeeper) sdk.Invariant { From 4e53600f9d07bb5c25ac1d0904134870314487f6 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 11:22:19 +0200 Subject: [PATCH 13/31] more cleanups + format --- x/gov/keeper/deposit.go | 3 +- x/gov/keeper/deposit_test.go | 3 +- x/gov/keeper/grpc_query.go | 1 + x/gov/keeper/proposal.go | 3 +- x/gov/migrations/v2/store_test.go | 5 +- x/gov/module.go | 2 +- x/gov/simulation/decoder.go | 68 ------------------ x/gov/simulation/decoder_test.go | 103 ---------------------------- x/gov/simulation/operations_test.go | 1 + 9 files changed, 12 insertions(+), 177 deletions(-) delete mode 100644 x/gov/simulation/decoder.go delete mode 100644 x/gov/simulation/decoder_test.go diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index da9663033817..4ec69be599c8 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -2,10 +2,11 @@ package keeper import ( "context" - "cosmossdk.io/collections" stderr "errors" "fmt" + "cosmossdk.io/collections" + "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 6e27a5607241..62fb6e378a01 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -1,10 +1,11 @@ package keeper_test import ( - "cosmossdk.io/collections" "fmt" "testing" + "cosmossdk.io/collections" + sdkmath "cosmossdk.io/math" "github.com/stretchr/testify/require" diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index d96e21b84f6d..4e6ed873dfe0 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "cosmossdk.io/collections" "google.golang.org/grpc/codes" diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 8509a58ee6e3..dc1176237b27 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -2,11 +2,12 @@ package keeper import ( "context" - "cosmossdk.io/collections" "errors" "fmt" "time" + "cosmossdk.io/collections" + errorsmod "cosmossdk.io/errors" storetypes "cosmossdk.io/store/types" diff --git a/x/gov/migrations/v2/store_test.go b/x/gov/migrations/v2/store_test.go index d2db23fc3c35..004014543fdb 100644 --- a/x/gov/migrations/v2/store_test.go +++ b/x/gov/migrations/v2/store_test.go @@ -2,11 +2,12 @@ package v2_test import ( "bytes" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" "testing" "time" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + "cosmossdk.io/math" "github.com/stretchr/testify/require" diff --git a/x/gov/module.go b/x/gov/module.go index 5df9eb11c870..d148eecab77d 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -353,7 +353,7 @@ func (AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.Weight // RegisterStoreDecoder registers a decoder for gov module's types func (am AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) { - sdr[govtypes.StoreKey] = simulation.NewDecodeStore(am.cdc) + sdr[govtypes.StoreKey] = simtypes.NewStoreDecoderFuncFromCollectionsSchema(am.keeper.Schema) } // WeightedOperations returns the all the gov module operations with their respective weights. diff --git a/x/gov/simulation/decoder.go b/x/gov/simulation/decoder.go deleted file mode 100644 index 6e0f99a292da..000000000000 --- a/x/gov/simulation/decoder.go +++ /dev/null @@ -1,68 +0,0 @@ -package simulation - -import ( - "bytes" - "encoding/binary" - "fmt" - - "github.com/cosmos/cosmos-sdk/codec" - "github.com/cosmos/cosmos-sdk/types/kv" - "github.com/cosmos/cosmos-sdk/x/gov/types" - v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" -) - -// NewDecodeStore returns a decoder function closure that unmarshals the KVPair's -// Value to the corresponding gov type. -func NewDecodeStore(cdc codec.Codec) func(kvA, kvB kv.Pair) string { - return func(kvA, kvB kv.Pair) string { - switch { - case bytes.Equal(kvA.Key[:1], types.ProposalsKeyPrefix): - var ( - proposalA v1beta1.Proposal - proposalB v1beta1.Proposal - - proposalD v1.Proposal - proposalC v1.Proposal - ) - if err := cdc.Unmarshal(kvA.Value, &proposalC); err != nil { - cdc.MustUnmarshal(kvA.Value, &proposalA) - } - - if err := cdc.Unmarshal(kvB.Value, &proposalD); err != nil { - cdc.MustUnmarshal(kvB.Value, &proposalB) - } - - // this is to check if the proposal has been unmarshalled as v1 correctly (and not v1beta1) - if proposalC.Title != "" || proposalD.Title != "" { - return fmt.Sprintf("%v\n%v", proposalC, proposalD) - } - - return fmt.Sprintf("%v\n%v", proposalA, proposalB) - case bytes.Equal(kvA.Key[:1], types.ActiveProposalQueuePrefix), - bytes.Equal(kvA.Key[:1], types.InactiveProposalQueuePrefix), - bytes.Equal(kvA.Key[:1], types.ProposalIDKey): - proposalIDA := binary.LittleEndian.Uint64(kvA.Value) - proposalIDB := binary.LittleEndian.Uint64(kvB.Value) - return fmt.Sprintf("proposalIDA: %d\nProposalIDB: %d", proposalIDA, proposalIDB) - - case bytes.Equal(kvA.Key[:1], types.DepositsKeyPrefix): - var depositA, depositB v1beta1.Deposit - cdc.MustUnmarshal(kvA.Value, &depositA) - cdc.MustUnmarshal(kvB.Value, &depositB) - return fmt.Sprintf("%v\n%v", depositA, depositB) - - case bytes.Equal(kvA.Key[:1], types.VotesKeyPrefix): - var voteA, voteB v1beta1.Vote - cdc.MustUnmarshal(kvA.Value, &voteA) - cdc.MustUnmarshal(kvB.Value, &voteB) - return fmt.Sprintf("%v\n%v", voteA, voteB) - - case bytes.Equal(kvA.Key[:1], types.VotingPeriodProposalKeyPrefix): - return fmt.Sprintf("%v\n%v", kvA.Value, kvB.Value) - - default: - panic(fmt.Sprintf("invalid governance key prefix %X", kvA.Key[:1])) - } - } -} diff --git a/x/gov/simulation/decoder_test.go b/x/gov/simulation/decoder_test.go deleted file mode 100644 index 0d6957249c6b..000000000000 --- a/x/gov/simulation/decoder_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package simulation_test - -import ( - "encoding/binary" - "fmt" - "testing" - "time" - - "cosmossdk.io/math" - "github.com/stretchr/testify/require" - - "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/kv" - moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" - "github.com/cosmos/cosmos-sdk/x/gov" - "github.com/cosmos/cosmos-sdk/x/gov/simulation" - "github.com/cosmos/cosmos-sdk/x/gov/types" - v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" -) - -var ( - delPk1 = ed25519.GenPrivKey().PubKey() - delAddr1 = sdk.AccAddress(delPk1.Address()) -) - -func TestDecodeStore(t *testing.T) { - cdc := moduletestutil.MakeTestEncodingConfig(gov.AppModuleBasic{}).Codec - dec := simulation.NewDecodeStore(cdc) - - endTime := time.Now().UTC() - content, ok := v1beta1.ContentFromProposalType("test", "test", v1beta1.ProposalTypeText) - require.True(t, ok) - proposalA, err := v1beta1.NewProposal(content, 1, endTime, endTime.Add(24*time.Hour)) - require.NoError(t, err) - proposalB, err := v1beta1.NewProposal(content, 2, endTime, endTime.Add(24*time.Hour)) - require.NoError(t, err) - proposalC, err := v1.NewProposal([]sdk.Msg{}, 3, endTime, endTime.Add(24*time.Hour), "metadata", "title", "summary", delAddr1, false) - require.NoError(t, err) - proposalD, err := v1.NewProposal([]sdk.Msg{}, 4, endTime, endTime.Add(24*time.Hour), "metadata", "title", "summary", delAddr1, true) - require.NoError(t, err) - - proposalIDBz := make([]byte, 8) - binary.LittleEndian.PutUint64(proposalIDBz, 1) - deposit := v1beta1.NewDeposit(1, delAddr1, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.OneInt()))) - vote := v1beta1.NewVote(1, delAddr1, v1beta1.NewNonSplitVoteOption(v1beta1.OptionYes)) - - tests := []struct { - name string - kvA, kvB kv.Pair - expectedLog string - wantPanic bool - }{ - { - "proposals v1beta", - kv.Pair{Key: types.ProposalKey(1), Value: cdc.MustMarshal(&proposalA)}, - kv.Pair{Key: types.ProposalKey(2), Value: cdc.MustMarshal(&proposalB)}, - fmt.Sprintf("%v\n%v", proposalA, proposalB), false, - }, - { - "proposals v1", - kv.Pair{Key: types.ProposalKey(3), Value: cdc.MustMarshal(&proposalC)}, - kv.Pair{Key: types.ProposalKey(4), Value: cdc.MustMarshal(&proposalD)}, - fmt.Sprintf("%v\n%v", proposalC, proposalD), false, - }, - { - "proposal IDs", - kv.Pair{Key: types.InactiveProposalQueueKey(1, endTime), Value: proposalIDBz}, - kv.Pair{Key: types.InactiveProposalQueueKey(1, endTime), Value: proposalIDBz}, - "proposalIDA: 1\nProposalIDB: 1", false, - }, - { - "deposits", - kv.Pair{Key: types.DepositKey(1, delAddr1), Value: cdc.MustMarshal(&deposit)}, - kv.Pair{Key: types.DepositKey(1, delAddr1), Value: cdc.MustMarshal(&deposit)}, - fmt.Sprintf("%v\n%v", deposit, deposit), false, - }, - { - "votes", - kv.Pair{Key: types.VoteKey(1, delAddr1), Value: cdc.MustMarshal(&vote)}, - kv.Pair{Key: types.VoteKey(1, delAddr1), Value: cdc.MustMarshal(&vote)}, - fmt.Sprintf("%v\n%v", vote, vote), false, - }, - { - "other", - kv.Pair{Key: []byte{0x99}, Value: []byte{0x99}}, - kv.Pair{Key: []byte{0x99}, Value: []byte{0x99}}, - "", true, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - if tt.wantPanic { - require.Panics(t, func() { dec(tt.kvA, tt.kvB) }, tt.name) - } else { - require.Equal(t, tt.expectedLog, dec(tt.kvA, tt.kvB), tt.name) - } - }) - } -} diff --git a/x/gov/simulation/operations_test.go b/x/gov/simulation/operations_test.go index 5aeacb5b8f17..ebed8e0b04d2 100644 --- a/x/gov/simulation/operations_test.go +++ b/x/gov/simulation/operations_test.go @@ -27,6 +27,7 @@ import ( _ "github.com/cosmos/cosmos-sdk/x/consensus" _ "github.com/cosmos/cosmos-sdk/x/distribution" dk "github.com/cosmos/cosmos-sdk/x/distribution/keeper" + _ "github.com/cosmos/cosmos-sdk/x/gov" govcodec "github.com/cosmos/cosmos-sdk/x/gov/codec" "github.com/cosmos/cosmos-sdk/x/gov/keeper" "github.com/cosmos/cosmos-sdk/x/gov/simulation" From 99066f324aba72447130c6710d2ae91f82fad497 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 12 May 2023 11:24:21 +0200 Subject: [PATCH 14/31] remove improt --- x/gov/keeper/deposit.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 4ec69be599c8..7a1c66afa365 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -2,7 +2,6 @@ package keeper import ( "context" - stderr "errors" "fmt" "cosmossdk.io/collections" @@ -82,7 +81,7 @@ func (keeper Keeper) IterateAllDeposits(ctx context.Context, cb func(deposit v1. func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) bool) error { pair := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID) err := keeper.Deposits.Walk(ctx, pair, cb) - if err != nil && !stderr.Is(err, collections.ErrInvalidIterator) { + if err != nil && !errors.IsOf(err, collections.ErrInvalidIterator) { return err } return nil From b23e38dfd8ee176b154ff04a148a7e6c795db42c Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:34:32 +0200 Subject: [PATCH 15/31] change the collections API to error on walking --- collections/indexed_map.go | 2 +- collections/indexes/multi.go | 4 ++-- collections/indexes/reverse_pair.go | 4 ++-- collections/indexes/unique.go | 2 +- collections/iter_test.go | 16 ++++++++++--- collections/keyset.go | 4 ++-- collections/map.go | 8 +++++-- x/bank/keeper/keeper.go | 16 +++++++++---- x/bank/keeper/send.go | 8 ++++++- x/bank/keeper/view.go | 10 ++++---- x/gov/keeper/deposit.go | 36 +++++++++++++---------------- x/gov/keeper/deposit_test.go | 4 ++-- 12 files changed, 69 insertions(+), 45 deletions(-) diff --git a/collections/indexed_map.go b/collections/indexed_map.go index 1625c47aec9c..4a45edbe45c0 100644 --- a/collections/indexed_map.go +++ b/collections/indexed_map.go @@ -93,7 +93,7 @@ func (m *IndexedMap[PrimaryKey, Value, Idx]) Remove(ctx context.Context, pk Prim } // Walk applies the same semantics as Map.Walk. -func (m *IndexedMap[PrimaryKey, Value, Idx]) Walk(ctx context.Context, ranger Ranger[PrimaryKey], walkFunc func(key PrimaryKey, value Value) bool) error { +func (m *IndexedMap[PrimaryKey, Value, Idx]) Walk(ctx context.Context, ranger Ranger[PrimaryKey], walkFunc func(key PrimaryKey, value Value) (stop bool, err error)) error { return m.m.Walk(ctx, ranger, walkFunc) } diff --git a/collections/indexes/multi.go b/collections/indexes/multi.go index 410aae00fd3b..9e567aaab425 100644 --- a/collections/indexes/multi.go +++ b/collections/indexes/multi.go @@ -82,9 +82,9 @@ func (m *Multi[ReferenceKey, PrimaryKey, Value]) Iterate(ctx context.Context, ra func (m *Multi[ReferenceKey, PrimaryKey, Value]) Walk( ctx context.Context, ranger collections.Ranger[collections.Pair[ReferenceKey, PrimaryKey]], - walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) bool, + walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) (stop bool, err error), ) error { - return m.refKeys.Walk(ctx, ranger, func(key collections.Pair[ReferenceKey, PrimaryKey]) bool { + return m.refKeys.Walk(ctx, ranger, func(key collections.Pair[ReferenceKey, PrimaryKey]) (bool, error) { return walkFunc(key.K1(), key.K2()) }) } diff --git a/collections/indexes/reverse_pair.go b/collections/indexes/reverse_pair.go index 55e4007e5f19..243b2d289ad8 100644 --- a/collections/indexes/reverse_pair.go +++ b/collections/indexes/reverse_pair.go @@ -67,9 +67,9 @@ func (i *ReversePair[K1, K2, Value]) Unreference(ctx context.Context, pk collect func (i *ReversePair[K1, K2, Value]) Walk( ctx context.Context, ranger collections.Ranger[collections.Pair[K2, K1]], - walkFunc func(indexingKey K2, indexedKey K1) bool, + walkFunc func(indexingKey K2, indexedKey K1) (stop bool, err error), ) error { - return i.refKeys.Walk(ctx, ranger, func(key collections.Pair[K2, K1]) bool { + return i.refKeys.Walk(ctx, ranger, func(key collections.Pair[K2, K1]) (bool, error) { return walkFunc(key.K1(), key.K2()) }) } diff --git a/collections/indexes/unique.go b/collections/indexes/unique.go index 561ffad930fb..b946ae7c4a5f 100644 --- a/collections/indexes/unique.go +++ b/collections/indexes/unique.go @@ -90,7 +90,7 @@ func (i *Unique[ReferenceKey, PrimaryKey, Value]) Iterate(ctx context.Context, r func (i *Unique[ReferenceKey, PrimaryKey, Value]) Walk( ctx context.Context, ranger collections.Ranger[ReferenceKey], - walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) bool, + walkFunc func(indexingKey ReferenceKey, indexedKey PrimaryKey) (stop bool, err error), ) error { return i.refKeys.Walk(ctx, ranger, walkFunc) } diff --git a/collections/iter_test.go b/collections/iter_test.go index 8569a6b93ffe..3f1ae44b0d42 100644 --- a/collections/iter_test.go +++ b/collections/iter_test.go @@ -175,14 +175,24 @@ func TestWalk(t *testing.T) { } u := uint64(0) - err = m.Walk(ctx, nil, func(key, value uint64) bool { + err = m.Walk(ctx, nil, func(key uint64, value uint64) (bool, error) { if key == 5 { - return true + return true, nil } require.Equal(t, u, key) require.Equal(t, u, value) u++ - return false + return false, nil }) require.NoError(t, err) + + sentinelErr := fmt.Errorf("sentinel error") + err = m.Walk(ctx, nil, func(key uint64, value uint64) (stop bool, err error) { + require.LessOrEqual(t, key, uint64(3)) // asserts that after the number three we stop + if key == 3 { + return false, sentinelErr + } + return false, nil + }) + require.ErrorIs(t, err, sentinelErr) // asserts correct error propagation } diff --git a/collections/keyset.go b/collections/keyset.go index 1ffa5afe6b08..408086b67262 100644 --- a/collections/keyset.go +++ b/collections/keyset.go @@ -53,8 +53,8 @@ func (k KeySet[K]) IterateRaw(ctx context.Context, start, end []byte, order Orde // Walk provides the same functionality as Map.Walk, but callbacks the walk // function only with the key. -func (k KeySet[K]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K) bool) error { - return (Map[K, NoValue])(k).Walk(ctx, ranger, func(key K, value NoValue) bool { return walkFunc(key) }) +func (k KeySet[K]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K) (stop bool, err error)) error { + return (Map[K, NoValue])(k).Walk(ctx, ranger, func(key K, value NoValue) (bool, error) { return walkFunc(key) }) } func (k KeySet[K]) KeyCodec() codec.KeyCodec[K] { return (Map[K, NoValue])(k).KeyCodec() } diff --git a/collections/map.go b/collections/map.go index 7623be6b14e5..9ada53c494cc 100644 --- a/collections/map.go +++ b/collections/map.go @@ -127,7 +127,7 @@ func (m Map[K, V]) Iterate(ctx context.Context, ranger Ranger[K]) (Iterator[K, V // walk function with the decoded key and value. If the callback function // returns true then the walking is stopped. // A nil ranger equals to walking over the entire key and value set. -func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(K, V) bool) error { +func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(key K, value V) (stop bool, err error)) error { iter, err := m.Iterate(ctx, ranger) if err != nil { return err @@ -139,7 +139,11 @@ func (m Map[K, V]) Walk(ctx context.Context, ranger Ranger[K], walkFunc func(K, if err != nil { return err } - if walkFunc(kv.Key, kv.Value) { + stop, err := walkFunc(kv.Key, kv.Value) + if err != nil { + return err + } + if stop { return nil } } diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index dd6ec47c79cb..1ac1fcc46a8d 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -2,6 +2,8 @@ package keeper import ( "context" + "cosmossdk.io/collections" + "errors" "fmt" "cosmossdk.io/core/store" @@ -256,9 +258,12 @@ func (k BaseKeeper) GetAllDenomMetaData(ctx context.Context) []types.Metadata { // provides the metadata to a callback. If true is returned from the // callback, iteration is halted. func (k BaseKeeper) IterateAllDenomMetaData(ctx context.Context, cb func(types.Metadata) bool) { - _ = k.BaseViewKeeper.DenomMetadata.Walk(ctx, nil, func(_ string, metadata types.Metadata) bool { - return cb(metadata) + err := k.BaseViewKeeper.DenomMetadata.Walk(ctx, nil, func(_ string, metadata types.Metadata) (stop bool, err error) { + return cb(metadata), nil }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } } // SetDenomMetaData sets the denominations metadata @@ -474,7 +479,10 @@ func (k BaseKeeper) trackUndelegation(ctx context.Context, addr sdk.AccAddress, // with the balance of each coin. // The iteration stops if the callback returns true. func (k BaseViewKeeper) IterateTotalSupply(ctx context.Context, cb func(sdk.Coin) bool) { - _ = k.Supply.Walk(ctx, nil, func(s string, m math.Int) bool { - return cb(sdk.NewCoin(s, m)) + err := k.Supply.Walk(ctx, nil, func(s string, m math.Int) (bool, error) { + return cb(sdk.NewCoin(s, m)), nil }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index a6bfff3f8ec2..06dc5b26ebd9 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -3,6 +3,7 @@ package keeper import ( "context" "fmt" + "github.com/pkg/errors" "cosmossdk.io/collections" "cosmossdk.io/core/store" @@ -376,7 +377,12 @@ func (k BaseSendKeeper) DeleteSendEnabled(ctx context.Context, denoms ...string) // IterateSendEnabledEntries iterates over all the SendEnabled entries. func (k BaseSendKeeper) IterateSendEnabledEntries(ctx context.Context, cb func(denom string, sendEnabled bool) bool) { - _ = k.SendEnabled.Walk(ctx, nil, cb) + err := k.SendEnabled.Walk(ctx, nil, func(key string, value bool) (stop bool, err error) { + return cb(key, value), nil + }) + if err != nil && errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } } // GetAllSendEnabledEntries gets all the SendEnabled entries that are stored. diff --git a/x/bank/keeper/view.go b/x/bank/keeper/view.go index db9852dd9f17..ca436d8b4540 100644 --- a/x/bank/keeper/view.go +++ b/x/bank/keeper/view.go @@ -157,8 +157,8 @@ func (k BaseViewKeeper) GetBalance(ctx context.Context, addr sdk.AccAddress, den // provides the token balance to a callback. If true is returned from the // callback, iteration is halted. func (k BaseViewKeeper) IterateAccountBalances(ctx context.Context, addr sdk.AccAddress, cb func(sdk.Coin) bool) { - err := k.Balances.Walk(ctx, collections.NewPrefixedPairRange[sdk.AccAddress, string](addr), func(key collections.Pair[sdk.AccAddress, string], value math.Int) bool { - return cb(sdk.NewCoin(key.K2(), value)) + err := k.Balances.Walk(ctx, collections.NewPrefixedPairRange[sdk.AccAddress, string](addr), func(key collections.Pair[sdk.AccAddress, string], value math.Int) (stop bool, err error) { + return cb(sdk.NewCoin(key.K2(), value)), nil }) if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { // TODO(tip): is this the correct strategy panic(err) @@ -169,10 +169,10 @@ func (k BaseViewKeeper) IterateAccountBalances(ctx context.Context, addr sdk.Acc // denominations that are provided to a callback. If true is returned from the // callback, iteration is halted. func (k BaseViewKeeper) IterateAllBalances(ctx context.Context, cb func(sdk.AccAddress, sdk.Coin) bool) { - err := k.Balances.Walk(ctx, nil, func(key collections.Pair[sdk.AccAddress, string], value math.Int) bool { - return cb(key.K1(), sdk.NewCoin(key.K2(), value)) + err := k.Balances.Walk(ctx, nil, func(key collections.Pair[sdk.AccAddress, string], value math.Int) (stop bool, err error) { + return cb(key.K1(), sdk.NewCoin(key.K2(), value)), nil }) - if err != nil { + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 7a1c66afa365..27e201d0e9ab 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -28,9 +28,9 @@ func (keeper Keeper) SetDeposit(ctx context.Context, deposit v1.Deposit) error { // GetDeposits returns all the deposits of a proposal func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposits v1.Deposits, err error) { - err = keeper.IterateDeposits(ctx, proposalID, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + err = keeper.IterateDeposits(ctx, proposalID, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) { deposits = append(deposits, &deposit) - return false + return false, nil }) return @@ -39,10 +39,9 @@ func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposi // DeleteAndBurnDeposits deletes and burns all the deposits on a specific proposal. func (keeper Keeper) DeleteAndBurnDeposits(ctx context.Context, proposalID uint64) error { coinsToBurn := sdk.NewCoins() - err := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + err := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (stop bool, err error) { coinsToBurn = coinsToBurn.Add(deposit.Amount...) - _ = keeper.Deposits.Remove(ctx, key) // can't error, otherwise the iterator wouldn't report it - return false + return false, keeper.Deposits.Remove(ctx, key) }) if err != nil { return err @@ -78,9 +77,9 @@ func (keeper Keeper) IterateAllDeposits(ctx context.Context, cb func(deposit v1. } // IterateDeposits iterates over all the proposals deposits and performs a callback function -func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) bool) error { - pair := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID) - err := keeper.Deposits.Walk(ctx, pair, cb) +func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) (bool, error)) error { + rng := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID) + err := keeper.Deposits.Walk(ctx, rng, cb) if err != nil && !errors.IsOf(err, collections.ErrInvalidIterator) { return err } @@ -213,7 +212,10 @@ func (keeper Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destA return err } } - return keeper.Deposits.Remove(ctx, collections.Join(deposit.ProposalId, sdk.AccAddress(depositerAddress))) + err = keeper.Deposits.Remove(ctx, collections.Join(deposit.ProposalId, sdk.AccAddress(depositerAddress))) + if err != nil { + return err + } } // burn the cancellation fee or sent the cancellation charges to destination address. @@ -251,21 +253,15 @@ func (keeper Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destA // RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal. func (keeper Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uint64) error { - var err error - iterErr := keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + return keeper.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) { depositor := key.K2() - err = keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount) + err := keeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount) if err != nil { - return true + return false, err } - _ = keeper.Deposits.Remove(ctx, key) // cannot error - return false + err = keeper.Deposits.Remove(ctx, key) + return false, err }) - if iterErr != nil { - return iterErr - } - - return err } // validateInitialDeposit validates if initial deposit is greater than or equal to the minimum diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 62fb6e378a01..3c6ab19c3d19 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -122,9 +122,9 @@ func TestDeposits(t *testing.T) { // Test deposit iterator // NOTE order of deposits is determined by the addresses var deposits v1.Deposits - err = govKeeper.Deposits.Walk(ctx, nil, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) bool { + err = govKeeper.Deposits.Walk(ctx, nil, func(_ collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) { deposits = append(deposits, &deposit) - return false + return false, nil }) require.NoError(t, err) require.Len(t, deposits, 2) From 6af4aa9d3a339ec31f02e50b6569edea9caf914e Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:36:32 +0200 Subject: [PATCH 16/31] chore: CHANGELOG.md --- collections/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/collections/CHANGELOG.md b/collections/CHANGELOG.md index 87e84a0f9ba9..1fa3b8ca67df 100644 --- a/collections/CHANGELOG.md +++ b/collections/CHANGELOG.md @@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#16074](https://github.com/cosmos/cosmos-sdk/pull/16074) – makes the generic Collection interface public, still highly unstable. +### API Breaking + +* [#16127](https://github.com/cosmos/cosmos-sdk/pull/16127) – In the `Walk` method the call back function being passed is allowed to error. + ## [v0.1.0](https://github.com/cosmos/cosmos-sdk/releases/tag/collections%2Fv0.1.0) Collections `v0.1.0` is released! Check out the [docs](https://docs.cosmos.network/main/packages/collections) to know how to use the APIs. \ No newline at end of file From 5191de7ec593db9abe3c3a68e6ea4ace332d8151 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:40:12 +0200 Subject: [PATCH 17/31] lint bank --- x/bank/keeper/keeper.go | 3 ++- x/bank/keeper/send.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index 1ac1fcc46a8d..2d6edfdf960f 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -2,10 +2,11 @@ package keeper import ( "context" - "cosmossdk.io/collections" "errors" "fmt" + "cosmossdk.io/collections" + "cosmossdk.io/core/store" "cosmossdk.io/log" "cosmossdk.io/math" diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 06dc5b26ebd9..42162494bef4 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -3,6 +3,7 @@ package keeper import ( "context" "fmt" + "github.com/pkg/errors" "cosmossdk.io/collections" From 4f9bea7ae06a0a8cd9b1b924366851913926af44 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:43:17 +0200 Subject: [PATCH 18/31] remove unused error --- x/gov/keeper/grpc_query.go | 6 +----- x/gov/types/errors.go | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index 4e6ed873dfe0..280ab044a4ee 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -229,11 +229,7 @@ func (q queryServer) Deposit(ctx context.Context, req *v1.QueryDepositRequest) ( } deposit, err := q.k.Deposits.Get(ctx, collections.Join(req.ProposalId, sdk.AccAddress(depositor))) if err != nil { - if errors.IsOf(err, types.ErrDepositNotFound) { - return nil, status.Errorf(codes.InvalidArgument, - "depositer: %v not found for proposal: %v", req.Depositor, req.ProposalId) - } - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.NotFound, err.Error()) } return &v1.QueryDepositResponse{Deposit: &deposit}, nil diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 8920fccfdf64..9223080a6789 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -27,6 +27,5 @@ var ( ErrNoDeposits = errors.Register(ModuleName, 19, "no deposits found") ErrVotingPeriodEnded = errors.Register(ModuleName, 20, "voting period already ended") ErrInvalidProposal = errors.Register(ModuleName, 21, "invalid proposal") - ErrDepositNotFound = errors.Register(ModuleName, 22, "deposit is not found") ErrVoteNotFound = errors.Register(ModuleName, 23, "vote is not found") ) From 3ee570e90b434edfbb5300e59e2d198d4873420b Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:43:43 +0200 Subject: [PATCH 19/31] remove unused errors --- x/gov/types/errors.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 9223080a6789..11bbb8fef516 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -19,12 +19,10 @@ var ( ErrNoProposalMsgs = errors.Register(ModuleName, 11, "no messages proposed") ErrInvalidProposalMsg = errors.Register(ModuleName, 12, "invalid proposal message") ErrInvalidSigner = errors.Register(ModuleName, 13, "expected gov account as only signer for proposal message") - ErrInvalidSignalMsg = errors.Register(ModuleName, 14, "signal message is invalid") ErrMetadataTooLong = errors.Register(ModuleName, 15, "metadata too long") ErrMinDepositTooSmall = errors.Register(ModuleName, 16, "minimum deposit is too small") ErrProposalNotFound = errors.Register(ModuleName, 17, "proposal is not found") ErrInvalidProposer = errors.Register(ModuleName, 18, "invalid proposer") - ErrNoDeposits = errors.Register(ModuleName, 19, "no deposits found") ErrVotingPeriodEnded = errors.Register(ModuleName, 20, "voting period already ended") ErrInvalidProposal = errors.Register(ModuleName, 21, "invalid proposal") ErrVoteNotFound = errors.Register(ModuleName, 23, "vote is not found") From 64bf12ffa1b0276e507577d61d137f210f88dac4 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:49:16 +0200 Subject: [PATCH 20/31] fix some minor thing --- x/bank/keeper/send.go | 2 +- x/bank/keeper/view.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 42162494bef4..fef012bdbe90 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -381,7 +381,7 @@ func (k BaseSendKeeper) IterateSendEnabledEntries(ctx context.Context, cb func(d err := k.SendEnabled.Walk(ctx, nil, func(key string, value bool) (stop bool, err error) { return cb(key, value), nil }) - if err != nil && errors.Is(err, collections.ErrInvalidIterator) { + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } } diff --git a/x/bank/keeper/view.go b/x/bank/keeper/view.go index ca436d8b4540..732ac3d17a75 100644 --- a/x/bank/keeper/view.go +++ b/x/bank/keeper/view.go @@ -160,7 +160,7 @@ func (k BaseViewKeeper) IterateAccountBalances(ctx context.Context, addr sdk.Acc err := k.Balances.Walk(ctx, collections.NewPrefixedPairRange[sdk.AccAddress, string](addr), func(key collections.Pair[sdk.AccAddress, string], value math.Int) (stop bool, err error) { return cb(sdk.NewCoin(key.K2(), value)), nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { // TODO(tip): is this the correct strategy + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { panic(err) } } From a4fe45c50e642f636dc907a083418c58dde486e9 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:54:22 +0200 Subject: [PATCH 21/31] more minor things --- x/gov/keeper/deposit.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index 27e201d0e9ab..ebcd4bdbf3be 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -32,8 +32,7 @@ func (keeper Keeper) GetDeposits(ctx context.Context, proposalID uint64) (deposi deposits = append(deposits, &deposit) return false, nil }) - - return + return deposits, err } // DeleteAndBurnDeposits deletes and burns all the deposits on a specific proposal. From 66b7866c67207dac10d11bf0dd5e2694f6758954 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:58:32 +0200 Subject: [PATCH 22/31] remove another method yet again --- x/gov/keeper/deposit.go | 29 ----------------------------- x/gov/keeper/invariants.go | 11 ++++++++--- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index ebcd4bdbf3be..f0be3fe7976a 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -8,9 +8,6 @@ import ( "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" - storetypes "cosmossdk.io/store/types" - - "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -49,32 +46,6 @@ func (keeper Keeper) DeleteAndBurnDeposits(ctx context.Context, proposalID uint6 return keeper.bankKeeper.BurnCoins(ctx, types.ModuleName, coinsToBurn) } -// IterateAllDeposits iterates over all the stored deposits and performs a callback function. -func (keeper Keeper) IterateAllDeposits(ctx context.Context, cb func(deposit v1.Deposit) error) error { - store := keeper.storeService.OpenKVStore(ctx) - iterator := storetypes.KVStorePrefixIterator(runtime.KVStoreAdapter(store), types.DepositsKeyPrefix) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - var deposit v1.Deposit - - err := keeper.cdc.Unmarshal(iterator.Value(), &deposit) - if err != nil { - return err - } - - err = cb(deposit) - // exit early without error if cb returns ErrStopIterating - if errors.IsOf(err, errors.ErrStopIterating) { - return nil - } else if err != nil { - return err - } - } - - return nil -} - // IterateDeposits iterates over all the proposals deposits and performs a callback function func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) (bool, error)) error { rng := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID) diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index 74bf6836947f..4fde70e64b39 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -1,6 +1,8 @@ package keeper import ( + "cosmossdk.io/collections" + "errors" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -19,10 +21,13 @@ func ModuleAccountInvariant(keeper *Keeper, bk types.BankKeeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { var expectedDeposits sdk.Coins - keeper.IterateAllDeposits(ctx, func(deposit v1.Deposit) error { - expectedDeposits = expectedDeposits.Add(deposit.Amount...) - return nil + err := keeper.Deposits.Walk(ctx, nil, func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) (stop bool, err error) { + expectedDeposits = expectedDeposits.Add(value.Amount...) + return false, nil }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } macc := keeper.GetGovernanceAccount(ctx) balances := bk.GetAllBalances(ctx, macc.GetAddress()) From 730116e56b58d85040cae002a32989aa0b023673 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Sat, 13 May 2023 20:58:47 +0200 Subject: [PATCH 23/31] fmt --- x/gov/keeper/invariants.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/gov/keeper/invariants.go b/x/gov/keeper/invariants.go index 4fde70e64b39..e8cd15d5737f 100644 --- a/x/gov/keeper/invariants.go +++ b/x/gov/keeper/invariants.go @@ -1,10 +1,11 @@ package keeper import ( - "cosmossdk.io/collections" "errors" "fmt" + "cosmossdk.io/collections" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" From 44906b072f21dc5f2bea942be3a51a869445ea3d Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 11:44:22 +0200 Subject: [PATCH 24/31] chore: changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 318c71625cf3..244c46dae806 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -196,6 +196,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (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. +* (x/gov) [](https://github.com/cosmos/cosmos-sdk/pull/16127) Use collections for deposit management: + - The following methods are removed from the gov keeper: `GetDeposit`, `GetAllDeposits`, `IterateAllDeposits`. + - The following functions are removed from the gov types: `DepositKey`, `DepositsKey`. ### Client Breaking Changes From a69ed21370d88772c20bd85e255437cb6caec7e2 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 11:46:26 +0200 Subject: [PATCH 25/31] supress lint --- x/gov/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index bfcb97505bc7..ac0c84bff53e 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -100,7 +100,7 @@ func NewKeeper( authority: authority, Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), - Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), + Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), // nolint: staticcheck, } schema, err := sb.Build() if err != nil { From 9aa0be47af880a7157a7ca1b91c3ae18922ee55d Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 12:38:51 +0200 Subject: [PATCH 26/31] fix lint maybe --- x/gov/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index ac0c84bff53e..2ea52d79a94c 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -100,7 +100,7 @@ func NewKeeper( authority: authority, Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), - Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), // nolint: staticcheck, + Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), //nolint: staticcheck // Needed to retain state compatibility } schema, err := sb.Build() if err != nil { From 21efcb5068e3dcd18e307f1e46e15c0e4d4123bd Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 13:27:06 +0200 Subject: [PATCH 27/31] fix lint 2 --- collections/iter_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/collections/iter_test.go b/collections/iter_test.go index 3f1ae44b0d42..dfe690bb97ba 100644 --- a/collections/iter_test.go +++ b/collections/iter_test.go @@ -175,7 +175,7 @@ func TestWalk(t *testing.T) { } u := uint64(0) - err = m.Walk(ctx, nil, func(key uint64, value uint64) (bool, error) { + err = m.Walk(ctx, nil, func(key, value uint64) (bool, error) { if key == 5 { return true, nil } @@ -187,7 +187,7 @@ func TestWalk(t *testing.T) { require.NoError(t, err) sentinelErr := fmt.Errorf("sentinel error") - err = m.Walk(ctx, nil, func(key uint64, value uint64) (stop bool, err error) { + err = m.Walk(ctx, nil, func(key, value uint64) (stop bool, err error) { require.LessOrEqual(t, key, uint64(3)) // asserts that after the number three we stop if key == 3 { return false, sentinelErr From 7f8b06644be7e4e664859b18a9cab4a9cb61021e Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 14:55:27 +0200 Subject: [PATCH 28/31] go mod tidy all --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c85987c55255..d7e1270d95af 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( github.com/magiconair/properties v1.8.7 github.com/manifoldco/promptui v0.9.0 github.com/mattn/go-isatty v0.0.18 + github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.15.1 github.com/prometheus/common v0.43.0 github.com/rs/zerolog v1.29.1 @@ -134,7 +135,6 @@ require ( github.com/oklog/run v1.1.0 // indirect github.com/pelletier/go-toml/v2 v2.0.7 // indirect github.com/petermattis/goid v0.0.0-20230317030725-371a4b8eda08 // indirect - github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect From 72dd01ba889686f693eb9b26011e4f80ea10f30c Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 15:08:05 +0200 Subject: [PATCH 29/31] remove dependency on another error pkg --- x/bank/keeper/send.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index fef012bdbe90..ecd0e015a6a5 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - "github.com/pkg/errors" - "cosmossdk.io/collections" "cosmossdk.io/core/store" "cosmossdk.io/log" @@ -381,7 +379,7 @@ func (k BaseSendKeeper) IterateSendEnabledEntries(ctx context.Context, cb func(d err := k.SendEnabled.Walk(ctx, nil, func(key string, value bool) (stop bool, err error) { return cb(key, value), nil }) - if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + if err != nil && !errorsmod.IsOf(err, collections.ErrInvalidIterator) { panic(err) } } From 98195ea2711fbcb1cd53c915df3d101b938daa9e Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 16:00:07 +0200 Subject: [PATCH 30/31] tidy again --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d7e1270d95af..c85987c55255 100644 --- a/go.mod +++ b/go.mod @@ -45,7 +45,6 @@ require ( github.com/magiconair/properties v1.8.7 github.com/manifoldco/promptui v0.9.0 github.com/mattn/go-isatty v0.0.18 - github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.15.1 github.com/prometheus/common v0.43.0 github.com/rs/zerolog v1.29.1 @@ -135,6 +134,7 @@ require ( github.com/oklog/run v1.1.0 // indirect github.com/pelletier/go-toml/v2 v2.0.7 // indirect github.com/petermattis/goid v0.0.0-20230317030725-371a4b8eda08 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect From f5eb34cb0e56054c1242752faee757795cbcd8f5 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 15 May 2023 16:47:00 +0200 Subject: [PATCH 31/31] try fix confix --- tools/confix/go.mod | 2 -- tools/confix/go.sum | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/confix/go.mod b/tools/confix/go.mod index 5483d7531bc2..cbb725647612 100644 --- a/tools/confix/go.mod +++ b/tools/confix/go.mod @@ -155,8 +155,6 @@ require ( sigs.k8s.io/yaml v1.3.0 // indirect ) -replace cosmossdk.io/collections => ../../collections - // Fix upstream GHSA-h395-qcrw-5vmq and GHSA-3vp4-m3rf-835h vulnerabilities. // TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409 // TODO investigate if we can outright delete this dependency, otherwise go install won't work :( diff --git a/tools/confix/go.sum b/tools/confix/go.sum index 2ccdb3a85f44..c3c2c03a85bf 100644 --- a/tools/confix/go.sum +++ b/tools/confix/go.sum @@ -37,6 +37,8 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= cosmossdk.io/api v0.4.1 h1:0ikaYM6GyxTYYcfBiyR8YnLCfhNnhKpEFnaSepCTmqg= cosmossdk.io/api v0.4.1/go.mod h1:jR7k5ok90LxW2lFUXvd8Vpo/dr4PpiyVegxdm7b1ZdE= +cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8= +cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo= cosmossdk.io/core v0.7.0 h1:GFss3qt2P9p23Cz24NnqLkslzb8n+B75A24x1JgJJp0= cosmossdk.io/core v0.7.0/go.mod h1:36hP0ZH/8ipsjzfcp0yKU4bqQXUGhS0/m1krWFCtwCc= cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=