diff --git a/tests/integration/gov/abci_test.go b/tests/integration/gov/abci_test.go index 7ff4bd12a4a2..dd0829ba20ac 100644 --- a/tests/integration/gov/abci_test.go +++ b/tests/integration/gov/abci_test.go @@ -34,7 +34,7 @@ func TestUnregisteredProposal_InactiveProposalFails(t *testing.T) { }, 1, startTime, startTime, "", "Unsupported proposal", "Unsupported proposal", addrs[0], v1.ProposalType_PROPOSAL_TYPE_STANDARD) require.NoError(t, err) - err = suite.GovKeeper.SetProposal(ctx, proposal) + err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal) require.NoError(t, err) // manually set proposal in inactive proposal queue @@ -62,7 +62,7 @@ func TestUnregisteredProposal_ActiveProposalFails(t *testing.T) { proposal.Status = v1.StatusVotingPeriod proposal.VotingEndTime = &endTime - err = suite.GovKeeper.SetProposal(ctx, proposal) + err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal) require.NoError(t, err) // manually set proposal in active proposal queue diff --git a/tests/integration/gov/keeper/grpc_query_test.go b/tests/integration/gov/keeper/grpc_query_test.go index 46750fe4d169..edd583c4dd9e 100644 --- a/tests/integration/gov/keeper/grpc_query_test.go +++ b/tests/integration/gov/keeper/grpc_query_test.go @@ -38,7 +38,7 @@ func TestLegacyGRPCQueryTally(t *testing.T) { proposal, err := f.govKeeper.SubmitProposal(ctx, TestProposal, "", "test", "description", addrs[0], v1.ProposalType_PROPOSAL_TYPE_STANDARD) assert.NilError(t, err) proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposal.Id, addrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) diff --git a/tests/integration/gov/keeper/tally_test.go b/tests/integration/gov/keeper/tally_test.go index 3449c7b8ea33..47b66fc17c6f 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) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) proposal, ok := f.govKeeper.Proposals.Get(ctx, proposalID) assert.Assert(t, ok) @@ -53,7 +53,7 @@ func TestTallyNoQuorum(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) err = f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "") assert.NilError(t, err) @@ -79,7 +79,7 @@ func TestTallyOnlyValidatorsAllYes(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -108,7 +108,7 @@ func TestTallyOnlyValidators51No(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -135,7 +135,7 @@ func TestTallyOnlyValidators51Yes(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -163,7 +163,7 @@ func TestTallyOnlyValidatorsVetoed(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -192,7 +192,7 @@ func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -221,7 +221,7 @@ func TestTallyOnlyValidatorsAbstainFails(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -251,7 +251,7 @@ func TestTallyOnlyValidatorsNonVoter(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddr1, v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, valAccAddr2, v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -288,7 +288,7 @@ func TestTallyDelgatorOverride(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[2], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -327,7 +327,7 @@ func TestTallyDelgatorInherit(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionNo), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -369,7 +369,7 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionYes), "")) @@ -414,7 +414,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -460,7 +460,7 @@ func TestTallyJailedValidator(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) @@ -496,7 +496,7 @@ func TestTallyValidatorMultipleDelegations(t *testing.T) { assert.NilError(t, err) proposalID := proposal.Id proposal.Status = v1.StatusVotingPeriod - err = f.govKeeper.SetProposal(ctx, proposal) + err = f.govKeeper.Proposals.Set(ctx, proposal.Id, proposal) assert.NilError(t, err) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "")) assert.NilError(t, f.govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionNo), "")) diff --git a/tests/sims/gov/operations_test.go b/tests/sims/gov/operations_test.go index f1b92a5cf363..d5469829c827 100644 --- a/tests/sims/gov/operations_test.go +++ b/tests/sims/gov/operations_test.go @@ -218,7 +218,7 @@ func TestSimulateMsgCancelProposal(t *testing.T) { proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "title", "summary", proposer, v1.ProposalType_PROPOSAL_TYPE_STANDARD) require.NoError(t, err) - err = suite.GovKeeper.SetProposal(ctx, proposal) + err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal) require.NoError(t, err) // execute operation @@ -261,7 +261,7 @@ func TestSimulateMsgDeposit(t *testing.T) { proposal, err := v1.NewProposal([]sdk.Msg{contentMsg}, 1, submitTime, submitTime.Add(*depositPeriod), "", "text proposal", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), v1.ProposalType_PROPOSAL_TYPE_STANDARD) require.NoError(t, err) - err = suite.GovKeeper.SetProposal(ctx, proposal) + err = suite.GovKeeper.Proposals.Set(ctx, proposal.Id, proposal) require.NoError(t, err) // execute operation diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 9c4539ce1a95..cc844666388d 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* [#19349](https://github.com/cosmos/cosmos-sdk/pull/19349) Simplify state management in `x/gov`. Note `k.VotingPeriodProposals` and `k.SetProposal` are no longer needed and have been removed. * [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) All functions that were taking an expedited bool parameter now take a `ProposalType` parameter instead. * [#17496](https://github.com/cosmos/cosmos-sdk/pull/17496) in `x/gov/types/v1beta1/vote.go` `NewVote` was removed, constructing the struct is required for this type. * [#19101](https://github.com/cosmos/cosmos-sdk/pull/19101) Move `QueryProposalVotesParams` and `QueryVoteParams` from the `types/v1` package to `utils` and remove unused `querier.go` file. diff --git a/x/gov/README.md b/x/gov/README.md index 9b0085dc8bd2..665fe7c69230 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -433,8 +433,6 @@ We will use one KVStore `Governance` to store four mappings: doing a range query on `proposalID:addresses`. * A mapping from `ParamsKey|'Params'` to `Params`. This map allows to query all x/gov params. -* A mapping from `VotingPeriodProposalKeyPrefix|proposalID` to a single byte. This allows - us to know if a proposal is in the voting period or not with very low gas cost. For pseudocode purposes, here are the two function we will use to read or write in stores: diff --git a/x/gov/abci.go b/x/gov/abci.go index 70074849a4d5..6f8c44f61d8f 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -245,8 +245,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { proposal.FinalTallyResult = &tallyResults - err = keeper.SetProposal(ctx, proposal) - if err != nil { + if err = keeper.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return false, err } @@ -307,7 +306,7 @@ func failUnsupportedProposal( proposal.FailedReason = fmt.Sprintf("proposal failed because it cannot be processed by gov: %s", errMsg) proposal.Messages = nil // clear out the messages - if err := keeper.SetProposal(ctx, proposal); err != nil { + if err := keeper.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return err } diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 156050299fbf..65fa70b4b216 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -68,8 +68,7 @@ func InitGenesis(ctx context.Context, ak types.AccountKeeper, bk types.BankKeepe panic(err) } } - err := k.SetProposal(ctx, *proposal) - if err != nil { + if err := k.Proposals.Set(ctx, proposal.Id, *proposal); err != nil { panic(err) } } diff --git a/x/gov/keeper/deposit.go b/x/gov/keeper/deposit.go index be79f0826373..34512ae253bf 100644 --- a/x/gov/keeper/deposit.go +++ b/x/gov/keeper/deposit.go @@ -141,8 +141,7 @@ func (k Keeper) AddDeposit(ctx context.Context, proposalID uint64, depositorAddr // Update proposal proposal.TotalDeposit = sdk.NewCoins(proposal.TotalDeposit...).Add(depositAmount...) - err = k.SetProposal(ctx, proposal) - if err != nil { + if err = k.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return false, err } diff --git a/x/gov/keeper/grpc_query_test.go b/x/gov/keeper/grpc_query_test.go index 9ecd4f0f44d0..f0fac0d4df6a 100644 --- a/x/gov/keeper/grpc_query_test.go +++ b/x/gov/keeper/grpc_query_test.go @@ -297,7 +297,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryProposals() { "request with filter of deposit address", func() { testProposals[1].Status = v1.StatusVotingPeriod - err := suite.govKeeper.SetProposal(ctx, *testProposals[1]) + err := suite.govKeeper.Proposals.Set(ctx, testProposals[1].Id, *testProposals[1]) suite.Require().NoError(err) suite.Require().NoError(suite.govKeeper.AddVote(ctx, testProposals[1].Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "")) @@ -520,7 +520,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryVote() { "valid request", func() { proposal.Status = v1.StatusVotingPeriod - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) suite.Require().NoError(suite.govKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "")) @@ -637,7 +637,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryVote() { "valid request", func() { proposal.Status = v1.StatusVotingPeriod - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) suite.Require().NoError(suite.govKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionAbstain), "")) @@ -743,7 +743,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryVotes() { "request after adding 2 votes", func() { proposal.Status = v1.StatusVotingPeriod - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) votes = []*v1.Vote{ {ProposalId: proposal.Id, Voter: addrs[0].String(), Options: v1.NewNonSplitVoteOption(v1.OptionAbstain)}, @@ -849,7 +849,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryVotes() { "request after adding 2 votes", func() { proposal.Status = v1.StatusVotingPeriod - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) votes = []v1beta1.Vote{ @@ -1503,7 +1503,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryDeposits() { func (suite *KeeperTestSuite) TestGRPCQueryTallyResult() { suite.reset() - ctx, queryClient := suite.ctx, suite.queryClient + queryClient := suite.queryClient var ( req *v1.QueryTallyResultRequest @@ -1559,7 +1559,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1590,7 +1590,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1621,7 +1621,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1664,7 +1664,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryTallyResult() { func (suite *KeeperTestSuite) TestLegacyGRPCQueryTallyResult() { suite.reset() - ctx, queryClient := suite.ctx, suite.legacyQueryClient + queryClient := suite.legacyQueryClient var ( req *v1beta1.QueryTallyResultRequest @@ -1716,7 +1716,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1beta1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1742,7 +1742,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1beta1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1768,7 +1768,7 @@ func (suite *KeeperTestSuite) TestLegacyGRPCQueryTallyResult() { VotingEndTime: &propTime, Metadata: "proposal metadata", } - err := suite.govKeeper.SetProposal(ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) req = &v1beta1.QueryTallyResultRequest{ProposalId: proposal.Id} @@ -1836,7 +1836,7 @@ func (suite *KeeperTestSuite) TestProposalVoteOptions() { Metadata: "proposal metadata", ProposalType: v1.ProposalType_PROPOSAL_TYPE_STANDARD, } - err := suite.govKeeper.SetProposal(suite.ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) }, req: &v1.QueryProposalVoteOptionsRequest{ProposalId: 1}, @@ -1864,7 +1864,7 @@ func (suite *KeeperTestSuite) TestProposalVoteOptions() { Metadata: "proposal metadata", ProposalType: v1.ProposalType_PROPOSAL_TYPE_MULTIPLE_CHOICE, } - err := suite.govKeeper.SetProposal(suite.ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) // multiple choice proposal, but no vote options set @@ -1895,7 +1895,7 @@ func (suite *KeeperTestSuite) TestProposalVoteOptions() { Metadata: "proposal metadata", ProposalType: v1.ProposalType_PROPOSAL_TYPE_MULTIPLE_CHOICE, } - err := suite.govKeeper.SetProposal(suite.ctx, proposal) + err := suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) err = suite.govKeeper.ProposalVoteOptions.Set(suite.ctx, proposal.Id, v1.ProposalVoteOptions{ OptionOne: "Vote for @tac0turle", diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 9728f2d1ca2d..6a82442a401c 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -71,8 +71,6 @@ type Keeper struct { ActiveProposalsQueue collections.Map[collections.Pair[time.Time, uint64], uint64] // TODO(tip): this should be simplified and go into an index. // InactiveProposalsQueue key: depositEndTime+proposalID | value: proposalID InactiveProposalsQueue collections.Map[collections.Pair[time.Time, uint64], uint64] // TODO(tip): this should be simplified and go into an index. - // VotingPeriodProposals key: proposalID | value: proposalStatus (votingPeriod or not) - VotingPeriodProposals collections.Map[uint64, []byte] // TODO(tip): this could be a keyset or index. } // GetAuthority returns the x/gov module's authority. @@ -136,7 +134,6 @@ func NewKeeper( ProposalVoteOptions: collections.NewMap(sb, types.ProposalVoteOptionsKeyPrefix, "proposal_vote_options", collections.Uint64Key, codec.CollValue[v1.ProposalVoteOptions](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 { diff --git a/x/gov/keeper/migrations.go b/x/gov/keeper/migrations.go index 5d7226a3b563..d8e02e0d3f8e 100644 --- a/x/gov/keeper/migrations.go +++ b/x/gov/keeper/migrations.go @@ -41,5 +41,5 @@ func (m Migrator) Migrate4to5(ctx context.Context) error { // Migrate4to5 migrates from version 5 to 6. func (m Migrator) Migrate5to6(ctx context.Context) error { - return v6.MigrateStore(ctx, m.keeper.Params, m.keeper.Proposals) + return v6.MigrateStore(ctx, m.keeper.storeService, m.keeper.Params, m.keeper.Proposals) } diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 943da3bd54b9..73d4566fa5a0 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -119,8 +119,7 @@ func (k Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, metadata return v1.Proposal{}, err } - err = k.SetProposal(ctx, proposal) - if err != nil { + if err = k.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return v1.Proposal{}, err } err = k.InactiveProposalsQueue.Set(ctx, collections.Join(*proposal.DepositEndTime, proposalID), proposalID) @@ -219,23 +218,6 @@ func (k Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer return nil } -// SetProposal sets a proposal to store. -func (k Keeper) SetProposal(ctx context.Context, proposal v1.Proposal) error { - if proposal.Status == v1.StatusVotingPeriod { - err := k.VotingPeriodProposals.Set(ctx, proposal.Id, []byte{1}) - if err != nil { - return err - } - } else { - err := k.VotingPeriodProposals.Remove(ctx, proposal.Id) - if err != nil { - return err - } - } - - return k.Proposals.Set(ctx, proposal.Id, proposal) -} - // DeleteProposal deletes a proposal from store. func (k Keeper) DeleteProposal(ctx context.Context, proposalID uint64) error { proposal, err := k.Proposals.Get(ctx, proposalID) @@ -254,11 +236,6 @@ func (k Keeper) DeleteProposal(ctx context.Context, proposalID uint64) error { if err != nil { return err } - - err = k.VotingPeriodProposals.Remove(ctx, proposalID) - if err != nil { - return err - } } return k.Proposals.Remove(ctx, proposalID) @@ -296,7 +273,7 @@ func (k Keeper) ActivateVotingPeriod(ctx context.Context, proposal v1.Proposal) endTime := proposal.VotingStartTime.Add(*votingPeriod) proposal.VotingEndTime = &endTime proposal.Status = v1.StatusVotingPeriod - if err = k.SetProposal(ctx, proposal); err != nil { + if err = k.Proposals.Set(ctx, proposal.Id, proposal); err != nil { return err } diff --git a/x/gov/keeper/proposal_test.go b/x/gov/keeper/proposal_test.go index 95ca0443f2bc..d691cf99b881 100644 --- a/x/gov/keeper/proposal_test.go +++ b/x/gov/keeper/proposal_test.go @@ -17,34 +17,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// TODO(tip): remove this -func (suite *KeeperTestSuite) TestGetSetProposal() { - testCases := map[string]struct { - proposalType v1.ProposalType - }{ - "unspecified proposal type": {}, - "regular proposal": { - proposalType: v1.ProposalType_PROPOSAL_TYPE_STANDARD, - }, - "expedited proposal": { - proposalType: v1.ProposalType_PROPOSAL_TYPE_EXPEDITED, - }, - } - - for _, tc := range testCases { - tp := TestProposal - proposal, err := suite.govKeeper.SubmitProposal(suite.ctx, tp, "", "test", "summary", suite.addrs[0], tc.proposalType) - suite.Require().NoError(err) - proposalID := proposal.Id - err = suite.govKeeper.SetProposal(suite.ctx, proposal) - suite.Require().NoError(err) - - 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 { @@ -67,7 +39,7 @@ func (suite *KeeperTestSuite) TestDeleteProposal() { proposal, err := suite.govKeeper.SubmitProposal(suite.ctx, tp, "", "test", "summary", suite.addrs[0], tc.proposalType) suite.Require().NoError(err) proposalID := proposal.Id - err = suite.govKeeper.SetProposal(suite.ctx, proposal) + err = suite.govKeeper.Proposals.Set(suite.ctx, proposal.Id, proposal) suite.Require().NoError(err) suite.Require().NotPanics(func() { @@ -274,7 +246,7 @@ func (suite *KeeperTestSuite) TestCancelProposal() { suite.Require().Nil(err) proposal2.Status = v1.ProposalStatus_PROPOSAL_STATUS_PASSED - err = suite.govKeeper.SetProposal(suite.ctx, proposal2) + err = suite.govKeeper.Proposals.Set(suite.ctx, proposal2.Id, proposal2) suite.Require().NoError(err) return proposal2ID, suite.addrs[1].String() }, diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index ebd3ae688770..b5415bd7145b 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -15,13 +15,18 @@ import ( // AddVote adds a vote on a specific proposal func (k Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr sdk.AccAddress, options v1.WeightedVoteOptions, metadata string) error { - // Check if proposal is in voting period. - inVotingPeriod, err := k.VotingPeriodProposals.Has(ctx, proposalID) + // get proposal + proposal, err := k.Proposals.Get(ctx, proposalID) if err != nil { + if stderrors.Is(err, collections.ErrNotFound) { + return errors.Wrapf(types.ErrInactiveProposal, "%d", proposalID) + } + return err } - if !inVotingPeriod { + // check if proposal is in voting period. + if proposal.Status != v1.StatusVotingPeriod { return errors.Wrapf(types.ErrInactiveProposal, "%d", proposalID) } @@ -29,12 +34,6 @@ func (k Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr sdk.Ac return err } - // get proposal - proposal, err := k.Proposals.Get(ctx, proposalID) - if err != nil { - return err - } - for _, option := range options { switch proposal.ProposalType { case v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC: diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index af2c46c1ccfb..caf3013afbc0 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -32,7 +32,7 @@ func TestVotes(t *testing.T) { require.Error(t, govKeeper.AddVote(ctx, 10, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), ""), "invalid proposal ID") proposal.Status = v1.StatusVotingPeriod - err = govKeeper.SetProposal(ctx, proposal) + err = govKeeper.Proposals.Set(ctx, proposal.Id, proposal) require.NoError(t, err) require.Error(t, govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(invalidOption), ""), "invalid option") @@ -123,7 +123,7 @@ func TestVotes_MultipleChoiceProposal(t *testing.T) { require.NoError(t, err) proposal.Status = v1.StatusVotingPeriod - require.NoError(t, govKeeper.SetProposal(ctx, proposal)) + require.NoError(t, govKeeper.Proposals.Set(ctx, proposal.Id, proposal)) proposalID := proposal.Id diff --git a/x/gov/migrations/v6/store.go b/x/gov/migrations/v6/store.go index 4676511700c7..be30f95f509b 100644 --- a/x/gov/migrations/v6/store.go +++ b/x/gov/migrations/v6/store.go @@ -5,16 +5,21 @@ import ( "fmt" "cosmossdk.io/collections" + corestoretypes "cosmossdk.io/core/store" v1 "cosmossdk.io/x/gov/types/v1" ) +var votingPeriodProposalKeyPrefix = collections.NewPrefix(4) // VotingPeriodProposalKeyPrefix stores which proposals are on voting period. + // MigrateStore performs in-place store migrations from v5 (v0.50) to v6 (v0.51). The // migration includes: // // Addition of new field in params to store types of proposals that can be submitted. // Addition of gov params for optimistic proposals. -func MigrateStore(ctx context.Context, paramsCollection collections.Item[v1.Params], proposalCollection collections.Map[uint64, v1.Proposal]) error { - // Migrate proposals +// Addition of gov params for proposal cancel max period. +// Cleanup of old proposal stores. +func MigrateStore(ctx context.Context, storeService corestoretypes.KVStoreService, paramsCollection collections.Item[v1.Params], proposalCollection collections.Map[uint64, v1.Proposal]) error { + // Migrate **all** proposals err := proposalCollection.Walk(ctx, nil, func(key uint64, proposal v1.Proposal) (bool, error) { if proposal.Expedited { proposal.ProposalType = v1.ProposalType_PROPOSAL_TYPE_EXPEDITED @@ -32,6 +37,13 @@ func MigrateStore(ctx context.Context, paramsCollection collections.Item[v1.Para return err } + // Clear old proposal stores + sb := collections.NewSchemaBuilder(storeService) + votingPeriodProposals := collections.NewMap(sb, votingPeriodProposalKeyPrefix, "voting_period_proposals", collections.Uint64Key, collections.BytesValue) + if err := votingPeriodProposals.Clear(ctx, nil); err != nil { + return err + } + // Migrate params govParams, err := paramsCollection.Get(ctx) if err != nil { diff --git a/x/gov/migrations/v6/store_test.go b/x/gov/migrations/v6/store_test.go index 5839ecc0e1a3..e4a784f7fb7d 100644 --- a/x/gov/migrations/v6/store_test.go +++ b/x/gov/migrations/v6/store_test.go @@ -37,7 +37,7 @@ func TestMigrateStore(t *testing.T) { require.NoError(t, err) // Run migrations. - err = v6.MigrateStore(ctx, paramsCollection, proposalCollection) + err = v6.MigrateStore(ctx, storeService, paramsCollection, proposalCollection) require.NoError(t, err) // Check params diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go index d37a9eef70c4..f7ba01b059ae 100644 --- a/x/gov/types/keys.go +++ b/x/gov/types/keys.go @@ -16,15 +16,19 @@ const ( ) var ( - 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. - ProposalVoteOptionsKeyPrefix = collections.NewPrefix(50) // ProposalVoteOptionsKeyPrefix stores the vote options of proposals. - MessageBasedParamsKey = collections.NewPrefix(51) // MessageBasedParamsKey stores the message based gov params. + 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. + 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. + ProposalVoteOptionsKeyPrefix = collections.NewPrefix(50) // ProposalVoteOptionsKeyPrefix stores the vote options of proposals. + MessageBasedParamsKey = collections.NewPrefix(51) // MessageBasedParamsKey stores the message based gov params. +) + +// Reserved kvstore keys +var ( + _ = collections.NewPrefix(4) ) diff --git a/x/gov/types/v1/proposal.go b/x/gov/types/v1/proposal.go index 61fde762248c..47615313fe11 100644 --- a/x/gov/types/v1/proposal.go +++ b/x/gov/types/v1/proposal.go @@ -5,7 +5,7 @@ import ( "strings" "time" - "github.com/cosmos/cosmos-sdk/codec/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdktx "github.com/cosmos/cosmos-sdk/types/tx" ) @@ -74,14 +74,14 @@ func (p Proposal) GetMinDepositFromParams(params Params) sdk.Coins { } // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces -func (p Proposal) UnpackInterfaces(unpacker types.AnyUnpacker) error { +func (p Proposal) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { return sdktx.UnpackInterfaces(unpacker, p.Messages) } // Proposals is an array of proposal type Proposals []*Proposal -var _ types.UnpackInterfacesMessage = Proposals{} +var _ codectypes.UnpackInterfacesMessage = Proposals{} // String implements stringer interface func (p Proposals) String() string { @@ -94,7 +94,7 @@ func (p Proposals) String() string { } // UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces -func (p Proposals) UnpackInterfaces(unpacker types.AnyUnpacker) error { +func (p Proposals) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { for _, x := range p { err := x.UnpackInterfaces(unpacker) if err != nil {