Skip to content

Commit

Permalink
refactor: remove deprecated vote option (#10854)
Browse files Browse the repository at this point in the history
## Description

Closes: #10792



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
atheeshp authored Jan 26, 2022
1 parent d9033e0 commit 8133ee8
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 301 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (deps) [\#10210](https://github.com/cosmos/cosmos-sdk/pull/10210) Bump Tendermint to [v0.35.0](https://github.com/tendermint/tendermint/releases/tag/v0.35.0).
* (deps) [\#10706](https://github.com/cosmos/cosmos-sdk/issues/10706) Bump rosetta-sdk-go to v0.7.2 and rosetta-cli to v0.7.3
* (types/errors) [\#10779](https://github.com/cosmos/cosmos-sdk/pull/10779) Move most functionality in `types/errors` to a standalone `errors` go module, except the `RootCodespace` errors and ABCI response helpers. All functions and types that used to live in `types/errors` are now aliased so this is not a breaking change.
* (gov) [\#10854](https://github.com/cosmos/cosmos-sdk/pull/10854) v1beta2's vote doesn't include the deprecate `option VoteOption` anymore. Instead, it only uses `WeightedVoteOption`.

### Bug Fixes

Expand Down
236 changes: 85 additions & 151 deletions api/cosmos/gov/v1beta2/gov.pulsar.go

Large diffs are not rendered by default.

10 changes: 4 additions & 6 deletions orm/internal/testpb/bank.pulsar.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@ package testpb

import (
fmt "fmt"
io "io"
reflect "reflect"
sync "sync"

runtime "github.com/cosmos/cosmos-proto/runtime"
_ "github.com/cosmos/cosmos-sdk/api/cosmos/orm/v1alpha1"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoiface "google.golang.org/protobuf/runtime/protoiface"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"

_ "github.com/cosmos/cosmos-sdk/api/cosmos/orm/v1alpha1"
io "io"
reflect "reflect"
sync "sync"
)

var (
Expand Down
5 changes: 1 addition & 4 deletions proto/cosmos/gov/v1beta2/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,7 @@ message TallyResult {
message Vote {
uint64 proposal_id = 1;
string voter = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// Deprecated: Prefer to use `options` instead. This field is set in queries
// if and only if `len(options) == 1` and that option has weight 1. In all
// other cases, this field will default to VOTE_OPTION_UNSPECIFIED.
VoteOption option = 3 [deprecated = true];
reserved 3;
repeated WeightedVoteOption options = 4;
}

Expand Down
1 change: 0 additions & 1 deletion x/gov/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ func (q Keeper) Votes(c context.Context, req *v1beta2.QueryVotesRequest) (*v1bet
if err := q.cdc.Unmarshal(value, &vote); err != nil {
return err
}
populateLegacyOption(&vote)

votes = append(votes, &vote)
return nil
Expand Down
6 changes: 3 additions & 3 deletions x/gov/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryVote() {
Voter: addrs[0].String(),
}

expRes = &v1beta2.QueryVoteResponse{Vote: &v1beta2.Vote{ProposalId: proposal.ProposalId, Voter: addrs[0].String(), Option: v1beta2.OptionAbstain, Options: []*v1beta2.WeightedVoteOption{{Option: v1beta2.OptionAbstain, Weight: sdk.MustNewDecFromStr("1.0").String()}}}}
expRes = &v1beta2.QueryVoteResponse{Vote: &v1beta2.Vote{ProposalId: proposal.ProposalId, Voter: addrs[0].String(), Options: []*v1beta2.WeightedVoteOption{{Option: v1beta2.OptionAbstain, Weight: sdk.MustNewDecFromStr("1.0").String()}}}}
},
true,
},
Expand Down Expand Up @@ -412,8 +412,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryVotes() {
app.GovKeeper.SetProposal(ctx, proposal)

votes = []*v1beta2.Vote{
{ProposalId: proposal.ProposalId, Voter: addrs[0].String(), Option: v1beta2.OptionAbstain, Options: v1beta2.NewNonSplitVoteOption(v1beta2.OptionAbstain)},
{ProposalId: proposal.ProposalId, Voter: addrs[1].String(), Option: v1beta2.OptionYes, Options: v1beta2.NewNonSplitVoteOption(v1beta2.OptionYes)},
{ProposalId: proposal.ProposalId, Voter: addrs[0].String(), Options: v1beta2.NewNonSplitVoteOption(v1beta2.OptionAbstain)},
{ProposalId: proposal.ProposalId, Voter: addrs[1].String(), Options: v1beta2.NewNonSplitVoteOption(v1beta2.OptionYes)},
}
accAddr1, err1 := sdk.AccAddressFromBech32(votes[0].Voter)
accAddr2, err2 := sdk.AccAddressFromBech32(votes[1].Voter)
Expand Down
18 changes: 0 additions & 18 deletions x/gov/keeper/vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A
// GetAllVotes returns all the votes from the store
func (keeper Keeper) GetAllVotes(ctx sdk.Context) (votes v1beta2.Votes) {
keeper.IterateAllVotes(ctx, func(vote v1beta2.Vote) bool {
populateLegacyOption(&vote)
votes = append(votes, &vote)
return false
})
Expand All @@ -55,7 +54,6 @@ func (keeper Keeper) GetAllVotes(ctx sdk.Context) (votes v1beta2.Votes) {
// GetVotes returns all the votes from a proposal
func (keeper Keeper) GetVotes(ctx sdk.Context, proposalID uint64) (votes v1beta2.Votes) {
keeper.IterateVotes(ctx, proposalID, func(vote v1beta2.Vote) bool {
populateLegacyOption(&vote)
votes = append(votes, &vote)
return false
})
Expand All @@ -71,18 +69,12 @@ func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A
}

keeper.cdc.MustUnmarshal(bz, &vote)
populateLegacyOption(&vote)

return vote, true
}

// SetVote sets a Vote to the gov store
func (keeper Keeper) SetVote(ctx sdk.Context, vote v1beta2.Vote) {
// vote.Option is a deprecated field, we don't set it in state
if vote.Option != v1beta2.OptionEmpty { // nolint
vote.Option = v1beta2.OptionEmpty // nolint
}

store := ctx.KVStore(keeper.storeKey)
bz := keeper.cdc.MustMarshal(&vote)
addr, err := sdk.AccAddressFromBech32(vote.Voter)
Expand All @@ -101,7 +93,6 @@ func (keeper Keeper) IterateAllVotes(ctx sdk.Context, cb func(vote v1beta2.Vote)
for ; iterator.Valid(); iterator.Next() {
var vote v1beta2.Vote
keeper.cdc.MustUnmarshal(iterator.Value(), &vote)
populateLegacyOption(&vote)

if cb(vote) {
break
Expand All @@ -118,7 +109,6 @@ func (keeper Keeper) IterateVotes(ctx sdk.Context, proposalID uint64, cb func(vo
for ; iterator.Valid(); iterator.Next() {
var vote v1beta2.Vote
keeper.cdc.MustUnmarshal(iterator.Value(), &vote)
populateLegacyOption(&vote)

if cb(vote) {
break
Expand All @@ -131,11 +121,3 @@ func (keeper Keeper) deleteVote(ctx sdk.Context, proposalID uint64, voterAddr sd
store := ctx.KVStore(keeper.storeKey)
store.Delete(types.VoteKey(proposalID, voterAddr))
}

// populateLegacyOption adds graceful fallback of deprecated `Option` field, in case
// there's only 1 VoteOption.
func populateLegacyOption(vote *v1beta2.Vote) {
if len(vote.Options) == 1 && sdk.MustNewDecFromStr(vote.Options[0].Weight).Equal(sdk.MustNewDecFromStr("1.0")) {
vote.Option = vote.Options[0].Option // nolint
}
}
4 changes: 0 additions & 4 deletions x/gov/keeper/vote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func TestVotes(t *testing.T) {
require.Equal(t, proposalID, vote.ProposalId)
require.True(t, len(vote.Options) == 1)
require.Equal(t, v1beta2.OptionAbstain, vote.Options[0].Option)
require.Equal(t, v1beta2.OptionAbstain, vote.Option)

// Test change of vote
require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], v1beta2.NewNonSplitVoteOption(v1beta2.OptionYes)))
Expand All @@ -50,7 +49,6 @@ func TestVotes(t *testing.T) {
require.Equal(t, proposalID, vote.ProposalId)
require.True(t, len(vote.Options) == 1)
require.Equal(t, v1beta2.OptionYes, vote.Options[0].Option)
require.Equal(t, v1beta2.OptionYes, vote.Option)

// Test second vote
require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], v1beta2.WeightedVoteOptions{
Expand All @@ -72,7 +70,6 @@ func TestVotes(t *testing.T) {
require.Equal(t, vote.Options[1].Weight, sdk.NewDecWithPrec(30, 2).String())
require.Equal(t, vote.Options[2].Weight, sdk.NewDecWithPrec(5, 2).String())
require.Equal(t, vote.Options[3].Weight, sdk.NewDecWithPrec(5, 2).String())
require.Equal(t, v1beta2.OptionEmpty, vote.Option)

// Test vote iterator
// NOTE order of deposits is determined by the addresses
Expand All @@ -90,5 +87,4 @@ func TestVotes(t *testing.T) {
require.Equal(t, votes[1].Options[1].Weight, sdk.NewDecWithPrec(30, 2).String())
require.Equal(t, votes[1].Options[2].Weight, sdk.NewDecWithPrec(5, 2).String())
require.Equal(t, votes[1].Options[3].Weight, sdk.NewDecWithPrec(5, 2).String())
require.Equal(t, v1beta2.OptionEmpty, vote.Option)
}
Loading

0 comments on commit 8133ee8

Please sign in to comment.