feat!: (x/gov) store an index of proposals that are in voting period (#…

…13576) * (x/gov) feat: move parts of Proposal into separate storages * fix some * fix others * fix unit tests * fix unit tests * fix conflicts * progress * do not overwrite messages * remove change * simplify methods * static data * fix tests * fix tests * fix tests * rollback some changes * rollback some changes * progress * progress * progress * progress * progress * progress * add delete * fix tests * use SetProposalWithoutContents whenever possible * add migrations * fix godoc * gofumpt * add changelog * rolling back changes * rolling back changes * progress * progress * progress * progress * fix tests * fix cl * fix cl * fix * fix test error * add store key in readme * add store key in readme
cosmos · Oct 26, 2022 · 91ca57b · 91ca57b
1 parent c833190
commit 91ca57b
Show file tree

Hide file tree

Showing 10 changed files with 142 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -535,6 +535,7 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v8.0
 
 ### State Machine Breaking
 
+* (x/gov) [#13576](https://github.com/cosmos/cosmos-sdk/pull/13576) Proposals in voting period are tracked in a separate store.
 * (baseapp) [#11985](https://github.com/cosmos/cosmos-sdk/pull/11985) Add a `postHandler` to baseapp. This `postHandler` is like antehandler, but is run _after_ the `runMsgs` execution. It is in the same store branch that `runMsgs`, meaning that both `runMsgs` and `postHandler`
 * (x/gov) [#11998](https://github.com/cosmos/cosmos-sdk/pull/11998) Tweak the `x/gov` `ModuleAccountInvariant` invariant to ensure deposits are `<=` total module account balance instead of strictly equal.
 * (x/upgrade) [#11800](https://github.com/cosmos/cosmos-sdk/pull/11800) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade.

diff --git a/x/gov/README.md b/x/gov/README.md
@@ -452,14 +452,16 @@ This type is used in a temp map when tallying
 
 *Note: Stores are KVStores in the multi-store. The key to find the store is the first parameter in the list*
 
-We will use one KVStore `Governance` to store two mappings:
+We will use one KVStore `Governance` to store four mappings:
 
 * A mapping from `proposalID|'proposal'` to `Proposal`.
 * A mapping from `proposalID|'addresses'|address` to `Vote`. This mapping allows
   us to query all addresses that voted on the proposal along with their vote by
   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/client/testutil/tx.go b/x/gov/client/testutil/tx.go
@@ -360,7 +360,7 @@ func (s *IntegrationTestSuite) TestNewCmdVote() {
 				fmt.Sprintf("--metadata=%s", "AQ=="),
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
 			},
-			false, 2,
+			false, 3,
 		},
 		{
 			"valid vote",
@@ -433,7 +433,7 @@ func (s *IntegrationTestSuite) TestNewCmdWeightedVote() {
 				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, 3,
 		},
 		{
 			"valid vote",

diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go
@@ -122,6 +122,13 @@ func (keeper Keeper) SetProposal(ctx sdk.Context, proposal v1.Proposal) {
 	}
 
 	store := ctx.KVStore(keeper.storeKey)
+
+	if proposal.Status == v1.StatusVotingPeriod {
+		store.Set(types.VotingPeriodProposalKey(proposal.Id), []byte{1})
+	} else {
+		store.Delete(types.VotingPeriodProposalKey(proposal.Id))
+	}
+
 	store.Set(types.ProposalKey(proposal.Id), bz)
 }
 

diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go
@@ -11,13 +11,12 @@ import (
 
 // AddVote adds a vote on a specific proposal
 func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress, options v1.WeightedVoteOptions, metadata string) error {
-	proposal, ok := keeper.GetProposal(ctx, proposalID)
-	if !ok {
-		return sdkerrors.Wrapf(types.ErrUnknownProposal, "%d", proposalID)
-	}
-	if proposal.Status != v1.StatusVotingPeriod {
+	// Check if proposal is in voting period.
+	store := ctx.KVStore(keeper.storeKey)
+	if !store.Has(types.VotingPeriodProposalKey(proposalID)) {
 		return sdkerrors.Wrapf(types.ErrInactiveProposal, "%d", proposalID)
 	}
+
 	err := keeper.assertMetadataLength(metadata)
 	if err != nil {
 		return err

diff --git a/x/gov/migrations/v4/keys.go b/x/gov/migrations/v4/keys.go
@@ -1,9 +1,28 @@
 package v4
 
+import "encoding/binary"
+
 const (
 	// ModuleName is the name of the module
 	ModuleName = "gov"
 )
 
-// ParamsKey is the key of x/gov params
-var ParamsKey = []byte{0x30}
+var (
+	// ParamsKey is the key of x/gov params
+	ParamsKey = []byte{0x30}
+
+	// - 0x04<proposalID_Bytes>: ProposalContents
+	VotingPeriodProposalKeyPrefix = []byte{0x04}
+)
+
+// VotingPeriodProposalKey gets if a proposal is in voting period.
+func VotingPeriodProposalKey(proposalID uint64) []byte {
+	return append(VotingPeriodProposalKeyPrefix, GetProposalIDBytes(proposalID)...)
+}
+
+// GetProposalIDBytes returns the byte representation of the proposalID
+func GetProposalIDBytes(proposalID uint64) (proposalIDBz []byte) {
+	proposalIDBz = make([]byte, 8)
+	binary.BigEndian.PutUint64(proposalIDBz, proposalID)
+	return
+}
diff --git a/x/gov/migrations/v4/store.go b/x/gov/migrations/v4/store.go
@@ -2,9 +2,11 @@ package v4
 
 import (
 	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov/exported"
+	v1 "github.com/cosmos/cosmos-sdk/x/gov/migrations/v1"
 	govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 )
 
@@ -38,11 +40,38 @@ func migrateParams(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace
 	return nil
 }
 
+func migrateProposalVotingPeriod(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
+	store := ctx.KVStore(storeKey)
+	propStore := prefix.NewStore(store, v1.ProposalsKeyPrefix)
+
+	iter := propStore.Iterator(nil, nil)
+	defer iter.Close()
+
+	for ; iter.Valid(); iter.Next() {
+		var prop govv1.Proposal
+		err := cdc.Unmarshal(iter.Value(), &prop)
+		if err != nil {
+			return err
+		}
+
+		if prop.Status == govv1.StatusVotingPeriod {
+			store.Set(VotingPeriodProposalKey(prop.Id), []byte{1})
+		}
+	}
+
+	return nil
+}
+
 // MigrateStore performs in-place store migrations from v3 (v0.46) to v4 (v0.47). The
 // migration includes:
 //
 // Params migrations from x/params to gov
 // Addition of the new min initial deposit ratio parameter that is set to 0 by default.
+// Proposals in voting period are tracked in a separate index.
 func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace exported.ParamSubspace, cdc codec.BinaryCodec) error {
+	if err := migrateProposalVotingPeriod(ctx, storeKey, cdc); err != nil {
+		return err
+	}
+
 	return migrateParams(ctx, storeKey, legacySubspace, cdc)
 }
diff --git a/x/gov/migrations/v4/store_test.go b/x/gov/migrations/v4/store_test.go
@@ -2,18 +2,32 @@ package v4_test
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
 	"github.com/cosmos/cosmos-sdk/testutil"
+	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	"github.com/cosmos/cosmos-sdk/x/bank"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"
+	v1gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v1"
 	v4 "github.com/cosmos/cosmos-sdk/x/gov/migrations/v4"
+	"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"
 	"github.com/cosmos/cosmos-sdk/x/upgrade"
 )
 
+var (
+	_, _, addr   = testdata.KeyTestPubAddr()
+	govAcct      = authtypes.NewModuleAddress(types.ModuleName)
+	TestProposal = getTestProposal()
+)
+
 type mockSubspace struct {
 	dp v1.DepositParams
 	vp v1.VotingParams
@@ -49,16 +63,36 @@ func (ms mockSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {
 }
 
 func TestMigrateStore(t *testing.T) {
-	cdc := moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}, gov.AppModuleBasic{}).Codec
+	cdc := moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}, gov.AppModuleBasic{}, bank.AppModuleBasic{}).Codec
 	govKey := sdk.NewKVStoreKey("gov")
 	ctx := testutil.DefaultContext(govKey, sdk.NewTransientStoreKey("transient_test"))
 	store := ctx.KVStore(govKey)
 
 	legacySubspace := newMockSubspace(v1.DefaultParams())
+
+	propTime := time.Unix(1e9, 0)
+
+	// Create 2 proposals
+	prop1Content, err := v1.NewLegacyContent(v1beta1.NewTextProposal("Test", "description"), authtypes.NewModuleAddress("gov").String())
+	require.NoError(t, err)
+	proposal1, err := v1.NewProposal([]sdk.Msg{prop1Content}, 1, "some metadata for the legacy content", propTime, propTime)
+	require.NoError(t, err)
+	prop1Bz, err := cdc.Marshal(&proposal1)
+	require.NoError(t, err)
+	store.Set(v1gov.ProposalKey(proposal1.Id), prop1Bz)
+
+	proposal2, err := v1.NewProposal(getTestProposal(), 2, "some metadata for the legacy content", propTime, propTime)
+	proposal2.Status = v1.StatusVotingPeriod
+	require.NoError(t, err)
+	prop2Bz, err := cdc.Marshal(&proposal2)
+	require.NoError(t, err)
+	store.Set(v1gov.ProposalKey(proposal2.Id), prop2Bz)
+
 	// Run migrations.
-	err := v4.MigrateStore(ctx, govKey, legacySubspace, cdc)
+	err = v4.MigrateStore(ctx, govKey, legacySubspace, cdc)
 	require.NoError(t, err)
 
+	// Check params
 	var params v1.Params
 	bz := store.Get(v4.ParamsKey)
 	require.NoError(t, cdc.Unmarshal(bz, &params))
@@ -70,4 +104,32 @@ func TestMigrateStore(t *testing.T) {
 	require.Equal(t, legacySubspace.tp.Threshold, params.Threshold)
 	require.Equal(t, legacySubspace.tp.VetoThreshold, params.VetoThreshold)
 	require.Equal(t, sdk.ZeroDec().String(), params.MinInitialDepositRatio)
+
+	// Check proposals' status
+	var migratedProp1 v1.Proposal
+	bz = store.Get(v1gov.ProposalKey(proposal1.Id))
+	require.NoError(t, cdc.Unmarshal(bz, &migratedProp1))
+	require.Equal(t, v1.StatusDepositPeriod, migratedProp1.Status)
+
+	var migratedProp2 v1.Proposal
+	bz = store.Get(v1gov.ProposalKey(proposal2.Id))
+	require.NoError(t, cdc.Unmarshal(bz, &migratedProp2))
+	require.Equal(t, v1.StatusVotingPeriod, migratedProp2.Status)
+
+	// Check if proposal 2 is in the new store but not proposal 1
+	require.Nil(t, store.Get(v4.VotingPeriodProposalKey(proposal1.Id)))
+	require.Equal(t, []byte{0x1}, store.Get(v4.VotingPeriodProposalKey(proposal2.Id)))
+
+}
+
+func getTestProposal() []sdk.Msg {
+	legacyProposalMsg, err := v1.NewLegacyContent(v1beta1.NewTextProposal("Title", "description"), authtypes.NewModuleAddress(types.ModuleName).String())
+	if err != nil {
+		panic(err)
+	}
+
+	return []sdk.Msg{
+		banktypes.NewMsgSend(govAcct, addr, sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000)))),
+		legacyProposalMsg,
+	}
 }
diff --git a/x/gov/types/keys.go b/x/gov/types/keys.go
@@ -37,10 +37,11 @@ const (
 //
 // - 0x30: Params
 var (
-	ProposalsKeyPrefix          = []byte{0x00}
-	ActiveProposalQueuePrefix   = []byte{0x01}
-	InactiveProposalQueuePrefix = []byte{0x02}
-	ProposalIDKey               = []byte{0x03}
+	ProposalsKeyPrefix            = []byte{0x00}
+	ActiveProposalQueuePrefix     = []byte{0x01}
+	InactiveProposalQueuePrefix   = []byte{0x02}
+	ProposalIDKey                 = []byte{0x03}
+	VotingPeriodProposalKeyPrefix = []byte{0x04}
 
 	DepositsKeyPrefix = []byte{0x10}
 
@@ -69,6 +70,11 @@ 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)...)

diff --git a/x/gov/types/v1/proposals_test.go b/x/gov/types/v1/proposals_test.go
@@ -8,7 +8,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/x/gov/types/v1"
+	v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 	"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 )