Skip to content

Commit

Permalink
fix(gov): Fix v3 votes migrations (backport #14214) (#14277)
Browse files Browse the repository at this point in the history
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
3 people committed Dec 13, 2022
1 parent 4153b12 commit f71df80
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
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
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
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
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
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
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

0 comments on commit f71df80

Please sign in to comment.