diff --git a/PENDING.md b/PENDING.md index 361ca70eb9cf..e39b15537c6d 100644 --- a/PENDING.md +++ b/PENDING.md @@ -30,8 +30,17 @@ exactly 9 atoms and transfer 1 atom, and `MsgSend` is disabled. ### SDK +* \#3750 Track outstanding rewards per-validator instead of globally, + and fix the main simulation issue, which was that slashes of + re-delegations to a validator were not correctly accounted for + in fee distribution when the redelegation in question had itself + been slashed (from a fault committed by a different validator) + in the same BeginBlock. Outstanding rewards are now available + on a per-validator basis in REST. * [\#3669] Ensure consistency in message naming, codec registration, and JSON tags. +* #3788 Change order of operations for greater accuracy when calculating delegation share token value +* #3788 DecCoins.Cap -> DecCoins.Intersect * [\#3666] Improve coins denom validation. * [\#3751] Disable (temporarily) support for ED25519 account key pairs. diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index ccb5895dad92..88845a731569 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -553,7 +553,7 @@ func TestBonding(t *testing.T) { // hence we utilize the exchange rate in the following test validator2 := getValidator(t, port, operAddrs[1]) - delTokensAfterRedelegation := delegatorDels[0].GetShares().Mul(validator2.DelegatorShareExRate()) + delTokensAfterRedelegation := validator2.ShareTokens(delegatorDels[0].GetShares()) require.Equal(t, rdTokens.ToDec(), delTokensAfterRedelegation) redelegation := getRedelegations(t, port, addr, operAddrs[0], operAddrs[1]) @@ -945,7 +945,7 @@ func TestDistributionFlow(t *testing.T) { operAddr := sdk.AccAddress(valAddr) var rewards sdk.DecCoins - res, body := Request(t, port, "GET", fmt.Sprintf("/distribution/outstanding_rewards"), nil) + res, body := Request(t, port, "GET", fmt.Sprintf("/distribution/validators/%s/outstanding_rewards", valAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) require.NoError(t, cdc.UnmarshalJSON([]byte(body), &rewards)) @@ -967,7 +967,7 @@ func TestDistributionFlow(t *testing.T) { require.Equal(t, uint32(0), resultTx.Code) // Query outstanding rewards changed - res, body = Request(t, port, "GET", fmt.Sprintf("/distribution/outstanding_rewards"), nil) + res, body = Request(t, port, "GET", fmt.Sprintf("/distribution/validators/%s/outstanding_rewards", valAddr), nil) require.Equal(t, http.StatusOK, res.StatusCode, body) require.NoError(t, cdc.UnmarshalJSON([]byte(body), &rewards)) diff --git a/client/lcd/swagger-ui/swagger.yaml b/client/lcd/swagger-ui/swagger.yaml index 803da4aedf3e..0c6b78da8c53 100644 --- a/client/lcd/swagger-ui/swagger.yaml +++ b/client/lcd/swagger-ui/swagger.yaml @@ -1558,6 +1558,28 @@ paths: description: Invalid validator address 500: description: Internal Server Error + /distribution/validators/{validatorAddr}/outstanding_rewards: + parameters: + - in: path + name: validatorAddr + description: Bech32 OperatorAddress of validator + required: true + type: string + get: + summary: Fee distribution outstanding rewards of a single validator + tags: + - ICS24 + produces: + - application/json + responses: + 200: + description: OK + schema: + type: array + items: + $ref: "#/definitions/Coin" + 500: + description: Internal Server Error /distribution/validators/{validatorAddr}/rewards: parameters: - in: path @@ -1566,8 +1588,8 @@ paths: required: true type: string get: - summary: Commission and self-delegation rewards of a single a validator - description: Query the commission and self-delegation rewards of a validator. + summary: Commission and self-delegation rewards of a single validator + description: Query the commission and self-delegation rewards of validator. tags: - ICS24 produces: @@ -1630,22 +1652,6 @@ paths: type: string 500: description: Internal Server Error - /distribution/outstanding_rewards: - get: - summary: Fee distribution outstanding rewards - tags: - - ICS24 - produces: - - application/json - responses: - 200: - description: OK - schema: - type: array - items: - $ref: "#/definitions/Coin" - 500: - description: Internal Server Error definitions: CheckTxResult: type: object diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index 978bcf92c538..3bd439396262 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -104,6 +104,13 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context, jailWhiteList []st // reinitialize all validators app.stakingKeeper.IterateValidators(ctx, func(_ int64, val sdk.Validator) (stop bool) { + + // donate any unwithdrawn outstanding reward fraction tokens to the community pool + scraps := app.distrKeeper.GetValidatorOutstandingRewards(ctx, val.GetOperator()) + feePool := app.distrKeeper.GetFeePool(ctx) + feePool.CommunityPool = feePool.CommunityPool.Add(scraps) + app.distrKeeper.SetFeePool(ctx, feePool) + app.distrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator()) return false }) diff --git a/types/dec_coin.go b/types/dec_coin.go index 91075235c7ed..19d5db2bd3cf 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -278,9 +278,12 @@ func (coins DecCoins) SafeSub(coinsB DecCoins) (DecCoins, bool) { return diff, diff.IsAnyNegative() } -// Trims any denom amount from coin which exceeds that of coinB, -// such that (coin.Cap(coinB)).IsLTE(coinB). -func (coins DecCoins) Cap(coinsB DecCoins) DecCoins { +// Intersect will return a new set of coins which contains the minimum DecCoin +// for common denoms found in both `coins` and `coinsB`. For denoms not common +// to both `coins` and `coinsB` the minimum is considered to be 0, thus they +// are not added to the final set.In other words, trim any denom amount from +// coin which exceeds that of coinB, such that (coin.Intersect(coinB)).IsLTE(coinB). +func (coins DecCoins) Intersect(coinsB DecCoins) DecCoins { res := make([]DecCoin, len(coins)) for i, coin := range coins { minCoin := DecCoin{ diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go index d5ddec2da5da..b7b6f4d0c28e 100644 --- a/types/dec_coin_test.go +++ b/types/dec_coin_test.go @@ -225,7 +225,7 @@ func TestDecCoinsString(t *testing.T) { } } -func TestDecCoinsCap(t *testing.T) { +func TestDecCoinsIntersect(t *testing.T) { testCases := []struct { input1 string input2 string @@ -252,7 +252,7 @@ func TestDecCoinsCap(t *testing.T) { exr, err := ParseDecCoins(tc.expectedResult) require.NoError(t, err, "unexpected parse error in %v", i) - require.True(t, in1.Cap(in2).IsEqual(exr), "in1.cap(in2) != exr in %v", i) - // require.Equal(t, tc.expectedResult, in1.Cap(in2).String(), "in1.cap(in2) != exr in %v", i) + require.True(t, in1.Intersect(in2).IsEqual(exr), "in1.cap(in2) != exr in %v", i) + // require.Equal(t, tc.expectedResult, in1.Intersect(in2).String(), "in1.cap(in2) != exr in %v", i) } } diff --git a/types/decimal.go b/types/decimal.go index 5f59a0f54857..bec27468c59b 100644 --- a/types/decimal.go +++ b/types/decimal.go @@ -291,6 +291,21 @@ func (d Dec) QuoTruncate(d2 Dec) Dec { return Dec{chopped} } +// quotient, round up +func (d Dec) QuoRoundUp(d2 Dec) Dec { + // multiply precision twice + mul := new(big.Int).Mul(d.Int, precisionReuse) + mul.Mul(mul, precisionReuse) + + quo := new(big.Int).Quo(mul, d2.Int) + chopped := chopPrecisionAndRoundUp(quo) + + if chopped.BitLen() > 255+DecimalPrecisionBits { + panic("Int overflow") + } + return Dec{chopped} +} + // quotient func (d Dec) QuoInt(i Int) Dec { mul := new(big.Int).Quo(d.Int, i.i) @@ -412,6 +427,29 @@ func chopPrecisionAndRound(d *big.Int) *big.Int { } } +func chopPrecisionAndRoundUp(d *big.Int) *big.Int { + + // remove the negative and add it back when returning + if d.Sign() == -1 { + // make d positive, compute chopped value, and then un-mutate d + d = d.Neg(d) + // truncate since d is negative... + d = chopPrecisionAndTruncate(d) + d = d.Neg(d) + return d + } + + // get the truncated quotient and remainder + quo, rem := d, big.NewInt(0) + quo, rem = quo.QuoRem(d, precisionReuse, rem) + + if rem.Sign() == 0 { // remainder is zero + return quo + } + + return quo.Add(quo, oneInt) +} + func chopPrecisionAndRoundNonMutative(d *big.Int) *big.Int { tmp := new(big.Int).Set(d) return chopPrecisionAndRound(tmp) diff --git a/types/decimal_test.go b/types/decimal_test.go index e0009236f50e..55aa31db9b58 100644 --- a/types/decimal_test.go +++ b/types/decimal_test.go @@ -156,44 +156,61 @@ func TestDecsEqual(t *testing.T) { func TestArithmetic(t *testing.T) { tests := []struct { - d1, d2 Dec - expMul, expQuo, expAdd, expSub Dec + d1, d2 Dec + expMul, expMulTruncate Dec + expQuo, expQuoRoundUp, expQuoTruncate Dec + expAdd, expSub Dec }{ - // d1 d2 MUL DIV ADD SUB - {NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0)}, - {NewDec(1), NewDec(0), NewDec(0), NewDec(0), NewDec(1), NewDec(1)}, - {NewDec(0), NewDec(1), NewDec(0), NewDec(0), NewDec(1), NewDec(-1)}, - {NewDec(0), NewDec(-1), NewDec(0), NewDec(0), NewDec(-1), NewDec(1)}, - {NewDec(-1), NewDec(0), NewDec(0), NewDec(0), NewDec(-1), NewDec(-1)}, - - {NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(2), NewDec(0)}, - {NewDec(-1), NewDec(-1), NewDec(1), NewDec(1), NewDec(-2), NewDec(0)}, - {NewDec(1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(0), NewDec(2)}, - {NewDec(-1), NewDec(1), NewDec(-1), NewDec(-1), NewDec(0), NewDec(-2)}, - - {NewDec(3), NewDec(7), NewDec(21), NewDecWithPrec(428571428571428571, 18), NewDec(10), NewDec(-4)}, - {NewDec(2), NewDec(4), NewDec(8), NewDecWithPrec(5, 1), NewDec(6), NewDec(-2)}, - {NewDec(100), NewDec(100), NewDec(10000), NewDec(1), NewDec(200), NewDec(0)}, - - {NewDecWithPrec(15, 1), NewDecWithPrec(15, 1), NewDecWithPrec(225, 2), - NewDec(1), NewDec(3), NewDec(0)}, - {NewDecWithPrec(3333, 4), NewDecWithPrec(333, 4), NewDecWithPrec(1109889, 8), - MustNewDecFromStr("10.009009009009009009"), NewDecWithPrec(3666, 4), NewDecWithPrec(3, 1)}, + // d1 d2 MUL MulTruncate QUO QUORoundUp QUOTrunctate ADD SUB + {NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0)}, + {NewDec(1), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(1), NewDec(1)}, + {NewDec(0), NewDec(1), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(1), NewDec(-1)}, + {NewDec(0), NewDec(-1), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(-1), NewDec(1)}, + {NewDec(-1), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(0), NewDec(-1), NewDec(-1)}, + + {NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(2), NewDec(0)}, + {NewDec(-1), NewDec(-1), NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(1), NewDec(-2), NewDec(0)}, + {NewDec(1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(0), NewDec(2)}, + {NewDec(-1), NewDec(1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(-1), NewDec(0), NewDec(-2)}, + + {NewDec(3), NewDec(7), NewDec(21), NewDec(21), + NewDecWithPrec(428571428571428571, 18), NewDecWithPrec(428571428571428572, 18), NewDecWithPrec(428571428571428571, 18), + NewDec(10), NewDec(-4)}, + {NewDec(2), NewDec(4), NewDec(8), NewDec(8), NewDecWithPrec(5, 1), NewDecWithPrec(5, 1), NewDecWithPrec(5, 1), + NewDec(6), NewDec(-2)}, + + {NewDec(100), NewDec(100), NewDec(10000), NewDec(10000), NewDec(1), NewDec(1), NewDec(1), NewDec(200), NewDec(0)}, + + {NewDecWithPrec(15, 1), NewDecWithPrec(15, 1), NewDecWithPrec(225, 2), NewDecWithPrec(225, 2), + NewDec(1), NewDec(1), NewDec(1), NewDec(3), NewDec(0)}, + {NewDecWithPrec(3333, 4), NewDecWithPrec(333, 4), NewDecWithPrec(1109889, 8), NewDecWithPrec(1109889, 8), + MustNewDecFromStr("10.009009009009009009"), MustNewDecFromStr("10.009009009009009010"), MustNewDecFromStr("10.009009009009009009"), + NewDecWithPrec(3666, 4), NewDecWithPrec(3, 1)}, } for tcIndex, tc := range tests { resAdd := tc.d1.Add(tc.d2) resSub := tc.d1.Sub(tc.d2) resMul := tc.d1.Mul(tc.d2) + resMulTruncate := tc.d1.MulTruncate(tc.d2) require.True(t, tc.expAdd.Equal(resAdd), "exp %v, res %v, tc %d", tc.expAdd, resAdd, tcIndex) require.True(t, tc.expSub.Equal(resSub), "exp %v, res %v, tc %d", tc.expSub, resSub, tcIndex) require.True(t, tc.expMul.Equal(resMul), "exp %v, res %v, tc %d", tc.expMul, resMul, tcIndex) + require.True(t, tc.expMulTruncate.Equal(resMulTruncate), "exp %v, res %v, tc %d", tc.expMulTruncate, resMulTruncate, tcIndex) if tc.d2.IsZero() { // panic for divide by zero require.Panics(t, func() { tc.d1.Quo(tc.d2) }) } else { resQuo := tc.d1.Quo(tc.d2) require.True(t, tc.expQuo.Equal(resQuo), "exp %v, res %v, tc %d", tc.expQuo.String(), resQuo.String(), tcIndex) + + resQuoRoundUp := tc.d1.QuoRoundUp(tc.d2) + require.True(t, tc.expQuoRoundUp.Equal(resQuoRoundUp), "exp %v, res %v, tc %d", + tc.expQuoRoundUp.String(), resQuoRoundUp.String(), tcIndex) + + resQuoTruncate := tc.d1.QuoTruncate(tc.d2) + require.True(t, tc.expQuoTruncate.Equal(resQuoTruncate), "exp %v, res %v, tc %d", + tc.expQuoTruncate.String(), resQuoTruncate.String(), tcIndex) } } } diff --git a/types/staking.go b/types/staking.go index 51961f39c531..428979a18658 100644 --- a/types/staking.go +++ b/types/staking.go @@ -73,7 +73,8 @@ type Validator interface { GetCommission() Dec // validator commission rate GetMinSelfDelegation() Int // validator minimum self delegation GetDelegatorShares() Dec // total outstanding delegator shares - GetDelegatorShareExRate() Dec // tokens per delegator share exchange rate + ShareTokens(Dec) Dec // token worth of provided delegator shares + ShareTokensTruncated(Dec) Dec // token worth of provided delegator shares, truncated } // validator which fulfills abci validator interface for use in Tendermint diff --git a/x/distribution/alias.go b/x/distribution/alias.go index 85b93fc1745b..34ad7dd57604 100644 --- a/x/distribution/alias.go +++ b/x/distribution/alias.go @@ -50,14 +50,15 @@ var ( NewMsgWithdrawDelegatorReward = types.NewMsgWithdrawDelegatorReward NewMsgWithdrawValidatorCommission = types.NewMsgWithdrawValidatorCommission - NewKeeper = keeper.NewKeeper - NewQuerier = keeper.NewQuerier - NewQueryValidatorCommissionParams = keeper.NewQueryValidatorCommissionParams - NewQueryValidatorSlashesParams = keeper.NewQueryValidatorSlashesParams - NewQueryDelegationRewardsParams = keeper.NewQueryDelegationRewardsParams - NewQueryDelegatorParams = keeper.NewQueryDelegatorParams - NewQueryDelegatorWithdrawAddrParams = keeper.NewQueryDelegatorWithdrawAddrParams - DefaultParamspace = keeper.DefaultParamspace + NewKeeper = keeper.NewKeeper + NewQuerier = keeper.NewQuerier + NewQueryValidatorOutstandingRewardsParams = keeper.NewQueryValidatorOutstandingRewardsParams + NewQueryValidatorCommissionParams = keeper.NewQueryValidatorCommissionParams + NewQueryValidatorSlashesParams = keeper.NewQueryValidatorSlashesParams + NewQueryDelegationRewardsParams = keeper.NewQueryDelegationRewardsParams + NewQueryDelegatorParams = keeper.NewQueryDelegatorParams + NewQueryDelegatorWithdrawAddrParams = keeper.NewQueryDelegatorWithdrawAddrParams + DefaultParamspace = keeper.DefaultParamspace RegisterCodec = types.RegisterCodec DefaultGenesisState = types.DefaultGenesisState diff --git a/x/distribution/client/cli/query.go b/x/distribution/client/cli/query.go index 4adcaa13098f..4ba5b03b1608 100644 --- a/x/distribution/client/cli/query.go +++ b/x/distribution/client/cli/query.go @@ -32,22 +32,22 @@ func GetCmdQueryParams(queryRoute string, cdc *codec.Codec) *cobra.Command { } } -// GetCmdQueryOutstandingRewards implements the query outstanding rewards command. -func GetCmdQueryOutstandingRewards(queryRoute string, cdc *codec.Codec) *cobra.Command { +// GetCmdQueryValidatorOutstandingRewards implements the query validator outstanding rewards command. +func GetCmdQueryValidatorOutstandingRewards(queryRoute string, cdc *codec.Codec) *cobra.Command { return &cobra.Command{ - Use: "outstanding-rewards", + Use: "validator-outstanding-rewards", Args: cobra.NoArgs, - Short: "Query distribution outstanding (un-withdrawn) rewards", + Short: "Query distribution outstanding (un-withdrawn) rewards for a validator and all their delegations", RunE: func(cmd *cobra.Command, args []string) error { cliCtx := context.NewCLIContext().WithCodec(cdc) - route := fmt.Sprintf("custom/%s/outstanding_rewards", queryRoute) + route := fmt.Sprintf("custom/%s/validator_outstanding_rewards", queryRoute) res, err := cliCtx.QueryWithData(route, []byte{}) if err != nil { return err } - var outstandingRewards types.OutstandingRewards + var outstandingRewards types.ValidatorOutstandingRewards cdc.MustUnmarshalJSON(res, &outstandingRewards) return cliCtx.PrintOutput(outstandingRewards) }, diff --git a/x/distribution/client/module_client.go b/x/distribution/client/module_client.go index b9dcb1c82bac..0acb62a72003 100644 --- a/x/distribution/client/module_client.go +++ b/x/distribution/client/module_client.go @@ -27,7 +27,7 @@ func (mc ModuleClient) GetQueryCmd() *cobra.Command { distQueryCmd.AddCommand(client.GetCommands( distCmds.GetCmdQueryParams(mc.storeKey, mc.cdc), - distCmds.GetCmdQueryOutstandingRewards(mc.storeKey, mc.cdc), + distCmds.GetCmdQueryValidatorOutstandingRewards(mc.storeKey, mc.cdc), distCmds.GetCmdQueryValidatorCommission(mc.storeKey, mc.cdc), distCmds.GetCmdQueryValidatorSlashes(mc.storeKey, mc.cdc), distCmds.GetCmdQueryDelegatorRewards(mc.storeKey, mc.cdc), diff --git a/x/distribution/client/rest/query.go b/x/distribution/client/rest/query.go index a8a4c98658fc..a8a6e13263e4 100644 --- a/x/distribution/client/rest/query.go +++ b/x/distribution/client/rest/query.go @@ -49,17 +49,18 @@ func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router, validatorRewardsHandlerFn(cliCtx, cdc, queryRoute), ).Methods("GET") + // Outstanding rewards of a single validator + r.HandleFunc( + "/distribution/validators/{validatorAddr}/outstanding_rewards", + outstandingRewardsHandlerFn(cliCtx, cdc, queryRoute), + ).Methods("GET") + // Get the current distribution parameter values r.HandleFunc( "/distribution/parameters", paramsHandlerFn(cliCtx, cdc, queryRoute), ).Methods("GET") - // Get the current distribution pool - r.HandleFunc( - "/distribution/outstanding_rewards", - outstandingRewardsHandlerFn(cliCtx, cdc, queryRoute), - ).Methods("GET") } // HTTP request handler to query the total rewards balance from all delegations @@ -211,7 +212,13 @@ func outstandingRewardsHandlerFn(cliCtx context.CLIContext, cdc *codec.Codec, queryRoute string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/outstanding_rewards", queryRoute), []byte{}) + validatorAddr, ok := checkValidatorAddressVar(w, r) + if !ok { + return + } + + bin := cdc.MustMarshalJSON(distribution.NewQueryValidatorOutstandingRewardsParams(validatorAddr)) + res, err := cliCtx.QueryWithData(fmt.Sprintf("custom/%s/validator_outstanding_rewards", queryRoute), bin) if err != nil { rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) return diff --git a/x/distribution/genesis.go b/x/distribution/genesis.go index eecaa612306a..524e2237cae0 100644 --- a/x/distribution/genesis.go +++ b/x/distribution/genesis.go @@ -16,7 +16,9 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) { keeper.SetDelegatorWithdrawAddr(ctx, dwi.DelegatorAddress, dwi.WithdrawAddress) } keeper.SetPreviousProposerConsAddr(ctx, data.PreviousProposer) - keeper.SetOutstandingRewards(ctx, data.OutstandingRewards) + for _, rew := range data.OutstandingRewards { + keeper.SetValidatorOutstandingRewards(ctx, rew.ValidatorAddress, rew.OutstandingRewards) + } for _, acc := range data.ValidatorAccumulatedCommissions { keeper.SetValidatorAccumulatedCommission(ctx, acc.ValidatorAddress, acc.Accumulated) } @@ -50,7 +52,16 @@ func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState { return false }) pp := keeper.GetPreviousProposerConsAddr(ctx) - outstanding := keeper.GetOutstandingRewards(ctx) + outstanding := make([]types.ValidatorOutstandingRewardsRecord, 0) + keeper.IterateValidatorOutstandingRewards(ctx, + func(addr sdk.ValAddress, rewards types.ValidatorOutstandingRewards) (stop bool) { + outstanding = append(outstanding, types.ValidatorOutstandingRewardsRecord{ + ValidatorAddress: addr, + OutstandingRewards: rewards, + }) + return false + }, + ) acc := make([]types.ValidatorAccumulatedCommissionRecord, 0) keeper.IterateValidatorAccumulatedCommissions(ctx, func(addr sdk.ValAddress, commission types.ValidatorAccumulatedCommission) (stop bool) { diff --git a/x/distribution/keeper/alias_functions.go b/x/distribution/keeper/alias_functions.go index 575eafe5399a..6081cf348a0d 100644 --- a/x/distribution/keeper/alias_functions.go +++ b/x/distribution/keeper/alias_functions.go @@ -5,8 +5,8 @@ import ( ) // get outstanding rewards -func (k Keeper) GetOutstandingRewardsCoins(ctx sdk.Context) sdk.DecCoins { - return k.GetOutstandingRewards(ctx) +func (k Keeper) GetValidatorOutstandingRewardsCoins(ctx sdk.Context, val sdk.ValAddress) sdk.DecCoins { + return k.GetValidatorOutstandingRewards(ctx, val) } // get the community coins diff --git a/x/distribution/keeper/allocation.go b/x/distribution/keeper/allocation.go index e151da2fc9fc..50dcd2e93d79 100644 --- a/x/distribution/keeper/allocation.go +++ b/x/distribution/keeper/allocation.go @@ -42,14 +42,17 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in proposerValidator := k.stakingKeeper.ValidatorByConsAddr(ctx, proposer) if proposerValidator != nil { k.AllocateTokensToValidator(ctx, proposerValidator, proposerReward) - remaining = feesCollected.Sub(proposerReward) + remaining = remaining.Sub(proposerReward) } else { // proposer can be unknown if say, the unbonding period is 1 block, so // e.g. a validator undelegates at block X, it's removed entirely by // block X+1's endblock, then X+2 we need to refer to the previous // proposer for X+1, but we've forgotten about them. logger.Error(fmt.Sprintf( - "WARNING: Attempt to allocate proposer rewards to unknown proposer %s. This should happen only if the proposer unbonded completely within a single block, which generally should not happen except in exceptional circumstances (or fuzz testing). We recommend you investigate immediately.", + "WARNING: Attempt to allocate proposer rewards to unknown proposer %s. "+ + "This should happen only if the proposer unbonded completely within a single block, "+ + "which generally should not happen except in exceptional circumstances (or fuzz testing). "+ + "We recommend you investigate immediately.", proposer.String())) } @@ -66,7 +69,7 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in // ref https://github.com/cosmos/cosmos-sdk/issues/2525#issuecomment-430838701 powerFraction := sdk.NewDec(vote.Validator.Power).QuoTruncate(sdk.NewDec(totalPower)) reward := feesCollected.MulDecTruncate(voteMultiplier).MulDecTruncate(powerFraction) - reward = reward.Cap(remaining) + reward = reward.Intersect(remaining) k.AllocateTokensToValidator(ctx, validator, reward) remaining = remaining.Sub(reward) } @@ -75,15 +78,11 @@ func (k Keeper) AllocateTokens(ctx sdk.Context, sumPrecommitPower, totalPower in feePool.CommunityPool = feePool.CommunityPool.Add(remaining) k.SetFeePool(ctx, feePool) - // update outstanding rewards - outstanding := k.GetOutstandingRewards(ctx) - outstanding = outstanding.Add(feesCollected.Sub(remaining)) - k.SetOutstandingRewards(ctx, outstanding) - } // allocate tokens to a particular validator, splitting according to commission func (k Keeper) AllocateTokensToValidator(ctx sdk.Context, val sdk.Validator, tokens sdk.DecCoins) { + // split tokens between validator and delegators according to commission commission := tokens.MulDec(val.GetCommission()) shared := tokens.Sub(commission) @@ -97,4 +96,9 @@ func (k Keeper) AllocateTokensToValidator(ctx sdk.Context, val sdk.Validator, to currentRewards := k.GetValidatorCurrentRewards(ctx, val.GetOperator()) currentRewards.Rewards = currentRewards.Rewards.Add(shared) k.SetValidatorCurrentRewards(ctx, val.GetOperator(), currentRewards) + + // update outstanding rewards + outstanding := k.GetValidatorOutstandingRewards(ctx, val.GetOperator()) + outstanding = outstanding.Add(tokens) + k.SetValidatorOutstandingRewards(ctx, val.GetOperator(), outstanding) } diff --git a/x/distribution/keeper/allocation_test.go b/x/distribution/keeper/allocation_test.go index 4ac5b3f656d8..8bbab7aef20a 100644 --- a/x/distribution/keeper/allocation_test.go +++ b/x/distribution/keeper/allocation_test.go @@ -14,9 +14,6 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -44,9 +41,6 @@ func TestAllocateTokensToManyValidators(t *testing.T) { ctx, _, k, sk, fck := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -69,7 +63,8 @@ func TestAllocateTokensToManyValidators(t *testing.T) { } // assert initial state: zero outstanding rewards, zero community pool, zero commission, zero current rewards - require.True(t, k.GetOutstandingRewards(ctx).IsZero()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr1).IsZero()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr2).IsZero()) require.True(t, k.GetFeePool(ctx).CommunityPool.IsZero()) require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1).IsZero()) require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr2).IsZero()) @@ -94,7 +89,8 @@ func TestAllocateTokensToManyValidators(t *testing.T) { k.AllocateTokens(ctx, 200, 200, valConsAddr2, votes) // 98 outstanding rewards (100 less 2 to community pool) - require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(98)}}, k.GetOutstandingRewards(ctx)) + require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDecWithPrec(465, 1)}}, k.GetValidatorOutstandingRewards(ctx, valOpAddr1)) + require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDecWithPrec(515, 1)}}, k.GetValidatorOutstandingRewards(ctx, valOpAddr2)) // 2 community pool coins require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(2)}}, k.GetFeePool(ctx).CommunityPool) // 50% commission for first proposer, (0.5 * 93%) * 100 / 2 = 23.25 @@ -112,9 +108,6 @@ func TestAllocateTokensTruncation(t *testing.T) { ctx, _, k, sk, fck := CreateTestInputAdvanced(t, false, 1000000, communityTax) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 10% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(1, 1), sdk.NewDecWithPrec(1, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -147,7 +140,9 @@ func TestAllocateTokensTruncation(t *testing.T) { } // assert initial state: zero outstanding rewards, zero community pool, zero commission, zero current rewards - require.True(t, k.GetOutstandingRewards(ctx).IsZero()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr1).IsZero()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr2).IsZero()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr3).IsZero()) require.True(t, k.GetFeePool(ctx).CommunityPool.IsZero()) require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1).IsZero()) require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr2).IsZero()) @@ -175,5 +170,7 @@ func TestAllocateTokensTruncation(t *testing.T) { } k.AllocateTokens(ctx, 31, 31, valConsAddr2, votes) - require.True(t, k.GetOutstandingRewards(ctx).IsValid()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr1).IsValid()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr2).IsValid()) + require.True(t, k.GetValidatorOutstandingRewards(ctx, valOpAddr3).IsValid()) } diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index d63d8cd2632a..00e7669bbd90 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -1,6 +1,8 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -20,7 +22,7 @@ func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sd // calculate delegation stake in tokens // we don't store directly, so multiply delegation shares * (tokens per share) // note: necessary to truncate so we don't allow withdrawing more rewards than owed - stake := delegation.GetShares().MulTruncate(validator.GetDelegatorShareExRate()) + stake := validator.ShareTokensTruncated(delegation.GetShares()) k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight()))) } @@ -53,32 +55,51 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Valid func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, del sdk.Delegation, endingPeriod uint64) (rewards sdk.DecCoins) { // fetch starting info for delegation startingInfo := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) + + if startingInfo.Height == uint64(ctx.BlockHeight()) { + // started this height, no rewards yet + return + } + startingPeriod := startingInfo.PreviousPeriod stake := startingInfo.Stake // iterate through slashes and withdraw with calculated staking for sub-intervals // these offsets are dependent on *when* slashes happen - namely, in BeginBlock, after rewards are allocated... - // ... so we don't reduce stake for slashes which happened in the *first* block, because the delegation wouldn't have existed - startingHeight := startingInfo.Height + 1 - // ... or slashes which happened in *this* block, since they would have happened after reward allocation - endingHeight := uint64(ctx.BlockHeight()) - 1 - if endingHeight >= startingHeight { + // slashes which happened in the first block would have been before this delegation existed, + // UNLESS they were slashes of a redelegation to this validator which was itself slashed + // (from a fault committed by the redelegation source validator) earlier in the same BeginBlock + startingHeight := startingInfo.Height + // slashes this block happened after reward allocation, but we have to account for them for the stake sanity check below + endingHeight := uint64(ctx.BlockHeight()) + if endingHeight > startingHeight { k.IterateValidatorSlashEventsBetween(ctx, del.GetValidatorAddr(), startingHeight, endingHeight, func(height uint64, event types.ValidatorSlashEvent) (stop bool) { endingPeriod := event.ValidatorPeriod - rewards = rewards.Add(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)) - // note: necessary to truncate so we don't allow withdrawing more rewards than owed - stake = stake.MulTruncate(sdk.OneDec().Sub(event.Fraction)) - startingPeriod = endingPeriod + if endingPeriod > startingPeriod { + rewards = rewards.Add(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)) + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + stake = stake.MulTruncate(sdk.OneDec().Sub(event.Fraction)) + startingPeriod = endingPeriod + } return false }, ) } + // a stake sanity check - recalculated final stake should be less than or equal to current stake + // here we cannot use Equals because stake is truncated when multiplied by slash fractions + // we could only use equals if we had arbitrary-precision rationals + currentStake := val.ShareTokens(del.GetShares()) + if stake.GT(currentStake) { + panic(fmt.Sprintf("calculated final stake for delegator %s greater than current stake: %s, %s", + del.GetDelegatorAddr(), stake, currentStake)) + } + // calculate rewards for final period rewards = rewards.Add(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)) - return + return rewards } func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, del sdk.Delegation) sdk.Error { @@ -90,7 +111,18 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de // end current period and calculate rewards endingPeriod := k.incrementValidatorPeriod(ctx, val) - rewards := k.calculateDelegationRewards(ctx, val, del, endingPeriod) + rewardsRaw := k.calculateDelegationRewards(ctx, val, del, endingPeriod) + outstanding := k.GetValidatorOutstandingRewards(ctx, del.GetValidatorAddr()) + + // defensive edge case may happen on the very final digits + // of the decCoins due to operation order of the distribution mechanism. + rewards := rewardsRaw.Intersect(outstanding) + if !rewards.IsEqual(rewardsRaw) { + logger := ctx.Logger().With("module", "x/distr") + logger.Info(fmt.Sprintf("missing rewards rounding error, delegator %v"+ + "withdrawing rewards from validator %v, should have received %v, got %v", + val.GetOperator(), del.GetDelegatorAddr(), rewardsRaw, rewards)) + } // decrement reference count of starting period startingInfo := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr()) @@ -99,9 +131,8 @@ func (k Keeper) withdrawDelegationRewards(ctx sdk.Context, val sdk.Validator, de // truncate coins, return remainder to community pool coins, remainder := rewards.TruncateDecimal() - outstanding := k.GetOutstandingRewards(ctx) - k.SetOutstandingRewards(ctx, outstanding.Sub(rewards)) + k.SetValidatorOutstandingRewards(ctx, del.GetValidatorAddr(), outstanding.Sub(rewards)) feePool := k.GetFeePool(ctx) feePool.CommunityPool = feePool.CommunityPool.Add(remainder) k.SetFeePool(ctx, feePool) diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index d62e4b8e1936..d6eb848e7d9a 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -13,9 +13,6 @@ func TestCalculateRewardsBasic(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -25,6 +22,9 @@ func TestCalculateRewardsBasic(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -66,9 +66,6 @@ func TestCalculateRewardsAfterSlash(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) valPower := int64(100) @@ -81,6 +78,9 @@ func TestCalculateRewardsAfterSlash(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -129,9 +129,6 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission power := int64(100) valTokens := sdk.TokensFromTendermintPower(power) @@ -143,6 +140,9 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -203,9 +203,6 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -215,6 +212,9 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del1 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -235,6 +235,9 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) { // end block staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) @@ -263,9 +266,6 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { ctx, ak, k, sk, _ := CreateTestInputDefault(t, false, balancePower) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission power := int64(100) valTokens := sdk.TokensFromTendermintPower(power) @@ -287,6 +287,9 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) @@ -294,7 +297,6 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { initial := sdk.TokensFromTendermintPower(10) tokens := sdk.DecCoins{sdk.NewDecCoin(sdk.DefaultBondDenom, initial)} - k.SetOutstandingRewards(ctx, tokens) k.AllocateTokensToValidator(ctx, val, tokens) // historical count should be 2 (initial + latest for delegation) @@ -328,9 +330,6 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission power := int64(100) valTokens := sdk.TokensFromTendermintPower(power) @@ -342,6 +341,9 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -395,9 +397,6 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) { ctx, _, k, sk, _ := CreateTestInputDefault(t, false, 1000) sh := staking.NewHandler(sk) - // initialize state - k.SetOutstandingRewards(ctx, sdk.DecCoins{}) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) power := int64(100) @@ -409,6 +408,9 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del1 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -433,6 +435,9 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) { // end block staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) @@ -471,9 +476,6 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { totalRewards := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDec(initial*2))} tokens := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDec(initial))} - // initialize state - k.SetOutstandingRewards(ctx, totalRewards) - // create validator with 50% commission commission := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, @@ -483,6 +485,9 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // end block to bond validator staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // fetch validator and delegation val := sk.Validator(ctx, valOpAddr1) del1 := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) @@ -507,6 +512,9 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // end block staking.EndBlocker(ctx, sk) + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) @@ -540,8 +548,10 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // commission should be zero require.True(t, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1).IsZero()) - totalRewards = k.GetOutstandingRewards(ctx).Add(tokens) - k.SetOutstandingRewards(ctx, totalRewards) + totalRewards = totalRewards.Add(tokens) + + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) @@ -567,8 +577,10 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { // commission should be half initial require.Equal(t, sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(initial / 2)}}, k.GetValidatorAccumulatedCommission(ctx, valOpAddr1)) - totalRewards = k.GetOutstandingRewards(ctx).Add(tokens) - k.SetOutstandingRewards(ctx, totalRewards) + totalRewards = k.GetValidatorOutstandingRewards(ctx, valOpAddr1).Add(tokens) + + // next block + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) // allocate some more rewards k.AllocateTokensToValidator(ctx, val, tokens) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index a0534bf59e21..1ce7a1fd63dc 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -22,9 +22,17 @@ func (h Hooks) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) { func (h Hooks) BeforeValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) { } func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr sdk.ValAddress) { + + // fetch outstanding + outstanding := h.k.GetValidatorOutstandingRewards(ctx, valAddr) + // force-withdraw commission commission := h.k.GetValidatorAccumulatedCommission(ctx, valAddr) if !commission.IsZero() { + // subtract from outstanding + outstanding = outstanding.Sub(commission) + + // split into integral & remainder coins, remainder := commission.TruncateDecimal() // remainder to community pool @@ -32,12 +40,9 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr feePool.CommunityPool = feePool.CommunityPool.Add(remainder) h.k.SetFeePool(ctx, feePool) - // update outstanding - outstanding := h.k.GetOutstandingRewards(ctx) - h.k.SetOutstandingRewards(ctx, outstanding.Sub(commission)) - // add to validator account if !coins.IsZero() { + accAddr := sdk.AccAddress(valAddr) withdrawAddr := h.k.GetDelegatorWithdrawAddr(ctx, accAddr) @@ -46,6 +51,15 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr } } } + + // add outstanding to community pool + feePool := h.k.GetFeePool(ctx) + feePool.CommunityPool = feePool.CommunityPool.Add(outstanding) + h.k.SetFeePool(ctx, feePool) + + // delete outstanding + h.k.DeleteValidatorOutstandingRewards(ctx, valAddr) + // remove commission record h.k.DeleteValidatorAccumulatedCommission(ctx, valAddr) diff --git a/x/distribution/keeper/keeper.go b/x/distribution/keeper/keeper.go index 40fe13e8ad6f..0652a5166468 100644 --- a/x/distribution/keeper/keeper.go +++ b/x/distribution/keeper/keeper.go @@ -84,8 +84,8 @@ func (k Keeper) WithdrawValidatorCommission(ctx sdk.Context, valAddr sdk.ValAddr k.SetValidatorAccumulatedCommission(ctx, valAddr, remainder) // update outstanding - outstanding := k.GetOutstandingRewards(ctx) - k.SetOutstandingRewards(ctx, outstanding.Sub(sdk.NewDecCoins(coins))) + outstanding := k.GetValidatorOutstandingRewards(ctx, valAddr) + k.SetValidatorOutstandingRewards(ctx, valAddr, outstanding.Sub(sdk.NewDecCoins(coins))) if !coins.IsZero() { accAddr := sdk.AccAddress(valAddr) diff --git a/x/distribution/keeper/keeper_test.go b/x/distribution/keeper/keeper_test.go index fc6f7cc0a1cc..59471a13287a 100644 --- a/x/distribution/keeper/keeper_test.go +++ b/x/distribution/keeper/keeper_test.go @@ -30,9 +30,6 @@ func TestWithdrawValidatorCommission(t *testing.T) { sdk.NewDecCoinFromDec("stake", sdk.NewDec(3).Quo(sdk.NewDec(2))), } - // set zero outstanding rewards - keeper.SetOutstandingRewards(ctx, valCommission) - // check initial balance balance := ak.GetAccount(ctx, sdk.AccAddress(valOpAddr3)).GetCoins() expTokens := sdk.TokensFromTendermintPower(1000) @@ -40,6 +37,9 @@ func TestWithdrawValidatorCommission(t *testing.T) { sdk.NewCoin("stake", sdk.TokensFromTendermintPower(1000)), }, balance) + // set outstanding rewards + keeper.SetValidatorOutstandingRewards(ctx, valOpAddr3, valCommission) + // set commission keeper.SetValidatorAccumulatedCommission(ctx, valOpAddr3, valCommission) diff --git a/x/distribution/keeper/key.go b/x/distribution/keeper/key.go index 1b8437b4cd95..068e05b857ec 100644 --- a/x/distribution/keeper/key.go +++ b/x/distribution/keeper/key.go @@ -13,9 +13,9 @@ const ( // keys var ( - FeePoolKey = []byte{0x00} // key for global distribution state - ProposerKey = []byte{0x01} // key for the proposer operator address - OutstandingRewardsKey = []byte{0x02} // key for outstanding rewards + FeePoolKey = []byte{0x00} // key for global distribution state + ProposerKey = []byte{0x01} // key for the proposer operator address + ValidatorOutstandingRewardsPrefix = []byte{0x02} // key for outstanding rewards DelegatorWithdrawAddrPrefix = []byte{0x03} // key for delegator withdraw address DelegatorStartingInfoPrefix = []byte{0x04} // key for delegator starting info @@ -30,6 +30,15 @@ var ( ParamStoreKeyWithdrawAddrEnabled = []byte("withdrawaddrenabled") ) +// gets an address from a validator's outstanding rewards key +func GetValidatorOutstandingRewardsAddress(key []byte) (valAddr sdk.ValAddress) { + addr := key[1:] + if len(addr) != sdk.AddrLen { + panic("unexpected key length") + } + return sdk.ValAddress(addr) +} + // gets an address from a delegator's withdraw info key func GetDelegatorWithdrawInfoAddress(key []byte) (delAddr sdk.AccAddress) { addr := key[1:] @@ -102,6 +111,11 @@ func GetValidatorSlashEventAddressHeight(key []byte) (valAddr sdk.ValAddress, he return } +// gets the outstanding rewards key for a validator +func GetValidatorOutstandingRewardsKey(valAddr sdk.ValAddress) []byte { + return append(ValidatorOutstandingRewardsPrefix, valAddr.Bytes()...) +} + // gets the key for a delegator's withdraw addr func GetDelegatorWithdrawAddrKey(delAddr sdk.AccAddress) []byte { return append(DelegatorWithdrawAddrPrefix, delAddr.Bytes()...) diff --git a/x/distribution/keeper/querier.go b/x/distribution/keeper/querier.go index 6099116fecd6..348c8b9a0e3f 100644 --- a/x/distribution/keeper/querier.go +++ b/x/distribution/keeper/querier.go @@ -12,14 +12,14 @@ import ( // nolint const ( - QueryParams = "params" - QueryOutstandingRewards = "outstanding_rewards" - QueryValidatorCommission = "validator_commission" - QueryValidatorSlashes = "validator_slashes" - QueryDelegationRewards = "delegation_rewards" - QueryDelegatorTotalRewards = "delegator_total_rewards" - QueryDelegatorValidators = "delegator_validators" - QueryWithdrawAddr = "withdraw_addr" + QueryParams = "params" + QueryValidatorOutstandingRewards = "validator_outstanding_rewards" + QueryValidatorCommission = "validator_commission" + QueryValidatorSlashes = "validator_slashes" + QueryDelegationRewards = "delegation_rewards" + QueryDelegatorTotalRewards = "delegator_total_rewards" + QueryDelegatorValidators = "delegator_validators" + QueryWithdrawAddr = "withdraw_addr" ParamCommunityTax = "community_tax" ParamBaseProposerReward = "base_proposer_reward" @@ -33,8 +33,8 @@ func NewQuerier(k Keeper) sdk.Querier { case QueryParams: return queryParams(ctx, path[1:], req, k) - case QueryOutstandingRewards: - return queryOutstandingRewards(ctx, path[1:], req, k) + case QueryValidatorOutstandingRewards: + return queryValidatorOutstandingRewards(ctx, path[1:], req, k) case QueryValidatorCommission: return queryValidatorCommission(ctx, path[1:], req, k) @@ -91,8 +91,25 @@ func queryParams(ctx sdk.Context, path []string, req abci.RequestQuery, k Keeper } } -func queryOutstandingRewards(ctx sdk.Context, path []string, req abci.RequestQuery, k Keeper) ([]byte, sdk.Error) { - bz, err := codec.MarshalJSONIndent(k.cdc, k.GetOutstandingRewards(ctx)) +// params for query 'custom/distr/validator_outstanding_rewards' +type QueryValidatorOutstandingRewardsParams struct { + ValidatorAddress sdk.ValAddress `json:"validator_address"` +} + +// creates a new instance of QueryValidatorOutstandingRewardsParams +func NewQueryValidatorOutstandingRewardsParams(validatorAddr sdk.ValAddress) QueryValidatorOutstandingRewardsParams { + return QueryValidatorOutstandingRewardsParams{ + ValidatorAddress: validatorAddr, + } +} + +func queryValidatorOutstandingRewards(ctx sdk.Context, path []string, req abci.RequestQuery, k Keeper) ([]byte, sdk.Error) { + var params QueryValidatorOutstandingRewardsParams + err := k.cdc.UnmarshalJSON(req.Data, ¶ms) + if err != nil { + return nil, sdk.ErrUnknownRequest(sdk.AppendMsgToErr("incorrectly formatted request data", err.Error())) + } + bz, err := codec.MarshalJSONIndent(k.cdc, k.GetValidatorOutstandingRewards(ctx, params.ValidatorAddress)) if err != nil { return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err.Error())) } diff --git a/x/distribution/keeper/querier_test.go b/x/distribution/keeper/querier_test.go index 874b85034a5b..9593f1bff472 100644 --- a/x/distribution/keeper/querier_test.go +++ b/x/distribution/keeper/querier_test.go @@ -57,13 +57,13 @@ func getQueriedParams(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier s return } -func getQueriedOutstandingRewards(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier) (outstandingRewards sdk.DecCoins) { +func getQueriedValidatorOutstandingRewards(t *testing.T, ctx sdk.Context, cdc *codec.Codec, querier sdk.Querier, validatorAddr sdk.ValAddress) (outstandingRewards sdk.DecCoins) { query := abci.RequestQuery{ - Path: strings.Join([]string{custom, types.QuerierRoute, QueryOutstandingRewards}, "/"), - Data: []byte{}, + Path: strings.Join([]string{custom, types.QuerierRoute, QueryValidatorOutstandingRewards}, "/"), + Data: cdc.MustMarshalJSON(NewQueryValidatorOutstandingRewardsParams(validatorAddr)), } - bz, err := querier(ctx, []string{QueryOutstandingRewards}, query) + bz, err := querier(ctx, []string{QueryValidatorOutstandingRewards}, query) require.Nil(t, err) require.Nil(t, cdc.UnmarshalJSON(bz, &outstandingRewards)) @@ -131,8 +131,8 @@ func TestQueries(t *testing.T) { // test outstanding rewards query outstandingRewards := sdk.DecCoins{{"mytoken", sdk.NewDec(3)}, {"myothertoken", sdk.NewDecWithPrec(3, 7)}} - keeper.SetOutstandingRewards(ctx, outstandingRewards) - retOutstandingRewards := getQueriedOutstandingRewards(t, ctx, cdc, querier) + keeper.SetValidatorOutstandingRewards(ctx, valOpAddr1, outstandingRewards) + retOutstandingRewards := getQueriedValidatorOutstandingRewards(t, ctx, cdc, querier, valOpAddr1) require.Equal(t, outstandingRewards, retOutstandingRewards) // test validator commission query @@ -155,7 +155,6 @@ func TestQueries(t *testing.T) { // test delegation rewards query sh := staking.NewHandler(sk) - keeper.SetOutstandingRewards(ctx, sdk.DecCoins{}) comm := staking.NewCommissionMsg(sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(5, 1), sdk.NewDec(0)) msg := staking.NewMsgCreateValidator(valOpAddr1, valConsPk1, sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)), staking.Description{}, comm, sdk.OneInt()) @@ -165,6 +164,7 @@ func TestQueries(t *testing.T) { rewards := getQueriedDelegationRewards(t, ctx, cdc, querier, sdk.AccAddress(valOpAddr1), valOpAddr1) require.True(t, rewards.IsZero()) initial := int64(10) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) tokens := sdk.DecCoins{{sdk.DefaultBondDenom, sdk.NewDec(initial)}} keeper.AllocateTokensToValidator(ctx, val, tokens) rewards = getQueriedDelegationRewards(t, ctx, cdc, querier, sdk.AccAddress(valOpAddr1), valOpAddr1) diff --git a/x/distribution/keeper/store.go b/x/distribution/keeper/store.go index ef1f7c783c34..1326c89d4548 100644 --- a/x/distribution/keeper/store.go +++ b/x/distribution/keeper/store.go @@ -263,19 +263,40 @@ func (k Keeper) IterateValidatorAccumulatedCommissions(ctx sdk.Context, handler } } -// get outstanding rewards -func (k Keeper) GetOutstandingRewards(ctx sdk.Context) (rewards types.OutstandingRewards) { +// get validator outstanding rewards +func (k Keeper) GetValidatorOutstandingRewards(ctx sdk.Context, val sdk.ValAddress) (rewards types.ValidatorOutstandingRewards) { store := ctx.KVStore(k.storeKey) - b := store.Get(OutstandingRewardsKey) + b := store.Get(GetValidatorOutstandingRewardsKey(val)) k.cdc.MustUnmarshalBinaryLengthPrefixed(b, &rewards) return } -// set outstanding rewards -func (k Keeper) SetOutstandingRewards(ctx sdk.Context, rewards types.OutstandingRewards) { +// set validator outstanding rewards +func (k Keeper) SetValidatorOutstandingRewards(ctx sdk.Context, val sdk.ValAddress, rewards types.ValidatorOutstandingRewards) { store := ctx.KVStore(k.storeKey) b := k.cdc.MustMarshalBinaryLengthPrefixed(rewards) - store.Set(OutstandingRewardsKey, b) + store.Set(GetValidatorOutstandingRewardsKey(val), b) +} + +// delete validator outstanding rewards +func (k Keeper) DeleteValidatorOutstandingRewards(ctx sdk.Context, val sdk.ValAddress) { + store := ctx.KVStore(k.storeKey) + store.Delete(GetValidatorOutstandingRewardsKey(val)) +} + +// iterate validator outstanding rewards +func (k Keeper) IterateValidatorOutstandingRewards(ctx sdk.Context, handler func(val sdk.ValAddress, rewards types.ValidatorOutstandingRewards) (stop bool)) { + store := ctx.KVStore(k.storeKey) + iter := sdk.KVStorePrefixIterator(store, ValidatorOutstandingRewardsPrefix) + defer iter.Close() + for ; iter.Valid(); iter.Next() { + var rewards types.ValidatorOutstandingRewards + k.cdc.MustUnmarshalBinaryLengthPrefixed(iter.Value(), &rewards) + addr := GetValidatorOutstandingRewardsAddress(iter.Key()) + if handler(addr, rewards) { + break + } + } } // get slash event for height diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index 21ffa24a304b..18c6cc6e87e0 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -16,6 +16,9 @@ func (k Keeper) initializeValidator(ctx sdk.Context, val sdk.Validator) { // set accumulated commission k.SetValidatorAccumulatedCommission(ctx, val.GetOperator(), types.InitialValidatorAccumulatedCommission()) + + // set outstanding rewards + k.SetValidatorOutstandingRewards(ctx, val.GetOperator(), sdk.DecCoins{}) } // increment validator period, returning the period just ended @@ -30,11 +33,11 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin // can't calculate ratio for zero-token validators // ergo we instead add to the community pool feePool := k.GetFeePool(ctx) - outstanding := k.GetOutstandingRewards(ctx) + outstanding := k.GetValidatorOutstandingRewards(ctx, val.GetOperator()) feePool.CommunityPool = feePool.CommunityPool.Add(rewards.Rewards) outstanding = outstanding.Sub(rewards.Rewards) k.SetFeePool(ctx, feePool) - k.SetOutstandingRewards(ctx, outstanding) + k.SetValidatorOutstandingRewards(ctx, val.GetOperator(), outstanding) current = sdk.DecCoins{} } else { diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index f72241065149..fa5f585c98d0 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -30,11 +30,23 @@ func AllInvariants(d distr.Keeper, stk types.StakingKeeper) sdk.Invariant { // NonNegativeOutstandingInvariant checks that outstanding unwithdrawn fees are never negative func NonNegativeOutstandingInvariant(k distr.Keeper) sdk.Invariant { return func(ctx sdk.Context) error { - outstanding := k.GetOutstandingRewards(ctx) + + var outstanding sdk.DecCoins + + k.IterateValidatorOutstandingRewards(ctx, func(_ sdk.ValAddress, rewards types.ValidatorOutstandingRewards) (stop bool) { + outstanding = rewards + if outstanding.IsAnyNegative() { + return true + } + return false + }) + if outstanding.IsAnyNegative() { return fmt.Errorf("negative outstanding coins: %v", outstanding) } + return nil + } } @@ -45,20 +57,29 @@ func CanWithdrawInvariant(k distr.Keeper, sk types.StakingKeeper) sdk.Invariant // cache, we don't want to write changes ctx, _ = ctx.CacheContext() - // iterate over all bonded validators, withdraw commission + var remaining sdk.DecCoins + + // iterate over all validators sk.IterateValidators(ctx, func(_ int64, val sdk.Validator) (stop bool) { _ = k.WithdrawValidatorCommission(ctx, val.GetOperator()) + // TODO fetch delegations just for the validator, requires sdk.ValidatorSet change + // iterate over all current delegations, withdraw rewards + dels := sk.GetAllSDKDelegations(ctx) + for _, delegation := range dels { + if delegation.GetValidatorAddr().String() == val.GetOperator().String() { + err := k.WithdrawDelegationRewards(ctx, delegation.GetDelegatorAddr(), delegation.GetValidatorAddr()) + if err != nil { + panic(err) + } + } + } + remaining = k.GetValidatorOutstandingRewards(ctx, val.GetOperator()) + if len(remaining) > 0 && remaining[0].Amount.LT(sdk.ZeroDec()) { + return true + } return false }) - // iterate over all current delegations, withdraw rewards - dels := sk.GetAllSDKDelegations(ctx) - for _, delegation := range dels { - _ = k.WithdrawDelegationRewards(ctx, delegation.GetDelegatorAddr(), delegation.GetValidatorAddr()) - } - - remaining := k.GetOutstandingRewards(ctx) - if len(remaining) > 0 && remaining[0].Amount.LT(sdk.ZeroDec()) { return fmt.Errorf("negative remaining coins: %v", remaining) } diff --git a/x/distribution/types/fee_pool.go b/x/distribution/types/fee_pool.go index bb34ab47cd4a..caf9905e1d8c 100644 --- a/x/distribution/types/fee_pool.go +++ b/x/distribution/types/fee_pool.go @@ -27,8 +27,3 @@ func (f FeePool) ValidateGenesis() error { return nil } - -// outstanding (un-withdrawn) rewards for everyone -// excludes the community pool -// inexpensive to track, allows simple sanity checks -type OutstandingRewards = sdk.DecCoins diff --git a/x/distribution/types/genesis.go b/x/distribution/types/genesis.go index 80d2ff22a1c9..e87d8a33fdeb 100644 --- a/x/distribution/types/genesis.go +++ b/x/distribution/types/genesis.go @@ -13,6 +13,12 @@ type DelegatorWithdrawInfo struct { WithdrawAddress sdk.AccAddress `json:"withdraw_address"` } +// used for import/export via genesis json +type ValidatorOutstandingRewardsRecord struct { + ValidatorAddress sdk.ValAddress `json:"validator_address"` + OutstandingRewards sdk.DecCoins `json:"outstanding_rewards"` +} + // used for import / export via genesis json type ValidatorAccumulatedCommissionRecord struct { ValidatorAddress sdk.ValAddress `json:"validator_address"` @@ -55,7 +61,7 @@ type GenesisState struct { WithdrawAddrEnabled bool `json:"withdraw_addr_enabled"` DelegatorWithdrawInfos []DelegatorWithdrawInfo `json:"delegator_withdraw_infos"` PreviousProposer sdk.ConsAddress `json:"previous_proposer"` - OutstandingRewards sdk.DecCoins `json:"outstanding_rewards"` + OutstandingRewards []ValidatorOutstandingRewardsRecord `json:"outstanding_rewards"` ValidatorAccumulatedCommissions []ValidatorAccumulatedCommissionRecord `json:"validator_accumulated_commissions"` ValidatorHistoricalRewards []ValidatorHistoricalRewardsRecord `json:"validator_historical_rewards"` ValidatorCurrentRewards []ValidatorCurrentRewardsRecord `json:"validator_current_rewards"` @@ -64,7 +70,7 @@ type GenesisState struct { } func NewGenesisState(feePool FeePool, communityTax, baseProposerReward, bonusProposerReward sdk.Dec, - withdrawAddrEnabled bool, dwis []DelegatorWithdrawInfo, pp sdk.ConsAddress, r OutstandingRewards, + withdrawAddrEnabled bool, dwis []DelegatorWithdrawInfo, pp sdk.ConsAddress, r []ValidatorOutstandingRewardsRecord, acc []ValidatorAccumulatedCommissionRecord, historical []ValidatorHistoricalRewardsRecord, cur []ValidatorCurrentRewardsRecord, dels []DelegatorStartingInfoRecord, slashes []ValidatorSlashEventRecord) GenesisState { @@ -96,7 +102,7 @@ func DefaultGenesisState() GenesisState { WithdrawAddrEnabled: true, DelegatorWithdrawInfos: []DelegatorWithdrawInfo{}, PreviousProposer: nil, - OutstandingRewards: sdk.DecCoins{}, + OutstandingRewards: []ValidatorOutstandingRewardsRecord{}, ValidatorAccumulatedCommissions: []ValidatorAccumulatedCommissionRecord{}, ValidatorHistoricalRewards: []ValidatorHistoricalRewardsRecord{}, ValidatorCurrentRewards: []ValidatorCurrentRewardsRecord{}, diff --git a/x/distribution/types/validator.go b/x/distribution/types/validator.go index 0bde7fc073ee..a7fc0dd2a9fb 100644 --- a/x/distribution/types/validator.go +++ b/x/distribution/types/validator.go @@ -91,3 +91,7 @@ func (vs ValidatorSlashEvents) String() string { } return strings.TrimSpace(out) } + +// outstanding (un-withdrawn) rewards for a validator +// inexpensive to track, allows simple sanity checks +type ValidatorOutstandingRewards = sdk.DecCoins diff --git a/x/slashing/handler.go b/x/slashing/handler.go index d9ae9f2d8127..fcc141a86c03 100644 --- a/x/slashing/handler.go +++ b/x/slashing/handler.go @@ -31,7 +31,7 @@ func handleMsgUnjail(ctx sdk.Context, msg MsgUnjail, k Keeper) sdk.Result { return ErrMissingSelfDelegation(k.codespace).Result() } - if validator.GetDelegatorShareExRate().Mul(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) { + if validator.ShareTokens(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) { return ErrSelfDelegationTooLowToUnjail(k.codespace).Result() } diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 97d306f6e161..73e8956372c8 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -301,8 +301,6 @@ func TestIncrementsMsgDelegate(t *testing.T) { require.Equal(t, bondAmount, bond.Shares.RoundInt()) pool := keeper.GetPool(ctx) - exRate := validator.DelegatorShareExRate() - require.True(t, exRate.Equal(sdk.OneDec()), "expected exRate 1 got %v", exRate) require.Equal(t, bondAmount, pool.BondedTokens) // just send the same msgbond multiple times @@ -320,9 +318,6 @@ func TestIncrementsMsgDelegate(t *testing.T) { bond, found := keeper.GetDelegation(ctx, delegatorAddr, validatorAddr) require.True(t, found) - exRate := validator.DelegatorShareExRate() - require.True(t, exRate.Equal(sdk.OneDec()), "expected exRate 1 got %v, i = %v", exRate, i) - expBond := bondAmount.MulRaw(i + 1) expDelegatorShares := bondAmount.MulRaw(i + 2) // (1 self delegation) expDelegatorAcc := initBond.Sub(expBond) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index a15b78d358ff..86adf577cb8a 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -450,7 +450,7 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.In // In some situations, the exchange rate becomes invalid, e.g. if // Validator loses all tokens due to slashing. In this case, // make all future delegations invalid. - if validator.DelegatorShareExRate().IsZero() { + if validator.InvalidExRate() { return sdk.ZeroDec(), types.ErrDelegatorShareExRateInvalid(k.Codespace()) } @@ -517,7 +517,9 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA // if the delegation is the operator of the validator and undelegating will decrease the validator's self delegation below their minimum // trigger a jail validator - if isValidatorOperator && !validator.Jailed && validator.DelegatorShareExRate().Mul(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) { + if isValidatorOperator && !validator.Jailed && + validator.ShareTokens(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) { + k.jailValidator(ctx, validator) validator = k.mustGetValidator(ctx, validator.OperatorAddress) } diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index dbc3b84eadfb..dbd19a9410e3 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -57,17 +57,6 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh // call the before-modification hook k.BeforeValidatorModified(ctx, operatorAddress) - // we need to calculate the *effective* slash fraction for distribution - if validator.Tokens.GT(sdk.ZeroInt()) { - effectiveFraction := slashAmountDec.Quo(validator.Tokens.ToDec()) - // possible if power has changed - if effectiveFraction.GT(sdk.OneDec()) { - effectiveFraction = sdk.OneDec() - } - // call the before-slashed hook - k.BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction) - } - // Track remaining slash amount for the validator // This will decrease when we slash unbondings and // redelegations, as that stake has since unbonded @@ -115,6 +104,17 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh tokensToBurn := sdk.MinInt(remainingSlashAmount, validator.Tokens) tokensToBurn = sdk.MaxInt(tokensToBurn, sdk.ZeroInt()) // defensive. + // we need to calculate the *effective* slash fraction for distribution + if validator.Tokens.GT(sdk.ZeroInt()) { + effectiveFraction := tokensToBurn.ToDec().QuoRoundUp(validator.Tokens.ToDec()) + // possible if power has changed + if effectiveFraction.GT(sdk.OneDec()) { + effectiveFraction = sdk.OneDec() + } + // call the before-slashed hook + k.BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction) + } + // Deduct from validator's bonded tokens and update the validator. // The deducted tokens are returned to pool.NotBondedTokens. // TODO: Move the token accounting outside of `RemoveValidatorTokens` so it is less confusing diff --git a/x/staking/simulation/invariants.go b/x/staking/simulation/invariants.go index d0d11e8124f2..db6249719f3f 100644 --- a/x/staking/simulation/invariants.go +++ b/x/staking/simulation/invariants.go @@ -67,6 +67,8 @@ func SupplyInvariants(k staking.Keeper, case sdk.Unbonding, sdk.Unbonded: loose = loose.Add(validator.GetTokens().ToDec()) } + // add yet-to-be-withdrawn + loose = loose.Add(d.GetValidatorOutstandingRewardsCoins(ctx, validator.GetOperator()).AmountOf(k.BondDenom(ctx))) return false }) @@ -76,9 +78,6 @@ func SupplyInvariants(k staking.Keeper, // add community pool loose = loose.Add(d.GetFeePoolCommunityCoins(ctx).AmountOf(k.BondDenom(ctx))) - // add yet-to-be-withdrawn - loose = loose.Add(d.GetOutstandingRewardsCoins(ctx).AmountOf(k.BondDenom(ctx))) - // Not-bonded tokens should equal coin supply plus unbonding delegations // plus tokens on unbonded validators if !pool.NotBondedTokens.ToDec().Equal(loose) { diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index affa2207076e..54c9bad62287 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -5,7 +5,7 @@ import sdk "github.com/cosmos/cosmos-sdk/types" // expected coin keeper type DistributionKeeper interface { GetFeePoolCommunityCoins(ctx sdk.Context) sdk.DecCoins - GetOutstandingRewardsCoins(ctx sdk.Context) sdk.DecCoins + GetValidatorOutstandingRewardsCoins(ctx sdk.Context, val sdk.ValAddress) sdk.DecCoins } // expected fee collection keeper diff --git a/x/staking/types/validator.go b/x/staking/types/validator.go index ebf51426d938..730d43073e29 100644 --- a/x/staking/types/validator.go +++ b/x/staking/types/validator.go @@ -348,10 +348,13 @@ func (v Validator) SetInitialCommission(commission Commission) (Validator, sdk.E // CONTRACT: Tokens are assumed to have come from not-bonded pool. func (v Validator) AddTokensFromDel(pool Pool, amount sdk.Int) (Validator, Pool, sdk.Dec) { - // bondedShare/delegatedShare - exRate := v.DelegatorShareExRate() - if exRate.IsZero() { - panic("zero exRate should not happen") + // calculate the shares to issue + var issuedShares sdk.Dec + if v.DelegatorShares.IsZero() { + // the first delegation to a validator sets the exchange rate to one + issuedShares = amount.ToDec() + } else { + issuedShares = v.DelegatorShares.MulInt(amount).QuoInt(v.Tokens) } if v.Status == sdk.Bonded { @@ -359,7 +362,6 @@ func (v Validator) AddTokensFromDel(pool Pool, amount sdk.Int) (Validator, Pool, } v.Tokens = v.Tokens.Add(amount) - issuedShares := amount.ToDec().Quo(exRate) v.DelegatorShares = v.DelegatorShares.Add(issuedShares) return v, pool, issuedShares @@ -382,7 +384,7 @@ func (v Validator) RemoveDelShares(pool Pool, delShares sdk.Dec) (Validator, Poo // leave excess tokens in the validator // however fully use all the delegator shares - issuedTokens = v.DelegatorShareExRate().Mul(delShares).TruncateInt() + issuedTokens = v.ShareTokens(delShares).TruncateInt() v.Tokens = v.Tokens.Sub(issuedTokens) if v.Tokens.IsNegative() { panic("attempting to remove more tokens than available in validator") @@ -397,14 +399,21 @@ func (v Validator) RemoveDelShares(pool Pool, delShares sdk.Dec) (Validator, Poo return v, pool, issuedTokens } -// DelegatorShareExRate gets the exchange rate of tokens over delegator shares. -// UNITS: tokens/delegator-shares -func (v Validator) DelegatorShareExRate() sdk.Dec { - if v.DelegatorShares.IsZero() { - // the first delegation to a validator sets the exchange rate to one - return sdk.OneDec() - } - return v.Tokens.ToDec().Quo(v.DelegatorShares) +// In some situations, the exchange rate becomes invalid, e.g. if +// Validator loses all tokens due to slashing. In this case, +// make all future delegations invalid. +func (v Validator) InvalidExRate() bool { + return v.Tokens.IsZero() && v.DelegatorShares.IsPositive() +} + +// calculate the token worth of provided shares +func (v Validator) ShareTokens(shares sdk.Dec) sdk.Dec { + return (shares.MulInt(v.Tokens)).Quo(v.DelegatorShares) +} + +// calculate the token worth of provided shares, truncated +func (v Validator) ShareTokensTruncated(shares sdk.Dec) sdk.Dec { + return (shares.MulInt(v.Tokens)).QuoTruncate(v.DelegatorShares) } // get the bonded tokens which the validator holds @@ -433,16 +442,15 @@ func (v Validator) PotentialTendermintPower() int64 { var _ sdk.Validator = Validator{} // nolint - for sdk.Validator -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.OperatorAddress } -func (v Validator) GetConsPubKey() crypto.PubKey { return v.ConsPubKey } -func (v Validator) GetConsAddr() sdk.ConsAddress { return sdk.ConsAddress(v.ConsPubKey.Address()) } -func (v Validator) GetTokens() sdk.Int { return v.Tokens } -func (v Validator) GetBondedTokens() sdk.Int { return v.BondedTokens() } -func (v Validator) GetTendermintPower() int64 { return v.TendermintPower() } -func (v Validator) GetCommission() sdk.Dec { return v.Commission.Rate } -func (v Validator) GetMinSelfDelegation() sdk.Int { return v.MinSelfDelegation } -func (v Validator) GetDelegatorShares() sdk.Dec { return v.DelegatorShares } -func (v Validator) GetDelegatorShareExRate() sdk.Dec { return v.DelegatorShareExRate() } +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.OperatorAddress } +func (v Validator) GetConsPubKey() crypto.PubKey { return v.ConsPubKey } +func (v Validator) GetConsAddr() sdk.ConsAddress { return sdk.ConsAddress(v.ConsPubKey.Address()) } +func (v Validator) GetTokens() sdk.Int { return v.Tokens } +func (v Validator) GetBondedTokens() sdk.Int { return v.BondedTokens() } +func (v Validator) GetTendermintPower() int64 { return v.TendermintPower() } +func (v Validator) GetCommission() sdk.Dec { return v.Commission.Rate } +func (v Validator) GetMinSelfDelegation() sdk.Int { return v.MinSelfDelegation } +func (v Validator) GetDelegatorShares() sdk.Dec { return v.DelegatorShares } diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index 5cb1992068de..b8b1fb5c7f57 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -1,7 +1,6 @@ package types import ( - "fmt" "testing" "github.com/cosmos/cosmos-sdk/codec" @@ -70,6 +69,21 @@ func TestABCIValidatorUpdateZero(t *testing.T) { require.Equal(t, int64(0), abciVal.Power) } +func TestShareTokens(t *testing.T) { + validator := Validator{ + OperatorAddress: addr1, + ConsPubKey: pk1, + Status: sdk.Bonded, + Tokens: sdk.NewInt(100), + DelegatorShares: sdk.NewDec(100), + } + assert.True(sdk.DecEq(t, sdk.NewDec(50), validator.ShareTokens(sdk.NewDec(50)))) + + validator.Tokens = sdk.NewInt(50) + assert.True(sdk.DecEq(t, sdk.NewDec(25), validator.ShareTokens(sdk.NewDec(50)))) + assert.True(sdk.DecEq(t, sdk.NewDec(5), validator.ShareTokens(sdk.NewDec(10)))) +} + func TestRemoveTokens(t *testing.T) { validator := Validator{ @@ -112,10 +126,9 @@ func TestAddTokensValidatorBonded(t *testing.T) { validator, pool = validator.UpdateStatus(pool, sdk.Bonded) validator, pool, delShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) - require.Equal(t, sdk.OneDec(), validator.DelegatorShareExRate()) - assert.True(sdk.DecEq(t, sdk.NewDec(10), delShares)) assert.True(sdk.IntEq(t, sdk.NewInt(10), validator.BondedTokens())) + assert.True(sdk.DecEq(t, sdk.NewDec(10), validator.DelegatorShares)) } func TestAddTokensValidatorUnbonding(t *testing.T) { @@ -125,11 +138,10 @@ func TestAddTokensValidatorUnbonding(t *testing.T) { validator, pool = validator.UpdateStatus(pool, sdk.Unbonding) validator, pool, delShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) - require.Equal(t, sdk.OneDec(), validator.DelegatorShareExRate()) - assert.True(sdk.DecEq(t, sdk.NewDec(10), delShares)) assert.Equal(t, sdk.Unbonding, validator.Status) assert.True(sdk.IntEq(t, sdk.NewInt(10), validator.Tokens)) + assert.True(sdk.DecEq(t, sdk.NewDec(10), validator.DelegatorShares)) } func TestAddTokensValidatorUnbonded(t *testing.T) { @@ -139,11 +151,10 @@ func TestAddTokensValidatorUnbonded(t *testing.T) { validator, pool = validator.UpdateStatus(pool, sdk.Unbonded) validator, pool, delShares := validator.AddTokensFromDel(pool, sdk.NewInt(10)) - require.Equal(t, sdk.OneDec(), validator.DelegatorShareExRate()) - assert.True(sdk.DecEq(t, sdk.NewDec(10), delShares)) assert.Equal(t, sdk.Unbonded, validator.Status) assert.True(sdk.IntEq(t, sdk.NewInt(10), validator.Tokens)) + assert.True(sdk.DecEq(t, sdk.NewDec(10), validator.DelegatorShares)) } // TODO refactor to make simpler like the AddToken tests above @@ -158,7 +169,6 @@ func TestRemoveDelShares(t *testing.T) { poolA := InitialPool() poolA.NotBondedTokens = sdk.NewInt(10) poolA.BondedTokens = valA.BondedTokens() - require.Equal(t, valA.DelegatorShareExRate(), sdk.OneDec()) // Remove delegator shares valB, poolB, coinsB := valA.RemoveDelShares(poolA, sdk.NewDec(10)) @@ -197,6 +207,26 @@ func TestRemoveDelShares(t *testing.T) { pool.NotBondedTokens.Add(pool.BondedTokens))) } +func TestAddTokensFromDel(t *testing.T) { + val := NewValidator(addr1, pk1, Description{}) + pool := InitialPool() + pool.NotBondedTokens = sdk.NewInt(10) + + val, pool, shares := val.AddTokensFromDel(pool, sdk.NewInt(6)) + require.True(sdk.DecEq(t, sdk.NewDec(6), shares)) + require.True(sdk.DecEq(t, sdk.NewDec(6), val.DelegatorShares)) + require.True(sdk.IntEq(t, sdk.NewInt(6), val.Tokens)) + require.True(sdk.IntEq(t, sdk.NewInt(0), pool.BondedTokens)) + require.True(sdk.IntEq(t, sdk.NewInt(10), pool.NotBondedTokens)) + + val, pool, shares = val.AddTokensFromDel(pool, sdk.NewInt(3)) + require.True(sdk.DecEq(t, sdk.NewDec(3), shares)) + require.True(sdk.DecEq(t, sdk.NewDec(9), val.DelegatorShares)) + require.True(sdk.IntEq(t, sdk.NewInt(9), val.Tokens)) + require.True(sdk.IntEq(t, sdk.NewInt(0), pool.BondedTokens)) + require.True(sdk.IntEq(t, sdk.NewInt(10), pool.NotBondedTokens)) +} + func TestUpdateStatus(t *testing.T) { pool := InitialPool() pool.NotBondedTokens = sdk.NewInt(100) @@ -236,13 +266,10 @@ func TestPossibleOverflow(t *testing.T) { BondedTokens: poolTokens, } tokens := int64(71) - msg := fmt.Sprintf("validator %#v", validator) newValidator, _, _ := validator.AddTokensFromDel(pool, sdk.NewInt(tokens)) - msg = fmt.Sprintf("Added %d tokens to %s", tokens, msg) - require.False(t, newValidator.DelegatorShareExRate().LT(sdk.ZeroDec()), - "Applying operation \"%s\" resulted in negative DelegatorShareExRate(): %v", - msg, newValidator.DelegatorShareExRate()) + require.False(t, newValidator.DelegatorShares.IsNegative()) + require.False(t, newValidator.Tokens.IsNegative()) } func TestValidatorMarshalUnmarshalJSON(t *testing.T) {