diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e157987b8c3..602fbb8849e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,9 @@ BREAKING CHANGES * [\#1894](https://github.com/cosmos/cosmos-sdk/pull/1894) `version` command now shows latest commit, vendor dir hash, and build machine info. * SDK + * [#3336](https://github.com/cosmos/cosmos-sdk/issues/3336) Ensure all SDK + messages have their signature bytes contain canonical fields `value` and `type`. + * [\#3333](https://github.com/cosmos/cosmos-sdk/issues/3333) - F1 storage efficiency improvements - automatic withdrawals when unbonded, historical reward reference counting * [staking] [\#2513](https://github.com/cosmos/cosmos-sdk/issues/2513) Validator power type from Dec -> Int * [staking] [\#3233](https://github.com/cosmos/cosmos-sdk/issues/3233) key and value now contain duplicate fields to simplify code * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. diff --git a/Dockerfile b/Dockerfile index b3a5ea7324af..aea2ecc9d54f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ COPY . . # Install minimum necessary dependencies, build Cosmos SDK, remove packages RUN apk add --no-cache $PACKAGES && \ make tools && \ - make get_vendor_deps && \ + make vendor-deps && \ make build && \ make install diff --git a/PENDING.md b/PENDING.md index 4d87301f0d5b..899c501a9ab2 100644 --- a/PENDING.md +++ b/PENDING.md @@ -3,49 +3,150 @@ BREAKING CHANGES * Gaia REST API (`gaiacli advanced rest-server`) + * [gaia-lite] [\#2182] Renamed and merged all redelegations endpoints into `/staking/redelegations` + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) `tx/sign` endpoint now expects `BaseReq` fields as nested object. + * [\#2222] all endpoints renamed from `/stake` -> `/staking` + * [\#1268] `LooseTokens` -> `NotBondedTokens` + * [\#3289] misc renames: + * `Validator.UnbondingMinTime` -> `Validator.UnbondingCompletionTime` + * `Delegation` -> `Value` in `MsgCreateValidator` and `MsgDelegate` + * `MsgBeginUnbonding` -> `MsgUndelegate` * Gaia CLI (`gaiacli`) + * [\#810](https://github.com/cosmos/cosmos-sdk/issues/810) Don't fallback to any default values for chain ID. + * Users need to supply chain ID either via config file or the `--chain-id` flag. + * Change `chain_id` and `trust_node` in `gaiacli` configuration to `chain-id` and `trust-node` respectively. + * [\#3069](https://github.com/cosmos/cosmos-sdk/pull/3069) `--fee` flag renamed to `--fees` to support multiple coins + * [\#3156](https://github.com/cosmos/cosmos-sdk/pull/3156) Remove unimplemented `gaiacli init` command + * [\#2222] `gaiacli tx stake` -> `gaiacli tx staking`, `gaiacli query stake` -> `gaiacli query staking` + * [\#1894](https://github.com/cosmos/cosmos-sdk/issues/1894) `version` command now shows latest commit, vendor dir hash, and build machine info. + * [\#3320](https://github.com/cosmos/cosmos-sdk/pull/3320) Ensure all `gaiacli query` commands respect the `--output` and `--indent` flags * Gaia + * https://github.com/cosmos/cosmos-sdk/issues/2838 - Move store keys to constants + * [\#3162](https://github.com/cosmos/cosmos-sdk/issues/3162) The `--gas` flag now takes `auto` instead of `simulate` + in order to trigger a simulation of the tx before the actual execution. + * [\#3285](https://github.com/cosmos/cosmos-sdk/pull/3285) New `gaiad tendermint version` to print libs versions + * [\#1894](https://github.com/cosmos/cosmos-sdk/pull/1894) `version` command now shows latest commit, vendor dir hash, and build machine info. * SDK + * [staking] \#2513 Validator power type from Dec -> Int + * [staking] \#3233 key and value now contain duplicate fields to simplify code + * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. + * [\#3195](https://github.com/cosmos/cosmos-sdk/issues/3195) Allows custom configuration for syncable strategy + * [\#3242](https://github.com/cosmos/cosmos-sdk/issues/3242) Fix infinite gas + meter utilization during aborted ante handler executions. + * [x/distribution] \#3292 Enable or disable withdraw addresses with a parameter in the param store + * [staking] \#2222 `/stake` -> `/staking` module rename + * [staking] \#1268 `LooseTokens` -> `NotBondedTokens` + * [staking] \#1402 Redelegation and unbonding-delegation structs changed to include multiple an array of entries + * [staking] \#3289 misc renames: + * `Validator.UnbondingMinTime` -> `Validator.UnbondingCompletionTime` + * `Delegation` -> `Value` in `MsgCreateValidator` and `MsgDelegate` + * `MsgBeginUnbonding` -> `MsgUndelegate` + * [\#3315] Increase decimal precision to 18 + * \#3333 - F1 storage efficiency improvements - automatic withdrawals when unbonded, historical reward reference counting + * \#3323 Update to Tendermint 0.29.0 + * [\#3328](https://github.com/cosmos/cosmos-sdk/issues/3328) [x/gov] Remove redundant action tag * Tendermint - + * [\#3298](https://github.com/cosmos/cosmos-sdk/issues/3298) Upgrade to Tendermint 0.28.0 FEATURES * Gaia REST API (`gaiacli advanced rest-server`) + * [\#3067](https://github.com/cosmos/cosmos-sdk/issues/3067) Add support for fees on transactions + * [\#3069](https://github.com/cosmos/cosmos-sdk/pull/3069) Add a custom memo on transactions + * [\#3027](https://github.com/cosmos/cosmos-sdk/issues/3027) Implement + `/gov/proposals/{proposalID}/proposer` to query for a proposal's proposer. * Gaia CLI (`gaiacli`) + * \#2399 Implement `params` command to query slashing parameters. + * [\#2730](https://github.com/cosmos/cosmos-sdk/issues/2730) Add tx search pagination parameter + * [\#3027](https://github.com/cosmos/cosmos-sdk/issues/3027) Implement + `query gov proposer [proposal-id]` to query for a proposal's proposer. + * [\#3198](https://github.com/cosmos/cosmos-sdk/issues/3198) New `keys add --multisig` flag to store multisig keys locally. + * [\#3198](https://github.com/cosmos/cosmos-sdk/issues/3198) New `multisign` command to generate multisig signatures. + * [\#3198](https://github.com/cosmos/cosmos-sdk/issues/3198) New `sign --multisig` flag to enable multisig mode. + * [\#2715](https://github.com/cosmos/cosmos-sdk/issues/2715) Reintroduce gaia server's insecure mode. + * [\#3334](https://github.com/cosmos/cosmos-sdk/pull/3334) New `gaiad completion` and `gaiacli completion` to generate Bash/Zsh completion scripts. + * [\#2607](https://github.com/cosmos/cosmos-sdk/issues/2607) Make `gaiacli config` handle the boolean `indent` flag to beautify commands JSON output. * Gaia + * [\#2182] [x/staking] Added querier for querying a single redelegation + * [\#3305](https://github.com/cosmos/cosmos-sdk/issues/3305) Add support for + vesting accounts at genesis. + * [\#3198](https://github.com/cosmos/cosmos-sdk/issues/3198) [x/auth] Add multisig transactions support + * [\#3198](https://github.com/cosmos/cosmos-sdk/issues/3198) `add-genesis-account` can take both account addresses and key names * SDK + - \#3099 Implement F1 fee distribution + - [\#2926](https://github.com/cosmos/cosmos-sdk/issues/2926) Add TxEncoder to client TxBuilder. + * \#2694 Vesting account implementation. + * \#2996 Update the `AccountKeeper` to contain params used in the context of + the ante handler. + * [\#3179](https://github.com/cosmos/cosmos-sdk/pull/3179) New CodeNoSignatures error code. + * \#3319 [x/distribution] Queriers for all distribution state worth querying; distribution query commands * Tendermint IMPROVEMENTS -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API + * [\#3176](https://github.com/cosmos/cosmos-sdk/issues/3176) Validate tx/sign endpoint POST body. + * [\#2948](https://github.com/cosmos/cosmos-sdk/issues/2948) Swagger UI now makes requests to light client node * Gaia CLI (`gaiacli`) + * [\#3224](https://github.com/cosmos/cosmos-sdk/pull/3224) Support adding offline public keys to the keystore * Gaia + * [\#2186](https://github.com/cosmos/cosmos-sdk/issues/2186) Add Address Interface + * [\#3158](https://github.com/cosmos/cosmos-sdk/pull/3158) Validate slashing genesis + * [\#3172](https://github.com/cosmos/cosmos-sdk/pull/3172) Support minimum fees in a local testnet. + * [\#3250](https://github.com/cosmos/cosmos-sdk/pull/3250) Refactor integration tests and increase coverage + * [\#3248](https://github.com/cosmos/cosmos-sdk/issues/3248) Refactor tx fee + model: + * Validators specify minimum gas prices instead of minimum fees + * Clients may provide either fees or gas prices directly + * The gas prices of a tx must meet a validator's minimum + * [\#2859](https://github.com/cosmos/cosmos-sdk/issues/2859) Rename `TallyResult` in gov proposals to `FinalTallyResult` + * [\#3286](https://github.com/cosmos/cosmos-sdk/pull/3286) Fix `gaiad gentx` printout of account's addresses, i.e. user bech32 instead of hex. * SDK + * [\#3137](https://github.com/cosmos/cosmos-sdk/pull/3137) Add tag documentation + for each module along with cleaning up a few existing tags in the governance, + slashing, and staking modules. + * [\#3093](https://github.com/cosmos/cosmos-sdk/issues/3093) Ante handler does no longer read all accounts in one go when processing signatures as signature + verification may fail before last signature is checked. + * [staking] \#1402 Add for multiple simultaneous redelegations or unbonding-delegations within an unbonding period + * [staking] \#1268 staking spec rewrite * Tendermint +* CI + * \#2498 Added macos CI job to CircleCI + * [#142](https://github.com/tendermint/devops/issues/142) Increased the number of blocks to be tested during multi-sim + * [#147](https://github.com/tendermint/devops/issues/142) Added docker image build to CI BUG FIXES -* Gaia REST API (`gaiacli advanced rest-server`) +* Gaia REST API * Gaia CLI (`gaiacli`) + * \#3141 Fix the bug in GetAccount when `len(res) == 0` and `err == nil` + * [\#810](https://github.com/cosmos/cosmos-sdk/pull/3316) Fix regression in gaiacli config file handling * Gaia + * \#3148 Fix `gaiad export` by adding a boolean to `NewGaiaApp` determining whether or not to load the latest version + * \#3181 Correctly reset total accum update height and jailed-validator bond height / unbonding height on export-for-zero-height + * [\#3172](https://github.com/cosmos/cosmos-sdk/pull/3172) Fix parsing `gaiad.toml` + when it already exists. + * \#3223 Fix unset governance proposal queues when importing state from old chain + * [#3187](https://github.com/cosmos/cosmos-sdk/issues/3187) Fix `gaiad export` + by resetting each validator's slashing period. + * [##3336](https://github.com/cosmos/cosmos-sdk/issues/3336) Ensure all SDK + messages have their signature bytes contain canonical fields `value` and `type`. * SDK diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index a6e6ce9cd708..f535d1373355 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -74,10 +74,10 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context) { } // clear validator slash events - app.distrKeeper.DeleteValidatorSlashEvents(ctx) + app.distrKeeper.DeleteAllValidatorSlashEvents(ctx) // clear validator historical rewards - app.distrKeeper.DeleteValidatorHistoricalRewards(ctx) + app.distrKeeper.DeleteAllValidatorHistoricalRewards(ctx) // set context height to zero height := ctx.BlockHeight() diff --git a/docs/spec/distribution/f1_reference_counting.md b/docs/spec/distribution/f1_reference_counting.md new file mode 100644 index 000000000000..2563be872f71 --- /dev/null +++ b/docs/spec/distribution/f1_reference_counting.md @@ -0,0 +1,18 @@ +## Reference Counting in F1 Fee Distribution + +In F1 fee distribution, in order to calculate the rewards a delegator ought to receive when they +withdraw their delegation, we must read the terms of the summation of rewards divided by tokens from +the period which they ended when they delegated, and the final period (created when they withdraw). + +Additionally, as slashes change the amount of tokens a delegation will have (but we calculate this lazily, +only when a delegator un-delegates), we must calculate rewards in separate periods before / after any slashes +which occurred in between when a delegator delegated and when they withdrew their rewards. Thus slashes, like +delegations, reference the period which was ended by the slash event. + +All stored historical rewards records for periods which are no longer referenced by any delegations +or any slashes can thus be safely removed, as they will never be read (future delegations and future +slashes will always reference future periods). This is implemented by tracking a `ReferenceCount` +along with each historical reward storage entry. Each time a new object (delegation or slash) +is created which might need to reference the historical record, the reference count is incremented. +Each time one object which previously needed to reference the historical record is deleted, the reference +count is decremented. If the reference count hits zero, the historical record is deleted. diff --git a/x/bank/msgs.go b/x/bank/msgs.go index e73dec3b306b..059b49525073 100644 --- a/x/bank/msgs.go +++ b/x/bank/msgs.go @@ -1,8 +1,6 @@ package bank import ( - "encoding/json" - sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -43,24 +41,8 @@ func (msg MsgSend) ValidateBasic() sdk.Error { // Implements Msg. func (msg MsgSend) GetSignBytes() []byte { - var inputs, outputs []json.RawMessage - for _, input := range msg.Inputs { - inputs = append(inputs, input.GetSignBytes()) - } - for _, output := range msg.Outputs { - outputs = append(outputs, output.GetSignBytes()) - } - b, err := msgCdc.MarshalJSON(struct { - Inputs []json.RawMessage `json:"inputs"` - Outputs []json.RawMessage `json:"outputs"` - }{ - Inputs: inputs, - Outputs: outputs, - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -81,15 +63,6 @@ type Input struct { Coins sdk.Coins `json:"coins"` } -// Return bytes to sign for Input -func (in Input) GetSignBytes() []byte { - bin, err := msgCdc.MarshalJSON(in) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(bin) -} - // ValidateBasic - validate transaction input func (in Input) ValidateBasic() sdk.Error { if len(in.Address) == 0 { @@ -106,11 +79,10 @@ func (in Input) ValidateBasic() sdk.Error { // NewInput - create a transaction input, used with MsgSend func NewInput(addr sdk.AccAddress, coins sdk.Coins) Input { - input := Input{ + return Input{ Address: addr, Coins: coins, } - return input } //---------------------------------------- @@ -122,15 +94,6 @@ type Output struct { Coins sdk.Coins `json:"coins"` } -// Return bytes to sign for Output -func (out Output) GetSignBytes() []byte { - bin, err := msgCdc.MarshalJSON(out) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(bin) -} - // ValidateBasic - validate transaction output func (out Output) ValidateBasic() sdk.Error { if len(out.Address) == 0 { @@ -147,11 +110,10 @@ func (out Output) ValidateBasic() sdk.Error { // NewOutput - create a transaction output, used with MsgSend func NewOutput(addr sdk.AccAddress, coins sdk.Coins) Output { - output := Output{ + return Output{ Address: addr, Coins: coins, } - return output } // ---------------------------------------------------------------------------- diff --git a/x/bank/msgs_test.go b/x/bank/msgs_test.go index 040d12bda4ba..3f8b68fd454e 100644 --- a/x/bank/msgs_test.go +++ b/x/bank/msgs_test.go @@ -179,7 +179,7 @@ func TestMsgSendGetSignBytes(t *testing.T) { } res := msg.GetSignBytes() - expected := `{"inputs":[{"address":"cosmos1d9h8qat57ljhcm","coins":[{"amount":"10","denom":"atom"}]}],"outputs":[{"address":"cosmos1da6hgur4wsmpnjyg","coins":[{"amount":"10","denom":"atom"}]}]}` + expected := `{"type":"cosmos-sdk/Send","value":{"inputs":[{"address":"cosmos1d9h8qat57ljhcm","coins":[{"amount":"10","denom":"atom"}]}],"outputs":[{"address":"cosmos1da6hgur4wsmpnjyg","coins":[{"amount":"10","denom":"atom"}]}]}}` require.Equal(t, expected, string(res)) } diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 2d8e21ce83ab..3381ee206f22 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -8,14 +8,16 @@ import ( // initialize starting info for a new delegation func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) { - // period has already been incremented - period := k.GetValidatorCurrentRewards(ctx, val).Period + // period has already been incremented - we want to store the period ended by this delegation action + previousPeriod := k.GetValidatorCurrentRewards(ctx, val).Period - 1 + validator := k.stakingKeeper.Validator(ctx, val) delegation := k.stakingKeeper.Delegation(ctx, del, val) + // calculate delegation stake in tokens // we don't store directly, so multiply delegation shares * (tokens per share) stake := delegation.GetShares().Mul(validator.GetDelegatorShareExRate()) - k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(period-1, stake, uint64(ctx.BlockHeight()))) + k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight()))) } // calculate the rewards accrued by a delegation between two periods @@ -29,7 +31,7 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Valid // return staking * (ending - starting) starting := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod) ending := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod) - difference := ending.Minus(starting) + difference := ending.CumulativeRewardRatio.Minus(starting.CumulativeRewardRatio) rewards = difference.MulDec(staking) return } @@ -50,7 +52,7 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d if endingHeight >= startingHeight { k.IterateValidatorSlashEventsBetween(ctx, del.GetValidatorAddr(), startingHeight, endingHeight, func(height uint64, event types.ValidatorSlashEvent) (stop bool) { - endingPeriod := event.ValidatorPeriod - 1 + endingPeriod := event.ValidatorPeriod rewards = rewards.Plus(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)) stake = stake.Mul(sdk.OneDec().Sub(event.Fraction)) startingPeriod = endingPeriod @@ -67,10 +69,20 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, del sdk.Delegation) sdk.Error { + // check existence of delegator starting info + if !k.HasDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) { + return types.ErrNoDelegationDistInfo(k.codespace) + } + // end current period and calculate rewards endingPeriod := k.incrementValidatorPeriod(ctx, val) rewards := k.calculateDelegationRewards(ctx, val, del, endingPeriod) + // decrement reference count of starting period + startingInfo := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) + startingPeriod := startingInfo.PreviousPeriod + k.decrementReferenceCount(ctx, del.GetValidatorAddr(), startingPeriod) + // truncate coins, return remainder to community pool coins, remainder := rewards.TruncateDecimal() outstanding := k.GetOutstandingRewards(ctx) @@ -85,5 +97,8 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de return err } + // remove delegator starting info + k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) + return nil } diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 0ebe068ba9b7..8ec514570a0d 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -29,9 +29,15 @@ func TestCalculateRewardsBasic(t *testing.T) { val := sk.Validator(ctx, valOpAddr1) del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) + // historical count should be 2 (once for validator init, once for delegation init) + require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + // end period endingPeriod := k.incrementValidatorPeriod(ctx, val) + // historical count should be 3 (since we ended the period, and haven't yet decremented a reference) + require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + // calculate delegation rewards rewards := k.calculateDelegationRewards(ctx, val, del, endingPeriod) @@ -276,9 +282,15 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { tokens := sdk.DecCoins{{staking.DefaultBondDenom, sdk.NewDec(initial)}} k.AllocateTokensToValidator(ctx, val, tokens) + // historical count should be 2 (initial + latest for delegation) + require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + // withdraw rewards require.Nil(t, k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1)) + // historical count should still be 2 (added one record, cleared one) + require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + // assert correct balance require.Equal(t, sdk.Coins{{staking.DefaultBondDenom, sdk.NewInt(balance - bond + (initial / 2))}}, ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins()) @@ -447,10 +459,16 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { tokens := sdk.DecCoins{{staking.DefaultBondDenom, sdk.NewDec(initial)}} k.AllocateTokensToValidator(ctx, val, tokens) + // historical count should be 2 (validator init, delegation init) + require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + // second delegation msg2 := staking.NewMsgDelegate(sdk.AccAddress(valOpAddr2), valOpAddr1, sdk.NewCoin(staking.DefaultBondDenom, sdk.NewInt(100))) require.True(t, sh(ctx, msg2).IsOK()) + // historical count should be 3 (second delegation init) + require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + // fetch updated validator val = sk.Validator(ctx, valOpAddr1) del2 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr2), valOpAddr1) @@ -467,6 +485,9 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // second delegator withdraws k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr2), valOpAddr1) + // historical count should be 3 (validator init + two delegations) + require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + // validator withdraws commission k.WithdrawValidatorCommission(ctx, valOpAddr1) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index a3ed466e2f37..3973b36ede6a 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -20,11 +20,41 @@ func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { h.k.initializeValidator(ctx, val) } func (h Hooks) BeforeValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { - val := h.k.stakingKeeper.Validator(ctx, valAddr) - // increment period - h.k.incrementValidatorPeriod(ctx, val) } func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { + // force-withdraw commission + commission := h.k.GetValidatorAccumulatedCommission(ctx, valAddr) + if !commission.IsZero() { + coins, remainder := commission.TruncateDecimal() + + // remainder to community pool + feePool := h.k.GetFeePool(ctx) + feePool.CommunityPool = feePool.CommunityPool.Plus(remainder) + h.k.SetFeePool(ctx, feePool) + + // update outstanding + outstanding := h.k.GetOutstandingRewards(ctx) + h.k.SetOutstandingRewards(ctx, outstanding.Minus(commission)) + + // add to validator account + accAddr := sdk.AccAddress(valAddr) + withdrawAddr := h.k.GetDelegatorWithdrawAddr(ctx, accAddr) + + if _, _, err := h.k.bankKeeper.AddCoins(ctx, withdrawAddr, coins); err != nil { + panic(err) + } + } + // remove commission record + h.k.DeleteValidatorAccumulatedCommission(ctx, valAddr) + + // clear slashes + h.k.DeleteValidatorSlashEvents(ctx, valAddr) + + // clear historical rewards + h.k.DeleteValidatorHistoricalRewards(ctx, valAddr) + + // clear current rewards + h.k.DeleteValidatorCurrentRewards(ctx, valAddr) } func (h Hooks) BeforeDelegationCreated(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { val := h.k.stakingKeeper.Validator(ctx, valAddr) @@ -42,7 +72,7 @@ func (h Hooks) BeforeDelegationSharesModified(ctx sdk.Context, delAddr sdk.AccAd } } func (h Hooks) BeforeDelegationRemoved(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { - // nothing needed here since OnDelegationSharesModified will always also be called + // nothing needed here since BeforeDelegationSharesModified will always also be called } func (h Hooks) AfterDelegationModified(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) { // create new delegation period record diff --git a/x/distribution/keeper/key.go b/x/distribution/keeper/key.go index 05bd7bf5cd48..1b8437b4cd95 100644 --- a/x/distribution/keeper/key.go +++ b/x/distribution/keeper/key.go @@ -112,6 +112,11 @@ func GetDelegatorStartingInfoKey(v sdk.ValAddress, d sdk.AccAddress) []byte { return append(append(DelegatorStartingInfoPrefix, v.Bytes()...), d.Bytes()...) } +// gets the prefix key for a validator's historical rewards +func GetValidatorHistoricalRewardsPrefix(v sdk.ValAddress) []byte { + return append(ValidatorHistoricalRewardsPrefix, v.Bytes()...) +} + // gets the key for a validator's historical rewards func GetValidatorHistoricalRewardsKey(v sdk.ValAddress, k uint64) []byte { b := make([]byte, 8) @@ -129,6 +134,11 @@ func GetValidatorAccumulatedCommissionKey(v sdk.ValAddress) []byte { return append(ValidatorAccumulatedCommissionPrefix, v.Bytes()...) } +// gets the prefix key for a validator's slash fractions +func GetValidatorSlashEventPrefix(v sdk.ValAddress) []byte { + return append(ValidatorSlashEventPrefix, v.Bytes()...) +} + // gets the key for a validator's slash fraction func GetValidatorSlashEventKey(v sdk.ValAddress, height uint64) []byte { b := make([]byte, 8) diff --git a/x/distribution/keeper/querier.go b/x/distribution/keeper/querier.go index 8aecb9780422..e1323aa34318 100644 --- a/x/distribution/keeper/querier.go +++ b/x/distribution/keeper/querier.go @@ -3,10 +3,11 @@ package keeper import ( "fmt" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" - abci "github.com/tendermint/tendermint/abci/types" ) // nolint diff --git a/x/distribution/keeper/querier_test.go b/x/distribution/keeper/querier_test.go index f4f99973acf8..eae5324edcd6 100644 --- a/x/distribution/keeper/querier_test.go +++ b/x/distribution/keeper/querier_test.go @@ -6,11 +6,12 @@ import ( "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/cosmos/cosmos-sdk/x/staking" - abci "github.com/tendermint/tendermint/abci/types" ) const custom = "custom" diff --git a/x/distribution/keeper/store.go b/x/distribution/keeper/store.go index 94bcf668c778..7bbf6a13ff2a 100644 --- a/x/distribution/keeper/store.go +++ b/x/distribution/keeper/store.go @@ -21,8 +21,8 @@ func (k Keeper) SetDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAddr store.Set(GetDelegatorWithdrawAddrKey(delAddr), withdrawAddr.Bytes()) } -// remove a delegator withdraw addr -func (k Keeper) RemoveDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAddr sdk.AccAddress) { +// delete a delegator withdraw addr +func (k Keeper) DeleteDelegatorWithdrawAddr(ctx sdk.Context, delAddr, withdrawAddr sdk.AccAddress) { store := ctx.KVStore(k.storeKey) store.Delete(GetDelegatorWithdrawAddrKey(delAddr)) } @@ -77,7 +77,7 @@ func (k Keeper) SetPreviousProposerConsAddr(ctx sdk.Context, consAddr sdk.ConsAd store.Set(ProposerKey, b) } -// get the starting period associated with a delegator +// get the starting info associated with a delegator func (k Keeper) GetDelegatorStartingInfo(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) (period types.DelegatorStartingInfo) { store := ctx.KVStore(k.storeKey) b := store.Get(GetDelegatorStartingInfoKey(val, del)) @@ -85,13 +85,25 @@ func (k Keeper) GetDelegatorStartingInfo(ctx sdk.Context, val sdk.ValAddress, de return } -// set the starting period associated with a delegator +// set the starting info associated with a delegator func (k Keeper) SetDelegatorStartingInfo(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress, period types.DelegatorStartingInfo) { store := ctx.KVStore(k.storeKey) b := k.cdc.MustMarshalBinaryLengthPrefixed(period) store.Set(GetDelegatorStartingInfoKey(val, del), b) } +// check existence of the starting info associated with a delegator +func (k Keeper) HasDelegatorStartingInfo(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) bool { + store := ctx.KVStore(k.storeKey) + return store.Has(GetDelegatorStartingInfoKey(val, del)) +} + +// delete the starting info associated with a delegator +func (k Keeper) DeleteDelegatorStartingInfo(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) { + store := ctx.KVStore(k.storeKey) + store.Delete(GetDelegatorStartingInfoKey(val, del)) +} + // iterate over delegator starting infos func (k Keeper) IterateDelegatorStartingInfos(ctx sdk.Context, handler func(val sdk.ValAddress, del sdk.AccAddress, info types.DelegatorStartingInfo) (stop bool)) { store := ctx.KVStore(k.storeKey) @@ -137,8 +149,24 @@ func (k Keeper) IterateValidatorHistoricalRewards(ctx sdk.Context, handler func( } } -// delete historical rewards -func (k Keeper) DeleteValidatorHistoricalRewards(ctx sdk.Context) { +// delete a historical reward +func (k Keeper) DeleteValidatorHistoricalReward(ctx sdk.Context, val sdk.ValAddress, period uint64) { + store := ctx.KVStore(k.storeKey) + store.Delete(GetValidatorHistoricalRewardsKey(val, period)) +} + +// delete historical rewards for a validator +func (k Keeper) DeleteValidatorHistoricalRewards(ctx sdk.Context, val sdk.ValAddress) { + store := ctx.KVStore(k.storeKey) + iter := sdk.KVStorePrefixIterator(store, GetValidatorHistoricalRewardsPrefix(val)) + defer iter.Close() + for ; iter.Valid(); iter.Next() { + store.Delete(iter.Key()) + } +} + +// delete all historical rewards +func (k Keeper) DeleteAllValidatorHistoricalRewards(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iter := sdk.KVStorePrefixIterator(store, ValidatorHistoricalRewardsPrefix) defer iter.Close() @@ -147,6 +175,17 @@ func (k Keeper) DeleteValidatorHistoricalRewards(ctx sdk.Context) { } } +// historical record count (used for testcases) +func (k Keeper) GetValidatorHistoricalRewardCount(ctx sdk.Context) (count uint64) { + store := ctx.KVStore(k.storeKey) + iter := sdk.KVStorePrefixIterator(store, ValidatorHistoricalRewardsPrefix) + defer iter.Close() + for ; iter.Valid(); iter.Next() { + count++ + } + return +} + // get current rewards for a validator func (k Keeper) GetValidatorCurrentRewards(ctx sdk.Context, val sdk.ValAddress) (rewards types.ValidatorCurrentRewards) { store := ctx.KVStore(k.storeKey) @@ -162,6 +201,12 @@ func (k Keeper) SetValidatorCurrentRewards(ctx sdk.Context, val sdk.ValAddress, store.Set(GetValidatorCurrentRewardsKey(val), b) } +// delete current rewards for a validator +func (k Keeper) DeleteValidatorCurrentRewards(ctx sdk.Context, val sdk.ValAddress) { + store := ctx.KVStore(k.storeKey) + store.Delete(GetValidatorCurrentRewardsKey(val)) +} + // iterate over current rewards func (k Keeper) IterateValidatorCurrentRewards(ctx sdk.Context, handler func(val sdk.ValAddress, rewards types.ValidatorCurrentRewards) (stop bool)) { store := ctx.KVStore(k.storeKey) @@ -195,6 +240,12 @@ func (k Keeper) SetValidatorAccumulatedCommission(ctx sdk.Context, val sdk.ValAd store.Set(GetValidatorAccumulatedCommissionKey(val), b) } +// delete accumulated commission for a validator +func (k Keeper) DeleteValidatorAccumulatedCommission(ctx sdk.Context, val sdk.ValAddress) { + store := ctx.KVStore(k.storeKey) + store.Delete(GetValidatorAccumulatedCommissionKey(val)) +} + // iterate over accumulated commissions func (k Keeper) IterateValidatorAccumulatedCommissions(ctx sdk.Context, handler func(val sdk.ValAddress, commission types.ValidatorAccumulatedCommission) (stop bool)) { store := ctx.KVStore(k.storeKey) @@ -277,8 +328,18 @@ func (k Keeper) IterateValidatorSlashEvents(ctx sdk.Context, handler func(val sd } } +// delete slash events for a particular validator +func (k Keeper) DeleteValidatorSlashEvents(ctx sdk.Context, val sdk.ValAddress) { + store := ctx.KVStore(k.storeKey) + iter := sdk.KVStorePrefixIterator(store, GetValidatorSlashEventPrefix(val)) + defer iter.Close() + for ; iter.Valid(); iter.Next() { + store.Delete(iter.Key()) + } +} + // delete all slash events -func (k Keeper) DeleteValidatorSlashEvents(ctx sdk.Context) { +func (k Keeper) DeleteAllValidatorSlashEvents(ctx sdk.Context) { store := ctx.KVStore(k.storeKey) iter := sdk.KVStorePrefixIterator(store, ValidatorSlashEventPrefix) defer iter.Close() diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 15bf49f0584f..d7bebb9c7ccb 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -8,8 +8,8 @@ import ( // initialize rewards for a new validator func (k Keeper) initializeValidator(ctx sdk.Context, val sdk.Validator) { - // set initial historical rewards (period 0) - k.SetValidatorHistoricalRewards(ctx, val.GetOperator(), 0, types.ValidatorHistoricalRewards{}) + // set initial historical rewards (period 0) with reference count of 1 + k.SetValidatorHistoricalRewards(ctx, val.GetOperator(), 0, types.NewValidatorHistoricalRewards(sdk.DecCoins{}, 1)) // set current rewards (starting at period 1) k.SetValidatorCurrentRewards(ctx, val.GetOperator(), types.NewValidatorCurrentRewards(sdk.DecCoins{}, 1)) @@ -42,10 +42,10 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin } // fetch historical rewards for last period - historical := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period-1) + historical := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period-1).CumulativeRewardRatio - // fet new historical rewards - k.SetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period, historical.Plus(current)) + // set new historical rewards with reference count of 1 + k.SetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period, types.NewValidatorHistoricalRewards(historical.Plus(current), 1)) // set current rewards, incrementing period by 1 k.SetValidatorCurrentRewards(ctx, val.GetOperator(), types.NewValidatorCurrentRewards(sdk.DecCoins{}, rewards.Period+1)) @@ -53,19 +53,47 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin return rewards.Period } +// increment the reference count for a historical rewards value +func (k Keeper) incrementReferenceCount(ctx sdk.Context, valAddr sdk.ValAddress, period uint64) { + historical := k.GetValidatorHistoricalRewards(ctx, valAddr, period) + if historical.ReferenceCount > 1 { + panic("reference count should never exceed 1") + } + historical.ReferenceCount++ + k.SetValidatorHistoricalRewards(ctx, valAddr, period, historical) +} + +// decrement the reference count for a historical rewards value, and delete if zero references remain +func (k Keeper) decrementReferenceCount(ctx sdk.Context, valAddr sdk.ValAddress, period uint64) { + historical := k.GetValidatorHistoricalRewards(ctx, valAddr, period) + if historical.ReferenceCount == 0 { + panic("cannot set negative reference count") + } + historical.ReferenceCount-- + if historical.ReferenceCount == 0 { + k.DeleteValidatorHistoricalReward(ctx, valAddr, period) + } else { + k.SetValidatorHistoricalRewards(ctx, valAddr, period, historical) + } +} + func (k Keeper) updateValidatorSlashFraction(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) { height := uint64(ctx.BlockHeight()) currentFraction := sdk.ZeroDec() - currentPeriod := k.GetValidatorCurrentRewards(ctx, valAddr).Period + endedPeriod := k.GetValidatorCurrentRewards(ctx, valAddr).Period - 1 current, found := k.GetValidatorSlashEvent(ctx, valAddr, height) if found { // there has already been a slash event this height, // and we don't need to store more than one, // so just update the current slash fraction currentFraction = current.Fraction + } else { + val := k.stakingKeeper.Validator(ctx, valAddr) + // increment current period + endedPeriod = k.incrementValidatorPeriod(ctx, val) } currentMultiplicand := sdk.OneDec().Sub(currentFraction) newMultiplicand := sdk.OneDec().Sub(fraction) updatedFraction := sdk.OneDec().Sub(currentMultiplicand.Mul(newMultiplicand)) - k.SetValidatorSlashEvent(ctx, valAddr, height, types.NewValidatorSlashEvent(currentPeriod, updatedFraction)) + k.SetValidatorSlashEvent(ctx, valAddr, height, types.NewValidatorSlashEvent(endedPeriod, updatedFraction)) } diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index 4c46d67dbb6b..5b2d0e341d77 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" distr "github.com/cosmos/cosmos-sdk/x/distribution" + "github.com/cosmos/cosmos-sdk/x/distribution/types" "github.com/cosmos/cosmos-sdk/x/mock/simulation" "github.com/cosmos/cosmos-sdk/x/staking" ) @@ -20,6 +21,10 @@ func AllInvariants(d distr.Keeper, stk staking.Keeper) simulation.Invariant { if err != nil { return err } + err = ReferenceCountInvariant(d, stk)(ctx) + if err != nil { + return err + } return nil } } @@ -29,7 +34,7 @@ func NonNegativeOutstandingInvariant(k distr.Keeper) simulation.Invariant { return func(ctx sdk.Context) error { outstanding := k.GetOutstandingRewards(ctx) if outstanding.HasNegative() { - return fmt.Errorf("Negative outstanding coins: %v", outstanding) + return fmt.Errorf("negative outstanding coins: %v", outstanding) } return nil } @@ -57,7 +62,35 @@ func CanWithdrawInvariant(k distr.Keeper, sk staking.Keeper) simulation.Invarian remaining := k.GetOutstandingRewards(ctx) if len(remaining) > 0 && remaining[0].Amount.LT(sdk.ZeroDec()) { - return fmt.Errorf("Negative remaining coins: %v", remaining) + return fmt.Errorf("negative remaining coins: %v", remaining) + } + + return nil + } +} + +// ReferenceCountInvariant checks that the number of historical rewards records is correct +func ReferenceCountInvariant(k distr.Keeper, sk staking.Keeper) simulation.Invariant { + return func(ctx sdk.Context) error { + + valCount := uint64(0) + sk.IterateValidators(ctx, func(_ int64, val sdk.Validator) (stop bool) { + valCount++ + return false + }) + dels := sk.GetAllDelegations(ctx) + slashCount := uint64(0) + k.IterateValidatorSlashEvents(ctx, func(_ sdk.ValAddress, _ uint64, _ types.ValidatorSlashEvent) (stop bool) { + slashCount++ + return false + }) + + // one record per validator (zeroeth period), one record per delegation (previous period), one record per slash (previous period) + expected := valCount + uint64(len(dels)) + slashCount + + count := k.GetValidatorHistoricalRewardCount(ctx) + if count != expected { + return fmt.Errorf("unexpected number of historical rewards records: expected %v, got %v", expected, count) } return nil diff --git a/x/distribution/types/msg.go b/x/distribution/types/msg.go index 8100e51562a3..9c977ea91085 100644 --- a/x/distribution/types/msg.go +++ b/x/distribution/types/msg.go @@ -34,11 +34,8 @@ func (msg MsgSetWithdrawAddress) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgSetWithdrawAddress) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -75,11 +72,8 @@ func (msg MsgWithdrawDelegatorReward) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgWithdrawDelegatorReward) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -114,11 +108,8 @@ func (msg MsgWithdrawValidatorCommission) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgWithdrawValidatorCommission) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check diff --git a/x/distribution/types/validator.go b/x/distribution/types/validator.go index 807a3ff44444..0bde7fc073ee 100644 --- a/x/distribution/types/validator.go +++ b/x/distribution/types/validator.go @@ -8,9 +8,28 @@ import ( ) // historical rewards for a validator -// TODO add reference counter, ref https://github.com/cosmos/cosmos-sdk/pull/3099#discussion_r245747051 // height is implicit within the store key -type ValidatorHistoricalRewards = sdk.DecCoins +// cumulative reward ratio is the sum from the zeroeth period +// until this period of rewards / tokens, per the spec +// The reference count indicates the number of objects +// which might need to reference this historical entry +// at any point. +// ReferenceCount = +// number of outstanding delegations which ended the associated period (and might need to read that record) +// + number of slashes which ended the associated period (and might need to read that record) +// + one per validator for the zeroeth period, set on initialization +type ValidatorHistoricalRewards struct { + CumulativeRewardRatio sdk.DecCoins `json:"cumulative_reward_ratio"` + ReferenceCount uint16 `json:"reference_count"` +} + +// create a new ValidatorHistoricalRewards +func NewValidatorHistoricalRewards(cumulativeRewardRatio sdk.DecCoins, referenceCount uint16) ValidatorHistoricalRewards { + return ValidatorHistoricalRewards{ + CumulativeRewardRatio: cumulativeRewardRatio, + ReferenceCount: referenceCount, + } +} // current rewards and current period for a validator // kept as a running counter and incremented each block diff --git a/x/gov/msgs.go b/x/gov/msgs.go index bec99f078b93..3be7ab583a15 100644 --- a/x/gov/msgs.go +++ b/x/gov/msgs.go @@ -73,11 +73,8 @@ func (msg MsgSubmitProposal) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgSubmitProposal) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -134,11 +131,8 @@ func (msg MsgDeposit) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgDeposit) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. @@ -192,11 +186,8 @@ func (msg MsgVote) Get(key interface{}) (value interface{}) { // Implements Msg. func (msg MsgVote) GetSignBytes() []byte { - b, err := msgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := msgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // Implements Msg. diff --git a/x/slashing/msg.go b/x/slashing/msg.go index 6721f8e9b876..551258627773 100644 --- a/x/slashing/msg.go +++ b/x/slashing/msg.go @@ -30,11 +30,8 @@ func (msg MsgUnjail) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgUnjail) GetSignBytes() []byte { - b, err := cdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := cdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check diff --git a/x/staking/types/msg.go b/x/staking/types/msg.go index 7fe7418dda56..9ef062104675 100644 --- a/x/staking/types/msg.go +++ b/x/staking/types/msg.go @@ -2,14 +2,21 @@ package types import ( "bytes" + "encoding/json" "github.com/tendermint/tendermint/crypto" sdk "github.com/cosmos/cosmos-sdk/types" ) -// Verify interface at compile time -var _, _, _ sdk.Msg = &MsgCreateValidator{}, &MsgEditValidator{}, &MsgDelegate{} +// ensure Msg interface compliance at compile time +var ( + _ sdk.Msg = &MsgCreateValidator{} + _ sdk.Msg = &MsgEditValidator{} + _ sdk.Msg = &MsgDelegate{} + _ sdk.Msg = &MsgUndelegate{} + _ sdk.Msg = &MsgBeginRedelegate{} +) //______________________________________________________________________ @@ -23,6 +30,15 @@ type MsgCreateValidator struct { Value sdk.Coin `json:"value"` } +type msgCreateValidatorJSON struct { + Description Description `json:"description"` + Commission CommissionMsg `json:"commission"` + DelegatorAddr sdk.AccAddress `json:"delegator_address"` + ValidatorAddr sdk.ValAddress `json:"validator_address"` + PubKey string `json:"pubkey"` + Value sdk.Coin `json:"value"` +} + // 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, commission CommissionMsg) MsgCreateValidator { @@ -62,26 +78,41 @@ func (msg MsgCreateValidator) GetSigners() []sdk.AccAddress { return addrs } -// TODO Remove use of custom struct (no longer necessary) -// get the bytes for the message signer to sign on -func (msg MsgCreateValidator) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - Description Description `json:"description"` - Commission CommissionMsg `json:"commission"` - DelegatorAddr sdk.AccAddress `json:"delegator_address"` - ValidatorAddr sdk.ValAddress `json:"validator_address"` - PubKey string `json:"pubkey"` - Value sdk.Coin `json:"value"` - }{ +// MarshalJSON implements the json.Marshaler interface to provide custom JSON +// serialization of the MsgCreateValidator type. +func (msg MsgCreateValidator) MarshalJSON() ([]byte, error) { + return json.Marshal(msgCreateValidatorJSON{ Description: msg.Description, + Commission: msg.Commission, + DelegatorAddr: msg.DelegatorAddr, ValidatorAddr: msg.ValidatorAddr, PubKey: sdk.MustBech32ifyConsPub(msg.PubKey), Value: msg.Value, }) - if err != nil { - panic(err) +} + +// UnmarshalJSON implements the json.Unmarshaler interface to provide custom +// JSON deserialization of the MsgCreateValidator type. +func (msg *MsgCreateValidator) UnmarshalJSON(bz []byte) error { + var msgCreateValJSON msgCreateValidatorJSON + if err := json.Unmarshal(bz, &msgCreateValJSON); err != nil { + return err } - return sdk.MustSortJSON(b) + + msg.Description = msgCreateValJSON.Description + msg.Commission = msgCreateValJSON.Commission + msg.DelegatorAddr = msgCreateValJSON.DelegatorAddr + msg.ValidatorAddr = msgCreateValJSON.ValidatorAddr + msg.PubKey = sdk.MustGetConsPubKeyBech32(msgCreateValJSON.PubKey) + msg.Value = msgCreateValJSON.Value + + return nil +} + +// GetSignBytes returns the message bytes to sign over. +func (msg MsgCreateValidator) GetSignBytes() []byte { + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -137,17 +168,8 @@ func (msg MsgEditValidator) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgEditValidator) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - Description - ValidatorAddr sdk.ValAddress `json:"address"` - }{ - Description: msg.Description, - ValidatorAddr: msg.ValidatorAddr, - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -189,11 +211,8 @@ func (msg MsgDelegate) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgDelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(msg) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -240,21 +259,8 @@ func (msg MsgBeginRedelegate) GetSigners() []sdk.AccAddress { // get the bytes for the message signer to sign on func (msg MsgBeginRedelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - DelegatorAddr sdk.AccAddress `json:"delegator_addr"` - ValidatorSrcAddr sdk.ValAddress `json:"validator_src_addr"` - ValidatorDstAddr sdk.ValAddress `json:"validator_dst_addr"` - SharesAmount string `json:"shares"` - }{ - DelegatorAddr: msg.DelegatorAddr, - ValidatorSrcAddr: msg.ValidatorSrcAddr, - ValidatorDstAddr: msg.ValidatorDstAddr, - SharesAmount: msg.SharesAmount.String(), - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check @@ -298,19 +304,8 @@ func (msg MsgUndelegate) GetSigners() []sdk.AccAddress { return []sdk.AccAddress // get the bytes for the message signer to sign on func (msg MsgUndelegate) GetSignBytes() []byte { - b, err := MsgCdc.MarshalJSON(struct { - DelegatorAddr sdk.AccAddress `json:"delegator_addr"` - ValidatorAddr sdk.ValAddress `json:"validator_addr"` - SharesAmount string `json:"shares_amount"` - }{ - DelegatorAddr: msg.DelegatorAddr, - ValidatorAddr: msg.ValidatorAddr, - SharesAmount: msg.SharesAmount.String(), - }) - if err != nil { - panic(err) - } - return sdk.MustSortJSON(b) + bz := MsgCdc.MustMarshalJSON(msg) + return sdk.MustSortJSON(bz) } // quick validity check diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index d216131b8440..2beba8e671d4 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -184,8 +184,8 @@ func TestRemoveDelShares(t *testing.T) { DelegatorShares: delShares, } pool := Pool{ - BondedTokens: sdk.NewInt(248305), - NotBondedTokens: sdk.NewInt(232147), + BondedTokens: sdk.NewInt(248305), + NotBondedTokens: sdk.NewInt(232147), } shares := sdk.NewDec(29) _, newPool, tokens := validator.RemoveDelShares(pool, shares) @@ -232,8 +232,8 @@ func TestPossibleOverflow(t *testing.T) { DelegatorShares: delShares, } pool := Pool{ - NotBondedTokens: sdk.NewInt(100), - BondedTokens: poolTokens, + NotBondedTokens: sdk.NewInt(100), + BondedTokens: poolTokens, } tokens := int64(71) msg := fmt.Sprintf("validator %#v", validator)