-
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 9 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,106 @@ | ||
## 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, wich `StartHeight` of the current block, `EndHeight` of `0` (sentinel value for not-yet-ended), | ||
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. wich -> 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. Thanks, fixed |
||
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 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) | ||
totalToSlash = max(slashingPeriod.SlashedSoFar, fraction) | ||
slashingPeriod.SlashedSoFar = totalToSlash | ||
setSlashingPeriod(slashingPeriod) | ||
|
||
remainderToSlash = slashingPeriod.SlashedSoFar - 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 | ||
JailedUntil int64 // Block height until which the validator is jailed, | ||
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. Won't 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. Agreed, now |
||
// or sentinel value of 0 for not jailed | ||
SignedBlocksCounter int64 // Running counter of signed blocks | ||
} | ||
|
||
``` | ||
|
@@ -49,3 +50,31 @@ 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 time 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. It's really a start and end block, right? 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. Yes, thanks, updated |
||
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 double-signing (but does not end if they are slashed & jailed for just missing blocks). | ||
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. It seems the last part is pretty important and long -- it doesn't seem to belong in parentheses. Thoughts? In other words, I think it'll read easier with just a comma :) 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. See below comment. |
||
When the validator voluntarily unjails themselves (and possibly changes signing keys), they reset the period. | ||
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. Can you help clarify for me one case? What if a validator misses enough blocks to be slashed and jailed -- the slashing period doesn't end. The validator can then, after some time, unjail themselves, correct? If so, is the period reset even though it never ended (they never double signed)? Does that mean they never get penalized for liveliness cost? I'm sure I'm missing something here, sorry. 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. bump @cwgoes 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. yup! you are right, this is unclear - addressing 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. Actually, I don't think we need this, we should just end the period when they're unbonded for any reason. If you think we do need it, please comment further... |
||
|
||
Slashing periods are indexed in the store as follows: | ||
|
||
- SlashingPeriod: ` 0x03 | ValTendermintAddr | StartHeight -> amino(slashingPeriod) ` | ||
|
||
This allows us to look up slashing period by validator address, the only lookup necessary, | ||
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. Slight rewording, 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. Updated |
||
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 | ||
SlashedSoFar 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.