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

refactor!: move v1beta2 gov types into types dir #10763

Merged
merged 19 commits into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from 14 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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ client/lcd/keys/*
coverage.txt
profile.out
sim_log_file
x/genutil/config
x/genutil/data

# Vagrant
.vagrant/
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/upgrade) [\#10189](https://github.com/cosmos/cosmos-sdk/issues/10189) Removed potential sources of non-determinism in upgrades
* [\#10393](https://github.com/cosmos/cosmos-sdk/pull/10422) Add `MinCommissionRate` param to `x/staking` module.
* [#10725](https://github.com/cosmos/cosmos-sdk/pull/10725) populate `ctx.ConsensusParams` for begin/end blockers.
* [#10763](https://github.com/cosmos/cosmos-sdk/pull/10763) modify the fields in `TallyParams` to use `string` instead of `bytes`

### Deprecated

Expand Down
6 changes: 3 additions & 3 deletions docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -6165,9 +6165,9 @@ TallyParams defines the params for tallying votes on governance proposals.

| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| `quorum` | [bytes](#bytes) | | Minimum percentage of total stake needed to vote for a result to be considered valid. |
| `threshold` | [bytes](#bytes) | | Minimum proportion of Yes votes for proposal to pass. Default value: 0.5. |
| `veto_threshold` | [bytes](#bytes) | | Minimum value of Veto votes to Total votes ratio for proposal to be vetoed. Default value: 1/3. |
| `quorum` | [string](#string) | | Minimum percentage of total stake needed to vote for a result to be considered valid. |
| `threshold` | [string](#string) | | Minimum proportion of Yes votes for proposal to pass. Default value: 0.5. |
| `veto_threshold` | [string](#string) | | Minimum value of Veto votes to Total votes ratio for proposal to be vetoed. Default value: 1/3. |



Expand Down
2 changes: 1 addition & 1 deletion proto/cosmos/gov/v1beta2/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package cosmos.gov.v1beta2;

import "cosmos/gov/v1beta2/gov.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2";
option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types";
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

// GenesisState defines the gov module's genesis state.
message GenesisState {
Expand Down
14 changes: 7 additions & 7 deletions proto/cosmos/gov/v1beta2/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "google/protobuf/any.proto";
import "google/protobuf/duration.proto";
import "cosmos_proto/cosmos.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2";
option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types";

// VoteOption enumerates the valid vote options for a given governance proposal.
enum VoteOption {
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -35,7 +35,7 @@ message WeightedVoteOption {
message Deposit {
uint64 proposal_id = 1;
string depositor = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
repeated cosmos.base.v1beta1.Coin amount = 3;
repeated cosmos.base.v1beta1.Coin amount = 3 [(gogoproto.nullable) = false];
}

// Proposal defines the core field members of a governance proposal.
Expand All @@ -46,7 +46,7 @@ message Proposal {
TallyResult final_tally_result = 4;
google.protobuf.Timestamp submit_time = 5 [(gogoproto.stdtime) = true];
google.protobuf.Timestamp deposit_end_time = 6 [(gogoproto.stdtime) = true];
repeated cosmos.base.v1beta1.Coin total_deposit = 7;
repeated cosmos.base.v1beta1.Coin total_deposit = 7 [(gogoproto.nullable) = false];
google.protobuf.Timestamp voting_start_time = 8 [(gogoproto.stdtime) = true];
google.protobuf.Timestamp voting_end_time = 9 [(gogoproto.stdtime) = true];
}
Expand Down Expand Up @@ -95,7 +95,7 @@ message Vote {
// DepositParams defines the params for deposits on governance proposals.
message DepositParams {
// Minimum deposit for a proposal to enter voting period.
repeated cosmos.base.v1beta1.Coin min_deposit = 1;
repeated cosmos.base.v1beta1.Coin min_deposit = 1 [(gogoproto.nullable) = false];

// Maximum period for Atom holders to deposit on a proposal. Initial value: 2
// months.
Expand All @@ -112,12 +112,12 @@ message VotingParams {
message TallyParams {
// Minimum percentage of total stake needed to vote for a result to be
// considered valid.
bytes quorum = 1;
string quorum = 1 [(cosmos_proto.scalar) = "cosmos.Dec"];
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved

// Minimum proportion of Yes votes for proposal to pass. Default value: 0.5.
bytes threshold = 2;
string threshold = 2 [(cosmos_proto.scalar) = "cosmos.Dec"];

// Minimum value of Veto votes to Total votes ratio for proposal to be
// vetoed. Default value: 1/3.
bytes veto_threshold = 3;
string veto_threshold = 3 [(cosmos_proto.scalar) = "cosmos.Dec"];
}
Comment on lines 112 to 123
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To reviewers: This is one of the major changes made in this PR

Copy link
Contributor

Choose a reason for hiding this comment

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

This might need a migration, we can add one in another PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. I plan to write up a migration for proposals so I can couple it with this as well

2 changes: 1 addition & 1 deletion proto/cosmos/gov/v1beta2/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "google/api/annotations.proto";
import "cosmos/gov/v1beta2/gov.proto";
import "cosmos_proto/cosmos.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2";
option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types";

// Query defines the gRPC querier service for gov module
service Query {
Expand Down
7 changes: 4 additions & 3 deletions proto/cosmos/gov/v1beta2/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package cosmos.gov.v1beta2;

import "cosmos/base/v1beta1/coin.proto";
import "cosmos/gov/v1beta2/gov.proto";
import "gogoproto/gogo.proto";
import "cosmos_proto/cosmos.proto";
import "google/protobuf/any.proto";

option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta2";
option go_package = "github.com/cosmos/cosmos-sdk/x/gov/types";

// Msg defines the gov Msg service.
service Msg {
Expand All @@ -29,7 +30,7 @@ service Msg {
// proposal Content.
message MsgSubmitProposal {
repeated google.protobuf.Any messages = 1;
repeated cosmos.base.v1beta1.Coin initial_deposit = 2;
repeated cosmos.base.v1beta1.Coin initial_deposit = 2 [(gogoproto.nullable) = false];
string proposer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
}

Expand Down Expand Up @@ -66,7 +67,7 @@ message MsgVoteWeightedResponse {}
message MsgDeposit {
uint64 proposal_id = 1;
string depositor = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
repeated cosmos.base.v1beta1.Coin amount = 3;
repeated cosmos.base.v1beta1.Coin amount = 3 [(gogoproto.nullable) = false];
}

// MsgDepositResponse defines the Msg/Deposit response type.
Expand Down
2 changes: 1 addition & 1 deletion x/genutil/migrations/v043/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/genutil/types"
v040gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v040"
v043gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v043"
gov "github.com/cosmos/cosmos-sdk/x/gov/types"
gov "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

// Migrate migrates exported state from v0.40 to a v0.43 genesis state.
Expand Down
2 changes: 1 addition & 1 deletion x/gov/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/version"
gcutils "github.com/cosmos/cosmos-sdk/x/gov/client/utils"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

// GetQueryCmd returns the cli query commands for this module
Expand Down
6 changes: 4 additions & 2 deletions x/gov/client/testutil/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ func TestIntegrationTestSuite(t *testing.T) {
cfg.NumValidators = 1
suite.Run(t, NewIntegrationTestSuite(cfg))

dp := types.NewDepositParams(sdk.NewCoins(sdk.NewCoin(cfg.BondDenom, types.DefaultMinDepositTokens)), time.Duration(15)*time.Second)
vp := types.NewVotingParams(time.Duration(5) * time.Second)
genesisState := types.DefaultGenesisState()
genesisState.DepositParams = types.NewDepositParams(sdk.NewCoins(sdk.NewCoin(cfg.BondDenom, types.DefaultMinDepositTokens)), time.Duration(15)*time.Second)
genesisState.VotingParams = types.NewVotingParams(time.Duration(5) * time.Second)
genesisState.DepositParams = &dp
genesisState.VotingParams = &vp
bz, err := cfg.Codec.MarshalJSON(genesisState)
require.NoError(t, err)
cfg.GenesisState["gov"] = bz
Expand Down
10 changes: 5 additions & 5 deletions x/gov/migrations/v043/json.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package v043

import (
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

// migrateWeightedVotes migrates the ADR-037 weighted votes.
func migrateJSONWeightedVotes(oldVotes types.Votes) types.Votes {
newVotes := make(types.Votes, len(oldVotes))
func migrateJSONWeightedVotes(oldVotes v1beta1.Votes) v1beta1.Votes {
newVotes := make(v1beta1.Votes, len(oldVotes))
for i, oldVote := range oldVotes {
newVotes[i] = migrateVote(oldVote)
}
Expand All @@ -18,8 +18,8 @@ func migrateJSONWeightedVotes(oldVotes types.Votes) types.Votes {
// v0.43 x/gov genesis state. The migration includes:
//
// - Gov weighted votes.
func MigrateJSON(oldState *types.GenesisState) *types.GenesisState {
return &types.GenesisState{
func MigrateJSON(oldState *v1beta1.GenesisState) *v1beta1.GenesisState {
return &v1beta1.GenesisState{
StartingProposalId: oldState.StartingProposalId,
Deposits: oldState.Deposits,
Votes: migrateJSONWeightedVotes(oldState.Votes),
Expand Down
16 changes: 8 additions & 8 deletions x/gov/migrations/v043/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
v043gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v043"
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

func TestMigrateJSON(t *testing.T) {
Expand All @@ -23,13 +23,13 @@ func TestMigrateJSON(t *testing.T) {

voter, err := sdk.AccAddressFromBech32("cosmos1fl48vsnmsdzcv85q5d2q4z5ajdha8yu34mf0eh")
require.NoError(t, err)
govGenState := &types.GenesisState{
Votes: types.Votes{
types.Vote{ProposalId: 1, Voter: voter.String(), Option: types.OptionAbstain},
types.Vote{ProposalId: 2, Voter: voter.String(), Option: types.OptionEmpty},
types.Vote{ProposalId: 3, Voter: voter.String(), Option: types.OptionNo},
types.Vote{ProposalId: 4, Voter: voter.String(), Option: types.OptionNoWithVeto},
types.Vote{ProposalId: 5, Voter: voter.String(), Option: types.OptionYes},
govGenState := &v1beta1.GenesisState{
Votes: v1beta1.Votes{
v1beta1.Vote{ProposalId: 1, Voter: voter.String(), Option: v1beta1.OptionAbstain},
v1beta1.Vote{ProposalId: 2, Voter: voter.String(), Option: v1beta1.OptionEmpty},
v1beta1.Vote{ProposalId: 3, Voter: voter.String(), Option: v1beta1.OptionNo},
v1beta1.Vote{ProposalId: 4, Voter: voter.String(), Option: v1beta1.OptionNoWithVeto},
v1beta1.Vote{ProposalId: 5, Voter: voter.String(), Option: v1beta1.OptionYes},
},
}

Expand Down
9 changes: 5 additions & 4 deletions x/gov/migrations/v043/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

const proposalIDLen = 8
Expand Down Expand Up @@ -37,11 +38,11 @@ func migratePrefixProposalAddress(store sdk.KVStore, prefixBz []byte) {
// migrateStoreWeightedVotes migrates a legacy vote to an ADR-037 weighted vote.
// Important: the `oldVote` has its `Option` field set, whereas the new weighted
// vote has its `Options` field set.
func migrateVote(oldVote types.Vote) types.Vote {
return types.Vote{
func migrateVote(oldVote v1beta1.Vote) v1beta1.Vote {
return v1beta1.Vote{
ProposalId: oldVote.ProposalId,
Voter: oldVote.Voter,
Options: types.NewNonSplitVoteOption(oldVote.Option),
Options: v1beta1.NewNonSplitVoteOption(oldVote.Option),
}
}

Expand All @@ -51,7 +52,7 @@ func migrateStoreWeightedVotes(store sdk.KVStore, cdc codec.BinaryCodec) error {

defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
var oldVote types.Vote
var oldVote v1beta1.Vote
err := cdc.Unmarshal(iterator.Value(), &oldVote)
if err != nil {
return err
Expand Down
5 changes: 3 additions & 2 deletions x/gov/migrations/v043/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
v040gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v040"
v043gov "github.com/cosmos/cosmos-sdk/x/gov/migrations/v043"
"github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

func TestMigrateStore(t *testing.T) {
Expand All @@ -28,9 +29,9 @@ func TestMigrateStore(t *testing.T) {
// Use dummy value for keys where we don't test values.
dummyValue := []byte("foo")
// Use real values for votes, as we're testing weighted votes.
oldVote := types.Vote{ProposalId: 1, Voter: "foobar", Option: types.OptionNoWithVeto}
oldVote := v1beta1.Vote{ProposalId: 1, Voter: "foobar", Option: v1beta1.OptionNoWithVeto}
oldVoteValue := cdc.MustMarshal(&oldVote)
newVote := types.Vote{ProposalId: 1, Voter: "foobar", Options: types.WeightedVoteOptions{{Option: types.OptionNoWithVeto, Weight: sdk.NewDec(1)}}}
newVote := v1beta1.Vote{ProposalId: 1, Voter: "foobar", Options: v1beta1.WeightedVoteOptions{{Option: v1beta1.OptionNoWithVeto, Weight: sdk.NewDec(1)}}}
newVoteValue := cdc.MustMarshal(&newVote)

testCases := []struct {
Expand Down
14 changes: 7 additions & 7 deletions x/gov/simulation/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ func TestRandomizedGenState(t *testing.T) {
dec2, _ := sdk.NewDecFromStr("0.512000000000000000")
dec3, _ := sdk.NewDecFromStr("0.267000000000000000")

require.Equal(t, "905stake", govGenesis.DepositParams.MinDeposit.String())
require.Equal(t, "905stake", govGenesis.DepositParams.MinDeposit[0].String())
require.Equal(t, "77h26m10s", govGenesis.DepositParams.MaxDepositPeriod.String())
require.Equal(t, float64(148296), govGenesis.VotingParams.VotingPeriod.Seconds())
require.Equal(t, dec1, govGenesis.TallyParams.Quorum)
require.Equal(t, dec2, govGenesis.TallyParams.Threshold)
require.Equal(t, dec3, govGenesis.TallyParams.VetoThreshold)
require.Equal(t, dec1.String(), govGenesis.TallyParams.Quorum)
require.Equal(t, dec2.String(), govGenesis.TallyParams.Threshold)
require.Equal(t, dec3.String(), govGenesis.TallyParams.VetoThreshold)
require.Equal(t, uint64(0x28), govGenesis.StartingProposalId)
require.Equal(t, types.Deposits{}, govGenesis.Deposits)
require.Equal(t, types.Votes{}, govGenesis.Votes)
require.Equal(t, types.Proposals{}, govGenesis.Proposals)
require.Equal(t, []*types.Deposit{}, govGenesis.Deposits)
require.Equal(t, []*types.Vote{}, govGenesis.Votes)
require.Equal(t, []*types.Proposal{}, govGenesis.Proposals)
}

// TestRandomizedGenState tests abnormal scenarios of applying RandomizedGenState.
Expand Down
12 changes: 0 additions & 12 deletions x/gov/types/deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package types
import (
"fmt"

"sigs.k8s.io/yaml"

sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand All @@ -14,11 +12,6 @@ func NewDeposit(proposalID uint64, depositor sdk.AccAddress, amount sdk.Coins) D
return Deposit{proposalID, depositor.String(), amount}
}

func (d Deposit) String() string {
out, _ := yaml.Marshal(d)
return string(out)
}

// Deposits is a collection of Deposit objects
type Deposits []Deposit

Expand Down Expand Up @@ -47,8 +40,3 @@ func (d Deposits) String() string {
}
return out
}

// Empty returns whether a deposit is empty.
func (d Deposit) Empty() bool {
return d.String() == Deposit{}.String()
}
39 changes: 13 additions & 26 deletions x/gov/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@ import (
"fmt"

"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// NewGenesisState creates a new genesis state for the governance module
func NewGenesisState(startingProposalID uint64, dp DepositParams, vp VotingParams, tp TallyParams) *GenesisState {
return &GenesisState{
StartingProposalId: startingProposalID,
DepositParams: dp,
VotingParams: vp,
TallyParams: tp,
DepositParams: &dp,
VotingParams: &vp,
TallyParams: &tp,
}
}

Expand All @@ -27,38 +26,26 @@ func DefaultGenesisState() *GenesisState {
)
}

func (data GenesisState) Equal(other GenesisState) bool {
return data.StartingProposalId == other.StartingProposalId &&
data.Deposits.Equal(other.Deposits) &&
data.Votes.Equal(other.Votes) &&
data.Proposals.Equal(other.Proposals) &&
data.DepositParams.Equal(other.DepositParams) &&
data.TallyParams.Equal(other.TallyParams) &&
data.VotingParams.Equal(other.VotingParams)
}

// Empty returns true if a GenesisState is empty
func (data GenesisState) Empty() bool {
return data.Equal(GenesisState{})
return data.StartingProposalId == 0 ||
data.DepositParams == nil ||
data.VotingParams == nil ||
data.TallyParams == nil
}

// ValidateGenesis checks if parameters are within valid ranges
func ValidateGenesis(data *GenesisState) error {
threshold := data.TallyParams.Threshold
if threshold.IsNegative() || threshold.GT(sdk.OneDec()) {
return fmt.Errorf("governance vote threshold should be positive and less or equal to one, is %s",
threshold.String())
if err := validateTallyParams(data.TallyParams); err != nil {
return fmt.Errorf("invalid tally params: %w", err)
}

veto := data.TallyParams.VetoThreshold
if veto.IsNegative() || veto.GT(sdk.OneDec()) {
return fmt.Errorf("governance vote veto threshold should be positive and less or equal to one, is %s",
veto.String())
if err := validateVotingParams(data.VotingParams); err != nil {
return fmt.Errorf("invalid voting params: %w", err)
}

if !data.DepositParams.MinDeposit.IsValid() {
return fmt.Errorf("governance deposit amount must be a valid sdk.Coins amount, is %s",
data.DepositParams.MinDeposit.String())
if err := validateDepositParams(data.DepositParams); err != nil {
return fmt.Errorf("invalid deposit params: %w", err)
}

return nil
Expand Down
Loading