Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 48 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
63c6057
(x/gov) feat: move parts of Proposal into separate storages
facundomedica Oct 18, 2022
d8219fc
fix some
facundomedica Oct 18, 2022
cb12f0e
fix others
facundomedica Oct 18, 2022
5514486
fix unit tests
facundomedica Oct 18, 2022
1d62ba4
fix unit tests
facundomedica Oct 18, 2022
4b97f7b
fix conflicts
facundomedica Oct 18, 2022
d9364d4
fix conflicts
facundomedica Oct 18, 2022
8e838f5
progress
facundomedica Oct 18, 2022
3fccb66
do not overwrite messages
facundomedica Oct 18, 2022
e2bef95
remove change
facundomedica Oct 18, 2022
82998ef
simplify methods
facundomedica Oct 19, 2022
da271d5
static data
facundomedica Oct 19, 2022
601c411
fix tests
facundomedica Oct 19, 2022
162353c
fix tests
facundomedica Oct 19, 2022
04e9308
fix tests
facundomedica Oct 19, 2022
d62d0c1
rollback some changes
facundomedica Oct 19, 2022
733f286
rollback some changes
facundomedica Oct 19, 2022
d7918b7
progress
facundomedica Oct 19, 2022
d9d339f
progress
facundomedica Oct 19, 2022
1f32225
progress
facundomedica Oct 19, 2022
b5b2ada
progress
facundomedica Oct 19, 2022
37af9c4
progress
facundomedica Oct 19, 2022
1a9a443
progress
facundomedica Oct 19, 2022
2edde57
add delete
facundomedica Oct 19, 2022
f4a128f
fix tests
facundomedica Oct 19, 2022
3f8936e
use SetProposalWithoutContents whenever possible
facundomedica Oct 19, 2022
46b087c
add migrations
facundomedica Oct 19, 2022
59dc8a7
fix godoc
facundomedica Oct 19, 2022
d7c8cd1
gofumpt
facundomedica Oct 19, 2022
78dbf6c
Merge branch 'main' into facu/gov-storage
facundomedica Oct 19, 2022
ece934d
add changelog
facundomedica Oct 19, 2022
c3af4cf
rolling back changes
facundomedica Oct 24, 2022
59434a7
rolling back changes
facundomedica Oct 24, 2022
2d07421
progress
facundomedica Oct 24, 2022
4ecb17c
progress
facundomedica Oct 24, 2022
c20b48f
progress
facundomedica Oct 24, 2022
dd38f3d
progress
facundomedica Oct 24, 2022
b6e24af
fix tests
facundomedica Oct 24, 2022
ecc8917
fix cl
facundomedica Oct 24, 2022
fc8b9d0
fix cl
facundomedica Oct 24, 2022
cd2ae66
fix
facundomedica Oct 24, 2022
ff62538
fix test error
facundomedica Oct 24, 2022
e23a08b
Merge branch 'main' into facu/gov-storage
facundomedica Oct 25, 2022
e42c114
add store key in readme
facundomedica Oct 26, 2022
97e54bd
add store key in readme
facundomedica Oct 26, 2022
0bda41c
Merge branch 'main' into facu/gov-storage
facundomedica Oct 26, 2022
8707966
Merge branch 'facu/gov-storage' of https://github.com/cosmos/cosmos-s…
facundomedica Oct 26, 2022
aa1ed8b
Merge branch 'main' into facu/gov-storage
facundomedica Oct 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ Ref: https://keepachangelog.com/en/1.0.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.
Expand Down
4 changes: 2 additions & 2 deletions x/gov/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
9 changes: 4 additions & 5 deletions x/gov/keeper/vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 21 additions & 2 deletions x/gov/migrations/v4/keys.go
Original file line number Diff line number Diff line change
@@ -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
}
29 changes: 29 additions & 0 deletions x/gov/migrations/v4/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
66 changes: 64 additions & 2 deletions x/gov/migrations/v4/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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,
}
}
14 changes: 10 additions & 4 deletions x/gov/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the spec/README.md file too? Just add some info about this new state

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing it now, ty! As a note I think the store section is not entirely accurate (or at least not very easy to understand), but I'll that there and re-visit later.


DepositsKeyPrefix = []byte{0x10}

Expand Down Expand Up @@ -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)...)
Expand Down
2 changes: 1 addition & 1 deletion x/gov/types/v1/proposals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down