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

validator slash event stored by period and height #4654

Merged
merged 2 commits into from
Jul 1, 2019
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
1 change: 1 addition & 0 deletions .pending/bugfixes/sdk/4654-validator-slash
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#4654 validator slash event stored by period and height
3 changes: 2 additions & 1 deletion x/distribution/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) {
keeper.SetDelegatorStartingInfo(ctx, del.ValidatorAddress, del.DelegatorAddress, del.StartingInfo)
}
for _, evt := range data.ValidatorSlashEvents {
keeper.SetValidatorSlashEvent(ctx, evt.ValidatorAddress, evt.Height, evt.Event)
keeper.SetValidatorSlashEvent(ctx, evt.ValidatorAddress, evt.Height, evt.Period, evt.Event)
}
}

Expand Down Expand Up @@ -110,6 +110,7 @@ func ExportGenesis(ctx sdk.Context, keeper Keeper) types.GenesisState {
slashes = append(slashes, types.ValidatorSlashEventRecord{
ValidatorAddress: val,
Height: height,
Period: event.ValidatorPeriod,
Copy link
Contributor

Choose a reason for hiding this comment

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

/cc @sabau

Event: event,
})
return false
Expand Down
28 changes: 20 additions & 8 deletions x/distribution/keeper/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,8 @@ func GetValidatorSlashEventAddressHeight(key []byte) (valAddr sdk.ValAddress, he
panic("unexpected key length")
}
valAddr = sdk.ValAddress(addr)
b := key[1+sdk.AddrLen:]
if len(b) != 8 {
panic("unexpected key length")
}
startB := 1 + sdk.AddrLen
b := key[startB : startB+8] // the next 8 bytes represent the height
height = binary.BigEndian.Uint64(b)
return
}
Expand Down Expand Up @@ -154,9 +152,23 @@ func GetValidatorSlashEventPrefix(v sdk.ValAddress) []byte {
return append(ValidatorSlashEventPrefix, v.Bytes()...)
}

// gets the prefix key for a validator's slash fraction (ValidatorSlashEventPrefix + height)
func GetValidatorSlashEventKeyPrefix(v sdk.ValAddress, height uint64) []byte {
heightBz := make([]byte, 8)
binary.BigEndian.PutUint64(heightBz, height)
return append(
ValidatorSlashEventPrefix,
append(
v.Bytes(),
heightBz...,
)...,
)
}

// gets the key for a validator's slash fraction
func GetValidatorSlashEventKey(v sdk.ValAddress, height uint64) []byte {
b := make([]byte, 8)
binary.BigEndian.PutUint64(b, height)
return append(append(ValidatorSlashEventPrefix, v.Bytes()...), b...)
func GetValidatorSlashEventKey(v sdk.ValAddress, height, period uint64) []byte {
periodBz := make([]byte, 8)
binary.BigEndian.PutUint64(periodBz, period)
prefix := GetValidatorSlashEventKeyPrefix(v, height)
return append(prefix, periodBz...)
}
4 changes: 2 additions & 2 deletions x/distribution/keeper/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ func TestQueries(t *testing.T) {
// test validator slashes query with height range
slashOne := types.NewValidatorSlashEvent(3, sdk.NewDecWithPrec(5, 1))
slashTwo := types.NewValidatorSlashEvent(7, sdk.NewDecWithPrec(6, 1))
keeper.SetValidatorSlashEvent(ctx, valOpAddr1, 3, slashOne)
keeper.SetValidatorSlashEvent(ctx, valOpAddr1, 7, slashTwo)
keeper.SetValidatorSlashEvent(ctx, valOpAddr1, 3, 0, slashOne)
keeper.SetValidatorSlashEvent(ctx, valOpAddr1, 7, 0, slashTwo)
slashes := getQueriedValidatorSlashes(t, ctx, cdc, querier, valOpAddr1, 0, 2)
require.Equal(t, 0, len(slashes))
slashes = getQueriedValidatorSlashes(t, ctx, cdc, querier, valOpAddr1, 0, 5)
Expand Down
12 changes: 6 additions & 6 deletions x/distribution/keeper/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,9 @@ func (k Keeper) IterateValidatorOutstandingRewards(ctx sdk.Context, handler func
}

// get slash event for height
func (k Keeper) GetValidatorSlashEvent(ctx sdk.Context, val sdk.ValAddress, height uint64) (event types.ValidatorSlashEvent, found bool) {
func (k Keeper) GetValidatorSlashEvent(ctx sdk.Context, val sdk.ValAddress, height, period uint64) (event types.ValidatorSlashEvent, found bool) {
store := ctx.KVStore(k.storeKey)
b := store.Get(GetValidatorSlashEventKey(val, height))
b := store.Get(GetValidatorSlashEventKey(val, height, period))
if b == nil {
return types.ValidatorSlashEvent{}, false
}
Expand All @@ -318,19 +318,19 @@ func (k Keeper) GetValidatorSlashEvent(ctx sdk.Context, val sdk.ValAddress, heig
}

// set slash event for height
func (k Keeper) SetValidatorSlashEvent(ctx sdk.Context, val sdk.ValAddress, height uint64, event types.ValidatorSlashEvent) {
func (k Keeper) SetValidatorSlashEvent(ctx sdk.Context, val sdk.ValAddress, height, period uint64, event types.ValidatorSlashEvent) {
store := ctx.KVStore(k.storeKey)
b := k.cdc.MustMarshalBinaryLengthPrefixed(event)
store.Set(GetValidatorSlashEventKey(val, height), b)
store.Set(GetValidatorSlashEventKey(val, height, period), b)
}

// iterate over slash events between heights, inclusive
func (k Keeper) IterateValidatorSlashEventsBetween(ctx sdk.Context, val sdk.ValAddress, startingHeight uint64, endingHeight uint64,
handler func(height uint64, event types.ValidatorSlashEvent) (stop bool)) {
store := ctx.KVStore(k.storeKey)
iter := store.Iterator(
GetValidatorSlashEventKey(val, startingHeight),
GetValidatorSlashEventKey(val, endingHeight+1),
GetValidatorSlashEventKeyPrefix(val, startingHeight),
GetValidatorSlashEventKeyPrefix(val, endingHeight+1),
)
defer iter.Close()
for ; iter.Valid(); iter.Next() {
Expand Down
41 changes: 15 additions & 26 deletions x/distribution/keeper/validator.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/cosmos-sdk/x/distribution/types"
Expand Down Expand Up @@ -86,33 +88,20 @@ func (k Keeper) decrementReferenceCount(ctx sdk.Context, valAddr sdk.ValAddress,
}

func (k Keeper) updateValidatorSlashFraction(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) {
if fraction.GT(sdk.OneDec()) {
panic("fraction greater than one")
if fraction.GT(sdk.OneDec()) || fraction.LT(sdk.ZeroDec()) {
panic(fmt.Sprintf("fraction must be >=0 and <=1, current fraction: %v", fraction))
}
height := uint64(ctx.BlockHeight())
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why all this logic was removed? Was calculating updatedFraction unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think this design decision was overlooked at inception seeing as the revised code eliminates the need for this complexity. By introducing the period into the validator-slash-record-key the fraction no longer needs to be "flattened" with the other fractions in that height, hence we remove all this flattening logic in this function

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is better and simpler. Mea culpa. I think originally we were concerned about iteration costs.

currentFraction := sdk.ZeroDec()
endedPeriod := k.GetValidatorCurrentRewards(ctx, valAddr).Period - 1
current, found := k.GetValidatorSlashEvent(ctx, valAddr, height)
if found {
// there has already been a slash event this height,
// and we don't need to store more than one,
// so just update the current slash fraction
currentFraction = current.Fraction
} else {
val := k.stakingKeeper.Validator(ctx, valAddr)
// increment current period
endedPeriod = k.incrementValidatorPeriod(ctx, val)
// increment reference count on period we need to track
k.incrementReferenceCount(ctx, valAddr, endedPeriod)
}
currentMultiplicand := sdk.OneDec().Sub(currentFraction)
newMultiplicand := sdk.OneDec().Sub(fraction)

// using MulTruncate here conservatively increases the slashing amount
updatedFraction := sdk.OneDec().Sub(currentMultiplicand.MulTruncate(newMultiplicand))
val := k.stakingKeeper.Validator(ctx, valAddr)

if updatedFraction.LT(sdk.ZeroDec()) {
panic("negative slash fraction")
}
k.SetValidatorSlashEvent(ctx, valAddr, height, types.NewValidatorSlashEvent(endedPeriod, updatedFraction))
// increment current period
newPeriod := k.incrementValidatorPeriod(ctx, val)

// increment reference count on period we need to track
k.incrementReferenceCount(ctx, valAddr, newPeriod)

slashEvent := types.NewValidatorSlashEvent(newPeriod, fraction)
height := uint64(ctx.BlockHeight())

k.SetValidatorSlashEvent(ctx, valAddr, height, newPeriod, slashEvent)
}
1 change: 1 addition & 0 deletions x/distribution/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type DelegatorStartingInfoRecord struct {
type ValidatorSlashEventRecord struct {
ValidatorAddress sdk.ValAddress `json:"validator_address"`
Height uint64 `json:"height"`
Period uint64 `json:"period"`
Event ValidatorSlashEvent `json:"validator_slash_event"`
}

Expand Down