diff --git a/examples/democoin/mock/validator.go b/examples/democoin/mock/validator.go index fd5ec85b14..a54cdfedf6 100644 --- a/examples/democoin/mock/validator.go +++ b/examples/democoin/mock/validator.go @@ -24,13 +24,13 @@ func (v Validator) GetOperator() sdk.ValAddress { } // Implements sdk.Validator -func (v Validator) GetPubKey() crypto.PubKey { +func (v Validator) GetConsPubKey() crypto.PubKey { return nil } // Implements sdk.Validator func (v Validator) GetConsAddr() sdk.ConsAddress { - return sdk.ConsAddress{} + return nil } // Implements sdk.Validator @@ -48,11 +48,6 @@ func (v Validator) GetDelegatorShares() sdk.Dec { return sdk.ZeroDec() } -// Implements sdk.Validator -func (v Validator) GetCommission() sdk.Dec { - return sdk.ZeroDec() -} - // Implements sdk.Validator func (v Validator) GetJailed() bool { return false @@ -98,12 +93,12 @@ func (vs *ValidatorSet) Validator(ctx sdk.Context, addr sdk.ValAddress) sdk.Vali } // ValidatorByPubKey implements sdk.ValidatorSet -func (vs *ValidatorSet) ValidatorByConsPubKey(ctx sdk.Context, _ crypto.PubKey) sdk.Validator { +func (vs *ValidatorSet) ValidatorByConsPubKey(_ sdk.Context, _ crypto.PubKey) sdk.Validator { panic("not implemented") } // ValidatorByPubKey implements sdk.ValidatorSet -func (vs *ValidatorSet) ValidatorByConsAddr(ctx sdk.Context, _ sdk.ConsAddress) sdk.Validator { +func (vs *ValidatorSet) ValidatorByConsAddr(_ sdk.Context, _ sdk.ConsAddress) sdk.Validator { panic("not implemented") } @@ -137,7 +132,7 @@ func (vs *ValidatorSet) RemoveValidator(addr sdk.AccAddress) { } // Implements sdk.ValidatorSet -func (vs *ValidatorSet) Slash(ctx sdk.Context, _ sdk.ConsAddress, height int64, power int64, amt sdk.Dec) { +func (vs *ValidatorSet) Slash(_ sdk.Context, _ sdk.ConsAddress, _ int64, _ int64, _ sdk.Dec) { panic("not implemented") } @@ -152,6 +147,6 @@ func (vs *ValidatorSet) Unjail(_ sdk.Context, _ sdk.ConsAddress) { } // Implements sdk.ValidatorSet -func (vs *ValidatorSet) Delegation(ctx sdk.Context, addrDel sdk.AccAddress, addrVal sdk.ValAddress) sdk.Delegation { +func (vs *ValidatorSet) Delegation(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) sdk.Delegation { panic("not implemented") } diff --git a/types/address.go b/types/address.go index 58b694f5da..ae13b2ad0a 100644 --- a/types/address.go +++ b/types/address.go @@ -292,6 +292,11 @@ func ConsAddressFromBech32(address string) (addr ConsAddress, err error) { return ConsAddress(bz), nil } +// get ConsAddress from pubkey +func GetConsAddress(pubkey crypto.PubKey) ConsAddress { + return ConsAddress(pubkey.Address()) +} + // Returns boolean for whether two ConsAddress are Equal func (ca ConsAddress) Equals(ca2 ConsAddress) bool { if ca.Empty() && ca2.Empty() { diff --git a/types/address_test.go b/types/address_test.go index e2ec36876c..6c6c78d6ec 100644 --- a/types/address_test.go +++ b/types/address_test.go @@ -10,7 +10,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/cosmos/cosmos-sdk/types" -) + ) var invalidStrs = []string{ "", diff --git a/types/decimal.go b/types/decimal.go index d6c93f617a..13a494ba0c 100644 --- a/types/decimal.go +++ b/types/decimal.go @@ -174,13 +174,15 @@ func NewDecFromStr(str string) (d Dec, err Error) { //______________________________________________________________________________________________ //nolint -func (d Dec) IsZero() bool { return (d.Int).Sign() == 0 } // Is equal to zero -func (d Dec) Equal(d2 Dec) bool { return (d.Int).Cmp(d2.Int) == 0 } +func (d Dec) IsNil() bool { return d.Int == nil } // is decimal nil +func (d Dec) IsZero() bool { return (d.Int).Sign() == 0 } // is equal to zero +func (d Dec) Equal(d2 Dec) bool { return (d.Int).Cmp(d2.Int) == 0 } // equal decimals func (d Dec) GT(d2 Dec) bool { return (d.Int).Cmp(d2.Int) > 0 } // greater than func (d Dec) GTE(d2 Dec) bool { return (d.Int).Cmp(d2.Int) >= 0 } // greater than or equal func (d Dec) LT(d2 Dec) bool { return (d.Int).Cmp(d2.Int) < 0 } // less than func (d Dec) LTE(d2 Dec) bool { return (d.Int).Cmp(d2.Int) <= 0 } // less than or equal func (d Dec) Neg() Dec { return Dec{new(big.Int).Neg(d.Int)} } // reverse the decimal sign +func (d Dec) Abs() Dec { return Dec{new(big.Int).Abs(d.Int)} } // absolute value // addition func (d Dec) Add(d2 Dec) Dec { @@ -333,7 +335,7 @@ func chopPrecisionAndTruncateNonMutative(d *big.Int) *big.Int { return chopPrecisionAndTruncate(tmp) } -// RoundInt64 rounds the decimal using bankers rounding +// TruncateInt64 truncates the decimals from the number and returns an int64 func (d Dec) TruncateInt64() int64 { chopped := chopPrecisionAndTruncateNonMutative(d.Int) if !chopped.IsInt64() { @@ -342,7 +344,7 @@ func (d Dec) TruncateInt64() int64 { return chopped.Int64() } -// RoundInt round the decimal using bankers rounding +// TruncateInt truncates the decimals from the number and returns an Int func (d Dec) TruncateInt() Int { return NewIntFromBigInt(chopPrecisionAndTruncateNonMutative(d.Int)) } diff --git a/types/errors.go b/types/errors.go index 1d4900d3c2..46bf748f48 100644 --- a/types/errors.go +++ b/types/errors.go @@ -73,7 +73,6 @@ func unknownCodeMsg(code CodeType) string { } // NOTE: Don't stringer this, we'll put better messages in later. -// nolint: gocyclo func CodeToDefaultMsg(code CodeType) string { switch code { case CodeInternal: diff --git a/types/stake.go b/types/stake.go index ded7715486..e794ea7349 100644 --- a/types/stake.go +++ b/types/stake.go @@ -37,24 +37,23 @@ func (b BondStatus) Equal(b2 BondStatus) bool { // validator for a delegated proof of stake system type Validator interface { - GetJailed() bool // whether the validator is jailed - GetMoniker() string // moniker of the validator - GetStatus() BondStatus // status of the validator - GetOperator() ValAddress // operator address to receive/return validators coins - GetPubKey() crypto.PubKey // validation pubkey - GetConsAddr() ConsAddress // validation consensus address - GetPower() Dec // validation power - GetTokens() Dec // validation tokens - GetCommission() Dec // validator commission rate - GetDelegatorShares() Dec // Total delegator shares - GetBondHeight() int64 // height in which the validator became active + GetJailed() bool // whether the validator is jailed + GetMoniker() string // moniker of the validator + GetStatus() BondStatus // status of the validator + GetOperator() ValAddress // operator address to receive/return validators coins + GetConsPubKey() crypto.PubKey // validation consensus pubkey + GetConsAddr() ConsAddress // validation consensus address + GetPower() Dec // validation power + GetTokens() Dec // validation tokens + GetDelegatorShares() Dec // Total out standing delegator shares + GetBondHeight() int64 // height in which the validator became active } // validator which fulfills abci validator interface for use in Tendermint func ABCIValidator(v Validator) abci.Validator { return abci.Validator{ - PubKey: tmtypes.TM2PB.PubKey(v.GetPubKey()), - Address: v.GetPubKey().Address(), + PubKey: tmtypes.TM2PB.PubKey(v.GetConsPubKey()), + Address: v.GetConsPubKey().Address(), Power: v.GetPower().RoundInt64(), } } @@ -69,10 +68,9 @@ type ValidatorSet interface { IterateValidatorsBonded(Context, func(index int64, validator Validator) (stop bool)) - Validator(Context, ValAddress) Validator // get a particular validator by operator - ValidatorByConsPubKey(Context, crypto.PubKey) Validator // get a particular validator by consensus address - ValidatorByConsAddr(Context, ConsAddress) Validator // get a particular validator by consensus address - TotalPower(Context) Dec // total power of the validator set + Validator(Context, ValAddress) Validator // get a particular validator by operator address + ValidatorByConsAddr(Context, ConsAddress) Validator // get a particular validator by consensus address + TotalPower(Context) Dec // total power of the validator set // slash the validator and delegators of the validator, specifying offence height, offence power, and slash fraction Slash(Context, ConsAddress, int64, int64, Dec) @@ -89,8 +87,8 @@ type ValidatorSet interface { // delegation bond for a delegated proof of stake system type Delegation interface { GetDelegator() AccAddress // delegator AccAddress for the bond - GetValidator() ValAddress // validator operator address TODO change to GetValAddr - GetShares() Dec // amount of validator's shares + GetValidator() ValAddress // validator operator address + GetShares() Dec // amount of validator's shares held in this delegation } // properties for the set of all delegations for a particular @@ -103,25 +101,12 @@ type DelegationSet interface { fn func(index int64, delegation Delegation) (stop bool)) } -//_______________________________________________________________________________ -// Event Hooks -// These can be utilized to communicate between a staking keeper and another -// keeper which must take particular actions when validators/delegators change -// state. The second keeper must implement this interface, which then the -// staking keeper can call. - -// TODO refactor event hooks out to the receiver modules - -// event hooks for staking validator object -type StakingHooks interface { - OnValidatorCreated(ctx Context, address ValAddress) // Must be called when a validator is created - OnValidatorCommissionChange(ctx Context, address ValAddress) // Must be called when a validator's commission is modified - OnValidatorRemoved(ctx Context, address ValAddress) // Must be called when a validator is deleted - +// validator event hooks +// These can be utilized to communicate between a staking keeper +// and another keeper which must take particular actions when +// validators are bonded and unbonded. The second keeper must implement +// this interface, which then the staking keeper can call. +type ValidatorHooks interface { OnValidatorBonded(ctx Context, address ConsAddress) // Must be called when a validator is bonded OnValidatorBeginUnbonding(ctx Context, address ConsAddress) // Must be called when a validator begins unbonding - - OnDelegationCreated(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation is created - OnDelegationSharesModified(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation's shares are modified - OnDelegationRemoved(ctx Context, delAddr AccAddress, valAddr ValAddress) // Must be called when a delegation is removed } diff --git a/x/gov/client/cli/tx.go b/x/gov/client/cli/tx.go index 77364665a1..1bd01b58ab 100644 --- a/x/gov/client/cli/tx.go +++ b/x/gov/client/cli/tx.go @@ -285,7 +285,6 @@ func GetCmdQueryProposal(queryRoute string, cdc *codec.Codec) *cobra.Command { return cmd } -// nolint: gocyclo // GetCmdQueryProposals implements a query proposals command. func GetCmdQueryProposals(queryRoute string, cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ diff --git a/x/gov/client/rest/rest.go b/x/gov/client/rest/rest.go index 059c0bb1cf..3f62691c74 100644 --- a/x/gov/client/rest/rest.go +++ b/x/gov/client/rest/rest.go @@ -334,7 +334,6 @@ func queryVoteHandlerFn(cdc *codec.Codec) http.HandlerFunc { } } -// nolint: gocyclo // todo: Split this functionality into helper functions to remove the above func queryVotesOnProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -373,7 +372,6 @@ func queryVotesOnProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { } } -// nolint: gocyclo // todo: Split this functionality into helper functions to remove the above func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -439,7 +437,6 @@ func queryProposalsWithParameterFn(cdc *codec.Codec) http.HandlerFunc { } } -// nolint: gocyclo // todo: Split this functionality into helper functions to remove the above func queryTallyOnProposalHandlerFn(cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { diff --git a/x/gov/endblocker_test.go b/x/gov/endblocker_test.go index 710ecb1dba..27eff15f64 100644 --- a/x/gov/endblocker_test.go +++ b/x/gov/endblocker_test.go @@ -2,11 +2,11 @@ package gov import ( "testing" + "time" "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/stake" abci "github.com/tendermint/tendermint/abci/types" ) @@ -28,12 +28,18 @@ func TestTickExpiredDepositPeriod(t *testing.T) { require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) - ctx = ctx.WithBlockHeight(10) + newHeader := ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + EndBlocker(ctx, keeper) require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) - ctx = ctx.WithBlockHeight(250) + newHeader = ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod) + ctx = ctx.WithBlockHeader(newHeader) + require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.True(t, shouldPopInactiveProposalQueue(ctx, keeper)) EndBlocker(ctx, keeper) @@ -59,7 +65,10 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) - ctx = ctx.WithBlockHeight(10) + newHeader := ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(2) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + EndBlocker(ctx, keeper) require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) @@ -68,14 +77,20 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) { res = govHandler(ctx, newProposalMsg2) require.True(t, res.IsOK()) - ctx = ctx.WithBlockHeight(205) + newHeader = ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod).Add(time.Duration(-1) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.True(t, shouldPopInactiveProposalQueue(ctx, keeper)) EndBlocker(ctx, keeper) require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) - ctx = ctx.WithBlockHeight(215) + newHeader = ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(5) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.True(t, shouldPopInactiveProposalQueue(ctx, keeper)) EndBlocker(ctx, keeper) @@ -105,7 +120,10 @@ func TestTickPassedDepositPeriod(t *testing.T) { require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) - ctx = ctx.WithBlockHeight(10) + newHeader := ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + EndBlocker(ctx, keeper) require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx)) require.False(t, shouldPopInactiveProposalQueue(ctx, keeper)) @@ -146,14 +164,20 @@ func TestTickPassedVotingPeriod(t *testing.T) { var proposalID int64 keeper.cdc.UnmarshalBinaryBare(res.Data, &proposalID) - ctx = ctx.WithBlockHeight(10) + newHeader := ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second) + ctx = ctx.WithBlockHeader(newHeader) + newDepositMsg := NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin("steak", 5)}) res = govHandler(ctx, newDepositMsg) require.True(t, res.IsOK()) EndBlocker(ctx, keeper) - ctx = ctx.WithBlockHeight(215) + newHeader = ctx.BlockHeader() + newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod).Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod) + ctx = ctx.WithBlockHeader(newHeader) + require.True(t, shouldPopActiveProposalQueue(ctx, keeper)) depositsIterator := keeper.GetDeposits(ctx, proposalID) require.True(t, depositsIterator.Valid()) @@ -169,56 +193,3 @@ func TestTickPassedVotingPeriod(t *testing.T) { require.Equal(t, StatusRejected, keeper.GetProposal(ctx, proposalID).GetStatus()) require.True(t, keeper.GetProposal(ctx, proposalID).GetTallyResult().Equals(EmptyTallyResult())) } - -func TestSlashing(t *testing.T) { - mapp, keeper, sk, addrs, _, _ := getMockApp(t, 10) - SortAddresses(addrs) - mapp.BeginBlock(abci.RequestBeginBlock{}) - ctx := mapp.BaseApp.NewContext(false, abci.Header{}) - govHandler := NewHandler(keeper) - stakeHandler := stake.NewHandler(sk) - - valAddrs := make([]sdk.ValAddress, len(addrs[:3])) - for i, addr := range addrs[:3] { - valAddrs[i] = sdk.ValAddress(addr) - } - - createValidators(t, stakeHandler, ctx, valAddrs, []int64{25, 6, 7}) - - initTotalPower := keeper.ds.GetValidatorSet().TotalPower(ctx) - val0Initial := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[0])).GetPower().Quo(initTotalPower) - val1Initial := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[1])).GetPower().Quo(initTotalPower) - val2Initial := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[2])).GetPower().Quo(initTotalPower) - - newProposalMsg := NewMsgSubmitProposal("Test", "test", ProposalTypeText, addrs[0], sdk.Coins{sdk.NewInt64Coin("steak", 15)}) - - res := govHandler(ctx, newProposalMsg) - require.True(t, res.IsOK()) - var proposalID int64 - keeper.cdc.UnmarshalBinaryBare(res.Data, &proposalID) - - ctx = ctx.WithBlockHeight(10) - require.Equal(t, StatusVotingPeriod, keeper.GetProposal(ctx, proposalID).GetStatus()) - - newVoteMsg := NewMsgVote(addrs[0], proposalID, OptionYes) - res = govHandler(ctx, newVoteMsg) - require.True(t, res.IsOK()) - - EndBlocker(ctx, keeper) - - ctx = ctx.WithBlockHeight(215) - require.Equal(t, StatusVotingPeriod, keeper.GetProposal(ctx, proposalID).GetStatus()) - - EndBlocker(ctx, keeper) - - require.False(t, keeper.GetProposal(ctx, proposalID).GetTallyResult().Equals(EmptyTallyResult())) - - endTotalPower := keeper.ds.GetValidatorSet().TotalPower(ctx) - val0End := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[0])).GetPower().Quo(endTotalPower) - val1End := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[1])).GetPower().Quo(endTotalPower) - val2End := keeper.ds.GetValidatorSet().Validator(ctx, sdk.ValAddress(addrs[2])).GetPower().Quo(endTotalPower) - - require.True(t, val0End.GTE(val0Initial)) - require.True(t, val1End.LT(val1Initial)) - require.True(t, val2End.LT(val2Initial)) -} diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 15f952c000..58273c8e84 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -1,6 +1,8 @@ package gov import ( + "time" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -27,10 +29,10 @@ func DefaultGenesisState() GenesisState { StartingProposalID: 1, DepositProcedure: DepositProcedure{ MinDeposit: sdk.Coins{sdk.NewInt64Coin("steak", 10)}, - MaxDepositPeriod: 200, + MaxDepositPeriod: time.Duration(172800) * time.Second, }, VotingProcedure: VotingProcedure{ - VotingPeriod: 200, + VotingPeriod: time.Duration(172800) * time.Second, }, TallyingProcedure: TallyingProcedure{ Threshold: sdk.NewDecWithPrec(5, 1), diff --git a/x/gov/handler.go b/x/gov/handler.go index 84964b20f6..3f8cd312b6 100644 --- a/x/gov/handler.go +++ b/x/gov/handler.go @@ -128,13 +128,13 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (resTags sdk.Tags) { for shouldPopActiveProposalQueue(ctx, keeper) { activeProposal := keeper.ActiveProposalQueuePop(ctx) - proposalStartBlock := activeProposal.GetVotingStartBlock() + proposalStartTime := activeProposal.GetVotingStartTime() votingPeriod := keeper.GetVotingProcedure(ctx).VotingPeriod - if ctx.BlockHeight() < proposalStartBlock+votingPeriod { + if ctx.BlockHeader().Time.Before(proposalStartTime.Add(votingPeriod)) { continue } - passes, tallyResults, nonVotingVals := tally(ctx, keeper, activeProposal) + passes, tallyResults := tally(ctx, keeper, activeProposal) proposalIDBytes := keeper.cdc.MustMarshalBinaryBare(activeProposal.GetProposalID()) var action []byte if passes { @@ -152,18 +152,6 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (resTags sdk.Tags) { logger.Info(fmt.Sprintf("proposal %d (%s) tallied; passed: %v", activeProposal.GetProposalID(), activeProposal.GetTitle(), passes)) - for _, valAddr := range nonVotingVals { - val := keeper.ds.GetValidatorSet().Validator(ctx, valAddr) - keeper.ds.GetValidatorSet().Slash(ctx, - val.GetConsAddr(), - ctx.BlockHeight(), - val.GetPower().RoundInt64(), - keeper.GetTallyingProcedure(ctx).GovernancePenalty) - - logger.Info(fmt.Sprintf("validator %s failed to vote on proposal %d; slashing", - val.GetOperator(), activeProposal.GetProposalID())) - } - resTags.AppendTag(tags.Action, action) resTags.AppendTag(tags.ProposalID, proposalIDBytes) } @@ -178,7 +166,7 @@ func shouldPopInactiveProposalQueue(ctx sdk.Context, keeper Keeper) bool { return false } else if peekProposal.GetStatus() != StatusDepositPeriod { return true - } else if ctx.BlockHeight() >= peekProposal.GetSubmitBlock()+depositProcedure.MaxDepositPeriod { + } else if !ctx.BlockHeader().Time.Before(peekProposal.GetSubmitTime().Add(depositProcedure.MaxDepositPeriod)) { return true } return false @@ -190,7 +178,7 @@ func shouldPopActiveProposalQueue(ctx sdk.Context, keeper Keeper) bool { if peekProposal == nil { return false - } else if ctx.BlockHeight() >= peekProposal.GetVotingStartBlock()+votingProcedure.VotingPeriod { + } else if !ctx.BlockHeader().Time.Before(peekProposal.GetVotingStartTime().Add(votingProcedure.VotingPeriod)) { return true } return false diff --git a/x/gov/keeper.go b/x/gov/keeper.go index bebcf51e78..80e5a205cd 100644 --- a/x/gov/keeper.go +++ b/x/gov/keeper.go @@ -38,7 +38,11 @@ type Keeper struct { codespace sdk.CodespaceType } -// NewGovernanceMapper returns a mapper that uses go-codec to (binary) encode and decode gov types. +// NewKeeper returns a governance keeper. It handles: +// - submitting governance proposals +// - depositing funds into proposals, and activating upon sufficient funds being deposited +// - users voting on proposals, with weight proportional to stake in the system +// - and tallying the result of the vote. func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ps params.Setter, ck bank.Keeper, ds sdk.DelegationSet, codespace sdk.CodespaceType) Keeper { return Keeper{ storeKey: key, @@ -51,11 +55,6 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, ps params.Setter, ck bank.Kee } } -// Returns the go-codec codec. -func (keeper Keeper) WireCodec() *codec.Codec { - return keeper.cdc -} - // ===================================================== // Proposals @@ -66,15 +65,14 @@ func (keeper Keeper) NewTextProposal(ctx sdk.Context, title string, description return nil } var proposal Proposal = &TextProposal{ - ProposalID: proposalID, - Title: title, - Description: description, - ProposalType: proposalType, - Status: StatusDepositPeriod, - TallyResult: EmptyTallyResult(), - TotalDeposit: sdk.Coins{}, - SubmitBlock: ctx.BlockHeight(), - VotingStartBlock: -1, // TODO: Make Time + ProposalID: proposalID, + Title: title, + Description: description, + ProposalType: proposalType, + Status: StatusDepositPeriod, + TallyResult: EmptyTallyResult(), + TotalDeposit: sdk.Coins{}, + SubmitTime: ctx.BlockHeader().Time, } keeper.SetProposal(ctx, proposal) keeper.InactiveProposalQueuePush(ctx, proposal) @@ -108,7 +106,6 @@ func (keeper Keeper) DeleteProposal(ctx sdk.Context, proposal Proposal) { store.Delete(KeyProposal(proposal.GetProposalID())) } -// nolint: gocyclo // Get Proposal from store by ProposalID func (keeper Keeper) GetProposalsFiltered(ctx sdk.Context, voterAddr sdk.AccAddress, depositerAddr sdk.AccAddress, status ProposalStatus, numLatest int64) []Proposal { @@ -200,7 +197,7 @@ func (keeper Keeper) peekCurrentProposalID(ctx sdk.Context) (proposalID int64, e } func (keeper Keeper) activateVotingPeriod(ctx sdk.Context, proposal Proposal) { - proposal.SetVotingStartBlock(ctx.BlockHeight()) + proposal.SetVotingStartTime(ctx.BlockHeader().Time) proposal.SetStatus(StatusVotingPeriod) keeper.SetProposal(ctx, proposal) keeper.ActiveProposalQueuePush(ctx, proposal) diff --git a/x/gov/keeper_test.go b/x/gov/keeper_test.go index a61292b93e..91c41d7d7d 100644 --- a/x/gov/keeper_test.go +++ b/x/gov/keeper_test.go @@ -2,6 +2,7 @@ package gov import ( "testing" + "time" "github.com/stretchr/testify/require" @@ -45,12 +46,12 @@ func TestActivateVotingPeriod(t *testing.T) { proposal := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText) - require.Equal(t, int64(-1), proposal.GetVotingStartBlock()) + require.True(t, proposal.GetVotingStartTime().Equal(time.Time{})) require.Nil(t, keeper.ActiveProposalQueuePeek(ctx)) keeper.activateVotingPeriod(ctx, proposal) - require.Equal(t, proposal.GetVotingStartBlock(), ctx.BlockHeight()) + require.True(t, proposal.GetVotingStartTime().Equal(ctx.BlockHeader().Time)) require.Equal(t, proposal.GetProposalID(), keeper.ActiveProposalQueuePeek(ctx).GetProposalID()) } @@ -77,7 +78,7 @@ func TestDeposits(t *testing.T) { // Check no deposits at beginning deposit, found := keeper.GetDeposit(ctx, proposalID, addrs[1]) require.False(t, found) - require.Equal(t, keeper.GetProposal(ctx, proposalID).GetVotingStartBlock(), int64(-1)) + require.True(t, keeper.GetProposal(ctx, proposalID).GetVotingStartTime().Equal(time.Time{})) require.Nil(t, keeper.ActiveProposalQueuePeek(ctx)) // Check first deposit @@ -114,7 +115,7 @@ func TestDeposits(t *testing.T) { require.Equal(t, addr1Initial.Minus(fourSteak), keeper.ck.GetCoins(ctx, addrs[1])) // Check that proposal moved to voting period - require.Equal(t, ctx.BlockHeight(), keeper.GetProposal(ctx, proposalID).GetVotingStartBlock()) + require.True(t, keeper.GetProposal(ctx, proposalID).GetVotingStartTime().Equal(ctx.BlockHeader().Time)) require.NotNil(t, keeper.ActiveProposalQueuePeek(ctx)) require.Equal(t, proposalID, keeper.ActiveProposalQueuePeek(ctx).GetProposalID()) diff --git a/x/gov/procedures.go b/x/gov/procedures.go index f74091c74f..e453add791 100644 --- a/x/gov/procedures.go +++ b/x/gov/procedures.go @@ -1,13 +1,15 @@ package gov import ( + "time" + sdk "github.com/cosmos/cosmos-sdk/types" ) // Procedure around Deposits for governance type DepositProcedure struct { - MinDeposit sdk.Coins `json:"min_deposit"` // Minimum deposit for a proposal to enter voting period. - MaxDepositPeriod int64 `json:"max_deposit_period"` // Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months + MinDeposit sdk.Coins `json:"min_deposit"` // Minimum deposit for a proposal to enter voting period. + MaxDepositPeriod time.Duration `json:"max_deposit_period"` // Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months } // Procedure around Tallying votes in governance @@ -19,5 +21,5 @@ type TallyingProcedure struct { // Procedure around Voting in governance type VotingProcedure struct { - VotingPeriod int64 `json:"voting_period"` // Length of the voting period. + VotingPeriod time.Duration `json:"voting_period"` // Length of the voting period. } diff --git a/x/gov/proposals.go b/x/gov/proposals.go index b4c6783673..37e29df704 100644 --- a/x/gov/proposals.go +++ b/x/gov/proposals.go @@ -3,6 +3,7 @@ package gov import ( "encoding/json" "fmt" + "time" "github.com/pkg/errors" @@ -30,14 +31,14 @@ type Proposal interface { GetTallyResult() TallyResult SetTallyResult(TallyResult) - GetSubmitBlock() int64 - SetSubmitBlock(int64) + GetSubmitTime() time.Time + SetSubmitTime(time.Time) GetTotalDeposit() sdk.Coins SetTotalDeposit(sdk.Coins) - GetVotingStartBlock() int64 - SetVotingStartBlock(int64) + GetVotingStartTime() time.Time + SetVotingStartTime(time.Time) } // checks if two proposals are equal @@ -48,9 +49,9 @@ func ProposalEqual(proposalA Proposal, proposalB Proposal) bool { proposalA.GetProposalType() == proposalB.GetProposalType() && proposalA.GetStatus() == proposalB.GetStatus() && proposalA.GetTallyResult().Equals(proposalB.GetTallyResult()) && - proposalA.GetSubmitBlock() == proposalB.GetSubmitBlock() && + proposalA.GetSubmitTime().Equal(proposalB.GetSubmitTime()) && proposalA.GetTotalDeposit().IsEqual(proposalB.GetTotalDeposit()) && - proposalA.GetVotingStartBlock() == proposalB.GetVotingStartBlock() { + proposalA.GetVotingStartTime().Equal(proposalB.GetVotingStartTime()) { return true } return false @@ -67,10 +68,10 @@ type TextProposal struct { Status ProposalStatus `json:"proposal_status"` // Status of the Proposal {Pending, Active, Passed, Rejected} TallyResult TallyResult `json:"tally_result"` // Result of Tallys - SubmitBlock int64 `json:"submit_block"` // Height of the block where TxGovSubmitProposal was included + SubmitTime time.Time `json:"submit_block"` // Height of the block where TxGovSubmitProposal was included TotalDeposit sdk.Coins `json:"total_deposit"` // Current deposit on this proposal. Initial value is set at InitialDeposit - VotingStartBlock int64 `json:"voting_start_block"` // Height of the block where MinDeposit was reached. -1 if MinDeposit is not reached + VotingStartTime time.Time `json:"voting_start_block"` // Height of the block where MinDeposit was reached. -1 if MinDeposit is not reached } // Implements Proposal Interface @@ -89,13 +90,13 @@ func (tp TextProposal) GetStatus() ProposalStatus { return tp.S func (tp *TextProposal) SetStatus(status ProposalStatus) { tp.Status = status } func (tp TextProposal) GetTallyResult() TallyResult { return tp.TallyResult } func (tp *TextProposal) SetTallyResult(tallyResult TallyResult) { tp.TallyResult = tallyResult } -func (tp TextProposal) GetSubmitBlock() int64 { return tp.SubmitBlock } -func (tp *TextProposal) SetSubmitBlock(submitBlock int64) { tp.SubmitBlock = submitBlock } +func (tp TextProposal) GetSubmitTime() time.Time { return tp.SubmitTime } +func (tp *TextProposal) SetSubmitTime(submitTime time.Time) { tp.SubmitTime = submitTime } func (tp TextProposal) GetTotalDeposit() sdk.Coins { return tp.TotalDeposit } func (tp *TextProposal) SetTotalDeposit(totalDeposit sdk.Coins) { tp.TotalDeposit = totalDeposit } -func (tp TextProposal) GetVotingStartBlock() int64 { return tp.VotingStartBlock } -func (tp *TextProposal) SetVotingStartBlock(votingStartBlock int64) { - tp.VotingStartBlock = votingStartBlock +func (tp TextProposal) GetVotingStartTime() time.Time { return tp.VotingStartTime } +func (tp *TextProposal) SetVotingStartTime(votingStartTime time.Time) { + tp.VotingStartTime = votingStartTime } //----------------------------------------------------------- @@ -191,8 +192,9 @@ func (pt ProposalKind) String() string { func (pt ProposalKind) Format(s fmt.State, verb rune) { switch verb { case 's': - s.Write([]byte(fmt.Sprintf("%s", pt.String()))) + s.Write([]byte(pt.String())) default: + // TODO: Do this conversion more directly s.Write([]byte(fmt.Sprintf("%v", byte(pt)))) } } @@ -294,8 +296,9 @@ func (status ProposalStatus) String() string { func (status ProposalStatus) Format(s fmt.State, verb rune) { switch verb { case 's': - s.Write([]byte(fmt.Sprintf("%s", status.String()))) + s.Write([]byte(status.String())) default: + // TODO: Do this conversion more directly s.Write([]byte(fmt.Sprintf("%v", byte(status)))) } } @@ -321,11 +324,8 @@ func EmptyTallyResult() TallyResult { // checks if two proposals are equal func (resultA TallyResult) Equals(resultB TallyResult) bool { - if resultA.Yes.Equal(resultB.Yes) && + return (resultA.Yes.Equal(resultB.Yes) && resultA.Abstain.Equal(resultB.Abstain) && resultA.No.Equal(resultB.No) && - resultA.NoWithVeto.Equal(resultB.NoWithVeto) { - return true - } - return false + resultA.NoWithVeto.Equal(resultB.NoWithVeto)) } diff --git a/x/gov/proposals_test.go b/x/gov/proposals_test.go new file mode 100644 index 0000000000..1d5f2cf389 --- /dev/null +++ b/x/gov/proposals_test.go @@ -0,0 +1,40 @@ +package gov + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestProposalKind_Format(t *testing.T) { + typeText, _ := ProposalTypeFromString("Text") + tests := []struct { + pt ProposalKind + sprintFArgs string + expectedStringOutput string + }{ + {typeText, "%s", "Text"}, + {typeText, "%v", "1"}, + } + for _, tt := range tests { + got := fmt.Sprintf(tt.sprintFArgs, tt.pt) + require.Equal(t, tt.expectedStringOutput, got) + } +} + +func TestProposalStatus_Format(t *testing.T) { + statusDepositPeriod, _ := ProposalStatusFromString("DepositPeriod") + tests := []struct { + pt ProposalStatus + sprintFArgs string + expectedStringOutput string + }{ + {statusDepositPeriod, "%s", "DepositPeriod"}, + {statusDepositPeriod, "%v", "1"}, + } + for _, tt := range tests { + got := fmt.Sprintf(tt.sprintFArgs, tt.pt) + require.Equal(t, tt.expectedStringOutput, got) + } +} diff --git a/x/gov/queryable.go b/x/gov/queryable.go index f20bb46f78..606f73a9ca 100644 --- a/x/gov/queryable.go +++ b/x/gov/queryable.go @@ -205,12 +205,12 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke var proposalID int64 err2 := keeper.cdc.UnmarshalJSON(req.Data, proposalID) if err2 != nil { - return []byte{}, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) + return res, sdk.ErrUnknownRequest(fmt.Sprintf("incorrectly formatted request data - %s", err2.Error())) } proposal := keeper.GetProposal(ctx, proposalID) if proposal == nil { - return []byte{}, ErrUnknownProposal(DefaultCodespace, proposalID) + return res, ErrUnknownProposal(DefaultCodespace, proposalID) } var tallyResult TallyResult @@ -220,7 +220,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke } else if proposal.GetStatus() == StatusPassed || proposal.GetStatus() == StatusRejected { tallyResult = proposal.GetTallyResult() } else { - _, tallyResult, _ = tally(ctx, keeper, proposal) + _, tallyResult = tally(ctx, keeper, proposal) } bz, err2 := codec.MarshalJSONIndent(keeper.cdc, tallyResult) diff --git a/x/gov/simulation/msgs.go b/x/gov/simulation/msgs.go index 168081b188..d5ff1881e4 100644 --- a/x/gov/simulation/msgs.go +++ b/x/gov/simulation/msgs.go @@ -4,8 +4,7 @@ import ( "fmt" "math" "math/rand" - - "github.com/tendermint/tendermint/crypto" + "time" "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" @@ -44,9 +43,9 @@ func SimulateSubmittingVotingAndSlashingForProposal(k gov.Keeper, sk stake.Keepe }) statePercentageArray := []float64{1, .9, .75, .4, .15, 0} curNumVotesState := 1 - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOps []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOps []simulation.FutureOperation, err error) { // 1) submit proposal now - sender := simulation.RandomKey(r, keys) + sender := simulation.RandomAcc(r, accs) msg, err := simulationCreateMsgSubmitProposal(r, sender) if err != nil { return "", nil, err @@ -60,16 +59,16 @@ func SimulateSubmittingVotingAndSlashingForProposal(k gov.Keeper, sk stake.Keepe // 2) Schedule operations for votes // 2.1) first pick a number of people to vote. curNumVotesState = numVotesTransitionMatrix.NextState(r, curNumVotesState) - numVotes := int(math.Ceil(float64(len(keys)) * statePercentageArray[curNumVotesState])) + numVotes := int(math.Ceil(float64(len(accs)) * statePercentageArray[curNumVotesState])) // 2.2) select who votes and when - whoVotes := r.Perm(len(keys)) + whoVotes := r.Perm(len(accs)) // didntVote := whoVotes[numVotes:] whoVotes = whoVotes[:numVotes] votingPeriod := k.GetVotingProcedure(ctx).VotingPeriod fops := make([]simulation.FutureOperation, numVotes+1) for i := 0; i < numVotes; i++ { - whenVote := ctx.BlockHeight() + r.Int63n(votingPeriod) - fops[i] = simulation.FutureOperation{BlockHeight: int(whenVote), Op: operationSimulateMsgVote(k, sk, keys[whoVotes[i]], proposalID)} + whenVote := ctx.BlockHeader().Time.Add(time.Duration(r.Int63n(int64(votingPeriod.Seconds()))) * time.Second) + fops[i] = simulation.FutureOperation{BlockTime: whenVote, Op: operationSimulateMsgVote(k, sk, accs[whoVotes[i]], proposalID)} } // 3) Make an operation to ensure slashes were done correctly. (Really should be a future invariant) // TODO: Find a way to check if a validator was slashed other than just checking their balance a block @@ -83,8 +82,8 @@ func SimulateSubmittingVotingAndSlashingForProposal(k gov.Keeper, sk stake.Keepe // Note: Currently doesn't ensure that the proposal txt is in JSON form func SimulateMsgSubmitProposal(k gov.Keeper, sk stake.Keeper) simulation.Operation { handler := gov.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOps []simulation.FutureOperation, err error) { - sender := simulation.RandomKey(r, keys) + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOps []simulation.FutureOperation, err error) { + sender := simulation.RandomAcc(r, accs) msg, err := simulationCreateMsgSubmitProposal(r, sender) if err != nil { return "", nil, err @@ -110,14 +109,13 @@ func simulateHandleMsgSubmitProposal(msg gov.MsgSubmitProposal, sk stake.Keeper, return } -func simulationCreateMsgSubmitProposal(r *rand.Rand, sender crypto.PrivKey) (msg gov.MsgSubmitProposal, err error) { - addr := sdk.AccAddress(sender.PubKey().Address()) +func simulationCreateMsgSubmitProposal(r *rand.Rand, sender simulation.Account) (msg gov.MsgSubmitProposal, err error) { deposit := randomDeposit(r) msg = gov.NewMsgSubmitProposal( simulation.RandStringOfLength(r, 5), simulation.RandStringOfLength(r, 5), gov.ProposalTypeText, - addr, + sender.Address, deposit, ) if msg.ValidateBasic() != nil { @@ -128,15 +126,14 @@ func simulationCreateMsgSubmitProposal(r *rand.Rand, sender crypto.PrivKey) (msg // SimulateMsgDeposit func SimulateMsgDeposit(k gov.Keeper, sk stake.Keeper) simulation.Operation { - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { - key := simulation.RandomKey(r, keys) - addr := sdk.AccAddress(key.PubKey().Address()) + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + acc := simulation.RandomAcc(r, accs) proposalID, ok := randomProposalID(r, k, ctx) if !ok { return "no-operation", nil, nil } deposit := randomDeposit(r) - msg := gov.NewMsgDeposit(addr, proposalID, deposit) + msg := gov.NewMsgDeposit(acc.Address, proposalID, deposit) if msg.ValidateBasic() != nil { return "", nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes()) } @@ -158,14 +155,14 @@ func SimulateMsgDeposit(k gov.Keeper, sk stake.Keeper) simulation.Operation { // SimulateMsgVote // nolint: unparam func SimulateMsgVote(k gov.Keeper, sk stake.Keeper) simulation.Operation { - return operationSimulateMsgVote(k, sk, nil, -1) + return operationSimulateMsgVote(k, sk, simulation.Account{}, -1) } // nolint: unparam -func operationSimulateMsgVote(k gov.Keeper, sk stake.Keeper, key crypto.PrivKey, proposalID int64) simulation.Operation { - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { - if key == nil { - key = simulation.RandomKey(r, keys) +func operationSimulateMsgVote(k gov.Keeper, sk stake.Keeper, acc simulation.Account, proposalID int64) simulation.Operation { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + if acc.Equals(simulation.Account{}) { + acc = simulation.RandomAcc(r, accs) } var ok bool @@ -176,10 +173,9 @@ func operationSimulateMsgVote(k gov.Keeper, sk stake.Keeper, key crypto.PrivKey, return "no-operation", nil, nil } } - addr := sdk.AccAddress(key.PubKey().Address()) option := randomVotingOption(r) - msg := gov.NewMsgVote(addr, proposalID, option) + msg := gov.NewMsgVote(acc.Address, proposalID, option) if msg.ValidateBasic() != nil { return "", nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes()) } diff --git a/x/gov/simulation/sim_test.go b/x/gov/simulation/sim_test.go index 35a11fb078..b0317d1161 100644 --- a/x/gov/simulation/sim_test.go +++ b/x/gov/simulation/sim_test.go @@ -6,7 +6,6 @@ import ( "testing" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/bank" @@ -43,12 +42,12 @@ func TestGovWithRandomMessages(t *testing.T) { panic(err) } - appStateFn := func(r *rand.Rand, keys []crypto.PrivKey, accs []sdk.AccAddress) json.RawMessage { - mock.RandomSetGenesis(r, mapp, accs, []string{"stake"}) + appStateFn := func(r *rand.Rand, accs []simulation.Account) json.RawMessage { + simulation.RandomSetGenesis(r, mapp, accs, []string{"stake"}) return json.RawMessage("{}") } - setup := func(r *rand.Rand, privKeys []crypto.PrivKey) { + setup := func(r *rand.Rand, accs []simulation.Account) { ctx := mapp.NewContext(false, abci.Header{}) stake.InitGenesis(ctx, stakeKeeper, stake.DefaultGenesisState()) gov.InitGenesis(ctx, govKeeper, gov.DefaultGenesisState()) diff --git a/x/gov/tally.go b/x/gov/tally.go index 55fd81da24..c5751258a0 100644 --- a/x/gov/tally.go +++ b/x/gov/tally.go @@ -13,7 +13,7 @@ type validatorGovInfo struct { Vote VoteOption // Vote of the validator } -func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tallyResults TallyResult, nonVoting []sdk.ValAddress) { +func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tallyResults TallyResult) { results := make(map[VoteOption]sdk.Dec) results[OptionYes] = sdk.ZeroDec() results[OptionAbstain] = sdk.ZeroDec() @@ -70,12 +70,9 @@ func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tall keeper.deleteVote(ctx, vote.ProposalID, vote.Voter) } - // iterate over the validators again to tally their voting power and see - // who didn't vote - nonVoting = []sdk.ValAddress{} + // iterate over the validators again to tally their voting power for _, val := range currValidators { if val.Vote == OptionEmpty { - nonVoting = append(nonVoting, val.Address) continue } @@ -98,19 +95,17 @@ func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tall // If no one votes, proposal fails if totalVotingPower.Sub(results[OptionAbstain]).Equal(sdk.ZeroDec()) { - return false, tallyResults, nonVoting + return false, tallyResults } // If more than 1/3 of voters veto, proposal fails if results[OptionNoWithVeto].Quo(totalVotingPower).GT(tallyingProcedure.Veto) { - return false, tallyResults, nonVoting + return false, tallyResults } // If more than 1/2 of non-abstaining voters vote Yes, proposal passes if results[OptionYes].Quo(totalVotingPower.Sub(results[OptionAbstain])).GT(tallyingProcedure.Threshold) { - return true, tallyResults, nonVoting + return true, tallyResults } // If more than 1/2 of non-abstaining voters vote No, proposal fails - SortValAddresses(nonVoting) - - return false, tallyResults, nonVoting + return false, tallyResults } diff --git a/x/gov/tally_test.go b/x/gov/tally_test.go index 9c5348b2c7..28fe0cb599 100644 --- a/x/gov/tally_test.go +++ b/x/gov/tally_test.go @@ -15,13 +15,19 @@ import ( var ( pubkeys = []crypto.PubKey{ed25519.GenPrivKey().PubKey(), ed25519.GenPrivKey().PubKey(), ed25519.GenPrivKey().PubKey()} + + testDescription = stake.NewDescription("T", "E", "S", "T") + testCommissionMsg = stake.NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) ) func createValidators(t *testing.T, stakeHandler sdk.Handler, ctx sdk.Context, addrs []sdk.ValAddress, coinAmt []int64) { require.True(t, len(addrs) <= len(pubkeys), "Not enough pubkeys specified at top of file.") - dummyDescription := stake.NewDescription("T", "E", "S", "T") + for i := 0; i < len(addrs); i++ { - valCreateMsg := stake.NewMsgCreateValidator(addrs[i], pubkeys[i], sdk.NewInt64Coin("steak", coinAmt[i]), dummyDescription) + valCreateMsg := stake.NewMsgCreateValidator( + addrs[i], pubkeys[i], sdk.NewInt64Coin("steak", coinAmt[i]), testDescription, testCommissionMsg, + ) + res := stakeHandler(ctx, valCreateMsg) require.True(t, res.IsOK()) } @@ -45,7 +51,7 @@ func TestTallyNoOneVotes(t *testing.T) { proposal.SetStatus(StatusVotingPeriod) keeper.SetProposal(ctx, proposal) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.True(t, tallyResults.Equals(EmptyTallyResult())) @@ -74,7 +80,7 @@ func TestTallyOnlyValidatorsAllYes(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[1], OptionYes) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.True(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -103,7 +109,7 @@ func TestTallyOnlyValidators51No(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[1], OptionNo) require.Nil(t, err) - passes, _, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) } @@ -133,7 +139,7 @@ func TestTallyOnlyValidators51Yes(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.True(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -164,7 +170,7 @@ func TestTallyOnlyValidatorsVetoed(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNoWithVeto) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -195,7 +201,7 @@ func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionYes) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.True(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -226,7 +232,7 @@ func TestTallyOnlyValidatorsAbstainFails(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -255,11 +261,9 @@ func TestTallyOnlyValidatorsNonVoter(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNo) require.Nil(t, err) - passes, tallyResults, nonVoting := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) - require.Equal(t, 1, len(nonVoting)) - require.Equal(t, sdk.ValAddress(addrs[0]), nonVoting[0]) require.False(t, tallyResults.Equals(EmptyTallyResult())) } @@ -293,7 +297,7 @@ func TestTallyDelgatorOverride(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[3], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -327,10 +331,9 @@ func TestTallyDelgatorInherit(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionYes) require.Nil(t, err) - passes, tallyResults, nonVoting := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.True(t, passes) - require.Equal(t, 0, len(nonVoting)) require.False(t, tallyResults.Equals(EmptyTallyResult())) } @@ -366,7 +369,7 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[3], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -378,20 +381,18 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { ctx := mapp.BaseApp.NewContext(false, abci.Header{}) stakeHandler := stake.NewHandler(sk) - dummyDescription := stake.NewDescription("T", "E", "S", "T") - val1CreateMsg := stake.NewMsgCreateValidator( - sdk.ValAddress(addrs[0]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 25), dummyDescription, + sdk.ValAddress(addrs[0]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 25), testDescription, testCommissionMsg, ) stakeHandler(ctx, val1CreateMsg) val2CreateMsg := stake.NewMsgCreateValidator( - sdk.ValAddress(addrs[1]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 6), dummyDescription, + sdk.ValAddress(addrs[1]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 6), testDescription, testCommissionMsg, ) stakeHandler(ctx, val2CreateMsg) val3CreateMsg := stake.NewMsgCreateValidator( - sdk.ValAddress(addrs[2]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 7), dummyDescription, + sdk.ValAddress(addrs[2]), ed25519.GenPrivKey().PubKey(), sdk.NewInt64Coin("steak", 7), testDescription, testCommissionMsg, ) stakeHandler(ctx, val3CreateMsg) @@ -413,7 +414,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.False(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) @@ -454,7 +455,7 @@ func TestTallyJailedValidator(t *testing.T) { err = keeper.AddVote(ctx, proposalID, addrs[2], OptionNo) require.Nil(t, err) - passes, tallyResults, _ := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) + passes, tallyResults := tally(ctx, keeper, keeper.GetProposal(ctx, proposalID)) require.True(t, passes) require.False(t, tallyResults.Equals(EmptyTallyResult())) diff --git a/x/slashing/app_test.go b/x/slashing/app_test.go index 497440a085..a3370b1d87 100644 --- a/x/slashing/app_test.go +++ b/x/slashing/app_test.go @@ -99,9 +99,12 @@ func TestSlashingMsgs(t *testing.T) { } accs := []auth.Account{acc1} mock.SetGenesis(mapp, accs) + description := stake.NewDescription("foo_moniker", "", "", "") + commission := stake.NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) + createValidatorMsg := stake.NewMsgCreateValidator( - sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, + sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, commission, ) mock.SignCheckDeliver(t, mapp.BaseApp, []sdk.Msg{createValidatorMsg}, []int64{0}, []int64{0}, true, true, priv1) mock.CheckBalance(t, mapp, addr1, sdk.Coins{genCoin.Minus(bondCoin)}) diff --git a/x/slashing/client/rest/tx.go b/x/slashing/client/rest/tx.go index 45b1b6471a..94537c2eb3 100644 --- a/x/slashing/client/rest/tx.go +++ b/x/slashing/client/rest/tx.go @@ -38,7 +38,6 @@ type UnjailBody struct { ValidatorAddr string `json:"validator_addr"` } -// nolint: gocyclo func unjailRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { var m UnjailBody diff --git a/x/slashing/genesis.go b/x/slashing/genesis.go index 6e2809bfcb..43ae6b0d0a 100644 --- a/x/slashing/genesis.go +++ b/x/slashing/genesis.go @@ -8,7 +8,7 @@ import ( // InitGenesis initializes the keeper's address to pubkey map. func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { for _, validator := range data.Validators { - keeper.addPubkey(ctx, validator.GetPubKey()) + keeper.addPubkey(ctx, validator.GetConsPubKey()) } return } diff --git a/x/slashing/handler.go b/x/slashing/handler.go index 4f342f62a0..740166d2af 100644 --- a/x/slashing/handler.go +++ b/x/slashing/handler.go @@ -34,7 +34,7 @@ func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result { return ErrValidatorNotJailed(k.codespace).Result() } - consAddr := sdk.ConsAddress(validator.GetPubKey().Address()) + consAddr := sdk.ConsAddress(validator.GetConsPubKey().Address()) info, found := k.getValidatorSigningInfo(ctx, consAddr) if !found { diff --git a/x/slashing/hooks.go b/x/slashing/hooks.go index 701a6b2cde..92c4cf85f8 100644 --- a/x/slashing/hooks.go +++ b/x/slashing/hooks.go @@ -29,10 +29,10 @@ type Hooks struct { k Keeper } -var _ sdk.StakingHooks = Hooks{} +var _ sdk.ValidatorHooks = Hooks{} // Return the wrapper struct -func (k Keeper) Hooks() Hooks { +func (k Keeper) ValidatorHooks() Hooks { return Hooks{k} } @@ -45,11 +45,3 @@ func (h Hooks) OnValidatorBonded(ctx sdk.Context, address sdk.ConsAddress) { func (h Hooks) OnValidatorBeginUnbonding(ctx sdk.Context, address sdk.ConsAddress) { h.k.onValidatorBeginUnbonding(ctx, address) } - -// nolint - unused hooks -func (h Hooks) OnValidatorCreated(_ sdk.Context, _ sdk.ValAddress) {} -func (h Hooks) OnValidatorCommissionChange(_ sdk.Context, _ sdk.ValAddress) {} -func (h Hooks) OnValidatorRemoved(_ sdk.Context, _ sdk.ValAddress) {} -func (h Hooks) OnDelegationCreated(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) {} -func (h Hooks) OnDelegationSharesModified(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) {} -func (h Hooks) OnDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) {} diff --git a/x/slashing/keeper.go b/x/slashing/keeper.go index e639182e18..6a5fa30148 100644 --- a/x/slashing/keeper.go +++ b/x/slashing/keeper.go @@ -79,7 +79,6 @@ func (k Keeper) handleDoubleSign(ctx sdk.Context, addr crypto.Address, infractio // handle a validator signature, must be called once per validator per block // TODO refactor to take in a consensus address, additionally should maybe just take in the pubkey too -// nolint gocyclo func (k Keeper) handleValidatorSignature(ctx sdk.Context, addr crypto.Address, power int64, signed bool) { logger := ctx.Logger().With("module", "x/slashing") height := ctx.BlockHeight() diff --git a/x/slashing/keeper_test.go b/x/slashing/keeper_test.go index 1d69f714f0..d4b1af3d51 100644 --- a/x/slashing/keeper_test.go +++ b/x/slashing/keeper_test.go @@ -27,7 +27,7 @@ func TestHandleDoubleSign(t *testing.T) { // initial setup ctx, ck, sk, _, keeper := createTestInput(t) - sk = sk.WithHooks(keeper.Hooks()) + sk = sk.WithValidatorHooks(keeper.ValidatorHooks()) amtInt := int64(100) addr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt) got := stake.NewHandler(sk)(ctx, newTestMsgCreateValidator(addr, val, amt)) @@ -69,7 +69,7 @@ func TestSlashingPeriodCap(t *testing.T) { // initial setup ctx, ck, sk, _, keeper := createTestInput(t) - sk = sk.WithHooks(keeper.Hooks()) + sk = sk.WithValidatorHooks(keeper.ValidatorHooks()) amtInt := int64(100) addr, amt := addrs[0], sdk.NewInt(amtInt) valConsPubKey, valConsAddr := pks[0], sdk.ConsAddress(pks[0].Address()) @@ -125,7 +125,7 @@ func TestHandleAbsentValidator(t *testing.T) { // initial setup ctx, ck, sk, _, keeper := createTestInput(t) - sk = sk.WithHooks(keeper.Hooks()) + sk = sk.WithValidatorHooks(keeper.ValidatorHooks()) amtInt := int64(100) addr, val, amt := addrs[0], pks[0], sdk.NewInt(amtInt) sh := stake.NewHandler(sk) @@ -167,7 +167,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.Equal(t, keeper.SignedBlocksWindow(ctx)-keeper.MinSignedPerWindow(ctx), info.SignedBlocksCounter) // validator should be bonded still - validator, _ := sk.GetValidatorByConsPubKey(ctx, val) + validator, _ := sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Bonded, validator.GetStatus()) pool := sk.GetPool(ctx) require.Equal(t, amtInt, pool.BondedTokens.RoundInt64()) @@ -181,7 +181,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.Equal(t, keeper.SignedBlocksWindow(ctx)-keeper.MinSignedPerWindow(ctx)-1, info.SignedBlocksCounter) // validator should have been jailed - validator, _ = sk.GetValidatorByConsPubKey(ctx, val) + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Unbonding, validator.GetStatus()) // unrevocation should fail prior to jail expiration @@ -194,7 +194,7 @@ func TestHandleAbsentValidator(t *testing.T) { require.True(t, got.IsOK()) // validator should be rebonded now - validator, _ = sk.GetValidatorByConsPubKey(ctx, val) + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Bonded, validator.GetStatus()) // validator should have been slashed @@ -212,7 +212,7 @@ func TestHandleAbsentValidator(t *testing.T) { height++ ctx = ctx.WithBlockHeight(height) keeper.handleValidatorSignature(ctx, val.Address(), amtInt, false) - validator, _ = sk.GetValidatorByConsPubKey(ctx, val) + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Bonded, validator.GetStatus()) // 500 signed blocks @@ -228,7 +228,7 @@ func TestHandleAbsentValidator(t *testing.T) { ctx = ctx.WithBlockHeight(height) keeper.handleValidatorSignature(ctx, val.Address(), amtInt, false) } - validator, _ = sk.GetValidatorByConsPubKey(ctx, val) + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Unbonding, validator.GetStatus()) } @@ -263,7 +263,7 @@ func TestHandleNewValidator(t *testing.T) { require.Equal(t, time.Unix(0, 0).UTC(), info.JailedUntil) // validator should be bonded still, should not have been jailed or slashed - validator, _ := sk.GetValidatorByConsPubKey(ctx, val) + validator, _ := sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Bonded, validator.GetStatus()) pool := sk.GetPool(ctx) require.Equal(t, int64(100), pool.BondedTokens.RoundInt64()) @@ -297,7 +297,7 @@ func TestHandleAlreadyJailed(t *testing.T) { } // validator should have been jailed and slashed - validator, _ := sk.GetValidatorByConsPubKey(ctx, val) + validator, _ := sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, sdk.Unbonding, validator.GetStatus()) // validator should have been slashed @@ -308,7 +308,7 @@ func TestHandleAlreadyJailed(t *testing.T) { keeper.handleValidatorSignature(ctx, val.Address(), amtInt, false) // validator should not have been slashed twice - validator, _ = sk.GetValidatorByConsPubKey(ctx, val) + validator, _ = sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(val)) require.Equal(t, amtInt-1, validator.GetTokens().RoundInt64()) } diff --git a/x/slashing/simulation/msgs.go b/x/slashing/simulation/msgs.go index da9340baf4..2b09226f23 100644 --- a/x/slashing/simulation/msgs.go +++ b/x/slashing/simulation/msgs.go @@ -4,8 +4,6 @@ import ( "fmt" "math/rand" - "github.com/tendermint/tendermint/crypto" - "github.com/cosmos/cosmos-sdk/baseapp" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/mock/simulation" @@ -14,9 +12,9 @@ import ( // SimulateMsgUnjail func SimulateMsgUnjail(k slashing.Keeper) simulation.Operation { - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { - key := simulation.RandomKey(r, keys) - address := sdk.ValAddress(key.PubKey().Address()) + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + acc := simulation.RandomAcc(r, accs) + address := sdk.ValAddress(acc.Address) msg := slashing.NewMsgUnjail(address) if msg.ValidateBasic() != nil { return "", nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes()) diff --git a/x/slashing/test_common.go b/x/slashing/test_common.go index db2293a2f9..7c97a85372 100644 --- a/x/slashing/test_common.go +++ b/x/slashing/test_common.go @@ -103,12 +103,14 @@ func testAddr(addr string) sdk.AccAddress { } func newTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt sdk.Int) stake.MsgCreateValidator { + commission := stake.NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) return stake.MsgCreateValidator{ Description: stake.Description{}, + Commission: commission, DelegatorAddr: sdk.AccAddress(address), ValidatorAddr: address, PubKey: pubKey, - Delegation: sdk.Coin{"steak", amt}, + Delegation: sdk.NewCoin("steak", amt), } } @@ -116,6 +118,6 @@ func newTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, delAmoun return stake.MsgDelegate{ DelegatorAddr: delAddr, ValidatorAddr: valAddr, - Delegation: sdk.Coin{"steak", delAmount}, + Delegation: sdk.NewCoin("steak", delAmount), } } diff --git a/x/slashing/tick_test.go b/x/slashing/tick_test.go index fe273be5eb..8225c96347 100644 --- a/x/slashing/tick_test.go +++ b/x/slashing/tick_test.go @@ -78,7 +78,7 @@ func TestBeginBlocker(t *testing.T) { } // validator should be jailed - validator, found := sk.GetValidatorByConsPubKey(ctx, pk) + validator, found := sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk)) require.True(t, found) require.Equal(t, sdk.Unbonding, validator.GetStatus()) } diff --git a/x/stake/app_test.go b/x/stake/app_test.go index 922ce8ac3a..f96408c11f 100644 --- a/x/stake/app_test.go +++ b/x/stake/app_test.go @@ -21,10 +21,12 @@ var ( priv4 = ed25519.GenPrivKey() addr4 = sdk.AccAddress(priv4.PubKey().Address()) coins = sdk.Coins{sdk.NewCoin("foocoin", sdk.NewInt(10))} - fee = auth.StdFee{ - sdk.Coins{sdk.NewCoin("foocoin", sdk.NewInt(0))}, + fee = auth.NewStdFee( 100000, - } + sdk.Coins{sdk.NewCoin("foocoin", sdk.NewInt(0))}..., + ) + + commissionMsg = NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) ) // getMockApp returns an initialized mock application for this module. @@ -129,7 +131,7 @@ func TestStakeMsgs(t *testing.T) { // create validator description := NewDescription("foo_moniker", "", "", "") createValidatorMsg := NewMsgCreateValidator( - sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, + sdk.ValAddress(addr1), priv1.PubKey(), bondCoin, description, commissionMsg, ) mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{createValidatorMsg}, []int64{0}, []int64{0}, true, true, priv1) @@ -143,7 +145,7 @@ func TestStakeMsgs(t *testing.T) { // addr1 create validator on behalf of addr2 createValidatorMsgOnBehalfOf := NewMsgCreateValidatorOnBehalfOf( - addr1, sdk.ValAddress(addr2), priv2.PubKey(), bondCoin, description, + addr1, sdk.ValAddress(addr2), priv2.PubKey(), bondCoin, description, commissionMsg, ) mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{createValidatorMsgOnBehalfOf}, []int64{0, 1}, []int64{1, 0}, true, true, priv1, priv2) @@ -160,7 +162,7 @@ func TestStakeMsgs(t *testing.T) { // edit the validator description = NewDescription("bar_moniker", "", "", "") - editValidatorMsg := NewMsgEditValidator(sdk.ValAddress(addr1), description) + editValidatorMsg := NewMsgEditValidator(sdk.ValAddress(addr1), description, nil) mock.SignCheckDeliver(t, mApp.BaseApp, []sdk.Msg{editValidatorMsg}, []int64{0}, []int64{2}, true, true, priv1) validator = checkValidator(t, mApp, keeper, sdk.ValAddress(addr1), true) diff --git a/x/stake/client/cli/flags.go b/x/stake/client/cli/flags.go index 4c36be0b54..bec76298f3 100644 --- a/x/stake/client/cli/flags.go +++ b/x/stake/client/cli/flags.go @@ -21,6 +21,10 @@ const ( FlagIdentity = "identity" FlagWebsite = "website" FlagDetails = "details" + + FlagCommissionRate = "commission-rate" + FlagCommissionMaxRate = "commission-max-rate" + FlagCommissionMaxChangeRate = "commission-max-change-rate" ) // common flagsets to add to various functions @@ -29,6 +33,8 @@ var ( fsAmount = flag.NewFlagSet("", flag.ContinueOnError) fsShares = flag.NewFlagSet("", flag.ContinueOnError) fsDescriptionCreate = flag.NewFlagSet("", flag.ContinueOnError) + fsCommissionCreate = flag.NewFlagSet("", flag.ContinueOnError) + fsCommissionUpdate = flag.NewFlagSet("", flag.ContinueOnError) fsDescriptionEdit = flag.NewFlagSet("", flag.ContinueOnError) fsValidator = flag.NewFlagSet("", flag.ContinueOnError) fsDelegator = flag.NewFlagSet("", flag.ContinueOnError) @@ -44,6 +50,10 @@ func init() { fsDescriptionCreate.String(FlagIdentity, "", "optional identity signature (ex. UPort or Keybase)") fsDescriptionCreate.String(FlagWebsite, "", "optional website") fsDescriptionCreate.String(FlagDetails, "", "optional details") + fsCommissionUpdate.String(FlagCommissionRate, "", "The new commission rate percentage") + fsCommissionCreate.String(FlagCommissionRate, "", "The initial commission rate percentage") + fsCommissionCreate.String(FlagCommissionMaxRate, "", "The maximum commission rate percentage") + fsCommissionCreate.String(FlagCommissionMaxChangeRate, "", "The maximum commission change rate percentage (per day)") fsDescriptionEdit.String(FlagMoniker, types.DoNotModifyDesc, "validator name") fsDescriptionEdit.String(FlagIdentity, types.DoNotModifyDesc, "optional identity signature (ex. UPort or Keybase)") fsDescriptionEdit.String(FlagWebsite, types.DoNotModifyDesc, "optional website") diff --git a/x/stake/client/cli/tx.go b/x/stake/client/cli/tx.go index f917173283..74f4c6d6e7 100644 --- a/x/stake/client/cli/tx.go +++ b/x/stake/client/cli/tx.go @@ -12,9 +12,7 @@ import ( authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" authtxb "github.com/cosmos/cosmos-sdk/x/auth/client/txbuilder" "github.com/cosmos/cosmos-sdk/x/stake" - "github.com/cosmos/cosmos-sdk/x/stake/types" - "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -66,6 +64,15 @@ func GetCmdCreateValidator(cdc *codec.Codec) *cobra.Command { Details: viper.GetString(FlagDetails), } + // get the initial validator commission parameters + rateStr := viper.GetString(FlagCommissionRate) + maxRateStr := viper.GetString(FlagCommissionMaxRate) + maxChangeRateStr := viper.GetString(FlagCommissionMaxChangeRate) + commissionMsg, err := buildCommissionMsg(rateStr, maxRateStr, maxChangeRateStr) + if err != nil { + return err + } + var msg sdk.Msg if viper.GetString(FlagAddressDelegator) != "" { delAddr, err := sdk.AccAddressFromBech32(viper.GetString(FlagAddressDelegator)) @@ -73,13 +80,19 @@ func GetCmdCreateValidator(cdc *codec.Codec) *cobra.Command { return err } - msg = stake.NewMsgCreateValidatorOnBehalfOf(delAddr, sdk.ValAddress(valAddr), pk, amount, description) + msg = stake.NewMsgCreateValidatorOnBehalfOf( + delAddr, sdk.ValAddress(valAddr), pk, amount, description, commissionMsg, + ) } else { - msg = stake.NewMsgCreateValidator(sdk.ValAddress(valAddr), pk, amount, description) + msg = stake.NewMsgCreateValidator( + sdk.ValAddress(valAddr), pk, amount, description, commissionMsg, + ) } + if cliCtx.GenerateOnly { return utils.PrintUnsignedStdTx(txBldr, cliCtx, []sdk.Msg{msg}) } + // build and sign the transaction, then broadcast to Tendermint return utils.SendTx(txBldr, cliCtx, []sdk.Msg{msg}) }, @@ -88,6 +101,7 @@ func GetCmdCreateValidator(cdc *codec.Codec) *cobra.Command { cmd.Flags().AddFlagSet(fsPk) cmd.Flags().AddFlagSet(fsAmount) cmd.Flags().AddFlagSet(fsDescriptionCreate) + cmd.Flags().AddFlagSet(fsCommissionCreate) cmd.Flags().AddFlagSet(fsDelegator) return cmd @@ -117,17 +131,31 @@ func GetCmdEditValidator(cdc *codec.Codec) *cobra.Command { Details: viper.GetString(FlagDetails), } - msg := stake.NewMsgEditValidator(sdk.ValAddress(valAddr), description) + var newRate *sdk.Dec + + commissionRate := viper.GetString(FlagCommissionRate) + if commissionRate != "" { + rate, err := sdk.NewDecFromStr(commissionRate) + if err != nil { + return fmt.Errorf("invalid new commission rate: %v", err) + } + + newRate = &rate + } + + msg := stake.NewMsgEditValidator(sdk.ValAddress(valAddr), description, newRate) if cliCtx.GenerateOnly { return utils.PrintUnsignedStdTx(txBldr, cliCtx, []sdk.Msg{msg}) } + // build and sign the transaction, then broadcast to Tendermint return utils.SendTx(txBldr, cliCtx, []sdk.Msg{msg}) }, } cmd.Flags().AddFlagSet(fsDescriptionEdit) + cmd.Flags().AddFlagSet(fsCommissionUpdate) return cmd } @@ -247,54 +275,6 @@ func GetCmdBeginRedelegate(storeName string, cdc *codec.Codec) *cobra.Command { return cmd } -// nolint: gocyclo -// TODO: Make this pass gocyclo linting -func getShares( - storeName string, cdc *codec.Codec, sharesAmountStr, - sharesPercentStr string, delAddr sdk.AccAddress, valAddr sdk.ValAddress, -) (sharesAmount sdk.Dec, err error) { - switch { - case sharesAmountStr != "" && sharesPercentStr != "": - return sharesAmount, errors.Errorf("can either specify the amount OR the percent of the shares, not both") - case sharesAmountStr == "" && sharesPercentStr == "": - return sharesAmount, errors.Errorf("can either specify the amount OR the percent of the shares, not both") - case sharesAmountStr != "": - sharesAmount, err = sdk.NewDecFromStr(sharesAmountStr) - if err != nil { - return sharesAmount, err - } - if !sharesAmount.GT(sdk.ZeroDec()) { - return sharesAmount, errors.Errorf("shares amount must be positive number (ex. 123, 1.23456789)") - } - case sharesPercentStr != "": - var sharesPercent sdk.Dec - sharesPercent, err = sdk.NewDecFromStr(sharesPercentStr) - if err != nil { - return sharesAmount, err - } - if !sharesPercent.GT(sdk.ZeroDec()) || !sharesPercent.LTE(sdk.OneDec()) { - return sharesAmount, errors.Errorf("shares percent must be >0 and <=1 (ex. 0.01, 0.75, 1)") - } - - // make a query to get the existing delegation shares - key := stake.GetDelegationKey(delAddr, valAddr) - cliCtx := context.NewCLIContext(). - WithCodec(cdc). - WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) - - resQuery, err := cliCtx.QueryStore(key, storeName) - if err != nil { - return sharesAmount, errors.Errorf("cannot find delegation to determine percent Error: %v", err) - } - delegation, err := types.UnmarshalDelegation(cdc, key, resQuery) - if err != nil { - return sdk.ZeroDec(), err - } - sharesAmount = sharesPercent.Mul(delegation.Shares) - } - return -} - // GetCmdCompleteRedelegate implements the complete redelegation command. func GetCmdCompleteRedelegate(cdc *codec.Codec) *cobra.Command { cmd := &cobra.Command{ diff --git a/x/stake/client/cli/utils.go b/x/stake/client/cli/utils.go new file mode 100644 index 0000000000..9aca2d8995 --- /dev/null +++ b/x/stake/client/cli/utils.go @@ -0,0 +1,88 @@ +package cli + +import ( + "github.com/cosmos/cosmos-sdk/client/context" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli" + "github.com/cosmos/cosmos-sdk/x/stake" + "github.com/cosmos/cosmos-sdk/x/stake/types" + "github.com/pkg/errors" +) + +func getShares( + storeName string, cdc *codec.Codec, sharesAmountStr, + sharesPercentStr string, delAddr sdk.AccAddress, valAddr sdk.ValAddress, +) (sharesAmount sdk.Dec, err error) { + + switch { + case sharesAmountStr != "" && sharesPercentStr != "": + return sharesAmount, errors.Errorf("can either specify the amount OR the percent of the shares, not both") + + case sharesAmountStr == "" && sharesPercentStr == "": + return sharesAmount, errors.Errorf("can either specify the amount OR the percent of the shares, not both") + + case sharesAmountStr != "": + sharesAmount, err = sdk.NewDecFromStr(sharesAmountStr) + if err != nil { + return sharesAmount, err + } + if !sharesAmount.GT(sdk.ZeroDec()) { + return sharesAmount, errors.Errorf("shares amount must be positive number (ex. 123, 1.23456789)") + } + + case sharesPercentStr != "": + var sharesPercent sdk.Dec + sharesPercent, err = sdk.NewDecFromStr(sharesPercentStr) + if err != nil { + return sharesAmount, err + } + if !sharesPercent.GT(sdk.ZeroDec()) || !sharesPercent.LTE(sdk.OneDec()) { + return sharesAmount, errors.Errorf("shares percent must be >0 and <=1 (ex. 0.01, 0.75, 1)") + } + + // make a query to get the existing delegation shares + key := stake.GetDelegationKey(delAddr, valAddr) + cliCtx := context.NewCLIContext(). + WithCodec(cdc). + WithAccountDecoder(authcmd.GetAccountDecoder(cdc)) + + resQuery, err := cliCtx.QueryStore(key, storeName) + if err != nil { + return sharesAmount, errors.Errorf("cannot find delegation to determine percent Error: %v", err) + } + + delegation, err := types.UnmarshalDelegation(cdc, key, resQuery) + if err != nil { + return sdk.ZeroDec(), err + } + + sharesAmount = sharesPercent.Mul(delegation.Shares) + } + + return +} + +func buildCommissionMsg(rateStr, maxRateStr, maxChangeRateStr string) (commission types.CommissionMsg, err error) { + if rateStr == "" || maxRateStr == "" || maxChangeRateStr == "" { + return commission, errors.Errorf("must specify all validator commission parameters") + } + + rate, err := sdk.NewDecFromStr(rateStr) + if err != nil { + return commission, err + } + + maxRate, err := sdk.NewDecFromStr(maxRateStr) + if err != nil { + return commission, err + } + + maxChangeRate, err := sdk.NewDecFromStr(maxChangeRateStr) + if err != nil { + return commission, err + } + + commission = types.NewCommissionMsg(rate, maxRate, maxChangeRate) + return commission, nil +} diff --git a/x/stake/client/rest/query.go b/x/stake/client/rest/query.go index 8d55bed8cf..a8fbfecf4d 100644 --- a/x/stake/client/rest/query.go +++ b/x/stake/client/rest/query.go @@ -120,7 +120,6 @@ func delegatorHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.Handle } } -// nolint gocyclo // HTTP request handler to query all staking txs (msgs) from a delegator func delegatorTxsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { diff --git a/x/stake/client/rest/tx.go b/x/stake/client/rest/tx.go index b716843602..324e0c914f 100644 --- a/x/stake/client/rest/tx.go +++ b/x/stake/client/rest/tx.go @@ -69,7 +69,6 @@ type EditDelegationsBody struct { CompleteRedelegates []msgCompleteRedelegateInput `json:"complete_redelegates"` } -// nolint: gocyclo // TODO: Split this up into several smaller functions, and remove the above nolint // TODO: use sdk.ValAddress instead of sdk.AccAddress for validators in messages func delegationsRequestHandlerFn(cdc *codec.Codec, kb keys.Keybase, cliCtx context.CLIContext) http.HandlerFunc { diff --git a/x/stake/client/rest/utils.go b/x/stake/client/rest/utils.go index 8ab5655009..e7b8891eab 100644 --- a/x/stake/client/rest/utils.go +++ b/x/stake/client/rest/utils.go @@ -3,11 +3,12 @@ package rest import ( "fmt" + "github.com/cosmos/cosmos-sdk/client/context" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/x/stake/tags" + rpcclient "github.com/tendermint/tendermint/rpc/client" - "github.com/cosmos/cosmos-sdk/client/context" ) // contains checks if the a given query contains one of the tx types diff --git a/x/stake/genesis.go b/x/stake/genesis.go index a7f8493081..58b7ed1b4f 100644 --- a/x/stake/genesis.go +++ b/x/stake/genesis.go @@ -75,7 +75,7 @@ func WriteGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { func WriteValidators(ctx sdk.Context, keeper Keeper) (vals []tmtypes.GenesisValidator) { keeper.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) (stop bool) { vals = append(vals, tmtypes.GenesisValidator{ - PubKey: validator.GetPubKey(), + PubKey: validator.GetConsPubKey(), Power: validator.GetPower().RoundInt64(), Name: validator.GetMoniker(), }) diff --git a/x/stake/handler.go b/x/stake/handler.go index 005b3af7d0..0524b2c117 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -62,49 +62,55 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) (ValidatorUpdates []abci.Valid // now we just perform action and save func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k keeper.Keeper) sdk.Result { - // check to see if the pubkey or sender has been registered before _, found := k.GetValidator(ctx, msg.ValidatorAddr) if found { return ErrValidatorOwnerExists(k.Codespace()).Result() } - _, found = k.GetValidatorByConsPubKey(ctx, msg.PubKey) + + _, found = k.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(msg.PubKey)) if found { return ErrValidatorPubKeyExists(k.Codespace()).Result() } + if msg.Delegation.Denom != k.GetParams(ctx).BondDenom { return ErrBadDenom(k.Codespace()).Result() } validator := NewValidator(msg.ValidatorAddr, msg.PubKey, msg.Description) + commission := NewCommissionWithTime( + msg.Commission.Rate, msg.Commission.MaxChangeRate, + msg.Commission.MaxChangeRate, ctx.BlockHeader().Time, + ) + + validator, err := validator.SetInitialCommission(commission) + if err != nil { + return err.Result() + } + k.SetValidator(ctx, validator) k.SetValidatorByConsAddr(ctx, validator) // move coins from the msg.Address account to a (self-delegation) delegator account // the validator account and global shares are updated within here - _, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true) + _, err = k.Delegate(ctx, msg.DelegatorAddr, msg.Delegation, validator, true) if err != nil { return err.Result() } - // call the hook if present - k.OnValidatorCreated(ctx, validator.OperatorAddr) - accAddr := sdk.AccAddress(validator.OperatorAddr) - k.OnDelegationCreated(ctx, accAddr, validator.OperatorAddr) - tags := sdk.NewTags( tags.Action, tags.ActionCreateValidator, tags.DstValidator, []byte(msg.ValidatorAddr.String()), tags.Moniker, []byte(msg.Description.Moniker), tags.Identity, []byte(msg.Description.Identity), ) + return sdk.Result{ Tags: tags, } } func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keeper.Keeper) sdk.Result { - // validator must already be registered validator, found := k.GetValidator(ctx, msg.ValidatorAddr) if !found { @@ -116,17 +122,26 @@ func handleMsgEditValidator(ctx sdk.Context, msg types.MsgEditValidator, k keepe if err != nil { return err.Result() } + validator.Description = description + if msg.CommissionRate != nil { + if err := k.UpdateValidatorCommission(ctx, validator, *msg.CommissionRate); err != nil { + return err.Result() + } + } + // We don't need to run through all the power update logic within k.UpdateValidator // We just need to override the entry in state, since only the description has changed. k.SetValidator(ctx, validator) + tags := sdk.NewTags( tags.Action, tags.ActionEditValidator, tags.DstValidator, []byte(msg.ValidatorAddr.String()), tags.Moniker, []byte(description.Moniker), tags.Identity, []byte(description.Identity), ) + return sdk.Result{ Tags: tags, } @@ -151,9 +166,6 @@ func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper) return err.Result() } - // call the hook if present - k.OnDelegationCreated(ctx, msg.DelegatorAddr, validator.OperatorAddr) - tags := sdk.NewTags( tags.Action, tags.ActionDelegate, tags.Delegator, []byte(msg.DelegatorAddr.String()), diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 4bc92c0b73..5780bf5431 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -17,7 +17,9 @@ import ( //______________________________________________________________________ func newTestMsgCreateValidator(address sdk.ValAddress, pubKey crypto.PubKey, amt int64) MsgCreateValidator { - return types.NewMsgCreateValidator(address, pubKey, sdk.NewCoin("steak", sdk.NewInt(amt)), Description{}) + return types.NewMsgCreateValidator( + address, pubKey, sdk.NewCoin("steak", sdk.NewInt(amt)), Description{}, commissionMsg, + ) } func newTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amt int64) MsgDelegate { @@ -31,6 +33,7 @@ func newTestMsgDelegate(delAddr sdk.AccAddress, valAddr sdk.ValAddress, amt int6 func newTestMsgCreateValidatorOnBehalfOf(delAddr sdk.AccAddress, valAddr sdk.ValAddress, valPubKey crypto.PubKey, amt int64) MsgCreateValidator { return MsgCreateValidator{ Description: Description{}, + Commission: commissionMsg, DelegatorAddr: delAddr, ValidatorAddr: valAddr, PubKey: valPubKey, diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 6ac51ac196..6efa4c8ed3 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -67,11 +67,6 @@ func (k Keeper) SetDelegation(ctx sdk.Context, delegation types.Delegation) { // remove a delegation from store func (k Keeper) RemoveDelegation(ctx sdk.Context, delegation types.Delegation) { - // call the hook if present - if k.hooks != nil { - k.hooks.OnDelegationRemoved(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) - } - store := ctx.KVStore(k.storeKey) store.Delete(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr)) } @@ -281,11 +276,6 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co k.SetDelegation(ctx, delegation) k.UpdateValidator(ctx, validator) - // call the hook if present - if k.hooks != nil { - k.hooks.OnDelegationSharesModified(ctx, delegation.DelegatorAddr, validator.OperatorAddr) - } - return } @@ -344,11 +334,6 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA k.RemoveValidator(ctx, validator.OperatorAddr) } - // call the hook if present - if k.hooks != nil { - k.hooks.OnDelegationSharesModified(ctx, delegation.DelegatorAddr, validator.OperatorAddr) - } - return amount, nil } diff --git a/x/stake/keeper/keeper.go b/x/stake/keeper/keeper.go index 82170a4aed..0f700f9ab8 100644 --- a/x/stake/keeper/keeper.go +++ b/x/stake/keeper/keeper.go @@ -14,7 +14,7 @@ type Keeper struct { storeTKey sdk.StoreKey cdc *codec.Codec bankKeeper bank.Keeper - hooks sdk.StakingHooks + hooks sdk.ValidatorHooks // codespace codespace sdk.CodespaceType @@ -33,7 +33,7 @@ func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey, ck bank.Keeper, codespa } // Set the validator hooks -func (k Keeper) WithHooks(sh sdk.StakingHooks) Keeper { +func (k Keeper) WithValidatorHooks(sh sdk.ValidatorHooks) Keeper { if k.hooks != nil { panic("cannot set validator hooks twice") } diff --git a/x/stake/keeper/sdk_types.go b/x/stake/keeper/sdk_types.go index 9872630a4e..d702e845da 100644 --- a/x/stake/keeper/sdk_types.go +++ b/x/stake/keeper/sdk_types.go @@ -5,7 +5,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/types" - "github.com/tendermint/tendermint/crypto" ) // Implements ValidatorSet @@ -67,15 +66,6 @@ func (k Keeper) ValidatorByConsAddr(ctx sdk.Context, addr sdk.ConsAddress) sdk.V return val } -// get the sdk.validator for a particular pubkey -func (k Keeper) ValidatorByConsPubKey(ctx sdk.Context, consPubKey crypto.PubKey) sdk.Validator { - val, found := k.GetValidatorByConsPubKey(ctx, consPubKey) - if !found { - return nil - } - return val -} - // total power from the bond func (k Keeper) TotalPower(ctx sdk.Context) sdk.Dec { pool := k.GetPool(ctx) diff --git a/x/stake/keeper/slash.go b/x/stake/keeper/slash.go index ea378678ae..486dc32869 100644 --- a/x/stake/keeper/slash.go +++ b/x/stake/keeper/slash.go @@ -21,8 +21,6 @@ import ( // CONTRACT: // Infraction committed at the current height or at a past height, // not at a height in the future -// -// nolint: gocyclo func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeight int64, power int64, slashFactor sdk.Dec) { logger := ctx.Logger().With("module", "x/stake") @@ -143,7 +141,7 @@ func (k Keeper) Unjail(ctx sdk.Context, consAddr sdk.ConsAddress) { // set the jailed flag on a validator func (k Keeper) setJailed(ctx sdk.Context, consAddr sdk.ConsAddress, isJailed bool) { - validator, found := k.GetValidatorByConsAddr(ctx, sdk.ConsAddress(consAddr)) + validator, found := k.GetValidatorByConsAddr(ctx, consAddr) if !found { panic(fmt.Errorf("validator with consensus-Address %s not found, cannot set jailed to %v", consAddr, isJailed)) } diff --git a/x/stake/keeper/slash_test.go b/x/stake/keeper/slash_test.go index c4a1edd415..0a6cccac2a 100644 --- a/x/stake/keeper/slash_test.go +++ b/x/stake/keeper/slash_test.go @@ -14,6 +14,7 @@ import ( // TODO integrate with test_common.go helper (CreateTestInput) // setup helper function - creates two validators func setupHelper(t *testing.T, amt int64) (sdk.Context, Keeper, types.Params) { + // setup ctx, _, keeper := CreateTestInput(t, false, amt) params := keeper.GetParams(ctx) @@ -472,7 +473,7 @@ func TestSlashBoth(t *testing.T) { // slash validator ctx = ctx.WithBlockHeight(12) oldPool := keeper.GetPool(ctx) - validator, found := keeper.GetValidatorByConsPubKey(ctx, PKs[0]) + validator, found := keeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(PKs[0])) require.True(t, found) consAddr0 := sdk.ConsAddress(PKs[0].Address()) keeper.Slash(ctx, consAddr0, 10, 10, fraction) @@ -489,7 +490,7 @@ func TestSlashBoth(t *testing.T) { // bonded tokens burned require.Equal(t, int64(3), oldPool.BondedTokens.Sub(newPool.BondedTokens).RoundInt64()) // read updated validator - validator, found = keeper.GetValidatorByConsPubKey(ctx, PKs[0]) + validator, found = keeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(PKs[0])) require.True(t, found) // power not decreased, all stake was bonded since require.Equal(t, sdk.NewDec(10), validator.GetPower()) diff --git a/x/stake/keeper/validator.go b/x/stake/keeper/validator.go index af7d77e207..9d8fb4362a 100644 --- a/x/stake/keeper/validator.go +++ b/x/stake/keeper/validator.go @@ -6,7 +6,6 @@ import ( "fmt" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/types" @@ -68,17 +67,6 @@ func (k Keeper) GetValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress return k.GetValidator(ctx, opAddr) } -// get a single validator by pubkey -func (k Keeper) GetValidatorByConsPubKey(ctx sdk.Context, consPubKey crypto.PubKey) (validator types.Validator, found bool) { - store := ctx.KVStore(k.storeKey) - consAddr := sdk.ConsAddress(consPubKey.Address()) - opAddr := store.Get(GetValidatorByConsAddrKey(consAddr)) - if opAddr == nil { - return validator, false - } - return k.GetValidator(ctx, opAddr) -} - // set the main record holding validator details func (k Keeper) SetValidator(ctx sdk.Context, validator types.Validator) { store := ctx.KVStore(k.storeKey) @@ -87,10 +75,9 @@ func (k Keeper) SetValidator(ctx sdk.Context, validator types.Validator) { } // validator index -// TODO change to SetValidatorByConsAddr? used for retrieving from ConsPubkey as well- kinda confusing func (k Keeper) SetValidatorByConsAddr(ctx sdk.Context, validator types.Validator) { store := ctx.KVStore(k.storeKey) - consAddr := sdk.ConsAddress(validator.OperatorAddr.Bytes()) + consAddr := sdk.ConsAddress(validator.ConsPubKey.Address()) store.Set(GetValidatorByConsAddrKey(consAddr), validator.OperatorAddr) } @@ -245,7 +232,6 @@ func (k Keeper) GetValidTendermintUpdates(ctx sdk.Context) (updates []abci.Valid // It may kick out validators if a new validator is entering the bonded validator // group. // -// nolint: gocyclo // TODO: Remove above nolint, function needs to be simplified! func (k Keeper) UpdateValidator(ctx sdk.Context, validator types.Validator) types.Validator { tstore := ctx.TransientStore(k.storeTKey) @@ -435,9 +421,6 @@ func (k Keeper) updateValidatorPower(ctx sdk.Context, oldFound bool, oldValidato // updated in store with the ValidatorsBondedIndexKey. This store is used to // determine if a validator is a validator without needing to iterate over all // validators. -// -// nolint: gocyclo -// TODO: Remove the above golint func (k Keeper) UpdateBondedValidators( ctx sdk.Context, affectedValidator types.Validator) ( updatedVal types.Validator, updated bool) { @@ -681,11 +664,6 @@ func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) types. // remove the validator record and associated indexes func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) { - // call the hook if present - if k.hooks != nil { - k.hooks.OnValidatorRemoved(ctx, address) - } - // first retrieve the old validator record validator, found := k.GetValidator(ctx, address) if !found { @@ -711,6 +689,23 @@ func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) { tstore.Set(GetTendermintUpdatesTKey(address), bz) } +// UpdateValidatorCommission attempts to update a validator's commission rate. +// An error is returned if the new commission rate is invalid. +func (k Keeper) UpdateValidatorCommission(ctx sdk.Context, validator types.Validator, newRate sdk.Dec) sdk.Error { + commission := validator.Commission + blockTime := ctx.BlockHeader().Time + + if err := commission.ValidateNewRate(newRate, blockTime); err != nil { + return err + } + + validator.Commission.Rate = newRate + validator.Commission.UpdateTime = blockTime + + k.SetValidator(ctx, validator) + return nil +} + //__________________________________________________________________________ // get the current validator on the cliff @@ -745,36 +740,3 @@ func ensureValidatorFound(found bool, ownerAddr []byte) { panic(fmt.Sprintf("validator record not found for address: %X\n", ownerAddr)) } } - -//__________________________________________________________________________ - -// XXX remove this code - this is should be superceded by commission work that bez is doing -// get a single validator -func (k Keeper) UpdateValidatorCommission(ctx sdk.Context, addr sdk.ValAddress, newCommission sdk.Dec) sdk.Error { - - // call the hook if present - if k.hooks != nil { - k.hooks.OnValidatorCommissionChange(ctx, addr) - } - - validator, found := k.GetValidator(ctx, addr) - - // check for errors - switch { - case !found: - return types.ErrNoValidatorFound(k.Codespace()) - case newCommission.LT(sdk.ZeroDec()): - return types.ErrCommissionNegative(k.Codespace()) - case newCommission.GT(validator.CommissionMax): - return types.ErrCommissionBeyondMax(k.Codespace()) - //case rateChange(Commission) > CommissionMaxChange: // XXX XXX XXX TODO implementation - //return types.ErrCommissionPastRate(k.Codespace()) - } - - // TODO adjust all the commission terms appropriately - - validator.Commission = newCommission - - k.SetValidator(ctx, validator) - return nil -} diff --git a/x/stake/keeper/validator_test.go b/x/stake/keeper/validator_test.go index a541600d9f..73a391acc7 100644 --- a/x/stake/keeper/validator_test.go +++ b/x/stake/keeper/validator_test.go @@ -3,9 +3,11 @@ package keeper import ( "fmt" "testing" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/stake/types" + abci "github.com/tendermint/tendermint/abci/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -305,10 +307,19 @@ func TestValidatorBasics(t *testing.T) { // set and retrieve a record validators[0] = keeper.UpdateValidator(ctx, validators[0]) + keeper.SetValidatorByConsAddr(ctx, validators[0]) resVal, found := keeper.GetValidator(ctx, addrVals[0]) require.True(t, found) assert.True(ValEq(t, validators[0], resVal)) + // retrieve from consensus + resVal, found = keeper.GetValidatorByConsAddr(ctx, sdk.ConsAddress(PKs[0].Address())) + require.True(t, found) + assert.True(ValEq(t, validators[0], resVal)) + resVal, found = keeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(PKs[0])) + require.True(t, found) + assert.True(ValEq(t, validators[0], resVal)) + resVals = keeper.GetValidatorsBonded(ctx) require.Equal(t, 1, len(resVals)) assert.True(ValEq(t, validators[0], resVals[0])) @@ -1044,3 +1055,55 @@ func TestGetValidTendermintUpdatesBondTransition(t *testing.T) { clearTendermintUpdates(ctx, keeper) require.Equal(t, 0, len(keeper.GetValidTendermintUpdates(ctx))) } + +func TestUpdateValidatorCommission(t *testing.T) { + ctx, _, keeper := CreateTestInput(t, false, 1000) + ctx = ctx.WithBlockHeader(abci.Header{Time: time.Now().UTC()}) + + commission1 := types.NewCommissionWithTime( + sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(3, 1), + sdk.NewDecWithPrec(1, 1), time.Now().UTC().Add(time.Duration(-1)*time.Hour), + ) + commission2 := types.NewCommission(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(3, 1), sdk.NewDecWithPrec(1, 1)) + + val1 := types.NewValidator(addrVals[0], PKs[0], types.Description{}) + val2 := types.NewValidator(addrVals[1], PKs[1], types.Description{}) + + val1, _ = val1.SetInitialCommission(commission1) + val2, _ = val2.SetInitialCommission(commission2) + + testCases := []struct { + validator types.Validator + newRate sdk.Dec + expectedErr bool + }{ + {val1, sdk.ZeroDec(), true}, + {val2, sdk.NewDecWithPrec(-1, 1), true}, + {val2, sdk.NewDecWithPrec(4, 1), true}, + {val2, sdk.NewDecWithPrec(3, 1), true}, + {val2, sdk.NewDecWithPrec(2, 1), false}, + } + + for i, tc := range testCases { + err := keeper.UpdateValidatorCommission(ctx, tc.validator, tc.newRate) + + if tc.expectedErr { + require.Error(t, err, "expected error for test case #%d with rate: %s", i, tc.newRate) + } else { + val, found := keeper.GetValidator(ctx, tc.validator.OperatorAddr) + + require.True(t, found, + "expected to find validator for test case #%d with rate: %s", i, tc.newRate, + ) + require.NoError(t, err, + "unexpected error for test case #%d with rate: %s", i, tc.newRate, + ) + require.Equal(t, tc.newRate, val.Commission.Rate, + "expected new validator commission rate for test case #%d with rate: %s", i, tc.newRate, + ) + require.Equal(t, ctx.BlockHeader().Time, val.Commission.UpdateTime, + "expected new validator commission update time for test case #%d with rate: %s", i, tc.newRate, + ) + } + } +} diff --git a/x/stake/simulation/invariants.go b/x/stake/simulation/invariants.go index 2ab0717042..cdd80a8c41 100644 --- a/x/stake/simulation/invariants.go +++ b/x/stake/simulation/invariants.go @@ -80,7 +80,7 @@ func PositivePowerInvariant(k stake.Keeper) simulation.Invariant { var err error k.IterateValidatorsBonded(ctx, func(_ int64, validator sdk.Validator) bool { if !validator.GetPower().GT(sdk.ZeroDec()) { - err = fmt.Errorf("validator with non-positive power stored. (pubkey %v)", validator.GetPubKey()) + err = fmt.Errorf("validator with non-positive power stored. (pubkey %v)", validator.GetConsPubKey()) return true } return false diff --git a/x/stake/simulation/msgs.go b/x/stake/simulation/msgs.go index 5b2bc1ee81..0cd4e08a92 100644 --- a/x/stake/simulation/msgs.go +++ b/x/stake/simulation/msgs.go @@ -11,44 +11,57 @@ import ( "github.com/cosmos/cosmos-sdk/x/mock/simulation" "github.com/cosmos/cosmos-sdk/x/stake" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/crypto" ) // SimulateMsgCreateValidator func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), } - key := simulation.RandomKey(r, keys) - pubkey := key.PubKey() - address := sdk.ValAddress(pubkey.Address()) - amount := m.GetAccount(ctx, sdk.AccAddress(address)).GetCoins().AmountOf(denom) + + maxCommission := sdk.NewInt(10) + commission := stake.NewCommissionMsg( + sdk.NewDecWithPrec(simulation.RandomAmount(r, maxCommission).Int64(), 1), + sdk.NewDecWithPrec(simulation.RandomAmount(r, maxCommission).Int64(), 1), + sdk.NewDecWithPrec(simulation.RandomAmount(r, maxCommission).Int64(), 1), + ) + + acc := simulation.RandomAcc(r, accs) + address := sdk.ValAddress(acc.Address) + amount := m.GetAccount(ctx, acc.Address).GetCoins().AmountOf(denom) if amount.GT(sdk.ZeroInt()) { amount = simulation.RandomAmount(r, amount) } + if amount.Equal(sdk.ZeroInt()) { return "no-operation", nil, nil } + msg := stake.MsgCreateValidator{ Description: description, + Commission: commission, ValidatorAddr: address, - DelegatorAddr: sdk.AccAddress(address), - PubKey: pubkey, + DelegatorAddr: acc.Address, + PubKey: acc.PubKey, Delegation: sdk.NewCoin(denom, amount), } + if msg.ValidateBasic() != nil { return "", nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes()) } + ctx, write := ctx.CacheContext() result := handler(ctx, msg) if result.IsOK() { write() } + event(fmt.Sprintf("stake/MsgCreateValidator/%v", result.IsOK())) + // require.True(t, result.IsOK(), "expected OK result but instead got %v", result) action = fmt.Sprintf("TestMsgCreateValidator: ok %v, msg %s", result.IsOK(), msg.GetSignBytes()) return action, nil, nil @@ -58,7 +71,7 @@ func SimulateMsgCreateValidator(m auth.AccountMapper, k stake.Keeper) simulation // SimulateMsgEditValidator func SimulateMsgEditValidator(k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { description := stake.Description{ Moniker: simulation.RandStringOfLength(r, 10), @@ -66,16 +79,22 @@ func SimulateMsgEditValidator(k stake.Keeper) simulation.Operation { Website: simulation.RandStringOfLength(r, 10), Details: simulation.RandStringOfLength(r, 10), } - key := simulation.RandomKey(r, keys) - pubkey := key.PubKey() - address := sdk.ValAddress(pubkey.Address()) + + maxCommission := sdk.NewInt(10) + newCommissionRate := sdk.NewDecWithPrec(simulation.RandomAmount(r, maxCommission).Int64(), 1) + + acc := simulation.RandomAcc(r, accs) + address := sdk.ValAddress(acc.Address) msg := stake.MsgEditValidator{ - Description: description, - ValidatorAddr: address, + Description: description, + ValidatorAddr: address, + CommissionRate: &newCommissionRate, } + if msg.ValidateBasic() != nil { return "", nil, fmt.Errorf("expected msg to pass ValidateBasic: %s", msg.GetSignBytes()) } + ctx, write := ctx.CacheContext() result := handler(ctx, msg) if result.IsOK() { @@ -90,13 +109,13 @@ func SimulateMsgEditValidator(k stake.Keeper) simulation.Operation { // SimulateMsgDelegate func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom - validatorKey := simulation.RandomKey(r, keys) - validatorAddress := sdk.ValAddress(validatorKey.PubKey().Address()) - delegatorKey := simulation.RandomKey(r, keys) - delegatorAddress := sdk.AccAddress(delegatorKey.PubKey().Address()) + validatorAcc := simulation.RandomAcc(r, accs) + validatorAddress := sdk.ValAddress(validatorAcc.Address) + delegatorAcc := simulation.RandomAcc(r, accs) + delegatorAddress := delegatorAcc.Address amount := m.GetAccount(ctx, delegatorAddress).GetCoins().AmountOf(denom) if amount.GT(sdk.ZeroInt()) { amount = simulation.RandomAmount(r, amount) @@ -126,13 +145,13 @@ func SimulateMsgDelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operat // SimulateMsgBeginUnbonding func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom - validatorKey := simulation.RandomKey(r, keys) - validatorAddress := sdk.ValAddress(validatorKey.PubKey().Address()) - delegatorKey := simulation.RandomKey(r, keys) - delegatorAddress := sdk.AccAddress(delegatorKey.PubKey().Address()) + validatorAcc := simulation.RandomAcc(r, accs) + validatorAddress := sdk.ValAddress(validatorAcc.Address) + delegatorAcc := simulation.RandomAcc(r, accs) + delegatorAddress := delegatorAcc.Address amount := m.GetAccount(ctx, delegatorAddress).GetCoins().AmountOf(denom) if amount.GT(sdk.ZeroInt()) { amount = simulation.RandomAmount(r, amount) @@ -162,12 +181,12 @@ func SimulateMsgBeginUnbonding(m auth.AccountMapper, k stake.Keeper) simulation. // SimulateMsgCompleteUnbonding func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { - validatorKey := simulation.RandomKey(r, keys) - validatorAddress := sdk.ValAddress(validatorKey.PubKey().Address()) - delegatorKey := simulation.RandomKey(r, keys) - delegatorAddress := sdk.AccAddress(delegatorKey.PubKey().Address()) + validatorAcc := simulation.RandomAcc(r, accs) + validatorAddress := sdk.ValAddress(validatorAcc.Address) + delegatorAcc := simulation.RandomAcc(r, accs) + delegatorAddress := delegatorAcc.Address msg := stake.MsgCompleteUnbonding{ DelegatorAddr: delegatorAddress, ValidatorAddr: validatorAddress, @@ -189,15 +208,15 @@ func SimulateMsgCompleteUnbonding(k stake.Keeper) simulation.Operation { // SimulateMsgBeginRedelegate func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { denom := k.GetParams(ctx).BondDenom - sourceValidatorKey := simulation.RandomKey(r, keys) - sourceValidatorAddress := sdk.ValAddress(sourceValidatorKey.PubKey().Address()) - destValidatorKey := simulation.RandomKey(r, keys) - destValidatorAddress := sdk.ValAddress(destValidatorKey.PubKey().Address()) - delegatorKey := simulation.RandomKey(r, keys) - delegatorAddress := sdk.AccAddress(delegatorKey.PubKey().Address()) + sourceValidatorAcc := simulation.RandomAcc(r, accs) + sourceValidatorAddress := sdk.ValAddress(sourceValidatorAcc.Address) + destValidatorAcc := simulation.RandomAcc(r, accs) + destValidatorAddress := sdk.ValAddress(destValidatorAcc.Address) + delegatorAcc := simulation.RandomAcc(r, accs) + delegatorAddress := delegatorAcc.Address // TODO amount := m.GetAccount(ctx, delegatorAddress).GetCoins().AmountOf(denom) if amount.GT(sdk.ZeroInt()) { @@ -229,14 +248,14 @@ func SimulateMsgBeginRedelegate(m auth.AccountMapper, k stake.Keeper) simulation // SimulateMsgCompleteRedelegate func SimulateMsgCompleteRedelegate(k stake.Keeper) simulation.Operation { handler := stake.NewHandler(k) - return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, keys []crypto.PrivKey, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { + return func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simulation.Account, event func(string)) (action string, fOp []simulation.FutureOperation, err error) { - validatorSrcKey := simulation.RandomKey(r, keys) - validatorSrcAddress := sdk.ValAddress(validatorSrcKey.PubKey().Address()) - validatorDstKey := simulation.RandomKey(r, keys) - validatorDstAddress := sdk.ValAddress(validatorDstKey.PubKey().Address()) - delegatorKey := simulation.RandomKey(r, keys) - delegatorAddress := sdk.AccAddress(delegatorKey.PubKey().Address()) + validatorSrcAcc := simulation.RandomAcc(r, accs) + validatorSrcAddress := sdk.ValAddress(validatorSrcAcc.Address) + validatorDstAcc := simulation.RandomAcc(r, accs) + validatorDstAddress := sdk.ValAddress(validatorDstAcc.Address) + delegatorAcc := simulation.RandomAcc(r, accs) + delegatorAddress := delegatorAcc.Address msg := stake.MsgCompleteRedelegate{ DelegatorAddr: delegatorAddress, ValidatorSrcAddr: validatorSrcAddress, @@ -259,7 +278,7 @@ func SimulateMsgCompleteRedelegate(k stake.Keeper) simulation.Operation { // Setup // nolint: errcheck func Setup(mapp *mock.App, k stake.Keeper) simulation.RandSetup { - return func(r *rand.Rand, privKeys []crypto.PrivKey) { + return func(r *rand.Rand, accs []simulation.Account) { ctx := mapp.NewContext(false, abci.Header{}) gen := stake.DefaultGenesisState() gen.Params.InflationMax = sdk.NewDec(0) diff --git a/x/stake/simulation/sim_test.go b/x/stake/simulation/sim_test.go index 53b9d826ce..b81555f030 100644 --- a/x/stake/simulation/sim_test.go +++ b/x/stake/simulation/sim_test.go @@ -6,7 +6,6 @@ import ( "testing" abci "github.com/tendermint/tendermint/abci/types" - "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/bank" @@ -38,8 +37,8 @@ func TestStakeWithRandomMessages(t *testing.T) { panic(err) } - appStateFn := func(r *rand.Rand, keys []crypto.PrivKey, accs []sdk.AccAddress) json.RawMessage { - mock.RandomSetGenesis(r, mapp, accs, []string{"stake"}) + appStateFn := func(r *rand.Rand, accs []simulation.Account) json.RawMessage { + simulation.RandomSetGenesis(r, mapp, accs, []string{"stake"}) return json.RawMessage("{}") } diff --git a/x/stake/stake.go b/x/stake/stake.go index 6b4036d281..7e60b3113a 100644 --- a/x/stake/stake.go +++ b/x/stake/stake.go @@ -12,6 +12,7 @@ type ( Keeper = keeper.Keeper Validator = types.Validator Description = types.Description + Commission = types.Commission Delegation = types.Delegation DelegationSummary = types.DelegationSummary UnbondingDelegation = types.UnbondingDelegation @@ -64,13 +65,16 @@ var ( GetREDsToValDstIndexKey = keeper.GetREDsToValDstIndexKey GetREDsByDelToValDstIndexKey = keeper.GetREDsByDelToValDstIndexKey - DefaultParams = types.DefaultParams - InitialPool = types.InitialPool - NewValidator = types.NewValidator - NewDescription = types.NewDescription - NewGenesisState = types.NewGenesisState - DefaultGenesisState = types.DefaultGenesisState - RegisterCodec = types.RegisterCodec + DefaultParams = types.DefaultParams + InitialPool = types.InitialPool + NewValidator = types.NewValidator + NewDescription = types.NewDescription + NewCommission = types.NewCommission + NewCommissionMsg = types.NewCommissionMsg + NewCommissionWithTime = types.NewCommissionWithTime + NewGenesisState = types.NewGenesisState + DefaultGenesisState = types.DefaultGenesisState + RegisterCodec = types.RegisterCodec NewMsgCreateValidator = types.NewMsgCreateValidator NewMsgCreateValidatorOnBehalfOf = types.NewMsgCreateValidatorOnBehalfOf diff --git a/x/stake/types/commission.go b/x/stake/types/commission.go new file mode 100644 index 0000000000..b76971faa0 --- /dev/null +++ b/x/stake/types/commission.go @@ -0,0 +1,128 @@ +package types + +import ( + "fmt" + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +type ( + // Commission defines a commission parameters for a given validator. + Commission struct { + Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators + MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which validator can ever charge + MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission + UpdateTime time.Time `json:"update_time"` // the last time the commission rate was changed + } + + // CommissionMsg defines a commission message to be used for creating a + // validator. + CommissionMsg struct { + Rate sdk.Dec `json:"rate"` // the commission rate charged to delegators + MaxRate sdk.Dec `json:"max_rate"` // maximum commission rate which validator can ever charge + MaxChangeRate sdk.Dec `json:"max_change_rate"` // maximum daily increase of the validator commission + } +) + +// NewCommissionMsg returns an initialized validator commission message. +func NewCommissionMsg(rate, maxRate, maxChangeRate sdk.Dec) CommissionMsg { + return CommissionMsg{ + Rate: rate, + MaxRate: maxRate, + MaxChangeRate: maxChangeRate, + } +} + +// NewCommission returns an initialized validator commission. +func NewCommission(rate, maxRate, maxChangeRate sdk.Dec) Commission { + return Commission{ + Rate: rate, + MaxRate: maxRate, + MaxChangeRate: maxChangeRate, + UpdateTime: time.Unix(0, 0).UTC(), + } +} + +// NewCommission returns an initialized validator commission with a specified +// update time which should be the current block BFT time. +func NewCommissionWithTime(rate, maxRate, maxChangeRate sdk.Dec, updatedAt time.Time) Commission { + return Commission{ + Rate: rate, + MaxRate: maxRate, + MaxChangeRate: maxChangeRate, + UpdateTime: updatedAt, + } +} + +// Equal checks if the given Commission object is equal to the receiving +// Commission object. +func (c Commission) Equal(c2 Commission) bool { + return c.Rate.Equal(c2.Rate) && + c.MaxRate.Equal(c2.MaxRate) && + c.MaxChangeRate.Equal(c2.MaxChangeRate) && + c.UpdateTime.Equal(c2.UpdateTime) +} + +// String implements the Stringer interface for a Commission. +func (c Commission) String() string { + return fmt.Sprintf("rate: %s, maxRate: %s, maxChangeRate: %s, updateTime: %s", + c.Rate, c.MaxRate, c.MaxChangeRate, c.UpdateTime, + ) +} + +// Validate performs basic sanity validation checks of initial commission +// parameters. If validation fails, an SDK error is returned. +func (c Commission) Validate() sdk.Error { + switch { + case c.MaxRate.LT(sdk.ZeroDec()): + // max rate cannot be negative + return ErrCommissionNegative(DefaultCodespace) + + case c.MaxRate.GT(sdk.OneDec()): + // max rate cannot be greater than 100% + return ErrCommissionHuge(DefaultCodespace) + + case c.Rate.LT(sdk.ZeroDec()): + // rate cannot be negative + return ErrCommissionNegative(DefaultCodespace) + + case c.Rate.GT(c.MaxRate): + // rate cannot be greater than the max rate + return ErrCommissionGTMaxRate(DefaultCodespace) + + case c.MaxChangeRate.LT(sdk.ZeroDec()): + // change rate cannot be negative + return ErrCommissionChangeRateNegative(DefaultCodespace) + + case c.MaxChangeRate.GT(c.MaxRate): + // change rate cannot be greater than the max rate + return ErrCommissionChangeRateGTMaxRate(DefaultCodespace) + } + + return nil +} + +// ValidateNewRate performs basic sanity validation checks of a new commission +// rate. If validation fails, an SDK error is returned. +func (c Commission) ValidateNewRate(newRate sdk.Dec, blockTime time.Time) sdk.Error { + switch { + case blockTime.Sub(c.UpdateTime).Hours() < 24: + // new rate cannot be changed more than once within 24 hours + return ErrCommissionUpdateTime(DefaultCodespace) + + case newRate.LT(sdk.ZeroDec()): + // new rate cannot be negative + return ErrCommissionNegative(DefaultCodespace) + + case newRate.GT(c.MaxRate): + // new rate cannot be greater than the max rate + return ErrCommissionGTMaxRate(DefaultCodespace) + + case newRate.Sub(c.Rate).Abs().GT(c.MaxChangeRate): + // new rate % points change cannot be greater than the max change rate + return ErrCommissionGTMaxChangeRate(DefaultCodespace) + } + + return nil +} diff --git a/x/stake/types/errors.go b/x/stake/types/errors.go index 541ad3d7b9..84a7e5ae68 100644 --- a/x/stake/types/errors.go +++ b/x/stake/types/errors.go @@ -65,12 +65,24 @@ func ErrCommissionHuge(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be more than 100%") } -func ErrCommissionBeyondMax(codespace sdk.CodespaceType) sdk.Error { - return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be more than preset commission maximum") +func ErrCommissionGTMaxRate(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be more than the max rate") } -func ErrCommissionPastRate(codespace sdk.CodespaceType) sdk.Error { - return sdk.NewError(codespace, CodeInvalidValidator, "commission change is greater than the commission rate, please wait before changing your commission more") +func ErrCommissionUpdateTime(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be changed more than once in 24h") +} + +func ErrCommissionChangeRateNegative(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidValidator, "commission change rate must be positive") +} + +func ErrCommissionChangeRateGTMaxRate(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidValidator, "commission change rate cannot be more than the max rate") +} + +func ErrCommissionGTMaxChangeRate(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidValidator, "commission cannot be changed more than max change rate") } func ErrNilDelegatorAddr(codespace sdk.CodespaceType) sdk.Error { diff --git a/x/stake/types/inflation_test.go b/x/stake/types/inflation_test.go index fd181af3c9..159ecb4c4e 100644 --- a/x/stake/types/inflation_test.go +++ b/x/stake/types/inflation_test.go @@ -107,7 +107,6 @@ func updateProvisions(t *testing.T, pool Pool, params Params, hr int) (sdk.Dec, } // Checks that The inflation will correctly increase or decrease after an update to the pool -// nolint: gocyclo func checkInflation(t *testing.T, pool Pool, previousInflation, updatedInflation sdk.Dec, msg string) { inflationChange := updatedInflation.Sub(previousInflation) diff --git a/x/stake/types/msg.go b/x/stake/types/msg.go index 558c913fc7..a313dd64f4 100644 --- a/x/stake/types/msg.go +++ b/x/stake/types/msg.go @@ -20,6 +20,7 @@ var _, _ sdk.Msg = &MsgBeginRedelegate{}, &MsgCompleteRedelegate{} // MsgCreateValidator - struct for unbonding transactions type MsgCreateValidator struct { Description + Commission CommissionMsg DelegatorAddr sdk.AccAddress `json:"delegator_address"` ValidatorAddr sdk.ValAddress `json:"validator_address"` PubKey crypto.PubKey `json:"pubkey"` @@ -28,22 +29,23 @@ type MsgCreateValidator struct { // Default way to create validator. Delegator address and validator address are the same func NewMsgCreateValidator(valAddr sdk.ValAddress, pubkey crypto.PubKey, - selfDelegation sdk.Coin, description Description) MsgCreateValidator { + selfDelegation sdk.Coin, description Description, commission CommissionMsg) MsgCreateValidator { return NewMsgCreateValidatorOnBehalfOf( - sdk.AccAddress(valAddr), valAddr, pubkey, selfDelegation, description, + sdk.AccAddress(valAddr), valAddr, pubkey, selfDelegation, description, commission, ) } // Creates validator msg by delegator address on behalf of validator address func NewMsgCreateValidatorOnBehalfOf(delAddr sdk.AccAddress, valAddr sdk.ValAddress, - pubkey crypto.PubKey, delegation sdk.Coin, description Description) MsgCreateValidator { + pubkey crypto.PubKey, delegation sdk.Coin, description Description, commission CommissionMsg) MsgCreateValidator { return MsgCreateValidator{ Description: description, DelegatorAddr: delAddr, ValidatorAddr: valAddr, PubKey: pubkey, Delegation: delegation, + Commission: commission, } } @@ -95,10 +97,13 @@ func (msg MsgCreateValidator) ValidateBasic() sdk.Error { if !(msg.Delegation.Amount.GT(sdk.ZeroInt())) { return ErrBadDelegationAmount(DefaultCodespace) } - empty := Description{} - if msg.Description == empty { + if msg.Description == (Description{}) { return sdk.NewError(DefaultCodespace, CodeInvalidInput, "description must be included") } + if msg.Commission == (CommissionMsg{}) { + return sdk.NewError(DefaultCodespace, CodeInvalidInput, "commission must be included") + } + return nil } @@ -108,12 +113,20 @@ func (msg MsgCreateValidator) ValidateBasic() sdk.Error { type MsgEditValidator struct { Description ValidatorAddr sdk.ValAddress `json:"address"` + + // We pass a reference to the new commission rate as it's not mandatory to + // update. If not updated, the deserialized rate will be zero with no way to + // distinguish if an update was intended. + // + // REF: #2373 + CommissionRate *sdk.Dec `json:"commission_rate"` } -func NewMsgEditValidator(valAddr sdk.ValAddress, description Description) MsgEditValidator { +func NewMsgEditValidator(valAddr sdk.ValAddress, description Description, newRate *sdk.Dec) MsgEditValidator { return MsgEditValidator{ - Description: description, - ValidatorAddr: valAddr, + Description: description, + CommissionRate: newRate, + ValidatorAddr: valAddr, } } @@ -144,10 +157,11 @@ func (msg MsgEditValidator) ValidateBasic() sdk.Error { if msg.ValidatorAddr == nil { return sdk.NewError(DefaultCodespace, CodeInvalidInput, "nil validator address") } - empty := Description{} - if msg.Description == empty { + + if msg.Description == (Description{}) { return sdk.NewError(DefaultCodespace, CodeInvalidInput, "transaction must include some information to modify") } + return nil } diff --git a/x/stake/types/msg_test.go b/x/stake/types/msg_test.go index eb66c04220..b5adbd0ad0 100644 --- a/x/stake/types/msg_test.go +++ b/x/stake/types/msg_test.go @@ -17,26 +17,30 @@ var ( // test ValidateBasic for MsgCreateValidator func TestMsgCreateValidator(t *testing.T) { + commission1 := NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) + commission2 := NewCommissionMsg(sdk.NewDec(5), sdk.NewDec(5), sdk.NewDec(5)) + tests := []struct { name, moniker, identity, website, details string + commissionMsg CommissionMsg validatorAddr sdk.ValAddress pubkey crypto.PubKey bond sdk.Coin expectPass bool }{ - {"basic good", "a", "b", "c", "d", addr1, pk1, coinPos, true}, - {"partial description", "", "", "c", "", addr1, pk1, coinPos, true}, - {"empty description", "", "", "", "", addr1, pk1, coinPos, false}, - {"empty address", "a", "b", "c", "d", emptyAddr, pk1, coinPos, false}, - {"empty pubkey", "a", "b", "c", "d", addr1, emptyPubkey, coinPos, true}, - {"empty bond", "a", "b", "c", "d", addr1, pk1, coinZero, false}, - {"negative bond", "a", "b", "c", "d", addr1, pk1, coinNeg, false}, - {"negative bond", "a", "b", "c", "d", addr1, pk1, coinNeg, false}, + {"basic good", "a", "b", "c", "d", commission1, addr1, pk1, coinPos, true}, + {"partial description", "", "", "c", "", commission1, addr1, pk1, coinPos, true}, + {"empty description", "", "", "", "", commission2, addr1, pk1, coinPos, false}, + {"empty address", "a", "b", "c", "d", commission2, emptyAddr, pk1, coinPos, false}, + {"empty pubkey", "a", "b", "c", "d", commission1, addr1, emptyPubkey, coinPos, true}, + {"empty bond", "a", "b", "c", "d", commission2, addr1, pk1, coinZero, false}, + {"negative bond", "a", "b", "c", "d", commission2, addr1, pk1, coinNeg, false}, + {"negative bond", "a", "b", "c", "d", commission1, addr1, pk1, coinNeg, false}, } for _, tc := range tests { description := NewDescription(tc.moniker, tc.identity, tc.website, tc.details) - msg := NewMsgCreateValidator(tc.validatorAddr, tc.pubkey, tc.bond, description) + msg := NewMsgCreateValidator(tc.validatorAddr, tc.pubkey, tc.bond, description, tc.commissionMsg) if tc.expectPass { require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) } else { @@ -60,7 +64,9 @@ func TestMsgEditValidator(t *testing.T) { for _, tc := range tests { description := NewDescription(tc.moniker, tc.identity, tc.website, tc.details) - msg := NewMsgEditValidator(tc.validatorAddr, description) + newRate := sdk.ZeroDec() + + msg := NewMsgEditValidator(tc.validatorAddr, description, &newRate) if tc.expectPass { require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) } else { @@ -71,28 +77,35 @@ func TestMsgEditValidator(t *testing.T) { // test ValidateBasic and GetSigners for MsgCreateValidatorOnBehalfOf func TestMsgCreateValidatorOnBehalfOf(t *testing.T) { + commission1 := NewCommissionMsg(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()) + commission2 := NewCommissionMsg(sdk.NewDec(5), sdk.NewDec(5), sdk.NewDec(5)) + tests := []struct { name, moniker, identity, website, details string + commissionMsg CommissionMsg delegatorAddr sdk.AccAddress validatorAddr sdk.ValAddress validatorPubKey crypto.PubKey bond sdk.Coin expectPass bool }{ - {"basic good", "a", "b", "c", "d", sdk.AccAddress(addr1), addr2, pk2, coinPos, true}, - {"partial description", "", "", "c", "", sdk.AccAddress(addr1), addr2, pk2, coinPos, true}, - {"empty description", "", "", "", "", sdk.AccAddress(addr1), addr2, pk2, coinPos, false}, - {"empty delegator address", "a", "b", "c", "d", sdk.AccAddress(emptyAddr), addr2, pk2, coinPos, false}, - {"empty validator address", "a", "b", "c", "d", sdk.AccAddress(addr1), emptyAddr, pk2, coinPos, false}, - {"empty pubkey", "a", "b", "c", "d", sdk.AccAddress(addr1), addr2, emptyPubkey, coinPos, true}, - {"empty bond", "a", "b", "c", "d", sdk.AccAddress(addr1), addr2, pk2, coinZero, false}, - {"negative bond", "a", "b", "c", "d", sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, - {"negative bond", "a", "b", "c", "d", sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, + {"basic good", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinPos, true}, + {"partial description", "", "", "c", "", commission2, sdk.AccAddress(addr1), addr2, pk2, coinPos, true}, + {"empty description", "", "", "", "", commission1, sdk.AccAddress(addr1), addr2, pk2, coinPos, false}, + {"empty delegator address", "a", "b", "c", "d", commission1, sdk.AccAddress(emptyAddr), addr2, pk2, coinPos, false}, + {"empty validator address", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), emptyAddr, pk2, coinPos, false}, + {"empty pubkey", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, emptyPubkey, coinPos, true}, + {"empty bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinZero, false}, + {"negative bond", "a", "b", "c", "d", commission1, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, + {"negative bond", "a", "b", "c", "d", commission2, sdk.AccAddress(addr1), addr2, pk2, coinNeg, false}, } for _, tc := range tests { description := NewDescription(tc.moniker, tc.identity, tc.website, tc.details) - msg := NewMsgCreateValidatorOnBehalfOf(tc.delegatorAddr, tc.validatorAddr, tc.validatorPubKey, tc.bond, description) + msg := NewMsgCreateValidatorOnBehalfOf( + tc.delegatorAddr, tc.validatorAddr, tc.validatorPubKey, tc.bond, description, tc.commissionMsg, + ) + if tc.expectPass { require.Nil(t, msg.ValidateBasic(), "test: %v", tc.name) } else { @@ -100,11 +113,11 @@ func TestMsgCreateValidatorOnBehalfOf(t *testing.T) { } } - msg := NewMsgCreateValidator(addr1, pk1, coinPos, Description{}) + msg := NewMsgCreateValidator(addr1, pk1, coinPos, Description{}, CommissionMsg{}) addrs := msg.GetSigners() require.Equal(t, []sdk.AccAddress{sdk.AccAddress(addr1)}, addrs, "Signers on default msg is wrong") - msg = NewMsgCreateValidatorOnBehalfOf(sdk.AccAddress(addr2), addr1, pk1, coinPos, Description{}) + msg = NewMsgCreateValidatorOnBehalfOf(sdk.AccAddress(addr2), addr1, pk1, coinPos, Description{}, CommissionMsg{}) addrs = msg.GetSigners() require.Equal(t, []sdk.AccAddress{sdk.AccAddress(addr2), sdk.AccAddress(addr1)}, addrs, "Signers for onbehalfof msg is wrong") } diff --git a/x/stake/types/validator.go b/x/stake/types/validator.go index c15d43396c..051ffa9e51 100644 --- a/x/stake/types/validator.go +++ b/x/stake/types/validator.go @@ -36,68 +36,56 @@ type Validator struct { UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding UnbondingMinTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding - Commission sdk.Dec `json:"commission"` // XXX the commission rate of fees charged to any delegators - CommissionMax sdk.Dec `json:"commission_max"` // XXX maximum commission rate which this validator can ever charge - CommissionChangeRate sdk.Dec `json:"commission_change_rate"` // XXX maximum daily increase of the validator commission - CommissionChangeToday sdk.Dec `json:"commission_change_today"` // XXX commission rate change today, reset each day (UTC time) + Commission Commission `json:"commission"` // commission parameters } // NewValidator - initialize a new validator func NewValidator(operator sdk.ValAddress, pubKey crypto.PubKey, description Description) Validator { return Validator{ - OperatorAddr: operator, - ConsPubKey: pubKey, - Jailed: false, - Status: sdk.Unbonded, - Tokens: sdk.ZeroDec(), - DelegatorShares: sdk.ZeroDec(), - Description: description, - BondHeight: int64(0), - BondIntraTxCounter: int16(0), - UnbondingHeight: int64(0), - UnbondingMinTime: time.Unix(0, 0).UTC(), - Commission: sdk.ZeroDec(), - CommissionMax: sdk.ZeroDec(), - CommissionChangeRate: sdk.ZeroDec(), - CommissionChangeToday: sdk.ZeroDec(), + OperatorAddr: operator, + ConsPubKey: pubKey, + Jailed: false, + Status: sdk.Unbonded, + Tokens: sdk.ZeroDec(), + DelegatorShares: sdk.ZeroDec(), + Description: description, + BondHeight: int64(0), + BondIntraTxCounter: int16(0), + UnbondingHeight: int64(0), + UnbondingMinTime: time.Unix(0, 0).UTC(), + Commission: NewCommission(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()), } } // what's kept in the store value type validatorValue struct { - ConsPubKey crypto.PubKey - Jailed bool - Status sdk.BondStatus - Tokens sdk.Dec - DelegatorShares sdk.Dec - Description Description - BondHeight int64 - BondIntraTxCounter int16 - UnbondingHeight int64 - UnbondingMinTime time.Time - Commission sdk.Dec - CommissionMax sdk.Dec - CommissionChangeRate sdk.Dec - CommissionChangeToday sdk.Dec + ConsPubKey crypto.PubKey + Jailed bool + Status sdk.BondStatus + Tokens sdk.Dec + DelegatorShares sdk.Dec + Description Description + BondHeight int64 + BondIntraTxCounter int16 + UnbondingHeight int64 + UnbondingMinTime time.Time + Commission Commission } // return the redelegation without fields contained within the key for the store func MustMarshalValidator(cdc *codec.Codec, validator Validator) []byte { val := validatorValue{ - ConsPubKey: validator.ConsPubKey, - Jailed: validator.Jailed, - Status: validator.Status, - Tokens: validator.Tokens, - DelegatorShares: validator.DelegatorShares, - Description: validator.Description, - BondHeight: validator.BondHeight, - BondIntraTxCounter: validator.BondIntraTxCounter, - UnbondingHeight: validator.UnbondingHeight, - UnbondingMinTime: validator.UnbondingMinTime, - Commission: validator.Commission, - CommissionMax: validator.CommissionMax, - CommissionChangeRate: validator.CommissionChangeRate, - CommissionChangeToday: validator.CommissionChangeToday, + ConsPubKey: validator.ConsPubKey, + Jailed: validator.Jailed, + Status: validator.Status, + Tokens: validator.Tokens, + DelegatorShares: validator.DelegatorShares, + Description: validator.Description, + BondHeight: validator.BondHeight, + BondIntraTxCounter: validator.BondIntraTxCounter, + UnbondingHeight: validator.UnbondingHeight, + UnbondingMinTime: validator.UnbondingMinTime, + Commission: validator.Commission, } return cdc.MustMarshalBinary(val) } @@ -124,21 +112,18 @@ func UnmarshalValidator(cdc *codec.Codec, operatorAddr, value []byte) (validator } return Validator{ - OperatorAddr: operatorAddr, - ConsPubKey: storeValue.ConsPubKey, - Jailed: storeValue.Jailed, - Tokens: storeValue.Tokens, - Status: storeValue.Status, - DelegatorShares: storeValue.DelegatorShares, - Description: storeValue.Description, - BondHeight: storeValue.BondHeight, - BondIntraTxCounter: storeValue.BondIntraTxCounter, - UnbondingHeight: storeValue.UnbondingHeight, - UnbondingMinTime: storeValue.UnbondingMinTime, - Commission: storeValue.Commission, - CommissionMax: storeValue.CommissionMax, - CommissionChangeRate: storeValue.CommissionChangeRate, - CommissionChangeToday: storeValue.CommissionChangeToday, + OperatorAddr: operatorAddr, + ConsPubKey: storeValue.ConsPubKey, + Jailed: storeValue.Jailed, + Tokens: storeValue.Tokens, + Status: storeValue.Status, + DelegatorShares: storeValue.DelegatorShares, + Description: storeValue.Description, + BondHeight: storeValue.BondHeight, + BondIntraTxCounter: storeValue.BondIntraTxCounter, + UnbondingHeight: storeValue.UnbondingHeight, + UnbondingMinTime: storeValue.UnbondingMinTime, + Commission: storeValue.Commission, }, nil } @@ -156,16 +141,13 @@ func (v Validator) HumanReadableString() (string, error) { resp += fmt.Sprintf("Validator Consensus Pubkey: %s\n", bechConsPubKey) resp += fmt.Sprintf("Jailed: %v\n", v.Jailed) resp += fmt.Sprintf("Status: %s\n", sdk.BondStatusToString(v.Status)) - resp += fmt.Sprintf("Tokens: %s\n", v.Tokens.String()) - resp += fmt.Sprintf("Delegator Shares: %s\n", v.DelegatorShares.String()) + resp += fmt.Sprintf("Tokens: %s\n", v.Tokens) + resp += fmt.Sprintf("Delegator Shares: %s\n", v.DelegatorShares) resp += fmt.Sprintf("Description: %s\n", v.Description) resp += fmt.Sprintf("Bond Height: %d\n", v.BondHeight) resp += fmt.Sprintf("Unbonding Height: %d\n", v.UnbondingHeight) resp += fmt.Sprintf("Minimum Unbonding Time: %v\n", v.UnbondingMinTime) - resp += fmt.Sprintf("Commission: %s\n", v.Commission.String()) - resp += fmt.Sprintf("Max Commission Rate: %s\n", v.CommissionMax.String()) - resp += fmt.Sprintf("Commission Change Rate: %s\n", v.CommissionChangeRate.String()) - resp += fmt.Sprintf("Commission Change Today: %s\n", v.CommissionChangeToday.String()) + resp += fmt.Sprintf("Commission: {%s}\n", v.Commission) return resp, nil } @@ -189,10 +171,7 @@ type bechValidator struct { UnbondingHeight int64 `json:"unbonding_height"` // if unbonding, height at which this validator has begun unbonding UnbondingMinTime time.Time `json:"unbonding_time"` // if unbonding, min time for the validator to complete unbonding - Commission sdk.Dec `json:"commission"` // XXX the commission rate of fees charged to any delegators - CommissionMax sdk.Dec `json:"commission_max"` // XXX maximum commission rate which this validator can ever charge - CommissionChangeRate sdk.Dec `json:"commission_change_rate"` // XXX maximum daily increase of the validator commission - CommissionChangeToday sdk.Dec `json:"commission_change_today"` // XXX commission rate change today, reset each day (UTC time) + Commission Commission `json:"commission"` // commission parameters } // MarshalJSON marshals the validator to JSON using Bech32 @@ -203,21 +182,18 @@ func (v Validator) MarshalJSON() ([]byte, error) { } return codec.Cdc.MarshalJSON(bechValidator{ - OperatorAddr: v.OperatorAddr, - ConsPubKey: bechConsPubKey, - Jailed: v.Jailed, - Status: v.Status, - Tokens: v.Tokens, - DelegatorShares: v.DelegatorShares, - Description: v.Description, - BondHeight: v.BondHeight, - BondIntraTxCounter: v.BondIntraTxCounter, - UnbondingHeight: v.UnbondingHeight, - UnbondingMinTime: v.UnbondingMinTime, - Commission: v.Commission, - CommissionMax: v.CommissionMax, - CommissionChangeRate: v.CommissionChangeRate, - CommissionChangeToday: v.CommissionChangeToday, + OperatorAddr: v.OperatorAddr, + ConsPubKey: bechConsPubKey, + Jailed: v.Jailed, + Status: v.Status, + Tokens: v.Tokens, + DelegatorShares: v.DelegatorShares, + Description: v.Description, + BondHeight: v.BondHeight, + BondIntraTxCounter: v.BondIntraTxCounter, + UnbondingHeight: v.UnbondingHeight, + UnbondingMinTime: v.UnbondingMinTime, + Commission: v.Commission, }) } @@ -232,21 +208,18 @@ func (v *Validator) UnmarshalJSON(data []byte) error { return err } *v = Validator{ - OperatorAddr: bv.OperatorAddr, - ConsPubKey: consPubKey, - Jailed: bv.Jailed, - Tokens: bv.Tokens, - Status: bv.Status, - DelegatorShares: bv.DelegatorShares, - Description: bv.Description, - BondHeight: bv.BondHeight, - BondIntraTxCounter: bv.BondIntraTxCounter, - UnbondingHeight: bv.UnbondingHeight, - UnbondingMinTime: bv.UnbondingMinTime, - Commission: bv.Commission, - CommissionMax: bv.CommissionMax, - CommissionChangeRate: bv.CommissionChangeRate, - CommissionChangeToday: bv.CommissionChangeToday, + OperatorAddr: bv.OperatorAddr, + ConsPubKey: consPubKey, + Jailed: bv.Jailed, + Tokens: bv.Tokens, + Status: bv.Status, + DelegatorShares: bv.DelegatorShares, + Description: bv.Description, + BondHeight: bv.BondHeight, + BondIntraTxCounter: bv.BondIntraTxCounter, + UnbondingHeight: bv.UnbondingHeight, + UnbondingMinTime: bv.UnbondingMinTime, + Commission: bv.Commission, } return nil } @@ -254,18 +227,14 @@ func (v *Validator) UnmarshalJSON(data []byte) error { //___________________________________________________________________ // only the vitals - does not check bond height of IntraTxCounter -// nolint gocyclo - why dis fail? -func (v Validator) Equal(c2 Validator) bool { - return v.ConsPubKey.Equals(c2.ConsPubKey) && - bytes.Equal(v.OperatorAddr, c2.OperatorAddr) && - v.Status.Equal(c2.Status) && - v.Tokens.Equal(c2.Tokens) && - v.DelegatorShares.Equal(c2.DelegatorShares) && - v.Description == c2.Description && - v.Commission.Equal(c2.Commission) && - v.CommissionMax.Equal(c2.CommissionMax) && - v.CommissionChangeRate.Equal(c2.CommissionChangeRate) && - v.CommissionChangeToday.Equal(c2.CommissionChangeToday) +func (v Validator) Equal(v2 Validator) bool { + return v.ConsPubKey.Equals(v2.ConsPubKey) && + bytes.Equal(v.OperatorAddr, v2.OperatorAddr) && + v.Status.Equal(v2.Status) && + v.Tokens.Equal(v2.Tokens) && + v.DelegatorShares.Equal(v2.DelegatorShares) && + v.Description == v2.Description && + v.Commission.Equal(v2.Commission) } // return the TM validator address @@ -400,6 +369,17 @@ func (v Validator) RemoveTokens(pool Pool, tokens sdk.Dec) (Validator, Pool) { return v, pool } +// SetInitialCommission attempts to set a validator's initial commission. An +// error is returned if the commission is invalid. +func (v Validator) SetInitialCommission(commission Commission) (Validator, sdk.Error) { + if err := commission.Validate(); err != nil { + return v, err + } + + v.Commission = commission + return v, nil +} + //_________________________________________________________________________________________________________ // AddTokensFromDel adds tokens to a validator @@ -474,10 +454,10 @@ func (v Validator) GetJailed() bool { return v.Jailed } func (v Validator) GetMoniker() string { return v.Description.Moniker } func (v Validator) GetStatus() sdk.BondStatus { return v.Status } func (v Validator) GetOperator() sdk.ValAddress { return v.OperatorAddr } -func (v Validator) GetPubKey() crypto.PubKey { return v.ConsPubKey } +func (v Validator) GetConsPubKey() crypto.PubKey { return v.ConsPubKey } func (v Validator) GetConsAddr() sdk.ConsAddress { return sdk.ConsAddress(v.ConsPubKey.Address()) } func (v Validator) GetPower() sdk.Dec { return v.BondedTokens() } func (v Validator) GetTokens() sdk.Dec { return v.Tokens } -func (v Validator) GetCommission() sdk.Dec { return v.Commission } +func (v Validator) GetCommission() sdk.Dec { return v.Commission.Rate } func (v Validator) GetDelegatorShares() sdk.Dec { return v.DelegatorShares } func (v Validator) GetBondHeight() int64 { return v.BondHeight } diff --git a/x/stake/types/validator_test.go b/x/stake/types/validator_test.go index eceffbad39..de9e184804 100644 --- a/x/stake/types/validator_test.go +++ b/x/stake/types/validator_test.go @@ -274,3 +274,37 @@ func TestValidatorMarshalUnmarshalJSON(t *testing.T) { assert.NoError(t, err) assert.Equal(t, validator, *got) } + +func TestValidatorSetInitialCommission(t *testing.T) { + val := NewValidator(addr1, pk1, Description{}) + testCases := []struct { + validator Validator + commission Commission + expectedErr bool + }{ + {val, NewCommission(sdk.ZeroDec(), sdk.ZeroDec(), sdk.ZeroDec()), false}, + {val, NewCommission(sdk.ZeroDec(), sdk.NewDecWithPrec(-1, 1), sdk.ZeroDec()), true}, + {val, NewCommission(sdk.ZeroDec(), sdk.NewDec(15000000000), sdk.ZeroDec()), true}, + {val, NewCommission(sdk.NewDecWithPrec(-1, 1), sdk.ZeroDec(), sdk.ZeroDec()), true}, + {val, NewCommission(sdk.NewDecWithPrec(2, 1), sdk.NewDecWithPrec(1, 1), sdk.ZeroDec()), true}, + {val, NewCommission(sdk.ZeroDec(), sdk.ZeroDec(), sdk.NewDecWithPrec(-1, 1)), true}, + {val, NewCommission(sdk.ZeroDec(), sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(2, 1)), true}, + } + + for i, tc := range testCases { + val, err := tc.validator.SetInitialCommission(tc.commission) + + if tc.expectedErr { + require.Error(t, err, + "expected error for test case #%d with commission: %s", i, tc.commission, + ) + } else { + require.NoError(t, err, + "unexpected error for test case #%d with commission: %s", i, tc.commission, + ) + require.Equal(t, tc.commission, val.Commission, + "invalid validator commission for test case #%d with commission: %s", i, tc.commission, + ) + } + } +}