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.

Sign up for GitHub

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

Jump to bottom

ADR 016: Validator consensus key rotation #5233

Merged
merged 36 commits into from
Jan 5, 2020
Merged

ADR 016: Validator consensus key rotation #5233

Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0db84f2
Create adr-016-validator-consensus-key-rotation.md
Hyung-bharvest Oct 23, 2019
8706fe8
typo line 10
Hyung-bharvest Oct 24, 2019
df3524c
typo line 26
Hyung-bharvest Oct 24, 2019
d0a8030
typo line 25
Hyung-bharvest Oct 24, 2019
352cac4
typo line 27
Hyung-bharvest Oct 24, 2019
65c1f40
typo line 29
Hyung-bharvest Oct 24, 2019
4181be9
remove rotation fee
Hyung-bharvest Oct 24, 2019
a73be81
add ADR 016 in ADR Table of Contents
Hyung-bharvest Oct 24, 2019
922fe9d
remove pruning and further implementation part
Hyung-bharvest Oct 24, 2019
1a62db4
add a step on procedure about HSM and KMS users
Hyung-bharvest Oct 24, 2019
d7e9c53
Merge branch 'master' into master
alexanderbez Oct 24, 2019
afffccd
RotateValConsensusKey -> MsgRotateConsPubkey
Hyung-bharvest Oct 25, 2019
7708e5e
Add workflow
dongsam Oct 25, 2019
953a8f8
Merge branch 'master' into master
alexanderbez Oct 28, 2019
4c932ee
Add checking MaxConsPubKeyRotations to workflow
dongsam Oct 29, 2019
889e382
Merge branch 'master' into master
alexanderbez Oct 29, 2019
a92ce8b
clearer statement regarding to HSM and KMS
Hyung-bharvest Oct 30, 2019
3e9f2d5
Update docs/architecture/adr-016-validator-consensus-key-rotation.md
Hyung-bharvest Nov 6, 2019
6992552
Update docs/architecture/adr-016-validator-consensus-key-rotation.md
Hyung-bharvest Nov 6, 2019
3fbc112
Merge branch 'master' into master
alexanderbez Nov 7, 2019
92e8b7e
add key rotation fee
Hyung-bharvest Nov 28, 2019
69b58a1
add new genesis parameters in considerations
Hyung-bharvest Nov 28, 2019
0da99f7
Merge branch 'master' into master
alexanderbez Dec 2, 2019
71ffdc9
Merge branch 'master' into master
dongsam Dec 9, 2019
ebdc907
update several things
Hyung-bharvest Dec 10, 2019
21f710a
Merge branch 'master' into master
dongsam Dec 11, 2019
0b65286
add suggested default value for genesis parameters
Hyung-bharvest Dec 11, 2019
897a2da
all additional features will be implemented on `staking` module
Hyung-bharvest Dec 12, 2019
744b2f1
Merge branch 'master' into master
Hyung-bharvest Dec 12, 2019
523a493
Merge branch 'master' into master
fedekunze Dec 12, 2019
f00e565
Update docs/architecture/adr-016-validator-consensus-key-rotation.md
dongsam Dec 16, 2019
4b81ab4
add "key rotation costs related to LCD and IBC" in considerations and…
Hyung-bharvest Dec 16, 2019
f190c9d
fix `KeyRotationFee` formula
Hyung-bharvest Dec 16, 2019
a4119f6
Merge branch 'master' into master
Hyung-bharvest Dec 16, 2019
c39d3a2
Merge branch 'master' into master
alexanderbez Dec 16, 2019
aa8b632
Merge branch 'master' into master
alexanderbez Jan 5, 2020
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
67 changes: 67 additions & 0 deletions docs/architecture/adr-016-validator-consensus-key-rotation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# ADR 016: Validator consensus key rotation

## Changelog

- 23-10-2019: initial draft

## Context

Validator consensus key rotation feature has been discussed and requested for a long time, for the sake of safer validator
key management policy.(https://github.com/tendermint/tendermint/issues/1136) So, we suggest one of the simplest form of
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
validator consensus key rotation implementation mostly onto Cosmos-SDK.
Copy link
Collaborator

Choose a reason for hiding this comment

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

consensus key rotation implementation mostly onto Cosmos-SDK.

I know this has been asked a few times already, but I'd be nice if you could add the reasoning of why does this feature fit into the SDK instead of Tendermint.

Copy link
Contributor Author

@Hyung-bharvest Hyung-bharvest Dec 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

thanks for noting that. I add below sentences to clarify the reason.

We don't need to make any update on consensus logic in Tendermint because Tendermint does not have any mapping information of consensus key and validator operator key, meaning that from Tendermint point of view, a consensus key rotation of a validator is simply a replacement of a consensus key to another.


## Decision

### Pseudo procedure for consensus key rotation

- create new random consensus key.
- create and broadcast a transaction(RotateValConsensusKey) that the new consensus key is now coupled with the validator operator with signature from validator wallet key.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- old consensus key becomes unable to participate on consensus immediately after the update of key mapping state on-chain.
- start validating with new consensus key.

alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
### Considerations

- consensus key mapping information management strategy
- stores history of each key mapping changes in kvstore.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- gaiad can search corresponding consensus key paired with given validator operator for any arbitrary height in recent unbonding period.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- gaiad does not need any historical mapping information which is past more than unbonding period.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- limits
- a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spams.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- a validator should contribute X atoms to community fund to rotate its consensus key, also to prevent spams.
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- parameters can be decided by governance and stored in genesis file.
- slash module
- slash module can search corresponding consensus key for any height so that it can decide which consensus key is supposed to be used for given height.
Copy link
Collaborator

Choose a reason for hiding this comment

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

the double sign evidence is now handled by the x/evidence module, which gets the registered pubkey from the x/slashing module. So ultimately the evidence module will have to look up the corresponding consensus address at the given height.

cc: @alexanderbez

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for pointing that out! as you said, evidence module gets pubkey from slashing keeper to use for double signing detection. edited as below

- evidence module
- evidence module can search corresponding consensus key for any height from slashing keeper so that it can decide which consensus key is supposed to be used for given height.

- data pruning
- database does not need to keep the historical key mapping changes after unbonding period past.
- when pruning is on, a node prunes all historical key mapping changes past unbonding period.
- further implementation
Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- introducing "validator control key" which has authority to sign transaction for given validator.
- validator control key can be replaced to another key by transaction(RotateValControlKey).

### Special note on implementation

Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
- tendermint already has ability to change a consensus key by ABCI communication(`ValidatorUpdate`).
- validator consensus key update can be done via creating new + delete old by change the power to zero.
- therefore, we expect we even do not need to change tendermint codebase at all to implement this feature.

## Status

Proposed

## Consequences

### Positive

- Validators can immediately or periodically rotate their consensus key to have better security policy

Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
### Negative
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing an important negative of it allows a validator to effectively sell their entity, in a way that before would require hardware assumptions. (They can change their key to give it to new management, without any oversight from their delegators)


- Slash module needs more computation because it needs to lookup corresponding consensus key of validators for each height

Hyung-bharvest marked this conversation as resolved.
Show resolved Hide resolved
### Neutral

## References

- on tendermint repo : https://github.com/tendermint/tendermint/issues/1136
- on cosmos-sdk repo : https://github.com/cosmos/cosmos-sdk/issues/5231
- about multiple consensus keys : https://github.com/tendermint/tendermint/issues/1758#issuecomment-545291698