R4R: Outstanding per-validator rewards; correctly handle same-BeginBlock redelegation-double-slash

PENDING.md

@@ -26,8 +26,17 @@
 
 ### 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.
 

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))
 

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

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
 	})

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{

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)
 	}
 }

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)

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)
 		}
 	}
 }

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

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

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)
 		},

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),