From 2188dd0f3cbed47fde5aa94f3620fb59382897e6 Mon Sep 17 00:00:00 2001 From: Tyler <48813565+technicallyty@users.noreply.github.com> Date: Mon, 5 Jul 2021 07:33:30 -0700 Subject: [PATCH] feat: add coins burned to slash event (#9458) ## 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 `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) --- CHANGELOG.md | 1 + x/distribution/types/expected_keepers.go | 2 +- x/slashing/keeper/infractions.go | 3 ++- x/slashing/keeper/keeper.go | 7 +++---- x/slashing/spec/06_events.md | 5 +++-- x/slashing/types/events.go | 1 + x/slashing/types/expected_keepers.go | 2 +- x/staking/keeper/slash.go | 5 +++-- x/staking/keeper/slash_test.go | 13 +++++++++++++ x/staking/types/expected_keepers.go | 2 +- 10 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62e475093860..b5a6bad54d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`. diff --git a/x/distribution/types/expected_keepers.go b/x/distribution/types/expected_keepers.go index ac9ebc3a4eea..be87675d208a 100644 --- a/x/distribution/types/expected_keepers.go +++ b/x/distribution/types/expected_keepers.go @@ -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 diff --git a/x/slashing/keeper/infractions.go b/x/slashing/keeper/infractions.go index 06baa48f82b4..426e54733a93 100644 --- a/x/slashing/keeper/infractions.go +++ b/x/slashing/keeper/infractions.go @@ -84,6 +84,7 @@ 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, @@ -91,9 +92,9 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre 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)) diff --git a/x/slashing/keeper/keeper.go b/x/slashing/keeper/keeper.go index 12a943c84c69..179bc860dd19 100644 --- a/x/slashing/keeper/keeper.go +++ b/x/slashing/keeper/keeper.go @@ -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) { diff --git a/x/slashing/spec/06_events.md b/x/slashing/spec/06_events.md index 8f259ae46c62..861af11f4b57 100644 --- a/x/slashing/spec/06_events.md +++ b/x/slashing/spec/06_events.md @@ -2,9 +2,9 @@ order: 6 --> -# Tags +# Events -The slashing module emits the following events/tags: +The slashing module emits the following events: ## MsgServer @@ -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. diff --git a/x/slashing/types/events.go b/x/slashing/types/events.go index e9fb254545f5..cf419ab6b999 100644 --- a/x/slashing/types/events.go +++ b/x/slashing/types/events.go @@ -11,6 +11,7 @@ const ( AttributeKeyReason = "reason" AttributeKeyJailed = "jailed" AttributeKeyMissedBlocks = "missed_blocks" + AttributeKeyBurnedCoins = "burned_coins" AttributeValueDoubleSign = "double_sign" AttributeValueMissingSignature = "missing_signature" diff --git a/x/slashing/types/expected_keepers.go b/x/slashing/types/expected_keepers.go index 7bddb6cebc61..5f9a4fee8c72 100644 --- a/x/slashing/types/expected_keepers.go +++ b/x/slashing/types/expected_keepers.go @@ -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 diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index 7b88fbfca17c..279af3a85a35 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -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() { @@ -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 @@ -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 diff --git a/x/staking/keeper/slash_test.go b/x/staking/keeper/slash_test.go index 81e5c8afb095..73fae5cce175 100644 --- a/x/staking/keeper/slash_test.go +++ b/x/staking/keeper/slash_test.go @@ -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)) +} diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go index f00971751c17..785a969f36e4 100644 --- a/x/staking/types/expected_keepers.go +++ b/x/staking/types/expected_keepers.go @@ -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