From b62a28aac041829da5ded4aeacfcd7a42873d1c8 Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Fri, 26 May 2023 11:49:12 +0200 Subject: [PATCH] refactor(gov)!: finalize collections migration (#16268) Co-authored-by: unknown unknown --- CHANGELOG.md | 5 + collections/pair.go | 6 + types/collections.go | 52 +++++++++ types/collections_test.go | 5 + x/gov/abci.go | 51 +++++--- x/gov/abci_test.go | 186 +++++++++++------------------- x/gov/genesis.go | 15 ++- x/gov/keeper/keeper.go | 151 +++++------------------- x/gov/keeper/keeper_test.go | 22 ++-- x/gov/keeper/proposal.go | 28 +++-- x/gov/keeper/proposal_test.go | 30 ++--- x/gov/keeper/vote.go | 3 +- x/gov/migrations/v2/store_test.go | 19 ++- x/gov/migrations/v4/store.go | 4 +- x/gov/types/keys.go | 118 ++----------------- x/gov/types/keys_test.go | 26 ----- 16 files changed, 267 insertions(+), 454 deletions(-) delete mode 100644 x/gov/types/keys_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ab681b2012..2fc7b9f4b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -235,6 +235,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/authx) [#15284](https://github.com/cosmos/cosmos-sdk/pull/15284) `NewKeeper` now requires `codec.Codec`. * (x/gov) [#15284](https://github.com/cosmos/cosmos-sdk/pull/15284) `NewKeeper` now requires `codec.Codec`. +* (x/gov) [#16268](https://github.com/cosmos/cosmos-sdk/pull/16268) Use collections for proposal state management (part 2): + * this finalizes the gov collections migration + * Removed: keeper `InsertActiveProposalsQueue`, `RemoveActiveProposalsQueue`, `InsertInactiveProposalsQueue`, `RemoveInactiveProposalsQueue`, `IterateInactiveProposalsQueue`, `IterateActiveProposalsQueue`, `ActiveProposalsQueueIterator`, `InactiveProposalsQueueIterator` + * Remove: types all the key related functions + ### Client Breaking Changes * (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) `HistoricalInfoKey` now has a binary format. diff --git a/collections/pair.go b/collections/pair.go index b0a789d553..f12aaac1b5 100644 --- a/collections/pair.go +++ b/collections/pair.go @@ -216,6 +216,12 @@ func (p pairKeyCodec[K1, K2]) DecodeJSON(b []byte) (Pair[K1, K2], error) { return Join(k1, k2), nil } +// NewPrefixUntilPairRange defines a collection query which ranges until the provided Pair prefix. +// Unstable: this API might change in the future. +func NewPrefixUntilPairRange[K1, K2 any](prefix K1) *PairRange[K1, K2] { + return &PairRange[K1, K2]{end: RangeKeyPrefixEnd(PairPrefix[K1, K2](prefix))} +} + // NewPrefixedPairRange creates a new PairRange which will prefix over all the keys // starting with the provided prefix. func NewPrefixedPairRange[K1, K2 any](prefix K1) *PairRange[K1, K2] { diff --git a/types/collections.go b/types/collections.go index f5e3f8a366..91d5fb7dcd 100644 --- a/types/collections.go +++ b/types/collections.go @@ -1,6 +1,9 @@ package types import ( + "fmt" + "time" + "cosmossdk.io/collections" collcodec "cosmossdk.io/collections/codec" "cosmossdk.io/math" @@ -28,6 +31,12 @@ var ( // IntValue represents a collections.ValueCodec to work with Int. IntValue collcodec.ValueCodec[math.Int] = intValueCodec{} + + // TimeKey represents a collections.KeyCodec to work with time.Time + // Deprecated: exists only for state compatibility reasons, should not + // be used for new storage keys using time. Please use the time KeyCodec + // provided in the collections package. + TimeKey collcodec.KeyCodec[time.Time] = timeKeyCodec{} ) type addressUnion interface { @@ -156,3 +165,46 @@ func (i intValueCodec) Stringify(value Int) string { func (i intValueCodec) ValueType() string { return "math.Int" } + +type timeKeyCodec struct{} + +func (timeKeyCodec) Encode(buffer []byte, key time.Time) (int, error) { + return copy(buffer, FormatTimeBytes(key)), nil +} + +var timeSize = len(FormatTimeBytes(time.Time{})) + +func (timeKeyCodec) Decode(buffer []byte) (int, time.Time, error) { + if len(buffer) != timeSize { + return 0, time.Time{}, fmt.Errorf("invalid time buffer buffer size") + } + t, err := ParseTimeBytes(buffer) + if err != nil { + return 0, time.Time{}, err + } + return timeSize, t, nil +} + +func (timeKeyCodec) Size(key time.Time) int { return timeSize } + +func (timeKeyCodec) EncodeJSON(value time.Time) ([]byte, error) { return value.MarshalJSON() } + +func (timeKeyCodec) DecodeJSON(b []byte) (time.Time, error) { + t := time.Time{} + err := t.UnmarshalJSON(b) + return t, err +} + +func (timeKeyCodec) Stringify(key time.Time) string { return key.String() } +func (timeKeyCodec) KeyType() string { return "sdk/time.Time" } +func (t timeKeyCodec) EncodeNonTerminal(buffer []byte, key time.Time) (int, error) { + return t.Encode(buffer, key) +} + +func (t timeKeyCodec) DecodeNonTerminal(buffer []byte) (int, time.Time, error) { + if len(buffer) < timeSize { + return 0, time.Time{}, fmt.Errorf("invalid time buffer size, wanted: %d at least, got: %d", timeSize, len(buffer)) + } + return t.Decode(buffer[:timeSize]) +} +func (t timeKeyCodec) SizeNonTerminal(key time.Time) int { return t.Size(key) } diff --git a/types/collections_test.go b/types/collections_test.go index 04e612fa2b..222566c27e 100644 --- a/types/collections_test.go +++ b/types/collections_test.go @@ -2,6 +2,7 @@ package types import ( "testing" + "time" "cosmossdk.io/collections/colltest" ) @@ -22,4 +23,8 @@ func TestCollectionsCorrectness(t *testing.T) { t.Run("AddressIndexingKey", func(t *testing.T) { colltest.TestKeyCodec(t, AddressKeyAsIndexKey(AccAddressKey), AccAddress{0x2, 0x5, 0x8}) }) + + t.Run("Time", func(t *testing.T) { + colltest.TestKeyCodec(t, TimeKey, time.Time{}) + }) } diff --git a/x/gov/abci.go b/x/gov/abci.go index eb5e4e9c67..48c83d1da6 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -1,9 +1,11 @@ package gov import ( + "errors" "fmt" "time" + "cosmossdk.io/collections" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/keeper" @@ -18,15 +20,20 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { logger := ctx.Logger().With("module", "x/"+types.ModuleName) // delete dead proposals from store and returns theirs deposits. // A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase. - err := keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error { - err := keeper.DeleteProposal(ctx, proposal.Id) + rng := collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()) + err := keeper.InactiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { + proposal, err := keeper.Proposals.Get(ctx, key.K2()) if err != nil { - return err + return false, err + } + err = keeper.DeleteProposal(ctx, proposal.Id) + if err != nil { + return false, err } params, err := keeper.Params.Get(ctx) if err != nil { - return err + return false, err } if !params.BurnProposalDepositPrevote { err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id) // refund deposit if proposal got removed without getting 100% of the proposal @@ -35,7 +42,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } if err != nil { - return err + return false, err } // called when proposal become inactive @@ -58,19 +65,25 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { "total_deposit", sdk.NewCoins(proposal.TotalDeposit...).String(), ) - return nil + return false, nil }) - if err != nil { + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { return err } // fetch active proposals whose voting periods have ended (are passed the block time) - return keeper.IterateActiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal v1.Proposal) error { + rng = collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()) + err = keeper.ActiveProposalsQueue.Walk(ctx, rng, func(key collections.Pair[time.Time, uint64], _ uint64) (bool, error) { + proposal, err := keeper.Proposals.Get(ctx, key.K2()) + if err != nil { + return false, err + } + var tagValue, logMsg string passes, burnDeposits, tallyResults, err := keeper.Tally(ctx, proposal) if err != nil { - return err + return false, err } // If an expedited proposal fails, we do not want to update @@ -85,13 +98,13 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { } if err != nil { - return err + return false, err } } - err = keeper.RemoveFromActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) + err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)) if err != nil { - return err + return false, err } switch { @@ -154,14 +167,14 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { proposal.Expedited = false params, err := keeper.Params.Get(ctx) if err != nil { - return err + return false, err } endTime := proposal.VotingStartTime.Add(*params.VotingPeriod) proposal.VotingEndTime = &endTime - err = keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) + err = keeper.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) if err != nil { - return err + return false, err } tagValue = types.AttributeValueExpeditedProposalRejected @@ -176,7 +189,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { err = keeper.SetProposal(ctx, proposal) if err != nil { - return err + return false, err } // when proposal become active @@ -200,6 +213,10 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { ), ) - return nil + return false, nil }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + return err + } + return nil } diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index fc551fda5b..204d8b5457 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "cosmossdk.io/collections" "cosmossdk.io/math" abci "github.com/cometbft/cometbft/abci/types" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" @@ -34,9 +35,7 @@ func TestTickExpiredDepositPeriod(t *testing.T) { govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - inactiveQueue, _ := suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newProposalMsg, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -53,32 +52,25 @@ func TestTickExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) params, _ := suite.GovKeeper.Params.Get(ctx) newHeader = ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) - gov.EndBlocker(ctx, suite.GovKeeper) + err = gov.EndBlocker(ctx, suite.GovKeeper) + require.NoError(t, err) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) } func TestTickMultipleExpiredDepositPeriod(t *testing.T) { @@ -94,9 +86,7 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - inactiveQueue, _ := suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newProposalMsg, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -113,17 +103,13 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(2) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newProposalMsg2, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, @@ -145,29 +131,17 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(-1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, inactiveQueue.Valid()) - inactiveQueue.Close() - - gov.EndBlocker(ctx, suite.GovKeeper) - - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) + require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper)) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newHeader = ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(5) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, inactiveQueue.Valid()) - inactiveQueue.Close() - - gov.EndBlocker(ctx, suite.GovKeeper) - - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, false) + require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper)) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) } func TestTickPassedDepositPeriod(t *testing.T) { @@ -183,13 +157,6 @@ func TestTickPassedDepositPeriod(t *testing.T) { govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - inactiveQueue, _ := suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() - activeQueue, _ := suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, activeQueue.Valid()) - activeQueue.Close() - newProposalMsg, err := v1.NewMsgSubmitProposal( []sdk.Msg{mkTestLegacyContent(t)}, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)}, @@ -207,17 +174,13 @@ func TestTickPassedDepositPeriod(t *testing.T) { proposalID := res.ProposalId - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newHeader := ctx.BlockHeader() newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 5)}) @@ -225,9 +188,7 @@ func TestTickPassedDepositPeriod(t *testing.T) { require.NoError(t, err) require.NotNil(t, res1) - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, activeQueue.Valid()) - activeQueue.Close() + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) } func TestTickPassedVotingPeriod(t *testing.T) { @@ -261,12 +222,8 @@ func TestTickPassedVotingPeriod(t *testing.T) { govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper) - inactiveQueue, _ := suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() - activeQueue, _ := suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, activeQueue.Valid()) - activeQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) proposalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 5*depositMultiplier))} newProposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{mkTestLegacyContent(t)}, proposalCoins, addrs[0].String(), "", "Proposal", "description of proposal", tc.expedited) @@ -298,40 +255,29 @@ func TestTickPassedVotingPeriod(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*votingPeriod) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, activeQueue.Valid()) - - activeProposalID := types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err := suite.GovKeeper.Proposals.Get(ctx, activeProposalID) - require.Nil(t, err) + proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) + require.NoError(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) - activeQueue.Close() + err = gov.EndBlocker(ctx, suite.GovKeeper) + require.NoError(t, err) - gov.EndBlocker(ctx, suite.GovKeeper) - - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) if !tc.expedited { - require.False(t, activeQueue.Valid()) - activeQueue.Close() + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) return } // If expedited, it should be converted to a regular proposal instead. - require.True(t, activeQueue.Valid()) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) - activeProposalID = types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) require.False(t, proposal.Expedited) require.Equal(t, proposal.VotingStartTime.Add(*params.VotingPeriod), *proposal.VotingEndTime) - - activeQueue.Close() }) } } @@ -518,12 +464,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { createValidators(t, stakingMsgSvr, ctx, []sdk.ValAddress{valAddr}, []int64{10}) suite.StakingKeeper.EndBlocker(ctx) - inactiveQueue, _ := suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() - activeQueue, _ := suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, activeQueue.Valid()) - activeQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) macc := suite.GovKeeper.GetGovernanceAccount(ctx) require.NotNil(t, macc) @@ -556,20 +498,13 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.ExpeditedVotingPeriod) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, activeQueue.Valid()) - - activeProposalID := types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err := suite.GovKeeper.Proposals.Get(ctx, activeProposalID) + proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) - activeQueue.Close() - if tc.expeditedPasses { // Validator votes YES, letting the expedited proposal pass. err = suite.GovKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "metadata") @@ -579,12 +514,10 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { // Here the expedited proposal is converted to regular after expiry. gov.EndBlocker(ctx, suite.GovKeeper) - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - if tc.expeditedPasses { - require.False(t, activeQueue.Valid()) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) - proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) require.Equal(t, v1.StatusPassed, proposal.Status) @@ -603,12 +536,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { } // Expedited proposal should be converted to a regular proposal instead. - require.True(t, activeQueue.Valid()) - - activeProposalID = types.GetProposalIDFromBytes(activeQueue.Value()) - activeQueue.Close() - - proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) require.False(t, proposal.Expedited) @@ -628,12 +557,8 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { newHeader.Time = ctx.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod) ctx = ctx.WithBlockHeader(newHeader) - inactiveQueue, _ = suite.GovKeeper.InactiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, inactiveQueue.Valid()) - inactiveQueue.Close() - - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.True(t, activeQueue.Valid()) + checkInactiveProposalsQueue(t, ctx, suite.GovKeeper, true) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, false) if tc.regularEventuallyPassing { // Validator votes YES, letting the converted regular proposal pass. @@ -651,10 +576,9 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { submitterEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[0]) depositorEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[1]) - activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) - require.False(t, activeQueue.Valid()) + checkActiveProposalsQueue(t, ctx, suite.GovKeeper, true) - proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId) require.Nil(t, err) if tc.regularEventuallyPassing { @@ -703,3 +627,25 @@ func getDepositMultiplier(expedited bool) int64 { return 1 } + +func checkActiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper, invalid bool) { + err := k.ActiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) { + return false, err + }) + if invalid { + require.ErrorIs(t, err, collections.ErrInvalidIterator) + } else { + require.NoError(t, err) + } +} + +func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper, invalid bool) { + err := k.InactiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.BlockTime()), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) { + return false, err + }) + if invalid { + require.ErrorIs(t, err, collections.ErrInvalidIterator) + } else { + require.NoError(t, err) + } +} diff --git a/x/gov/genesis.go b/x/gov/genesis.go index bc86a0cb3e..8877324d65 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -58,11 +58,20 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k for _, proposal := range data.Proposals { switch proposal.Status { case v1.StatusDepositPeriod: - k.InsertInactiveProposalQueue(ctx, proposal.Id, *proposal.DepositEndTime) + err := k.InactiveProposalsQueue.Set(ctx, collections.Join(*proposal.DepositEndTime, proposal.Id), proposal.Id) + if err != nil { + panic(err) + } case v1.StatusVotingPeriod: - k.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) + err := k.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) + if err != nil { + panic(err) + } + } + err := k.SetProposal(ctx, *proposal) + if err != nil { + panic(err) } - k.SetProposal(ctx, *proposal) } // if account has zero balance it probably means it's not set, so we set it diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index db95a0a89a..6fdeb324fa 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -8,10 +8,7 @@ import ( "cosmossdk.io/collections" corestoretypes "cosmossdk.io/core/store" - "cosmossdk.io/errors" "cosmossdk.io/log" - storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -50,13 +47,16 @@ type Keeper struct { // should be the x/gov module account. authority string - Schema collections.Schema - Constitution collections.Item[string] - Params collections.Item[v1.Params] - Deposits collections.Map[collections.Pair[uint64, sdk.AccAddress], v1.Deposit] - Votes collections.Map[collections.Pair[uint64, sdk.AccAddress], v1.Vote] - ProposalID collections.Sequence - Proposals collections.Map[uint64, v1.Proposal] + Schema collections.Schema + Constitution collections.Item[string] + Params collections.Item[v1.Params] + Deposits collections.Map[collections.Pair[uint64, sdk.AccAddress], v1.Deposit] + Votes collections.Map[collections.Pair[uint64, sdk.AccAddress], v1.Vote] + ProposalID collections.Sequence + Proposals collections.Map[uint64, v1.Proposal] + ActiveProposalsQueue collections.Map[collections.Pair[time.Time, uint64], uint64] // TODO(tip): this should be simplified and go into an index. + InactiveProposalsQueue collections.Map[collections.Pair[time.Time, uint64], uint64] // TODO(tip): this should be simplified and go into an index. + VotingPeriodProposals collections.Map[uint64, []byte] // TODO(tip): this could be a keyset or index. } // GetAuthority returns the x/gov module's authority. @@ -92,21 +92,24 @@ func NewKeeper( sb := collections.NewSchemaBuilder(storeService) k := &Keeper{ - storeService: storeService, - authKeeper: authKeeper, - bankKeeper: bankKeeper, - distrKeeper: distrKeeper, - sk: sk, - cdc: cdc, - router: router, - config: config, - authority: authority, - Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), - Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), - 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 - Votes: collections.NewMap(sb, types.VotesKeyPrefix, "votes", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Vote](cdc)), //nolint: staticcheck // Needed to retain state compatibility - ProposalID: collections.NewSequence(sb, types.ProposalIDKey, "proposal_id"), - Proposals: collections.NewMap(sb, types.ProposalsKeyPrefix, "proposals", collections.Uint64Key, codec.CollValue[v1.Proposal](cdc)), + storeService: storeService, + authKeeper: authKeeper, + bankKeeper: bankKeeper, + distrKeeper: distrKeeper, + sk: sk, + cdc: cdc, + router: router, + config: config, + authority: authority, + Constitution: collections.NewItem(sb, types.ConstitutionKey, "constitution", collections.StringValue), + Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[v1.Params](cdc)), + Deposits: collections.NewMap(sb, types.DepositsKeyPrefix, "deposits", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Deposit](cdc)), // nolint: staticcheck // sdk.AddressKeyAsIndexKey is needed to retain state compatibility + Votes: collections.NewMap(sb, types.VotesKeyPrefix, "votes", collections.PairKeyCodec(collections.Uint64Key, sdk.AddressKeyAsIndexKey(sdk.AccAddressKey)), codec.CollValue[v1.Vote](cdc)), // nolint: staticcheck // sdk.AddressKeyAsIndexKey is needed to retain state compatibility + ProposalID: collections.NewSequence(sb, types.ProposalIDKey, "proposal_id"), + Proposals: collections.NewMap(sb, types.ProposalsKeyPrefix, "proposals", collections.Uint64Key, codec.CollValue[v1.Proposal](cdc)), + ActiveProposalsQueue: collections.NewMap(sb, types.ActiveProposalQueuePrefix, "active_proposals_queue", collections.PairKeyCodec(sdk.TimeKey, collections.Uint64Key), collections.Uint64Value), // sdk.TimeKey is needed to retain state compatibility + InactiveProposalsQueue: collections.NewMap(sb, types.InactiveProposalQueuePrefix, "inactive_proposals_queue", collections.PairKeyCodec(sdk.TimeKey, collections.Uint64Key), collections.Uint64Value), // sdk.TimeKey is needed to retain state compatibility + VotingPeriodProposals: collections.NewMap(sb, types.VotingPeriodProposalKeyPrefix, "voting_period_proposals", collections.Uint64Key, collections.BytesValue), } schema, err := sb.Build() if err != nil { @@ -167,104 +170,6 @@ func (k Keeper) GetGovernanceAccount(ctx context.Context) sdk.ModuleAccountI { return k.authKeeper.GetModuleAccount(ctx, types.ModuleName) } -// ProposalQueues - -// InsertActiveProposalQueue inserts a proposalID into the active proposal queue at endTime -func (k Keeper) InsertActiveProposalQueue(ctx context.Context, proposalID uint64, endTime time.Time) error { - store := k.storeService.OpenKVStore(ctx) - bz := types.GetProposalIDBytes(proposalID) - return store.Set(types.ActiveProposalQueueKey(proposalID, endTime), bz) -} - -// RemoveFromActiveProposalQueue removes a proposalID from the Active Proposal Queue -func (k Keeper) RemoveFromActiveProposalQueue(ctx context.Context, proposalID uint64, endTime time.Time) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.ActiveProposalQueueKey(proposalID, endTime)) -} - -// InsertInactiveProposalQueue inserts a proposalID into the inactive proposal queue at endTime -func (k Keeper) InsertInactiveProposalQueue(ctx context.Context, proposalID uint64, endTime time.Time) error { - store := k.storeService.OpenKVStore(ctx) - bz := types.GetProposalIDBytes(proposalID) - return store.Set(types.InactiveProposalQueueKey(proposalID, endTime), bz) -} - -// RemoveFromInactiveProposalQueue removes a proposalID from the Inactive Proposal Queue -func (k Keeper) RemoveFromInactiveProposalQueue(ctx context.Context, proposalID uint64, endTime time.Time) error { - store := k.storeService.OpenKVStore(ctx) - return store.Delete(types.InactiveProposalQueueKey(proposalID, endTime)) -} - -// Iterators - -// IterateActiveProposalsQueue iterates over the proposals in the active proposal queue -// and performs a callback function -func (k Keeper) IterateActiveProposalsQueue(ctx context.Context, endTime time.Time, cb func(proposal v1.Proposal) error) error { - iterator, err := k.ActiveProposalQueueIterator(ctx, endTime) - if err != nil { - return err - } - - defer iterator.Close() - for ; iterator.Valid(); iterator.Next() { - proposalID, _ := types.SplitActiveProposalQueueKey(iterator.Key()) - proposal, err := k.Proposals.Get(ctx, proposalID) - if err != nil { - return err - } - - err = cb(proposal) - // exit early without error if cb returns ErrStopIterating - if errors.IsOf(err, errors.ErrStopIterating) { - return nil - } else if err != nil { - return err - } - } - - return nil -} - -// IterateInactiveProposalsQueue iterates over the proposals in the inactive proposal queue -// and performs a callback function -func (k Keeper) IterateInactiveProposalsQueue(ctx context.Context, endTime time.Time, cb func(proposal v1.Proposal) error) error { - iterator, err := k.InactiveProposalQueueIterator(ctx, endTime) - if err != nil { - return err - } - - defer iterator.Close() - for ; iterator.Valid(); iterator.Next() { - proposalID, _ := types.SplitInactiveProposalQueueKey(iterator.Key()) - proposal, err := k.Proposals.Get(ctx, proposalID) - if err != nil { - return err - } - - err = cb(proposal) - // exit early without error if cb returns ErrStopIterating - if errors.IsOf(err, errors.ErrStopIterating) { - return nil - } else if err != nil { - return err - } - } - - return nil -} - -// ActiveProposalQueueIterator returns an corestoretypes.Iterator for all the proposals in the Active Queue that expire by endTime -func (k Keeper) ActiveProposalQueueIterator(ctx context.Context, endTime time.Time) (corestoretypes.Iterator, error) { - store := k.storeService.OpenKVStore(ctx) - return store.Iterator(types.ActiveProposalQueuePrefix, storetypes.PrefixEndBytes(types.ActiveProposalByTimeKey(endTime))) -} - -// InactiveProposalQueueIterator returns an corestoretypes.Iterator for all the proposals in the Inactive Queue that expire by endTime -func (k Keeper) InactiveProposalQueueIterator(ctx context.Context, endTime time.Time) (corestoretypes.Iterator, error) { - store := k.storeService.OpenKVStore(ctx) - return store.Iterator(types.InactiveProposalQueuePrefix, storetypes.PrefixEndBytes(types.InactiveProposalByTimeKey(endTime))) -} - // ModuleAccountAddress returns gov module account address func (k Keeper) ModuleAccountAddress() sdk.AccAddress { return k.authKeeper.GetModuleAddress(types.ModuleName) diff --git a/x/gov/keeper/keeper_test.go b/x/gov/keeper/keeper_test.go index 7979c62d4e..6c309be482 100644 --- a/x/gov/keeper/keeper_test.go +++ b/x/gov/keeper/keeper_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "testing" + "cosmossdk.io/collections" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -118,25 +119,18 @@ func TestProposalQueues(t *testing.T) { proposal, err := govKeeper.SubmitProposal(ctx, tp, "", "test", "summary", addrBz, false) require.NoError(t, err) - inactiveIterator, _ := govKeeper.InactiveProposalQueueIterator(ctx, *proposal.DepositEndTime) - require.True(t, inactiveIterator.Valid()) + has, err := govKeeper.InactiveProposalsQueue.Has(ctx, collections.Join(*proposal.DepositEndTime, proposal.Id)) + require.NoError(t, err) + require.True(t, has) - proposalID := types.GetProposalIDFromBytes(inactiveIterator.Value()) - require.Equal(t, proposalID, proposal.Id) - inactiveIterator.Close() - - govKeeper.ActivateVotingPeriod(ctx, proposal) + require.NoError(t, govKeeper.ActivateVotingPeriod(ctx, proposal)) proposal, err = govKeeper.Proposals.Get(ctx, proposal.Id) require.Nil(t, err) - activeIterator, _ := govKeeper.ActiveProposalQueueIterator(ctx, *proposal.VotingEndTime) - require.True(t, activeIterator.Valid()) - - proposalID, _ = types.SplitActiveProposalQueueKey(activeIterator.Key()) - require.Equal(t, proposalID, proposal.Id) - - activeIterator.Close() + has, err = govKeeper.ActiveProposalsQueue.Has(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)) + require.NoError(t, err) + require.True(t, has) } func TestKeeperTestSuite(t *testing.T) { diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index c6487b8537..d48f60c4a1 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "cosmossdk.io/collections" errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/types" @@ -102,8 +103,14 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met return v1.Proposal{}, err } - keeper.SetProposal(ctx, proposal) - keeper.InsertInactiveProposalQueue(ctx, proposalID, *proposal.DepositEndTime) + err = keeper.SetProposal(ctx, proposal) + if err != nil { + return v1.Proposal{}, err + } + err = keeper.InactiveProposalsQueue.Set(ctx, collections.Join(*proposal.DepositEndTime, proposalID), proposalID) + if err != nil { + return v1.Proposal{}, err + } // called right after a proposal is submitted keeper.Hooks().AfterProposalSubmission(ctx, proposalID) @@ -183,15 +190,13 @@ func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, prop // SetProposal sets a proposal to store. func (keeper Keeper) SetProposal(ctx context.Context, proposal v1.Proposal) error { - store := keeper.storeService.OpenKVStore(ctx) - if proposal.Status == v1.StatusVotingPeriod { - err := store.Set(types.VotingPeriodProposalKey(proposal.Id), []byte{1}) + err := keeper.VotingPeriodProposals.Set(ctx, proposal.Id, []byte{1}) if err != nil { return err } } else { - err := store.Delete(types.VotingPeriodProposalKey(proposal.Id)) + err := keeper.VotingPeriodProposals.Remove(ctx, proposal.Id) if err != nil { return err } @@ -202,25 +207,24 @@ func (keeper Keeper) SetProposal(ctx context.Context, proposal v1.Proposal) erro // DeleteProposal deletes a proposal from store. func (keeper Keeper) DeleteProposal(ctx context.Context, proposalID uint64) error { - store := keeper.storeService.OpenKVStore(ctx) proposal, err := keeper.Proposals.Get(ctx, proposalID) if err != nil { return err } if proposal.DepositEndTime != nil { - err := keeper.RemoveFromInactiveProposalQueue(ctx, proposalID, *proposal.DepositEndTime) + err := keeper.InactiveProposalsQueue.Remove(ctx, collections.Join(*proposal.DepositEndTime, proposalID)) if err != nil { return err } } if proposal.VotingEndTime != nil { - err := keeper.RemoveFromActiveProposalQueue(ctx, proposalID, *proposal.VotingEndTime) + err := keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposalID)) if err != nil { return err } - err = store.Delete(types.VotingPeriodProposalKey(proposalID)) + err = keeper.VotingPeriodProposals.Remove(ctx, proposalID) if err != nil { return err } @@ -253,10 +257,10 @@ func (keeper Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Propo return err } - err = keeper.RemoveFromInactiveProposalQueue(ctx, proposal.Id, *proposal.DepositEndTime) + err = keeper.InactiveProposalsQueue.Remove(ctx, collections.Join(*proposal.DepositEndTime, proposal.Id)) if err != nil { return err } - return keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) + return keeper.ActiveProposalsQueue.Set(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id), proposal.Id) } diff --git a/x/gov/keeper/proposal_test.go b/x/gov/keeper/proposal_test.go index 72ac648948..0089904264 100644 --- a/x/gov/keeper/proposal_test.go +++ b/x/gov/keeper/proposal_test.go @@ -88,17 +88,10 @@ func (suite *KeeperTestSuite) TestActivateVotingPeriod() { suite.Require().Nil(err) suite.Require().True(proposal.VotingStartTime.Equal(suite.ctx.BlockHeader().Time)) - activeIterator, _ := suite.govKeeper.ActiveProposalQueueIterator(suite.ctx, *proposal.VotingEndTime) - suite.Require().True(activeIterator.Valid()) - - proposalID := types.GetProposalIDFromBytes(activeIterator.Value()) - suite.Require().Equal(proposalID, proposal.Id) - activeIterator.Close() - - // delete the proposal to avoid issues with other tests - suite.Require().NotPanics(func() { - suite.govKeeper.DeleteProposal(suite.ctx, proposalID) - }, "") + has, err := suite.govKeeper.ActiveProposalsQueue.Has(suite.ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)) + require.NoError(suite.T(), err) + require.True(suite.T(), has) + require.NoError(suite.T(), suite.govKeeper.DeleteProposal(suite.ctx, proposal.Id)) } } @@ -118,27 +111,22 @@ func (suite *KeeperTestSuite) TestDeleteProposalInVotingPeriod() { suite.Require().NoError(err) suite.Require().Nil(proposal.VotingStartTime) - suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal) + require.NoError(suite.T(), suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal)) proposal, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal.Id) suite.Require().Nil(err) suite.Require().True(proposal.VotingStartTime.Equal(suite.ctx.BlockHeader().Time)) - activeIterator, _ := suite.govKeeper.ActiveProposalQueueIterator(suite.ctx, *proposal.VotingEndTime) - suite.Require().True(activeIterator.Valid()) - - proposalID := types.GetProposalIDFromBytes(activeIterator.Value()) - suite.Require().Equal(proposalID, proposal.Id) - activeIterator.Close() + has, err := suite.govKeeper.ActiveProposalsQueue.Has(suite.ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)) + require.NoError(suite.T(), err) + require.True(suite.T(), has) // add vote voteOptions := []*v1.WeightedVoteOption{{Option: v1.OptionYes, Weight: "1.0"}} err = suite.govKeeper.AddVote(suite.ctx, proposal.Id, suite.addrs[0], voteOptions, "") suite.Require().NoError(err) - suite.Require().NotPanics(func() { - suite.govKeeper.DeleteProposal(suite.ctx, proposalID) - }, "") + require.NoError(suite.T(), suite.govKeeper.DeleteProposal(suite.ctx, proposal.Id)) // add vote but proposal is deleted along with its VotingPeriodProposalKey err = suite.govKeeper.AddVote(suite.ctx, proposal.Id, suite.addrs[0], voteOptions, "") diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index e236f0127d..84ae0f5ebc 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -15,8 +15,7 @@ import ( // AddVote adds a vote on a specific proposal func (keeper Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress, options v1.WeightedVoteOptions, metadata string) error { // Check if proposal is in voting period. - store := keeper.storeService.OpenKVStore(ctx) - inVotingPeriod, err := store.Has(types.VotingPeriodProposalKey(proposalID)) + inVotingPeriod, err := keeper.VotingPeriodProposals.Has(ctx, proposalID) if err != nil { return err } diff --git a/x/gov/migrations/v2/store_test.go b/x/gov/migrations/v2/store_test.go index a7099eae26..8a4ec143d4 100644 --- a/x/gov/migrations/v2/store_test.go +++ b/x/gov/migrations/v2/store_test.go @@ -47,17 +47,17 @@ func TestMigrateStore(t *testing.T) { { "ProposalKey", v1.ProposalKey(proposalID), dummyValue, - types.ProposalKey(proposalID), dummyValue, + append(types.ProposalsKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...), dummyValue, }, { "ActiveProposalQueue", v1.ActiveProposalQueueKey(proposalID, now), dummyValue, - types.ActiveProposalQueueKey(proposalID, now), dummyValue, + activeProposalQueueKey(proposalID, now), dummyValue, }, { "InactiveProposalQueue", v1.InactiveProposalQueueKey(proposalID, now), dummyValue, - types.InactiveProposalQueueKey(proposalID, now), dummyValue, + inactiveProposalQueueKey(proposalID, now), dummyValue, }, { "ProposalIDKey", @@ -98,8 +98,8 @@ func TestMigrateStore(t *testing.T) { } } -// depositKey key of a specific deposit from the store. -// NOTE(tip): legacy, eventually remove me. +// TODO(tip): remove all the functions below once we delete the migrations + func depositKey(proposalID uint64, depositorAddr sdk.AccAddress) []byte { return append(append(types.DepositsKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...), address.MustLengthPrefix(depositorAddr.Bytes())...) } @@ -107,3 +107,12 @@ func depositKey(proposalID uint64, depositorAddr sdk.AccAddress) []byte { func voteKey(proposalID uint64, addr sdk.AccAddress) []byte { return append(append(types.VotesKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...), address.MustLengthPrefix(addr.Bytes())...) } + +func activeProposalQueueKey(proposalID uint64, endTime time.Time) []byte { + return append(append(types.ActiveProposalQueuePrefix, sdk.FormatTimeBytes(endTime)...), sdk.Uint64ToBigEndian(proposalID)...) +} + +// InactiveProposalQueueKey returns the key for a proposalID in the inactiveProposalQueue +func inactiveProposalQueueKey(proposalID uint64, endTime time.Time) []byte { + return append(append(types.InactiveProposalQueuePrefix, sdk.FormatTimeBytes(endTime)...), sdk.Uint64ToBigEndian(proposalID)...) +} diff --git a/x/gov/migrations/v4/store.go b/x/gov/migrations/v4/store.go index 0124b4167d..cb41e51222 100644 --- a/x/gov/migrations/v4/store.go +++ b/x/gov/migrations/v4/store.go @@ -112,7 +112,7 @@ func AddProposerAddressToProposal(ctx sdk.Context, storeService corestoretypes.K return fmt.Errorf("invalid proposer address : %s", proposals[proposalID]) } - bz := store.Get(types.ProposalKey(proposalID)) + bz := store.Get(append(types.ProposalsKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...)) var proposal govv1.Proposal if err := cdc.Unmarshal(bz, &proposal); err != nil { panic(err) @@ -131,7 +131,7 @@ func AddProposerAddressToProposal(ctx sdk.Context, storeService corestoretypes.K if err != nil { panic(err) } - store.Set(types.ProposalKey(proposal.Id), bz) + store.Set(append(types.ProposalsKeyPrefix, sdk.Uint64ToBigEndian(proposalID)...), bz) } return nil diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index 2fca0466e1..53ab369280 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -1,13 +1,7 @@ package types import ( - "encoding/binary" - "time" - "cosmossdk.io/collections" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/kv" ) const ( @@ -21,108 +15,14 @@ const ( RouterKey = ModuleName ) -// Keys for governance store -// Items are stored with the following key: values -// -// - 0x00: Proposal -// -// - 0x01: activeProposalID -// -// - 0x02: inactiveProposalID -// -// - 0x03: nextProposalID -// -// - 0x04: []byte{0x01} if proposalID is in the voting period -// -// - 0x10: Deposit -// -// - 0x20: Voter -// -// - 0x30: Params var ( - ProposalsKeyPrefix = collections.NewPrefix(0) - ActiveProposalQueuePrefix = []byte{0x01} - InactiveProposalQueuePrefix = []byte{0x02} - ProposalIDKey = collections.NewPrefix(3) - VotingPeriodProposalKeyPrefix = []byte{0x04} - - DepositsKeyPrefix = collections.NewPrefix(16) - - VotesKeyPrefix = collections.NewPrefix(32) - - // ParamsKey is the key to query all gov params - ParamsKey = collections.NewPrefix(48) - - // ConstitutionKey is the key string used to store the chain's constitution - ConstitutionKey = collections.NewPrefix(49) + ProposalsKeyPrefix = collections.NewPrefix(0) // ProposalsKeyPrefix stores the proposals raw bytes. + ActiveProposalQueuePrefix = collections.NewPrefix(1) // ActiveProposalQueuePrefix stores the active proposals. + InactiveProposalQueuePrefix = collections.NewPrefix(2) // InactiveProposalQueuePrefix stores the inactive proposals. + ProposalIDKey = collections.NewPrefix(3) // ProposalIDKey stores the sequence representing the next proposal ID. + VotingPeriodProposalKeyPrefix = collections.NewPrefix(4) // VotingPeriodProposalKeyPrefix stores which proposals are on voting period. + DepositsKeyPrefix = collections.NewPrefix(16) // DepositsKeyPrefix stores deposits. + VotesKeyPrefix = collections.NewPrefix(32) // VotesKeyPrefix stores the votes of proposals. + ParamsKey = collections.NewPrefix(48) // ParamsKey stores the module's params. + ConstitutionKey = collections.NewPrefix(49) // ConstitutionKey stores a chain's constitution. ) - -var lenTime = len(sdk.FormatTimeBytes(time.Now())) - -// GetProposalIDBytes returns the byte representation of the proposalID -func GetProposalIDBytes(proposalID uint64) (proposalIDBz []byte) { - proposalIDBz = make([]byte, 8) - binary.BigEndian.PutUint64(proposalIDBz, proposalID) - return -} - -// GetProposalIDFromBytes returns proposalID in uint64 format from a byte array -func GetProposalIDFromBytes(bz []byte) (proposalID uint64) { - return binary.BigEndian.Uint64(bz) -} - -// ProposalKey gets a specific proposal from the store -func ProposalKey(proposalID uint64) []byte { - return append(ProposalsKeyPrefix, GetProposalIDBytes(proposalID)...) -} - -// VotingPeriodProposalKey gets if a proposal is in voting period. -func VotingPeriodProposalKey(proposalID uint64) []byte { - return append(VotingPeriodProposalKeyPrefix, GetProposalIDBytes(proposalID)...) -} - -// ActiveProposalByTimeKey gets the active proposal queue key by endTime -func ActiveProposalByTimeKey(endTime time.Time) []byte { - return append(ActiveProposalQueuePrefix, sdk.FormatTimeBytes(endTime)...) -} - -// ActiveProposalQueueKey returns the key for a proposalID in the activeProposalQueue -func ActiveProposalQueueKey(proposalID uint64, endTime time.Time) []byte { - return append(ActiveProposalByTimeKey(endTime), GetProposalIDBytes(proposalID)...) -} - -// InactiveProposalByTimeKey gets the inactive proposal queue key by endTime -func InactiveProposalByTimeKey(endTime time.Time) []byte { - return append(InactiveProposalQueuePrefix, sdk.FormatTimeBytes(endTime)...) -} - -// InactiveProposalQueueKey returns the key for a proposalID in the inactiveProposalQueue -func InactiveProposalQueueKey(proposalID uint64, endTime time.Time) []byte { - return append(InactiveProposalByTimeKey(endTime), GetProposalIDBytes(proposalID)...) -} - -// Split keys function; used for iterators - -// SplitActiveProposalQueueKey split the active proposal key and returns the proposal id and endTime -func SplitActiveProposalQueueKey(key []byte) (proposalID uint64, endTime time.Time) { - return splitKeyWithTime(key) -} - -// SplitInactiveProposalQueueKey split the inactive proposal key and returns the proposal id and endTime -func SplitInactiveProposalQueueKey(key []byte) (proposalID uint64, endTime time.Time) { - return splitKeyWithTime(key) -} - -// private functions - -func splitKeyWithTime(key []byte) (proposalID uint64, endTime time.Time) { - kv.AssertKeyLength(key[1:], 8+lenTime) - - endTime, err := sdk.ParseTimeBytes(key[1 : 1+lenTime]) - if err != nil { - panic(err) - } - - proposalID = GetProposalIDFromBytes(key[1+lenTime:]) - return -} diff --git a/x/gov/types/keys_test.go b/x/gov/types/keys_test.go deleted file mode 100644 index 3cd66ac2dc..0000000000 --- a/x/gov/types/keys_test.go +++ /dev/null @@ -1,26 +0,0 @@ -package types - -import ( - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -func TestProposalKeys(t *testing.T) { - // key active proposal queue - now := time.Now() - key := ActiveProposalQueueKey(3, now) - proposalID, expTime := SplitActiveProposalQueueKey(key) - require.Equal(t, int(proposalID), 3) - require.True(t, now.Equal(expTime)) - - // key inactive proposal queue - key = InactiveProposalQueueKey(3, now) - proposalID, expTime = SplitInactiveProposalQueueKey(key) - require.Equal(t, int(proposalID), 3) - require.True(t, now.Equal(expTime)) - - // invalid key - require.Panics(t, func() { SplitInactiveProposalQueueKey([]byte("test")) }) -}