Skip to content

Commit

Permalink
feat: add coins burned to slash event (#9458)
Browse files Browse the repository at this point in the history
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Adds the amount of coins slashed from the validator to the `Slash` event. Additionally, before this PR, the jail/slash events were emitted **BEFORE** the slash/jail functions were even ran, which could result in a false positive event if an error occurred in these functions. ~~These events were moved to the end of the functions that implement the logic for them instead.~~ This PR moves the events to be emitted after the logic is executed. 

Closes: #9138 

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

- +Add `amount_slashed` to slash event
- +Slash now returns the amount of tokens burned
- +Events moved to be emitted after the logic is executed
- +Add test to test the slash return amount
~~- +Add EventType `Jail` to separate it from the `Slash` event~~
~~- +Move slash/jail events into the functions that execute the logic for it~~
~~- -Remove `Reason` attribute from slash event (didn't appear to be consistent with what was happening in code)~~

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [x] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - @ryanchristo 
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [x] reviewed state machine logic - not applicable
- [x] reviewed API design and naming - @ryanchristo 
- [x] reviewed documentation is accurate - @ryanchristo 
- [x] reviewed tests and test coverage - @ryanchristo 
- [ ] manually tested (if applicable)
  • Loading branch information
technicallyty committed Jul 5, 2021
1 parent 7f90374 commit 2188dd0
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#9427](https://github.com/cosmos/cosmos-sdk/pull/9427) Move simapp `FundAccount` and `FundModuleAccount` to `x/bank/testutil`
* (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to
the Tx Factory as methods.
* (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event.
* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.
* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`.
* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`.
Expand Down
2 changes: 1 addition & 1 deletion x/distribution/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type StakingKeeper interface {
ValidatorByConsAddr(sdk.Context, sdk.ConsAddress) stakingtypes.ValidatorI // get a particular validator by consensus address

// slash the validator and delegators of the validator, specifying offence height, offence power, and slash fraction
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec)
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec) sdk.Int
Jail(sdk.Context, sdk.ConsAddress) // jail a validator
Unjail(sdk.Context, sdk.ConsAddress) // unjail a validator

Expand Down
3 changes: 2 additions & 1 deletion x/slashing/keeper/infractions.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,17 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre
// That's fine since this is just used to filter unbonding delegations & redelegations.
distributionHeight := height - sdk.ValidatorUpdateDelay - 1

coinsBurned := k.sk.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx))
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeSlash,
sdk.NewAttribute(types.AttributeKeyAddress, consAddr.String()),
sdk.NewAttribute(types.AttributeKeyPower, fmt.Sprintf("%d", power)),
sdk.NewAttribute(types.AttributeKeyReason, types.AttributeValueMissingSignature),
sdk.NewAttribute(types.AttributeKeyJailed, consAddr.String()),
sdk.NewAttribute(types.AttributeKeyBurnedCoins, coinsBurned.String()),
),
)
k.sk.Slash(ctx, consAddr, distributionHeight, power, k.SlashFractionDowntime(ctx))
k.sk.Jail(ctx, consAddr)

signInfo.JailedUntil = ctx.BlockHeader().Time.Add(k.DowntimeJailDuration(ctx))
Expand Down
7 changes: 3 additions & 4 deletions x/slashing/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,29 +65,28 @@ func (k Keeper) GetPubkey(ctx sdk.Context, a cryptotypes.Address) (cryptotypes.P
// Slash attempts to slash a validator. The slash is delegated to the staking
// module to make the necessary validator changes.
func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, fraction sdk.Dec, power, distributionHeight int64) {
coinsBurned := k.sk.Slash(ctx, consAddr, distributionHeight, power, fraction)
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeSlash,
sdk.NewAttribute(types.AttributeKeyAddress, consAddr.String()),
sdk.NewAttribute(types.AttributeKeyPower, fmt.Sprintf("%d", power)),
sdk.NewAttribute(types.AttributeKeyReason, types.AttributeValueDoubleSign),
sdk.NewAttribute(types.AttributeKeyBurnedCoins, coinsBurned.String()),
),
)

k.sk.Slash(ctx, consAddr, distributionHeight, power, fraction)
}

// Jail attempts to jail a validator. The slash is delegated to the staking module
// to make the necessary validator changes.
func (k Keeper) Jail(ctx sdk.Context, consAddr sdk.ConsAddress) {
k.sk.Jail(ctx, consAddr)
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeSlash,
sdk.NewAttribute(types.AttributeKeyJailed, consAddr.String()),
),
)

k.sk.Jail(ctx, consAddr)
}

func (k Keeper) deleteAddrPubkeyRelation(ctx sdk.Context, addr cryptotypes.Address) {
Expand Down
5 changes: 3 additions & 2 deletions x/slashing/spec/06_events.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
order: 6
-->

# Tags
# Events

The slashing module emits the following events/tags:
The slashing module emits the following events:

## MsgServer

Expand All @@ -25,6 +25,7 @@ The slashing module emits the following events/tags:
| slash | power | {validatorPower} |
| slash | reason | {slashReason} |
| slash | jailed [0] | {validatorConsensusAddress} |
| slash | burned coins | {sdk.Int} |

- [0] Only included if the validator is jailed.

Expand Down
1 change: 1 addition & 0 deletions x/slashing/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const (
AttributeKeyReason = "reason"
AttributeKeyJailed = "jailed"
AttributeKeyMissedBlocks = "missed_blocks"
AttributeKeyBurnedCoins = "burned_coins"

AttributeValueDoubleSign = "double_sign"
AttributeValueMissingSignature = "missing_signature"
Expand Down
2 changes: 1 addition & 1 deletion x/slashing/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type StakingKeeper interface {
ValidatorByConsAddr(sdk.Context, sdk.ConsAddress) stakingtypes.ValidatorI // get a particular validator by consensus address

// slash the validator and delegators of the validator, specifying offence height, offence power, and slash fraction
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec)
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec) sdk.Int
Jail(sdk.Context, sdk.ConsAddress) // jail a validator
Unjail(sdk.Context, sdk.ConsAddress) // unjail a validator

Expand Down
5 changes: 3 additions & 2 deletions x/staking/keeper/slash.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
// CONTRACT:
// Infraction was committed at the current height or at a past height,
// not at a height in the future
func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeight int64, power int64, slashFactor sdk.Dec) {
func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeight int64, power int64, slashFactor sdk.Dec) sdk.Int {
logger := k.Logger(ctx)

if slashFactor.IsNegative() {
Expand All @@ -45,7 +45,7 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh
"WARNING: ignored attempt to slash a nonexistent validator; we recommend you investigate immediately",
"validator", consAddr.String(),
)
return
return sdk.NewInt(0)
}

// should not be slashing an unbonded validator
Expand Down Expand Up @@ -140,6 +140,7 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh
"slash_factor", slashFactor.String(),
"burned", tokensToBurn,
)
return tokensToBurn
}

// jail a validator
Expand Down
13 changes: 13 additions & 0 deletions x/staking/keeper/slash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,3 +605,16 @@ func TestSlashBoth(t *testing.T) {
// power not decreased, all stake was bonded since
require.Equal(t, int64(10), validator.GetConsensusPower(app.StakingKeeper.PowerReduction(ctx)))
}

func TestSlashAmount(t *testing.T) {
app, ctx, _, _ := bootstrapSlashTest(t, 10)
consAddr := sdk.ConsAddress(PKs[0].Address())
fraction := sdk.NewDecWithPrec(5, 1)
burnedCoins := app.StakingKeeper.Slash(ctx, consAddr, ctx.BlockHeight(), 10, fraction)
require.True(t, burnedCoins.GT(sdk.ZeroInt()))

// test the case where the validator was not found, which should return no coins
_, addrVals := generateAddresses(app, ctx, 100)
noBurned := app.StakingKeeper.Slash(ctx, sdk.ConsAddress(addrVals[0]), ctx.BlockHeight(), 10, fraction)
require.True(t, sdk.NewInt(0).Equal(noBurned))
}
2 changes: 1 addition & 1 deletion x/staking/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type ValidatorSet interface {
StakingTokenSupply(sdk.Context) sdk.Int // total staking token supply

// slash the validator and delegators of the validator, specifying offence height, offence power, and slash fraction
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec)
Slash(sdk.Context, sdk.ConsAddress, int64, int64, sdk.Dec) sdk.Int
Jail(sdk.Context, sdk.ConsAddress) // jail a validator
Unjail(sdk.Context, sdk.ConsAddress) // unjail a validator

Expand Down

0 comments on commit 2188dd0

Please sign in to comment.