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

fix(gov): Fix v3 votes migrations (backport #14214) #14277

Merged
merged 4 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### State Machine Breaking

* (x/gov) [#14214](https://github.com/cosmos/cosmos-sdk/pull/14214) Fix gov v0.46 migration to v1 votes.
* Also provide a helper function `govv046.Migrate_V0466_To_V0467` for migrating a chain already on v0.46 with versions <=v0.46.6 to the latest v0.46.7 correct state.
* (x/group) [#14071](https://github.com/cosmos/cosmos-sdk/pull/14071) Don't re-tally proposal after voting period end if they have been marked as ACCEPTED or REJECTED.

### API Breaking Changes
Expand Down
4 changes: 4 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Please read the release notes of [v0.46.5](https://github.com/cosmos/cosmos-sdk/

A critical vulnerability has been fixed in the group module. For safety, `v0.46.5` and `v0.46.6` are retracted, even though chains not using the group module are not affected. When using the group module, please upgrade immediately to `v0.46.7`.

An issue has been discovered in the gov module's votes migration. It does not impact proposals and votes tallying, but the gRPC queries on votes are incorrect. This issue is fixed in `v0.46.7`, however:
- if your chain is already on v0.46 using `<= v0.46.6`, a **coordinated upgrade** to v0.46.7 is required. You can use the helper function `govv046.Migrate_V046_6_To_V046_7` for migrating a chain **already on v0.46 with versions <=v0.46.6** to the latest v0.46.7 correct state.
- if your chain is on a previous version <= v0.45, then simply use v0.46.7 when upgrading to v0.46.

**NOTE**: The changes mentioned in `v0.46.3` are no longer required. The following replace directive can be removed from the chains.

```go
Expand Down
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,7 @@ replace (
)

retract (
// subject to a bug in the group module,
// these versions are safe to use when not using the group module
// subject to a bug in the group module and gov module migration
[v0.46.5, v0.46.6]
// subject to the dragonberry vulnerability
// and/or the bank coin metadata migration issue
Expand Down
2 changes: 2 additions & 0 deletions x/gov/migrations/v046/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

var voter = sdk.MustAccAddressFromBech32("cosmos1fl48vsnmsdzcv85q5d2q4z5ajdha8yu34mf0eh")

func TestMigrateJSON(t *testing.T) {
encodingConfig := simapp.MakeTestEncodingConfig()
clientCtx := client.Context{}.
Expand Down
64 changes: 61 additions & 3 deletions x/gov/migrations/v046/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
v042 "github.com/cosmos/cosmos-sdk/x/gov/migrations/v042"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

// migrateProposals migrates all legacy proposals into MsgExecLegacyContent
Expand All @@ -18,7 +19,7 @@ func migrateProposals(store sdk.KVStore, cdc codec.BinaryCodec) error {
defer iter.Close()

for ; iter.Valid(); iter.Next() {
var oldProp v1beta1.Proposal
var oldProp govv1beta1.Proposal
err := cdc.Unmarshal(iter.Value(), &oldProp)
if err != nil {
return err
Expand All @@ -40,12 +41,69 @@ func migrateProposals(store sdk.KVStore, cdc codec.BinaryCodec) error {
return nil
}

// MigrateStore performs in-place store migrations from v0.43 to v0.46. The
// migrateVotes migrates all v1beta1 weighted votes (with sdk.Dec as weight)
// to v1 weighted votes (with string as weight)
func migrateVotes(store sdk.KVStore, cdc codec.BinaryCodec) error {
votesStore := prefix.NewStore(store, v042.VotesKeyPrefix)

iter := votesStore.Iterator(nil, nil)
defer iter.Close()

for ; iter.Valid(); iter.Next() {
var oldVote govv1beta1.Vote
err := cdc.Unmarshal(iter.Value(), &oldVote)
if err != nil {
return err
}

newVote := govv1.Vote{
ProposalId: oldVote.ProposalId,
Voter: oldVote.Voter,
}
newOptions := make([]*govv1.WeightedVoteOption, len(oldVote.Options))
for i, o := range oldVote.Options {
newOptions[i] = &govv1.WeightedVoteOption{
Option: govv1.VoteOption(o.Option),
Weight: o.Weight.String(), // Convert to decimal string
}
}
newVote.Options = newOptions
bz, err := cdc.Marshal(&newVote)
if err != nil {
return err
}

// Set new value on store.
votesStore.Set(iter.Key(), bz)
}

return nil
}

// MigrateStore performs in-place store migrations from v2 (v0.43) to v3 (v0.46). The
// migration includes:
//
// - Migrate proposals to be Msg-based.
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)

if err := migrateVotes(store, cdc); err != nil {
return err
}

return migrateProposals(store, cdc)
}

// Migrate_V046_4_To_V046_5 is a helper function to migrate chains from <=v0.46.6
// to v0.46.7 ONLY.
//
// IMPORTANT: Please do not use this function if you are upgrading to v0.46
// from <=v0.45.
//
// This function migrates the store in-place by fixing the gov votes weight to
// be stored as decimals strings (instead of the sdk.Dec BigInt representation).
//
// The store is expected to be the gov store, and not any prefixed substore.
func Migrate_V046_6_To_V046_7(store sdk.KVStore, cdc codec.BinaryCodec) error {
return migrateVotes(store, cdc)
}
17 changes: 17 additions & 0 deletions x/gov/migrations/v046/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ func TestMigrateStore(t *testing.T) {
store.Set(v042gov.ProposalKey(prop1.ProposalId), prop1Bz)
store.Set(v042gov.ProposalKey(prop2.ProposalId), prop2Bz)

// Vote on prop 1
options := []v1beta1.WeightedVoteOption{
{Option: v1beta1.OptionNo, Weight: sdk.MustNewDecFromStr("0.3")},
{Option: v1beta1.OptionYes, Weight: sdk.MustNewDecFromStr("0.7")},
}
vote1 := v1beta1.NewVote(1, voter, options)
vote1Bz := cdc.MustMarshal(&vote1)
store.Set(v042gov.VoteKey(1, voter), vote1Bz)

// Run migrations.
err = v046gov.MigrateStore(ctx, govKey, cdc)
require.NoError(t, err)
Expand All @@ -52,6 +61,14 @@ func TestMigrateStore(t *testing.T) {
err = cdc.Unmarshal(store.Get(v042gov.ProposalKey(prop2.ProposalId)), &newProp2)
require.NoError(t, err)
compareProps(t, prop2, newProp2)

var newVote1 v1.Vote
err = cdc.Unmarshal(store.Get(v042gov.VoteKey(prop1.ProposalId, voter)), &newVote1)
require.NoError(t, err)
// Without the votes migration, we would have 300000000000000000 in state,
// because of how sdk.Dec stores itself in state.
require.Equal(t, "0.300000000000000000", newVote1.Options[0].Weight)
require.Equal(t, "0.700000000000000000", newVote1.Options[1].Weight)
}

func compareProps(t *testing.T, oldProp v1beta1.Proposal, newProp v1.Proposal) {
Expand Down