diff --git a/CHANGELOG.md b/CHANGELOG.md index 29ad05def725..931e6b78a25a 100644 --- 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 index 2ab24cfb48e7..0d625bf601ea 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -452,7 +452,7 @@ 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 @@ -460,6 +460,8 @@ We will use one KVStore `Governance` to store two 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/client/testutil/tx.go b/x/gov/client/testutil/tx.go index e87024ef0643..8ac10c5125ef 100644 --- 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 index cb1807410927..946531534212 100644 --- 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 index 038310a916a4..7f43bec1266d 100644 --- 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 index 1c5163a82c4a..7815a688d6e2 100644 --- 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: 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 index 5047b3cc0194..eb8ebc5004cd 100644 --- 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 index cf206b7ebced..74fb64bcd1a7 100644 --- 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, ¶ms)) @@ -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 index dd1fb906335e..dc78b0590f5e 100644 --- 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 index 1f808bdefbaf..08ad5164900a 100644 --- 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" )