-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
R4R: Update slashing spec for slashing-by-period #2001
Changes from 13 commits
4f8c9e4
84e9b21
98a278d
ff01cbb
53fa4a2
a8af4a4
07a7db7
2445718
8b7d6e0
52475b1
32263ff
a2463d0
21be609
79e3c05
94dc512
e3cb1e1
b8d6465
da92b1b
d0c87ff
c9e5745
f188955
41df6db
73e1965
bb9c265
4cc2054
4475a8c
63a85aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# Slashing module specification | ||
|
||
## Abstract | ||
|
||
This section specifies the slashing module of the Cosmos SDK, which implements functionality | ||
first outlined in the [Cosmos Whitepaper](https://cosmos.network/about/whitepaper) in June 2016. | ||
|
||
The slashing module enables Cosmos SDK-based blockchains to disincentivize any attributable action | ||
by a protocol-recognized actor with value at stake by "slashing" them: burning some amount of their | ||
stake - and possibly also removing their ability to vote on future blocks for a period of time. | ||
|
||
This module will be used by the Cosmos Hub, the first hub in the Cosmos ecosystem. | ||
|
||
## Contents | ||
|
||
1. **[State](state.md)** | ||
1. [SigningInfo](state.md#signing-info) | ||
1. [SlashingPeriod](state.md#slashing-period) | ||
1. **[State Machine](state-machine.md)** | ||
1. [Transactions](state-machine.md#transactions) | ||
1. Unjail | ||
1. [Interactions](state-machine.md#interactions) | ||
1. Validator Bonded | ||
1. Validator Unbonding | ||
1. Validator Slashed | ||
1. [State Cleanup](state-machine.md#state-cleanup) | ||
1. **[Begin Block](begin-block.md)** | ||
1. [Evidence handling](begin-block.md#evidence-handling) | ||
1. [Uptime tracking](begin-block.md#uptime-tracking) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
## Transaction & State Machine Interaction Overview | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably be an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure; updated & also updated in |
||
|
||
### Transactions | ||
|
||
In this section we describe the processing of transactions for the `slashing` module. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unnecessary intro, transactions should be its own file ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure; moved the intro to |
||
|
||
#### TxUnjail | ||
|
||
If a validator was automatically unbonded due to downtime and wishes to come back online & | ||
possibly rejoin the bonded set, it must send `TxUnjail`: | ||
|
||
```golang | ||
type TxUnjail struct { | ||
ValidatorAddr sdk.AccAddress | ||
} | ||
|
||
handleMsgUnjail(tx TxUnjail) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be a mixed bag of pseudo-code and Go -- I think we should just stick to one. I guess in this case, can we just add braces? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah, thanks - I'd rather it be pseudocode, updated to be less Go-like, let me know if it's better now. |
||
|
||
validator := getValidator(tx.ValidatorAddr) | ||
if validator == nil | ||
fail with "No validator found" | ||
|
||
if !validator.Jailed | ||
fail with "Validator not jailed, cannot unjail" | ||
|
||
info := getValidatorSigningInfo(operator) | ||
if BlockHeader.Time.Before(info.JailedUntil) | ||
fail with "Validator still jailed, cannot unjail until period has expired" | ||
|
||
// Update the start height so the validator won't be immediately unbonded again | ||
info.StartHeight = BlockHeight | ||
setValidatorSigningInfo(info) | ||
|
||
validator.Jailed = false | ||
setValidator(validator) | ||
|
||
return | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the return statement is implied here and thus can be removed (and all the other pseudo functions which do not have early returns in this file) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this I did to maintain consistency with the other specs, e.g. https://github.com/cosmos/cosmos-sdk/blob/develop/docs/spec/staking/transactions.md |
||
``` | ||
|
||
If the validater has enough stake to be in the top hundred, they will be automatically rebonded, | ||
and all delegators still delegated to the validator will be rebonded and begin to again collect | ||
provisions and rewards. | ||
|
||
### Interactions | ||
|
||
In this section we describe the "hooks" - slashing module code that runs when other events happen. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Interactions or "hooks" should maybe be separated into a new file - in distribution I use "triggers" - not sure if that's the most ideal - but we should def stay consistent between the two specs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Within piggy bank, I also included "triggered-by" section for each "trigger" could be good to outline these here as well for each hook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I think I'm using the standard terminology here, e.g. https://en.wikipedia.org/wiki/Hooking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh interesting - I'll update piggy bank to use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
#### Validator Bonded | ||
|
||
Upon successful bonding of a validator (a given validator changing from "unbonded" state to "bonded" state, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that we have two terms: "unbonded" == "not bonded", and "unbonded == sdk.Unbonded state", because we call the state itself unbonded it's unclear what to refer to the former by. I like your wording better anyways but maybe we should consider renaming the state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but "unbonded" is also an English word, e.g. https://www.dictionary.com/browse/unbonded or https://www.collinsdictionary.com/dictionary/english/unbonded, which means "not bonded" (is the antonym of "bonded") - and I presume this etymology is the same root as for Cosmos; e.g. https://en.wikipedia.org/wiki/Bonded_warehouse (perhaps)? What do you think we should call the state of "not bonded", if not "unbonded"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I don't use "unbonded" this way in the spec anymore though) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay - yeah confusing. We should probably just completely avoid the language unbonded and only refer to things AS bonded/unbonding/unbonded - and define this strictly as meaning one of the 3 states of tokens. However I will draw the subtle generalized distinction (as I understand it) between the prefix "un-" and the the adverb "not". The word "not" is rooted in the idea of exclusion "exclude a person or part of a group", whereas "un-" when used as "subtle" lol |
||
which may happen on delegation, on unjailing, etc), we create a new `SlashingPeriod` structure for the | ||
now-bonded validator, which `StartHeight` of the current block, `EndHeight` of `0` (sentinel value for not-yet-ended), | ||
and `SlashedSoFar` of `0`: | ||
|
||
```golang | ||
onValidatorBonded(address sdk.ValAddress) | ||
|
||
slashingPeriod := SlashingPeriod{ | ||
ValidatorAddr : address, | ||
StartHeight : CurrentHeight, | ||
EndHeight : 0, | ||
SlashedSoFar : 0, | ||
} | ||
setSlashingPeriod(slashingPeriod) | ||
|
||
return | ||
``` | ||
|
||
#### Validator Unbonded | ||
|
||
When a validator is unbonded, we update the in-progress `SlashingPeriod` with the current block as the `EndHeight`: | ||
|
||
```golang | ||
onValidatorUnbonded(address sdk.ValAddress) | ||
|
||
slashingPeriod = getSlashingPeriod(address, CurrentHeight) | ||
slashingPeriod.EndHeight = CurrentHeight | ||
setSlashingPeriod(slashingPeriod) | ||
|
||
return | ||
``` | ||
|
||
#### Validator Power Changed | ||
|
||
When a validator's power changes, we update the in-progress `SlashingPeriod` with the validator's current power: | ||
|
||
```golang | ||
onValidatorPowerChanged(address sdk.ValAddress, stakeBonded sdk.Rat) | ||
|
||
slashingPeriod = getSlashingPeriod(address, CurrentHeight) | ||
slashingPeriod.MaxStakeBonded = max(slashingPeriod.MaxStakeBonded, stakeBonded) | ||
setSlashingPeriod(slashingPeriod) | ||
|
||
return | ||
``` | ||
|
||
#### Validator Slashed | ||
|
||
When a validator is slashed, we look up the appropriate `SlashingPeriod` based on the validator | ||
address and the time of infraction, cap the fraction slashed as `max(SlashFraction, SlashedSoFar)` | ||
(which may be `0`), and update the `SlashingPeriod` with the increased `SlashedSoFar`: | ||
|
||
```golang | ||
beforeValidatorSlashed(address sdk.ValAddress, fraction sdk.Rat, infractionHeight int64) | ||
|
||
slashingPeriod = getSlashingPeriod(address, infractionHeight) | ||
totalFractionToSlash = max(slashingPeriod.SlashedSoFar, fraction) | ||
slashingPeriod.FractionSlashedSoFar = totalToSlash | ||
setSlashingPeriod(slashingPeriod) | ||
|
||
remainderToSlash = slashingPeriod.FractionSlashedSoFar - totalToSlash | ||
fraction = remainderToSlash | ||
|
||
continue with slashing | ||
``` | ||
|
||
### State Cleanup | ||
|
||
Once no evidence for a given slashing period can possibly be valid (the end time plus the unbonding period is less than the current time), | ||
old slashing periods should be cleaned up. This will be implemented post-launch. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,10 +36,11 @@ The information stored for tracking validator liveness is as follows: | |
|
||
```go | ||
type ValidatorSigningInfo struct { | ||
StartHeight int64 | ||
IndexOffset int64 | ||
JailedUntil int64 | ||
SignedBlocksCounter int64 | ||
StartHeight int64 // Height at which the validator became able to sign blocks | ||
IndexOffset int64 // Offset into the signed block bit array | ||
JailedUntilHeight int64 // Block height until which the validator is jailed, | ||
// or sentinel value of 0 for not jailed | ||
SignedBlocksCounter int64 // Running counter of signed blocks | ||
} | ||
|
||
``` | ||
|
@@ -49,3 +50,33 @@ Where: | |
* `IndexOffset` is incremented each time the candidate was a bonded validator in a block (and may have signed a precommit or not). | ||
* `JailedUntil` is set whenever the candidate is revoked due to downtime | ||
* `SignedBlocksCounter` is a counter kept to avoid unnecessary array reads. `SignedBlocksBitArray.Sum() == SignedBlocksCounter` always. | ||
|
||
### Slashing Period | ||
|
||
A slashing period is a start and end block height associated with a particular validator, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
within which only the "worst infraction counts": the total amount of slashing for | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "worst-infraction-counts" is a good principal to outline in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. then it could just be linked here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So linked. |
||
infractions committed within the period (and discovered whenever) is capped at the | ||
penalty for the worst offense. | ||
|
||
This period starts when a validator is first bonded and ends when a validator is slashed & jailed | ||
for any reason. When the validator rejoins the validator set (perhaps through unjailing themselves, | ||
and perhaps also changing signing keys), they enter into a new period. | ||
|
||
Slashing periods are indexed in the store as follows: | ||
|
||
- SlashingPeriod: ` 0x03 | ValTendermintAddr | StartHeight -> amino(slashingPeriod) ` | ||
|
||
This allows us to look up slashing period by a validator's address, the only lookup necessary, | ||
and iterate over start height to efficiently retrieve the most recent slashing period(s) | ||
or those beginning after a given height. | ||
|
||
```go | ||
type SlashingPeriod struct { | ||
ValidatorAddr sdk.ValAddress // Tendermint address of the validator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this is the Tendermint Addr - I almost think the variable should be renamed to something a bit more explicit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like what? It's called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ack. |
||
StartHeight int64 // Block height at which slashing period begin | ||
EndHeight int64 // Block height at which slashing period ended | ||
MaxBondedStake sdk.Rat // Maximum bonded stake during period | ||
StakeSlashedSoFar sdk.Rat // Amount of stake slashed so far | ||
FractionSlashedSoFar sdk.Rat // Fraction slashed so far, cumulative | ||
} | ||
``` |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be more clear to include these two items as numbered bullets - also use of "possibly" seems a bit out of place - I think slashing and jailing are independant penalties which are both optional to a penalty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, changed to a list.