From 91d14c04accdd5ded86888514401f1cdd0949eb2 Mon Sep 17 00:00:00 2001 From: testinginprod <98415576+testinginprod@users.noreply.github.com> Date: Tue, 16 May 2023 11:55:08 +0200 Subject: [PATCH] refactor(gov)!: use collections for Proposal state management (part 1) (#16171) Co-authored-by: unknown unknown --- CHANGELOG.md | 3 + tests/e2e/gov/tx.go | 4 +- tests/integration/gov/genesis_test.go | 14 +- .../integration/gov/keeper/grpc_query_test.go | 4 +- tests/integration/gov/keeper/tally_test.go | 30 +-- x/gov/abci_test.go | 14 +- x/gov/genesis.go | 12 +- x/gov/keeper/common_test.go | 2 +- x/gov/keeper/deposit.go | 5 +- x/gov/keeper/deposit_test.go | 10 +- x/gov/keeper/grpc_query.go | 85 ++++----- x/gov/keeper/keeper.go | 8 +- x/gov/keeper/keeper_test.go | 2 +- x/gov/keeper/msg_server_test.go | 6 +- x/gov/keeper/proposal.go | 173 +----------------- x/gov/keeper/proposal_test.go | 71 +------ x/gov/simulation/operations.go | 20 +- x/gov/types/errors.go | 3 - x/gov/types/keys.go | 10 +- x/gov/types/keys_test.go | 8 +- 20 files changed, 130 insertions(+), 354 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75bb86d11e..dd6e4f6129 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -202,6 +202,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#16164](https://github.com/cosmos/cosmos-sdk/pull/16164) Use collections for vote state management: - Removed: types `VoteKey`, `VoteKeys` - Removed: keeper `IterateVotes`, `IterateAllVotes`, `GetVotes`, `GetVote`, `SetVote` +* (x/gov) [#16171](https://github.com/cosmos/cosmos-sdk/pull/16171) Use collections for proposal state management (part 1): + - Removed: keeper: `GetProposal`, `UnmarshalProposal`, `MarshalProposal`, `IterateProposal`, `GetProposal`, `GetProposalFiltered`, `GetProposals`, `GetProposalID`, `SetProposalID` + - Remove: errors unused errors * (sims) [#16155](https://github.com/cosmos/cosmos-sdk/pull/16155) * `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes. diff --git a/tests/e2e/gov/tx.go b/tests/e2e/gov/tx.go index e53d538ed5..cdf255344f 100644 --- a/tests/e2e/gov/tx.go +++ b/tests/e2e/gov/tx.go @@ -339,7 +339,7 @@ func (s *E2ETestSuite) TestNewCmdCancelProposal() { fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), }, - false, 17, + false, 1, }, } @@ -430,7 +430,7 @@ func (s *E2ETestSuite) TestNewCmdDeposit() { fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), }, - false, 2, + false, 1, }, { "deposit on existing proposal", diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index 68821ba65f..8135e98339 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -94,9 +94,9 @@ func TestImportExportQueues(t *testing.T) { assert.NilError(t, err) assert.Assert(t, votingStarted) - proposal1, err = s1.GovKeeper.GetProposal(ctx, proposalID1) + proposal1, err = s1.GovKeeper.Proposals.Get(ctx, proposalID1) assert.NilError(t, err) - proposal2, err = s1.GovKeeper.GetProposal(ctx, proposalID2) + proposal2, err = s1.GovKeeper.Proposals.Get(ctx, proposalID2) assert.NilError(t, err) assert.Assert(t, proposal1.Status == v1.StatusDepositPeriod) assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod) @@ -152,9 +152,9 @@ func TestImportExportQueues(t *testing.T) { ctx2 = ctx2.WithBlockTime(ctx2.BlockHeader().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod)) // Make sure that they are still in the DepositPeriod and VotingPeriod respectively - proposal1, err = s2.GovKeeper.GetProposal(ctx2, proposalID1) + proposal1, err = s2.GovKeeper.Proposals.Get(ctx2, proposalID1) assert.NilError(t, err) - proposal2, err = s2.GovKeeper.GetProposal(ctx2, proposalID2) + proposal2, err = s2.GovKeeper.Proposals.Get(ctx2, proposalID2) assert.NilError(t, err) assert.Assert(t, proposal1.Status == v1.StatusDepositPeriod) assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod) @@ -166,10 +166,10 @@ func TestImportExportQueues(t *testing.T) { err = gov.EndBlocker(ctx2, s2.GovKeeper) assert.NilError(t, err) - proposal1, err = s2.GovKeeper.GetProposal(ctx2, proposalID1) - assert.ErrorContains(t, err, "proposal 1 doesn't exist") + proposal1, err = s2.GovKeeper.Proposals.Get(ctx2, proposalID1) + assert.ErrorContains(t, err, "not found") - proposal2, err = s2.GovKeeper.GetProposal(ctx2, proposalID2) + proposal2, err = s2.GovKeeper.Proposals.Get(ctx2, proposalID2) assert.NilError(t, err) assert.Assert(t, proposal2.Status == v1.StatusRejected) } diff --git a/tests/integration/gov/keeper/grpc_query_test.go b/tests/integration/gov/keeper/grpc_query_test.go index 298270afaf..2d75ee69ce 100644 --- a/tests/integration/gov/keeper/grpc_query_test.go +++ b/tests/integration/gov/keeper/grpc_query_test.go @@ -104,7 +104,7 @@ func TestGRPCQueryTally(t *testing.T) { func() { proposal.Status = v1.StatusPassed app.GovKeeper.SetProposal(ctx, proposal) - proposal, _ = app.GovKeeper.GetProposal(ctx, proposal.Id) + proposal, _ = app.GovKeeper.Proposals.Get(ctx, proposal.Id) req = &v1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -225,7 +225,7 @@ func TestLegacyGRPCQueryTally(t *testing.T) { func() { proposal.Status = v1.StatusPassed app.GovKeeper.SetProposal(ctx, proposal) - proposal, _ = app.GovKeeper.GetProposal(ctx, proposal.Id) + proposal, _ = app.GovKeeper.Proposals.Get(ctx, proposal.Id) req = &v1beta1.QueryTallyResultRequest{ProposalId: proposal.Id} diff --git a/tests/integration/gov/keeper/tally_test.go b/tests/integration/gov/keeper/tally_test.go index 725241d671..5507a1de84 100644 --- a/tests/integration/gov/keeper/tally_test.go +++ b/tests/integration/gov/keeper/tally_test.go @@ -26,7 +26,7 @@ func TestTallyNoOneVotes(t *testing.T) { proposal.Status = v1.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -55,7 +55,7 @@ func TestTallyNoQuorum(t *testing.T) { err = app.GovKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "") assert.NilError(t, err) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, _, _ := app.GovKeeper.Tally(ctx, proposal) assert.Assert(t, passes == false) @@ -81,7 +81,7 @@ func TestTallyOnlyValidatorsAllYes(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -108,7 +108,7 @@ func TestTallyOnlyValidators51No(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, _, _ := app.GovKeeper.Tally(ctx, proposal) @@ -134,7 +134,7 @@ func TestTallyOnlyValidators51Yes(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -162,7 +162,7 @@ func TestTallyOnlyValidatorsVetoed(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], v1.NewNonSplitVoteOption(v1.OptionNoWithVeto), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -190,7 +190,7 @@ func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -218,7 +218,7 @@ func TestTallyOnlyValidatorsAbstainFails(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -246,7 +246,7 @@ func TestTallyOnlyValidatorsNonVoter(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddr1, v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddr2, v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -284,7 +284,7 @@ func TestTallyDelgatorOverride(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[3], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[4], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -321,7 +321,7 @@ func TestTallyDelgatorInherit(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -363,7 +363,7 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[3], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -406,7 +406,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -451,7 +451,7 @@ func TestTallyJailedValidator(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionNo), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) @@ -486,7 +486,7 @@ func TestTallyValidatorMultipleDelegations(t *testing.T) { assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) - proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) + proposal, ok := app.GovKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) passes, burnDeposits, tallyResults, _ := app.GovKeeper.Tally(ctx, proposal) diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go index 095d12c345..fd81c6de6c 100644 --- a/x/gov/abci_test.go +++ b/x/gov/abci_test.go @@ -298,7 +298,7 @@ func TestTickPassedVotingPeriod(t *testing.T) { require.True(t, activeQueue.Valid()) activeProposalID := types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err := suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err := suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) @@ -317,7 +317,7 @@ func TestTickPassedVotingPeriod(t *testing.T) { require.True(t, activeQueue.Valid()) activeProposalID = types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err = suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) require.False(t, proposal.Expedited) @@ -449,7 +449,7 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) { require.True(t, eventOk) require.Contains(t, attr[0].Value, "failed on execution") - proposal, err = suite.GovKeeper.GetProposal(ctx, proposal.Id) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, proposal.Id) require.Nil(t, err) require.Equal(t, v1.StatusFailed, proposal.Status) } @@ -549,7 +549,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { require.True(t, activeQueue.Valid()) activeProposalID := types.GetProposalIDFromBytes(activeQueue.Value()) - proposal, err := suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err := suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) @@ -569,7 +569,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { if tc.expeditedPasses { require.False(t, activeQueue.Valid()) - proposal, err = suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) require.Equal(t, v1.StatusPassed, proposal.Status) @@ -593,7 +593,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { activeProposalID = types.GetProposalIDFromBytes(activeQueue.Value()) activeQueue.Close() - proposal, err = suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) require.Equal(t, v1.StatusVotingPeriod, proposal.Status) require.False(t, proposal.Expedited) @@ -639,7 +639,7 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) { activeQueue, _ = suite.GovKeeper.ActiveProposalQueueIterator(ctx, ctx.BlockHeader().Time) require.False(t, activeQueue.Valid()) - proposal, err = suite.GovKeeper.GetProposal(ctx, activeProposalID) + proposal, err = suite.GovKeeper.Proposals.Get(ctx, activeProposalID) require.Nil(t, err) if tc.regularEventuallyPassing { diff --git a/x/gov/genesis.go b/x/gov/genesis.go index eb8b3bda93..18ad7043ca 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -14,7 +14,7 @@ import ( // InitGenesis - store genesis parameters func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k *keeper.Keeper, data *v1.GenesisState) { - err := k.SetProposalID(ctx, data.StartingProposalId) + err := k.ProposalID.Set(ctx, data.StartingProposalId) if err != nil { panic(err) } @@ -79,13 +79,17 @@ func InitGenesis(ctx sdk.Context, ak types.AccountKeeper, bk types.BankKeeper, k // ExportGenesis - output genesis parameters func ExportGenesis(ctx sdk.Context, k *keeper.Keeper) (*v1.GenesisState, error) { - startingProposalID, err := k.GetProposalID(ctx) + startingProposalID, err := k.ProposalID.Peek(ctx) if err != nil { return nil, err } - proposals, err := k.GetProposals(ctx) - if err != nil { + var proposals v1.Proposals + err = k.Proposals.Walk(ctx, nil, func(_ uint64, value v1.Proposal) (stop bool, err error) { + proposals = append(proposals, &value) + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { return nil, err } diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index c6738cc66f..ca9a7d84dd 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -100,7 +100,7 @@ func setupGovKeeper(t *testing.T) ( // Gov keeper initializations govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, acctKeeper, bankKeeper, stakingKeeper, distributionKeeper, msr, types.DefaultConfig(), govAcct.String()) - govKeeper.SetProposalID(ctx, 1) + require.NoError(t, govKeeper.ProposalID.Set(ctx, 1)) govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too. govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler) govKeeper.SetLegacyRouter(govRouter) diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index f0be3fe797..4ec4a5c9de 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -60,11 +60,8 @@ func (keeper Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb // Activates voting period when appropriate and returns true in that case, else returns false. func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr sdk.AccAddress, depositAmount sdk.Coins) (bool, error) { // Checks to see if proposal exists - proposal, err := keeper.GetProposal(ctx, proposalID) + proposal, err := keeper.Proposals.Get(ctx, proposalID) if err != nil { - if errors.IsOf(err, types.ErrProposalNotFound) { - return false, errors.Wrapf(types.ErrUnknownProposal, "%d", proposalID) - } return false, err } diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 3c6ab19c3d..3c6c8384c9 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -71,7 +71,7 @@ func TestDeposits(t *testing.T) { // Check no deposits at beginning _, err = govKeeper.Deposits.Get(ctx, collections.Join(proposalID, TestAddrs[1])) require.ErrorIs(t, err, collections.ErrNotFound) - proposal, err = govKeeper.GetProposal(ctx, proposalID) + proposal, err = govKeeper.Proposals.Get(ctx, proposalID) require.Nil(t, err) require.Nil(t, proposal.VotingStartTime) @@ -83,7 +83,7 @@ func TestDeposits(t *testing.T) { require.Nil(t, err) require.Equal(t, fourStake, sdk.NewCoins(deposit.Amount...)) require.Equal(t, TestAddrs[0].String(), deposit.Depositor) - proposal, err = govKeeper.GetProposal(ctx, proposalID) + proposal, err = govKeeper.Proposals.Get(ctx, proposalID) require.Nil(t, err) require.Equal(t, fourStake, sdk.NewCoins(proposal.TotalDeposit...)) require.Equal(t, addr0Initial.Sub(fourStake...), bankKeeper.GetAllBalances(ctx, TestAddrs[0])) @@ -96,7 +96,7 @@ func TestDeposits(t *testing.T) { require.Nil(t, err) require.Equal(t, fourStake.Add(fiveStake...), sdk.NewCoins(deposit.Amount...)) require.Equal(t, TestAddrs[0].String(), deposit.Depositor) - proposal, err = govKeeper.GetProposal(ctx, proposalID) + proposal, err = govKeeper.Proposals.Get(ctx, proposalID) require.Nil(t, err) require.Equal(t, fourStake.Add(fiveStake...), sdk.NewCoins(proposal.TotalDeposit...)) require.Equal(t, addr0Initial.Sub(fourStake...).Sub(fiveStake...), bankKeeper.GetAllBalances(ctx, TestAddrs[0])) @@ -109,13 +109,13 @@ func TestDeposits(t *testing.T) { require.Nil(t, err) require.Equal(t, TestAddrs[1].String(), deposit.Depositor) require.Equal(t, fourStake, sdk.NewCoins(deposit.Amount...)) - proposal, err = govKeeper.GetProposal(ctx, proposalID) + proposal, err = govKeeper.Proposals.Get(ctx, proposalID) require.Nil(t, err) require.Equal(t, fourStake.Add(fiveStake...).Add(fourStake...), sdk.NewCoins(proposal.TotalDeposit...)) require.Equal(t, addr1Initial.Sub(fourStake...), bankKeeper.GetAllBalances(ctx, TestAddrs[1])) // Check that proposal moved to voting period - proposal, err = govKeeper.GetProposal(ctx, proposalID) + proposal, err = govKeeper.Proposals.Get(ctx, proposalID) require.Nil(t, err) require.True(t, proposal.VotingStartTime.Equal(ctx.BlockHeader().Time)) diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go index b6abd174d4..286d2dc1cd 100644 --- a/x/gov/keeper/grpc_query.go +++ b/x/gov/keeper/grpc_query.go @@ -10,13 +10,9 @@ import ( "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" - "cosmossdk.io/store/prefix" - - "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" v3 "github.com/cosmos/cosmos-sdk/x/gov/migrations/v3" - "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" ) @@ -47,9 +43,9 @@ func (q queryServer) Proposal(ctx context.Context, req *v1.QueryProposalRequest) return nil, status.Error(codes.InvalidArgument, "proposal id can not be 0") } - proposal, err := q.k.GetProposal(ctx, req.ProposalId) + proposal, err := q.k.Proposals.Get(ctx, req.ProposalId) if err != nil { - if errors.IsOf(err, types.ErrProposalNotFound) { + if errors.IsOf(err, collections.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId) } return nil, status.Error(codes.Internal, err.Error()) @@ -60,53 +56,46 @@ func (q queryServer) Proposal(ctx context.Context, req *v1.QueryProposalRequest) // Proposals implements the Query/Proposals gRPC method func (q queryServer) Proposals(ctx context.Context, req *v1.QueryProposalsRequest) (*v1.QueryProposalsResponse, error) { - store := q.k.storeService.OpenKVStore(ctx) - proposalStore := prefix.NewStore(runtime.KVStoreAdapter(store), types.ProposalsKeyPrefix) + var filteredProposals []*v1.Proposal + _, pageRes, err := query.CollectionFilteredPaginate(ctx, q.k.Proposals, req.Pagination, func(key uint64, p v1.Proposal) (bool, error) { + matchVoter, matchDepositor, matchStatus := true, true, true - filteredProposals, pageRes, err := query.GenericFilteredPaginate( - q.k.cdc, - proposalStore, - req.Pagination, - func(key []byte, p *v1.Proposal) (*v1.Proposal, error) { - matchVoter, matchDepositor, matchStatus := true, true, true + // match status (if supplied/valid) + if v1.ValidProposalStatus(req.ProposalStatus) { + matchStatus = p.Status == req.ProposalStatus + } - // match status (if supplied/valid) - if v1.ValidProposalStatus(req.ProposalStatus) { - matchStatus = p.Status == req.ProposalStatus + // match voter address (if supplied) + if len(req.Voter) > 0 { + voter, err := q.k.authKeeper.StringToBytes(req.Voter) + if err != nil { + return false, err } - // match voter address (if supplied) - if len(req.Voter) > 0 { - voter, err := q.k.authKeeper.StringToBytes(req.Voter) - if err != nil { - return nil, err - } + has, err := q.k.Votes.Has(ctx, collections.Join(p.Id, sdk.AccAddress(voter))) + // if no error, vote found, matchVoter = true + matchVoter = err == nil && has + } - has, err := q.k.Votes.Has(ctx, collections.Join(p.Id, sdk.AccAddress(voter))) - // if no error, vote found, matchVoter = true - matchVoter = err == nil && has + // match depositor (if supplied) + if len(req.Depositor) > 0 { + depositor, err := q.k.authKeeper.StringToBytes(req.Depositor) + if err != nil { + return false, err } + has, err := q.k.Deposits.Has(ctx, collections.Join(p.Id, sdk.AccAddress(depositor))) + // if no error, deposit found, matchDepositor = true + matchDepositor = err == nil && has + } - // match depositor (if supplied) - if len(req.Depositor) > 0 { - depositor, err := q.k.authKeeper.StringToBytes(req.Depositor) - if err != nil { - return nil, err - } - has, err := q.k.Deposits.Has(ctx, collections.Join(p.Id, sdk.AccAddress(depositor))) - // if no error, deposit found, matchDepositor = true - matchDepositor = err == nil && has - } - - if matchVoter && matchDepositor && matchStatus { - return p, nil - } - - return nil, nil - }, func() *v1.Proposal { - return &v1.Proposal{} - }) - if err != nil { + // if all match, append to results + if matchVoter && matchDepositor && matchStatus { + filteredProposals = append(filteredProposals, &p) + } + // continue to next item, do not include because we're appending results above. + return false, nil + }) + if err != nil && !errors.IsOf(err, collections.ErrInvalidIterator) { return nil, status.Error(codes.Internal, err.Error()) } @@ -259,9 +248,9 @@ func (q queryServer) TallyResult(ctx context.Context, req *v1.QueryTallyResultRe return nil, status.Error(codes.InvalidArgument, "proposal id can not be 0") } - proposal, err := q.k.GetProposal(ctx, req.ProposalId) + proposal, err := q.k.Proposals.Get(ctx, req.ProposalId) if err != nil { - if errors.IsOf(err, types.ErrProposalNotFound) { + if errors.IsOf(err, collections.ErrNotFound) { return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId) } return nil, status.Error(codes.Internal, err.Error()) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 7bfcd83263..7a390c2ca8 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -55,6 +55,8 @@ type Keeper struct { 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] } // GetAuthority returns the x/gov module's authority. @@ -103,6 +105,8 @@ func NewKeeper( 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)), } schema, err := sb.Build() if err != nil { @@ -204,7 +208,7 @@ func (k Keeper) IterateActiveProposalsQueue(ctx context.Context, endTime time.Ti defer iterator.Close() for ; iterator.Valid(); iterator.Next() { proposalID, _ := types.SplitActiveProposalQueueKey(iterator.Key()) - proposal, err := k.GetProposal(ctx, proposalID) + proposal, err := k.Proposals.Get(ctx, proposalID) if err != nil { return err } @@ -232,7 +236,7 @@ func (k Keeper) IterateInactiveProposalsQueue(ctx context.Context, endTime time. defer iterator.Close() for ; iterator.Valid(); iterator.Next() { proposalID, _ := types.SplitInactiveProposalQueueKey(iterator.Key()) - proposal, err := k.GetProposal(ctx, proposalID) + proposal, err := k.Proposals.Get(ctx, proposalID) if err != nil { return err } diff --git a/x/gov/keeper/keeper_test.go b/x/gov/keeper/keeper_test.go index 2f4fc4a681..0e403519b6 100644 --- a/x/gov/keeper/keeper_test.go +++ b/x/gov/keeper/keeper_test.go @@ -131,7 +131,7 @@ func TestProposalQueues(t *testing.T) { govKeeper.ActivateVotingPeriod(ctx, proposal) - proposal, err = govKeeper.GetProposal(ctx, proposal.Id) + proposal, err = govKeeper.Proposals.Get(ctx, proposal.Id) require.Nil(t, err) activeIterator, _ := govKeeper.ActiveProposalQueueIterator(ctx, *proposal.VotingEndTime) diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index 9769636a66..bb7e114051 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -256,7 +256,7 @@ func (suite *KeeperTestSuite) TestCancelProposalReq() { }, depositor: proposer, expErr: true, - expErrMsg: "proposal is not found", + expErrMsg: "not found", }, "valid proposal but invalid proposer": { preRun: func() uint64 { @@ -752,7 +752,7 @@ func (suite *KeeperTestSuite) TestDepositReq() { depositor: proposer, deposit: coins, expErr: true, - expErrMsg: "0: unknown proposal", + expErrMsg: "not found", }, "empty depositor": { preRun: func() uint64 { @@ -1338,7 +1338,7 @@ func (suite *KeeperTestSuite) TestLegacyMsgDeposit() { depositor: proposer, deposit: coins, expErr: true, - expErrMsg: "unknown proposal", + expErrMsg: "not found", }, "empty depositer": { preRun: func() uint64 { diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index b2cc297e15..032e800cd7 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -6,13 +6,7 @@ import ( "fmt" "time" - "cosmossdk.io/collections" - errorsmod "cosmossdk.io/errors" - storetypes "cosmossdk.io/store/types" - - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/runtime" 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" @@ -86,7 +80,7 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met } - proposalID, err := keeper.GetProposalID(ctx) + proposalID, err := keeper.ProposalID.Next(ctx) if err != nil { return v1.Proposal{}, err } @@ -106,7 +100,6 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met keeper.SetProposal(ctx, proposal) keeper.InsertInactiveProposalQueue(ctx, proposalID, *proposal.DepositEndTime) - keeper.SetProposalID(ctx, proposalID+1) // called right after a proposal is submitted keeper.Hooks().AfterProposalSubmission(ctx, proposalID) @@ -125,7 +118,7 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met // CancelProposal will cancel proposal before the voting period ends func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer string) error { sdkCtx := sdk.UnwrapSDKContext(ctx) - proposal, err := keeper.GetProposal(ctx, proposalID) + proposal, err := keeper.Proposals.Get(ctx, proposalID) if err != nil { return err } @@ -184,54 +177,29 @@ func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, prop return nil } -// GetProposal gets a proposal from store by ProposalID. -func (keeper Keeper) GetProposal(ctx context.Context, proposalID uint64) (proposal v1.Proposal, err error) { - store := keeper.storeService.OpenKVStore(ctx) - - bz, err := store.Get(types.ProposalKey(proposalID)) - if err != nil { - return - } - - if bz == nil { - return proposal, types.ErrProposalNotFound.Wrapf("proposal %d doesn't exist", proposalID) - } - - if err = keeper.UnmarshalProposal(bz, &proposal); err != nil { - return - } - - return proposal, nil -} - // SetProposal sets a proposal to store. func (keeper Keeper) SetProposal(ctx context.Context, proposal v1.Proposal) error { - bz, err := keeper.MarshalProposal(proposal) - if err != nil { - return err - } - store := keeper.storeService.OpenKVStore(ctx) if proposal.Status == v1.StatusVotingPeriod { - err = store.Set(types.VotingPeriodProposalKey(proposal.Id), []byte{1}) + err := store.Set(types.VotingPeriodProposalKey(proposal.Id), []byte{1}) if err != nil { return err } } else { - err = store.Delete(types.VotingPeriodProposalKey(proposal.Id)) + err := store.Delete(types.VotingPeriodProposalKey(proposal.Id)) if err != nil { return err } } - return store.Set(types.ProposalKey(proposal.Id), bz) + return keeper.Proposals.Set(ctx, proposal.Id, proposal) } // DeleteProposal deletes a proposal from store. func (keeper Keeper) DeleteProposal(ctx context.Context, proposalID uint64) error { store := keeper.storeService.OpenKVStore(ctx) - proposal, err := keeper.GetProposal(ctx, proposalID) + proposal, err := keeper.Proposals.Get(ctx, proposalID) if err != nil { return err } @@ -254,116 +222,7 @@ func (keeper Keeper) DeleteProposal(ctx context.Context, proposalID uint64) erro } } - return store.Delete(types.ProposalKey(proposalID)) -} - -// IterateProposals iterates over all the proposals and performs a callback function. -func (keeper Keeper) IterateProposals(ctx context.Context, cb func(proposal v1.Proposal) error) error { - store := keeper.storeService.OpenKVStore(ctx) - iterator := storetypes.KVStorePrefixIterator(runtime.KVStoreAdapter(store), types.ProposalsKeyPrefix) - defer iterator.Close() - - for ; iterator.Valid(); iterator.Next() { - var proposal v1.Proposal - err := keeper.UnmarshalProposal(iterator.Value(), &proposal) - if err != nil { - return err - } - - err = cb(proposal) - // exit early without error if cb returns ErrStopIterating - if errorsmod.IsOf(err, errorsmod.ErrStopIterating) { - return nil - } else if err != nil { - return err - } - } - - return nil -} - -// GetProposals returns all the proposals from store -func (keeper Keeper) GetProposals(ctx context.Context) (proposals v1.Proposals, err error) { - err = keeper.IterateProposals(ctx, func(proposal v1.Proposal) error { - proposals = append(proposals, &proposal) - return nil - }) - return -} - -// GetProposalsFiltered retrieves proposals filtered by a given set of params which -// include pagination parameters along with voter and depositor addresses and a -// proposal status. The voter address will filter proposals by whether or not -// that address has voted on proposals. The depositor address will filter proposals -// by whether or not that address has deposited to them. Finally, status will filter -// proposals by status. -// -// NOTE: If no filters are provided, all proposals will be returned in paginated -// form. -func (keeper Keeper) GetProposalsFiltered(ctx context.Context, params v1.QueryProposalsParams) (v1.Proposals, error) { - proposals, err := keeper.GetProposals(ctx) - if err != nil { - return nil, err - } - - filteredProposals := make([]*v1.Proposal, 0, len(proposals)) - - for _, p := range proposals { - matchVoter, matchDepositor, matchStatus := true, true, true - - // match status (if supplied/valid) - if v1.ValidProposalStatus(params.ProposalStatus) { - matchStatus = p.Status == params.ProposalStatus - } - - // match voter address (if supplied) - if len(params.Voter) > 0 { - has, err := keeper.Votes.Has(ctx, collections.Join(p.Id, params.Voter)) - // if no error, vote found, matchVoter = true - matchVoter = err == nil && has - } - - // match depositor (if supplied) - if len(params.Depositor) > 0 { - _, err = keeper.Deposits.Get(ctx, collections.Join(p.Id, params.Depositor)) - // if no error, deposit found, matchDepositor = true - matchDepositor = err == nil - } - - if matchVoter && matchDepositor && matchStatus { - filteredProposals = append(filteredProposals, p) - } - } - - start, end := client.Paginate(len(filteredProposals), params.Page, params.Limit, 100) - if start < 0 || end < 0 { - filteredProposals = []*v1.Proposal{} - } else { - filteredProposals = filteredProposals[start:end] - } - - return filteredProposals, nil -} - -// GetProposalID gets the highest proposal ID -func (keeper Keeper) GetProposalID(ctx context.Context) (proposalID uint64, err error) { - store := keeper.storeService.OpenKVStore(ctx) - bz, err := store.Get(types.ProposalIDKey) - if err != nil { - return 0, err - } - if bz == nil { - return 0, errorsmod.Wrap(types.ErrInvalidGenesis, "initial proposal ID hasn't been set") - } - - proposalID = types.GetProposalIDFromBytes(bz) - return proposalID, nil -} - -// SetProposalID sets the new proposal ID to the store -func (keeper Keeper) SetProposalID(ctx context.Context, proposalID uint64) error { - store := keeper.storeService.OpenKVStore(ctx) - return store.Set(types.ProposalIDKey, types.GetProposalIDBytes(proposalID)) + return keeper.Proposals.Remove(ctx, proposalID) } // ActivateVotingPeriod activates the voting period of a proposal @@ -397,21 +256,3 @@ func (keeper Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Propo return keeper.InsertActiveProposalQueue(ctx, proposal.Id, *proposal.VotingEndTime) } - -// MarshalProposal marshals the proposal and returns binary encoded bytes. -func (keeper Keeper) MarshalProposal(proposal v1.Proposal) ([]byte, error) { - bz, err := keeper.cdc.Marshal(&proposal) - if err != nil { - return nil, err - } - return bz, nil -} - -// UnmarshalProposal unmarshals the proposal. -func (keeper Keeper) UnmarshalProposal(bz []byte, proposal *v1.Proposal) error { - err := keeper.cdc.Unmarshal(bz, proposal) - if err != nil { - return err - } - return nil -} diff --git a/x/gov/keeper/proposal_test.go b/x/gov/keeper/proposal_test.go index 11ac24e578..72ac648948 100644 --- a/x/gov/keeper/proposal_test.go +++ b/x/gov/keeper/proposal_test.go @@ -2,10 +2,8 @@ package keeper_test import ( "errors" - "fmt" "strings" "testing" - "time" "cosmossdk.io/collections" @@ -18,6 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) +// TODO(tip): remove this func (suite *KeeperTestSuite) TestGetSetProposal() { testCases := map[string]struct { expedited bool @@ -35,12 +34,13 @@ func (suite *KeeperTestSuite) TestGetSetProposal() { proposalID := proposal.Id suite.govKeeper.SetProposal(suite.ctx, proposal) - gotProposal, err := suite.govKeeper.GetProposal(suite.ctx, proposalID) + gotProposal, err := suite.govKeeper.Proposals.Get(suite.ctx, proposalID) suite.Require().Nil(err) suite.Require().Equal(proposal, gotProposal) } } +// TODO(tip): remove this func (suite *KeeperTestSuite) TestDeleteProposal() { testCases := map[string]struct { expedited bool @@ -53,7 +53,7 @@ func (suite *KeeperTestSuite) TestDeleteProposal() { for _, tc := range testCases { // delete non-existing proposal - suite.Require().ErrorIs(suite.govKeeper.DeleteProposal(suite.ctx, 10), types.ErrProposalNotFound.Wrapf("proposal_id %d", 10)) + suite.Require().ErrorIs(suite.govKeeper.DeleteProposal(suite.ctx, 10), collections.ErrNotFound) tp := TestProposal proposal, err := suite.govKeeper.SubmitProposal(suite.ctx, tp, "", "test", "summary", suite.addrs[0], tc.expedited) @@ -84,7 +84,7 @@ func (suite *KeeperTestSuite) TestActivateVotingPeriod() { suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal) - proposal, err = suite.govKeeper.GetProposal(suite.ctx, proposal.Id) + proposal, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal.Id) suite.Require().Nil(err) suite.Require().True(proposal.VotingStartTime.Equal(suite.ctx.BlockHeader().Time)) @@ -120,7 +120,7 @@ func (suite *KeeperTestSuite) TestDeleteProposalInVotingPeriod() { suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal) - proposal, err = suite.govKeeper.GetProposal(suite.ctx, proposal.Id) + proposal, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal.Id) suite.Require().Nil(err) suite.Require().True(proposal.VotingStartTime.Equal(suite.ctx.BlockHeader().Time)) @@ -185,63 +185,6 @@ func (suite *KeeperTestSuite) TestSubmitProposal() { } } -func (suite *KeeperTestSuite) TestGetProposalsFiltered() { - proposalID := uint64(1) - status := []v1.ProposalStatus{v1.StatusDepositPeriod, v1.StatusVotingPeriod} - - addr1 := suite.addrs[1] - - for _, s := range status { - for i := 0; i < 50; i++ { - p, err := v1.NewProposal(TestProposal, proposalID, time.Now(), time.Now(), "metadata", "title", "summary", suite.addrs[0], false) - suite.Require().NoError(err) - - p.Status = s - - if i%2 == 0 { - d := v1.NewDeposit(proposalID, addr1, nil) - v := v1.NewVote(proposalID, addr1, v1.NewNonSplitVoteOption(v1.OptionYes), "") - suite.govKeeper.SetDeposit(suite.ctx, d) - require.NoError(suite.T(), suite.govKeeper.Votes.Set(suite.ctx, collections.Join(proposalID, addr1), v)) - } - - suite.govKeeper.SetProposal(suite.ctx, p) - proposalID++ - } - } - - testCases := []struct { - params v1.QueryProposalsParams - expectedNumResults int - }{ - {v1.NewQueryProposalsParams(1, 50, v1.StatusNil, nil, nil), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusDepositPeriod, nil, nil), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusVotingPeriod, nil, nil), 50}, - {v1.NewQueryProposalsParams(1, 25, v1.StatusNil, nil, nil), 25}, - {v1.NewQueryProposalsParams(2, 25, v1.StatusNil, nil, nil), 25}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusRejected, nil, nil), 0}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusNil, addr1, nil), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusNil, nil, addr1), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusNil, addr1, addr1), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusDepositPeriod, addr1, addr1), 25}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusDepositPeriod, nil, nil), 50}, - {v1.NewQueryProposalsParams(1, 50, v1.StatusVotingPeriod, nil, nil), 50}, - } - - for i, tc := range testCases { - suite.Run(fmt.Sprintf("Test Case %d", i), func() { - proposals, _ := suite.govKeeper.GetProposalsFiltered(suite.ctx, tc.params) - suite.Require().Len(proposals, tc.expectedNumResults) - - for _, p := range proposals { - if v1.ValidProposalStatus(tc.params.ProposalStatus) { - suite.Require().Equal(tc.params.ProposalStatus, p.Status) - } - } - }) - } -} - func (suite *KeeperTestSuite) TestCancelProposal() { govAcct := suite.govKeeper.GetGovernanceAccount(suite.ctx).GetAddress().String() tp := v1beta1.TextProposal{Title: "title", Description: "description"} @@ -254,7 +197,7 @@ func (suite *KeeperTestSuite) TestCancelProposal() { proposal2Resp, err := suite.govKeeper.SubmitProposal(suite.ctx, []sdk.Msg{prop}, "", "title", "summary", suite.addrs[1], true) proposal2ID := proposal2Resp.Id makeProposalPass := func() { - proposal2, err := suite.govKeeper.GetProposal(suite.ctx, proposal2ID) + proposal2, err := suite.govKeeper.Proposals.Get(suite.ctx, proposal2ID) suite.Require().Nil(err) proposal2.Status = v1.ProposalStatus_PROPOSAL_STATUS_PASSED diff --git a/x/gov/simulation/operations.go b/x/gov/simulation/operations.go index 8402ed6fe9..5fa12549c3 100644 --- a/x/gov/simulation/operations.go +++ b/x/gov/simulation/operations.go @@ -1,10 +1,13 @@ package simulation import ( + "errors" "math" "math/rand" "time" + "cosmossdk.io/collections" + sdkmath "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/baseapp" @@ -276,7 +279,7 @@ func simulateMsgSubmitProposal( opMsg := simtypes.NewOperationMsg(msg, true, "") // get the submitted proposal ID - proposalID, err := k.GetProposalID(ctx) + proposalID, err := k.ProposalID.Peek(ctx) if err != nil { return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "unable to generate proposalID"), nil, err } @@ -595,9 +598,16 @@ func randomDeposit( return sdk.Coins{sdk.NewCoin(denom, amount)}, false, nil } -// randomProposal +// randomProposal returns a random proposal stored in state func randomProposal(r *rand.Rand, k *keeper.Keeper, ctx sdk.Context) *v1.Proposal { - proposals, _ := k.GetProposals(ctx) + var proposals []*v1.Proposal + err := k.Proposals.Walk(ctx, nil, func(key uint64, value v1.Proposal) (stop bool, err error) { + proposals = append(proposals, &value) + return false, nil + }) + if err != nil && !errors.Is(err, collections.ErrInvalidIterator) { + panic(err) + } if len(proposals) == 0 { return nil } @@ -610,7 +620,7 @@ func randomProposal(r *rand.Rand, k *keeper.Keeper, ctx sdk.Context) *v1.Proposa // that matches a given Status. // It does not provide a default ID. func randomProposalID(r *rand.Rand, k *keeper.Keeper, ctx sdk.Context, status v1.ProposalStatus) (proposalID uint64, found bool) { - proposalID, _ = k.GetProposalID(ctx) + proposalID, _ = k.ProposalID.Peek(ctx) switch { case proposalID > initialProposalID: @@ -623,7 +633,7 @@ func randomProposalID(r *rand.Rand, k *keeper.Keeper, ctx sdk.Context, status v1 initialProposalID = proposalID } - proposal, err := k.GetProposal(ctx, proposalID) + proposal, err := k.Proposals.Get(ctx, proposalID) if err != nil || proposal.Status != status { return proposalID, false } diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 11bbb8fef5..fa584da934 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -6,7 +6,6 @@ import ( // x/gov module sentinel errors var ( - ErrUnknownProposal = errors.Register(ModuleName, 2, "unknown proposal") ErrInactiveProposal = errors.Register(ModuleName, 3, "inactive proposal") ErrAlreadyActiveProposal = errors.Register(ModuleName, 4, "proposal already active") // Errors 5 & 6 are legacy errors related to v1beta1.Proposal. @@ -21,9 +20,7 @@ var ( ErrInvalidSigner = errors.Register(ModuleName, 13, "expected gov account as only signer for proposal message") 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") 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") ) diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index fd1a290310..2fca0466e1 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -40,10 +40,10 @@ const ( // // - 0x30: Params var ( - ProposalsKeyPrefix = []byte{0x00} + ProposalsKeyPrefix = collections.NewPrefix(0) ActiveProposalQueuePrefix = []byte{0x01} InactiveProposalQueuePrefix = []byte{0x02} - ProposalIDKey = []byte{0x03} + ProposalIDKey = collections.NewPrefix(3) VotingPeriodProposalKeyPrefix = []byte{0x04} DepositsKeyPrefix = collections.NewPrefix(16) @@ -103,12 +103,6 @@ func InactiveProposalQueueKey(proposalID uint64, endTime time.Time) []byte { // Split keys function; used for iterators -// SplitProposalKey split the proposal key and returns the proposal id -func SplitProposalKey(key []byte) (proposalID uint64) { - kv.AssertKeyLength(key[1:], 8) - return GetProposalIDFromBytes(key[1:]) -} - // 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) diff --git a/x/gov/types/keys_test.go b/x/gov/types/keys_test.go index bf8d098d9f..3cd66ac2dc 100644 --- a/x/gov/types/keys_test.go +++ b/x/gov/types/keys_test.go @@ -8,14 +8,9 @@ import ( ) func TestProposalKeys(t *testing.T) { - // key proposal - key := ProposalKey(1) - proposalID := SplitProposalKey(key) - require.Equal(t, int(proposalID), 1) - // key active proposal queue now := time.Now() - key = ActiveProposalQueueKey(3, now) + key := ActiveProposalQueueKey(3, now) proposalID, expTime := SplitActiveProposalQueueKey(key) require.Equal(t, int(proposalID), 3) require.True(t, now.Equal(expTime)) @@ -27,6 +22,5 @@ func TestProposalKeys(t *testing.T) { require.True(t, now.Equal(expTime)) // invalid key - require.Panics(t, func() { SplitProposalKey([]byte("test")) }) require.Panics(t, func() { SplitInactiveProposalQueueKey([]byte("test")) }) }