Fix updateValidatorDistInfoFromPool (#3046)

Fixes regression introduced by #2984.
Continuiation of #3033 , which didn't fix the simulation issues.
(candidate) Complete solution for #3019, 9002 halt bug.

From #2984, it isn't sufficient to take the fee pool rewards of a validator. Since we don't track delegator accums (as we do with validator accums), and because onValidatorModified >updateValidatorDistInfoFromPool is also being called upon delegation updates (or at least I believe this is the reason), it is necessary to also withdraw self delegation.

TODO: I don't think self-delegation should be required to be modified here... consider using a delegation hook to do the self-delegation withdraw part instead, e.g. splitting the updateValidatorDistInfoFromPool function into two. It might not result in cleaner code, however. Think hard.

Makefile

@@ -53,6 +53,7 @@ else
 	go build $(BUILD_FLAGS) -o build/gaiad ./cmd/gaia/cmd/gaiad
 	go build $(BUILD_FLAGS) -o build/gaiacli ./cmd/gaia/cmd/gaiacli
 	go build $(BUILD_FLAGS) -o build/gaiareplay ./cmd/gaia/cmd/gaiareplay
+	go build $(BUILD_FLAGS) -o build/gaiakeyutil ./cmd/gaia/cmd/gaiakeyutil
 endif
 
 build-linux:
@@ -85,6 +86,7 @@ install: check-ledger update_gaia_lite_docs
 	go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiad
 	go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiacli
 	go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiareplay
+	go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiakeyutil
 
 install_examples:
 	go install $(BUILD_FLAGS) ./docs/examples/basecoin/cmd/basecoind

cmd/gaia/cmd/gaiakeyutil/main.go

@@ -0,0 +1,51 @@
+package main
+
+import (
+	"encoding/hex"
+	"fmt"
+	"os"
+
+	"github.com/tendermint/tendermint/libs/bech32"
+)
+
+var bech32Prefixes = []string{"cosmos", "cosmospub", "cosmosvaloper", "cosmosvaloperpub", "cosmosvalcons", "cosmosvalconspub"}
+
+func main() {
+	if len(os.Args) < 2 {
+		fmt.Println("Must specify an input string")
+	}
+	arg := os.Args[1]
+	runFromBech32(arg)
+	runFromHex(arg)
+}
+
+// Print info from bech32.
+func runFromBech32(bech32str string) {
+	hrp, bz, err := bech32.DecodeAndConvert(bech32str)
+	if err != nil {
+		fmt.Println("Not a valid bech32 string")
+		return
+	}
+	fmt.Println("Bech32 parse:")
+	fmt.Printf("Human readible part: %v\nBytes (hex): %X\n",
+		hrp,
+		bz,
+	)
+}
+
+func runFromHex(hexaddr string) {
+	bz, err := hex.DecodeString(hexaddr)
+	if err != nil {
+		fmt.Println("Not a valid hex string")
+		return
+	}
+	fmt.Println("Hex parse:")
+	fmt.Println("Bech32 formats:")
+	for _, prefix := range bech32Prefixes {
+		bech32Addr, err := bech32.ConvertAndEncode(prefix, bz)
+		if err != nil {
+			panic(err)
+		}
+		fmt.Println("  - " + bech32Addr)
+	}
+}

types/decimal.go

@@ -400,6 +400,11 @@ func (d Dec) TruncateInt() Int {
 	return NewIntFromBigInt(chopPrecisionAndTruncateNonMutative(d.Int))
 }
 
+// TruncateDec truncates the decimals from the number and returns a Dec
+func (d Dec) TruncateDec() Dec {
+	return NewDecFromBigInt(chopPrecisionAndTruncateNonMutative(d.Int))
+}
+
 //___________________________________________________________________________________
 
 // reuse nil values

x/distribution/keeper/hooks.go

@@ -32,7 +32,7 @@ func (k Keeper) onValidatorModified(ctx sdk.Context, valAddr sdk.ValAddress) {
 	// (dist info), but without actually withdrawing the rewards. This does not
 	// need to happen during the genesis block.
 	if ctx.BlockHeight() > 0 {
-		if err := k.updateValidatorDistInfoFromPool(ctx, valAddr); err != nil {
+		if err := k.takeValidatorFeePoolRewards(ctx, valAddr); err != nil {
 			panic(err)
 		}
 	}

x/distribution/keeper/validator.go

@@ -91,27 +91,36 @@ func (k Keeper) GetValidatorAccum(ctx sdk.Context, operatorAddr sdk.ValAddress)
 	return accum, nil
 }
 
-// updateValidatorDistInfoFromPool updates the validator's distribution info
+// takeValidatorFeePoolRewards updates the validator's distribution info
 // from the global fee pool without withdrawing any rewards. This will be called
 // from a onValidatorModified hook.
-func (k Keeper) updateValidatorDistInfoFromPool(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error {
+func (k Keeper) takeValidatorFeePoolRewards(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error {
 	if !k.HasValidatorDistInfo(ctx, operatorAddr) {
 		return types.ErrNoValidatorDistInfo(k.codespace)
 	}
+	accAddr := sdk.AccAddress(operatorAddr.Bytes())
 
+	// withdraw reward for self-delegation
+	if k.HasDelegationDistInfo(ctx, accAddr, operatorAddr) {
+		fp, vi, di, withdraw :=
+			k.withdrawDelegationReward(ctx, accAddr, operatorAddr)
+		k.SetFeePool(ctx, fp)
+		k.SetValidatorDistInfo(ctx, vi)
+		k.SetDelegationDistInfo(ctx, di)
+		k.WithdrawToDelegator(ctx, fp, accAddr, withdraw)
+	}
+
+	// withdrawal validator commission rewards
 	valInfo := k.GetValidatorDistInfo(ctx, operatorAddr)
 	wc := k.GetWithdrawContext(ctx, operatorAddr)
 	valInfo, feePool := valInfo.TakeFeePoolRewards(wc)
-
 	k.SetFeePool(ctx, feePool)
 	k.SetValidatorDistInfo(ctx, valInfo)
-
 	return nil
 }
 
 // withdrawal all the validator rewards including the commission
 func (k Keeper) WithdrawValidatorRewardsAll(ctx sdk.Context, operatorAddr sdk.ValAddress) sdk.Error {
-
 	if !k.HasValidatorDistInfo(ctx, operatorAddr) {
 		return types.ErrNoValidatorDistInfo(k.codespace)
 	}

x/distribution/keeper/validator_test.go

@@ -13,7 +13,7 @@ func TestWithdrawValidatorRewardsAllNoDelegator(t *testing.T) {
 	stakeHandler := stake.NewHandler(sk)
 	denom := sk.GetParams(ctx).BondDenom
 
-	//first make a validator
+	// first make a validator
 	msgCreateValidator := stake.NewTestMsgCreateValidator(valOpAddr1, valConsPk1, 10)
 	got := stakeHandler(ctx, msgCreateValidator)
 	require.True(t, got.IsOK(), "expected msg to be ok, got %v", got)
@@ -107,17 +107,20 @@ func TestWithdrawValidatorRewardsAllMultipleValidator(t *testing.T) {
 	stakeHandler := stake.NewHandler(sk)
 	denom := sk.GetParams(ctx).BondDenom
 
-	//make some  validators with different commissions
+	// Make some  validators with different commissions.
+	// Bond 10 of 100 with 0.1 commission.
 	msgCreateValidator := stake.NewTestMsgCreateValidatorWithCommission(
 		valOpAddr1, valConsPk1, 10, sdk.NewDecWithPrec(1, 1))
 	got := stakeHandler(ctx, msgCreateValidator)
 	require.True(t, got.IsOK(), "expected msg to be ok, got %v", got)
 
+	// Bond 50 of 100 with 0.2 commission.
 	msgCreateValidator = stake.NewTestMsgCreateValidatorWithCommission(
 		valOpAddr2, valConsPk2, 50, sdk.NewDecWithPrec(2, 1))
 	got = stakeHandler(ctx, msgCreateValidator)
 	require.True(t, got.IsOK(), "expected msg to be ok, got %v", got)
 
+	// Bond 40 of 100 with 0.3 commission.
 	msgCreateValidator = stake.NewTestMsgCreateValidatorWithCommission(
 		valOpAddr3, valConsPk3, 40, sdk.NewDecWithPrec(3, 1))
 	got = stakeHandler(ctx, msgCreateValidator)
@@ -125,22 +128,27 @@ func TestWithdrawValidatorRewardsAllMultipleValidator(t *testing.T) {
 
 	_ = sk.ApplyAndReturnValidatorSetUpdates(ctx)
 
-	// allocate 100 denom of fees
+	// Allocate 1000 denom of fees.
 	feeInputs := sdk.NewInt(1000)
 	fck.SetCollectedFees(sdk.Coins{sdk.NewCoin(denom, feeInputs)})
 	require.Equal(t, feeInputs, fck.GetCollectedFees(ctx).AmountOf(denom))
+	// Collect proposer reward for 100% of votes.
 	keeper.AllocateTokens(ctx, sdk.OneDec(), valConsAddr1)
 
-	// withdraw validator reward
+	// Withdraw validator reward.
 	ctx = ctx.WithBlockHeight(1)
 	keeper.WithdrawValidatorRewardsAll(ctx, valOpAddr1)
 	amt := accMapper.GetAccount(ctx, valAccAddr1).GetCoins().AmountOf(denom)
 
 	feesInNonProposer := sdk.NewDecFromInt(feeInputs).Mul(sdk.NewDecWithPrec(95, 2))
 	feesInProposer := sdk.NewDecFromInt(feeInputs).Mul(sdk.NewDecWithPrec(5, 2))
-	expRes := sdk.NewDec(90). // orig tokens (100 - 10)
-					Add(feesInNonProposer.Quo(sdk.NewDec(10))). // validator 1 has 1/10 total power
-					Add(feesInProposer).
+	// NOTE: the non-proposer rewards (95) and proposer rewards (50) add up to
+	// 145.  During computation, this is further split into 130.5 and 14.5,
+	// which is the non-commission and commission respectively, but the
+	// commission is for self so the result is just 145.
+	expRes := sdk.NewDec(90). // orig tokens (100) - bonded (10)
+					Add(feesInNonProposer.Quo(sdk.NewDec(10))). // validator 1 has 1/10 total power (non-proposer rewards = 95)
+					Add(feesInProposer).                        // (proposer rewards = 50)
 					TruncateInt()
 	require.True(sdk.IntEq(t, expRes, amt))
 }

x/distribution/simulation/invariants.go

@@ -101,9 +101,10 @@ func DelAccumInvariants(k distr.Keeper, sk distr.StakeKeeper) simulation.Invaria
 						delegation := sk.Delegation(ctx, ddi.DelegatorAddr, ddi.ValOperatorAddr)
 						accum := ddi.GetDelAccum(height, delegation.GetShares())
 						if accum.IsPositive() {
-							logDelAccums += fmt.Sprintf("\n\t\tdel: %v, accum: %v",
+							logDelAccums += fmt.Sprintf("\n\t\tdel: %v, accum: %v, power: %v",
 								ddi.DelegatorAddr.String(),
-								accum.String())
+								accum.String(),
+								delegation.GetShares())
 						}
 					}
 					return false