Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix updateValidatorDistInfoFromPool #3046

Merged
merged 3 commits into from
Dec 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
51 changes: 51 additions & 0 deletions cmd/gaia/cmd/gaiakeyutil/main.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
5 changes: 5 additions & 0 deletions types/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
19 changes: 14 additions & 5 deletions x/distribution/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I think this reintroduces the attack we were trying to prevent - #2914

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)
}
Expand Down
22 changes: 15 additions & 7 deletions x/distribution/keeper/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -107,40 +107,48 @@ 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)
require.True(t, got.IsOK(), "expected msg to be ok, got %v", got)

_ = 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))
}
Expand Down
5 changes: 3 additions & 2 deletions x/distribution/simulation/invariants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down