From 0db84f20f16489c29b54443c2ddbd974fc7242ee Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Wed, 23 Oct 2019 16:33:00 +0900 Subject: [PATCH 01/24] Create adr-016-validator-consensus-key-rotation.md --- ...dr-016-validator-consensus-key-rotation.md | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 docs/architecture/adr-016-validator-consensus-key-rotation.md diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md new file mode 100644 index 000000000000..890e35268825 --- /dev/null +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -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 +validator consensus key rotation implementation mostly onto Cosmos-SDK. + +## 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. +- 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. + +### Considerations + +- consensus key mapping information management strategy + - stores history of each key mapping changes in kvstore. + - gaiad can search corresponding consensus key paired with given validator operator for any arbitrary height in recent unbonding period. + - gaiad does not need any historical mapping information which is past more than unbonding period. +- limits + - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spams. + - a validator should contribute X atoms to community fund to rotate its consensus key, also to prevent spams. + - 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. +- 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 + - 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 + +- 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 + +### Negative + +- Slash module needs more computation because it needs to lookup corresponding consensus key of validators for each height + +### 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 From 8706fe8fb369fa38136491a14e84abf23b871fb5 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:40:37 +0900 Subject: [PATCH 02/24] typo line 10 Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 890e35268825..83e410b8117d 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -7,7 +7,7 @@ ## 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 +key management policy (e.g. https://github.com/tendermint/tendermint/issues/1136). So, we suggest one of the simplest form of validator consensus key rotation implementation mostly onto Cosmos-SDK. ## Decision From df3524ce48cf07c1ccd2ec7eb3ced9988079e794 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:41:19 +0900 Subject: [PATCH 03/24] typo line 26 Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 83e410b8117d..068669bd6a84 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -23,7 +23,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - consensus key mapping information management strategy - stores history of each key mapping changes in kvstore. - - gaiad can search corresponding consensus key paired with given validator operator for any arbitrary height in recent unbonding period. + - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - gaiad does not need any historical mapping information which is past more than unbonding period. - limits - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spams. From d0a803012b679193975c4f3c9e2b5bb6780a807d Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:41:28 +0900 Subject: [PATCH 04/24] typo line 25 Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 068669bd6a84..44379c8b7919 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -22,7 +22,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. ### Considerations - consensus key mapping information management strategy - - stores history of each key mapping changes in kvstore. + - store history of each key mapping changes in the kvstore. - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - gaiad does not need any historical mapping information which is past more than unbonding period. - limits From 352cac4b7d874e671f1f3c925253b1585763885a Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:41:38 +0900 Subject: [PATCH 05/24] typo line 27 Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 44379c8b7919..0ba7cb91b96d 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -24,7 +24,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - consensus key mapping information management strategy - store history of each key mapping changes in the kvstore. - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - - gaiad does not need any historical mapping information which is past more than unbonding period. + - the state machine does not need any historical mapping information which is past more than unbonding period. - limits - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spams. - a validator should contribute X atoms to community fund to rotate its consensus key, also to prevent spams. From 65c1f40ee19885ed1b736c00ce25aadfc0e72511 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:41:46 +0900 Subject: [PATCH 06/24] typo line 29 Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 0ba7cb91b96d..8e00f734eed7 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -26,7 +26,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - the state machine does not need any historical mapping information which is past more than unbonding period. - limits - - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spams. + - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spam. - a validator should contribute X atoms to community fund to rotate its consensus key, also to prevent spams. - parameters can be decided by governance and stored in genesis file. - slash module From 4181be989a28b9e98536c350c2f6a70c4eb8a264 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:41:59 +0900 Subject: [PATCH 07/24] remove rotation fee Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 8e00f734eed7..8c039fea5aee 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -27,7 +27,6 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - the state machine does not need any historical mapping information which is past more than unbonding period. - limits - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spam. - - a validator should contribute X atoms to community fund to rotate its consensus key, also to prevent spams. - 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. From a73be81f62aabe26ea2b5b4cbe590263e0b3e0c7 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 10:57:12 +0900 Subject: [PATCH 08/24] add ADR 016 in ADR Table of Contents --- docs/architecture/README.MD | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/architecture/README.MD b/docs/architecture/README.MD index 99d8204b213f..51dabad3152d 100644 --- a/docs/architecture/README.MD +++ b/docs/architecture/README.MD @@ -31,3 +31,4 @@ Please add a entry below in your Pull Request for an ADR. - [ADR 010: Modular AnteHandler](./adr-010-modular-antehandler.md) - [ADR 011: Generalize Genesis Accounts](./adr-011-generalize-genesis-accounts.md) - [ADR 012: State Accessors](./adr-012-state-accessors.md) +- [ADR 016: Validator Consensus Key Rotation](./adr-016-validator-consensus-key-rotation.md) From 922fe9d76f4fec66c0b4182b2252cb5b5eac4aa4 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 11:03:00 +0900 Subject: [PATCH 09/24] remove pruning and further implementation part --- .../adr-016-validator-consensus-key-rotation.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 8c039fea5aee..7559320e6320 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -1,4 +1,4 @@ -# ADR 016: Validator consensus key rotation +# ADR 016: Validator Consensus Key Rotation ## Changelog @@ -30,12 +30,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - 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. -- 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 - - 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 From 1a62db4e28e8d1c7569cbf815f7ff5985fca8635 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 24 Oct 2019 11:07:47 +0900 Subject: [PATCH 10/24] add a step on procedure about HSM and KMS users --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 7559320e6320..af6ead8fdd05 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -18,6 +18,8 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - create and broadcast a transaction(RotateValConsensusKey) that the new consensus key is now coupled with the validator operator with signature from validator wallet key. - 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. +- validators using HSM and KMS should update the consensus key in HSM to use the new rotated key for signing votes. + ### Considerations From afffccdb68ac4f2d2d143c707a182096f24b9cc0 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Fri, 25 Oct 2019 12:40:47 +0900 Subject: [PATCH 11/24] RotateValConsensusKey -> MsgRotateConsPubkey Co-Authored-By: Alexander Bezobchuk --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index af6ead8fdd05..7e43dbaae7b0 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -15,7 +15,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. ### 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. +- create and broadcast a transaction with a `MsgRotateConsPubKey` that states the new consensus key is now coupled with the validator operator with signature from the validator's operator key. - 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. - validators using HSM and KMS should update the consensus key in HSM to use the new rotated key for signing votes. From 7708e5e0f61ab426f1951e85afc8a27fb5d07364 Mon Sep 17 00:00:00 2001 From: dongsam Date: Fri, 25 Oct 2019 13:13:15 +0900 Subject: [PATCH 12/24] Add workflow --- ...dr-016-validator-consensus-key-rotation.md | 59 ++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 7e43dbaae7b0..88053a02f6ad 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -2,7 +2,7 @@ ## Changelog -- 23-10-2019: initial draft +- 2019 Oct 23: Initial draft ## Context @@ -32,14 +32,59 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - 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. +- abci.ValidatorUpdate + - 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. + + +### Workflow + +1. The validator generates a new consensus keypair. +2. The validator generates and signs a `MsgRotateConsPubKey` tx with their operator key and new ConsPubKey + + ```go + type MsgRotateConsPubKey struct { + ValidatorAddress sdk.ValAddress + NewPubKey crypto.PubKey + } + ``` + +3. `handleMsgRotateConsPubKey` gets `MsgRotateConsPubKey`, calls `RotateConsPubKey` with emits event +4. `RotateConsPubKey` + - checks if `NewPubKey` is not duplicated on `ValidatorsByConsAddr` + - overwrites `NewPubKey` in `validator.ConsPubKey` + - deletes old `ValidatorByConsAddr` + - `SetValidatorByConsAddr` for `NewPubKey` + - Add `ConsPubKeyRotationHistory` for tracking rotation + + ```go + type ConsPubKeyRotationHistory struct { + OperatorAddress sdk.ValAddress + OldConsPubKey crypto.PubKey + NewConsPubKey crypto.PubKey + RotatedHeight int64 + } + ``` + +5. `ApplyAndReturnValidatorSetUpdates` checks if there is `ConsPubKeyRotationHistory` with `ConsPubKeyRotationHistory.RotatedHeight == ctx.BlockHeight()` and if so, generates 2 `ValidatorUpdate` , one for a remove validator and one for create new validator + + ```go + abci.ValidatorUpdate{ + PubKey: tmtypes.TM2PB.PubKey(OldConsPubKey), + Power: 0, + } + + abci.ValidatorUpdate{ + PubKey: tmtypes.TM2PB.PubKey(NewConsPubKey), + Power: v.ConsensusPower(), + } + ``` + +6. at `previousVotes` Iteration logic of `AllocateTokens`, `previousVote` using `OldConsPubKey` match up with `ConsPubKeyRotationHistory`, and replace validator for token allocation +7. Migrate `ValidatorSigningInfo` and `ValidatorMissedBlockBitArray` from `OldConsPubKey` to `NewConsPubKey` -### Special note on implementation - -- 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 From 4c932ee39d75b321dfdc3c8d462d2f8c61c5b69d Mon Sep 17 00:00:00 2001 From: dongsam Date: Tue, 29 Oct 2019 17:32:25 +0900 Subject: [PATCH 13/24] Add checking MaxConsPubKeyRotations to workflow --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 88053a02f6ad..2304e6b14c67 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -53,6 +53,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. 3. `handleMsgRotateConsPubKey` gets `MsgRotateConsPubKey`, calls `RotateConsPubKey` with emits event 4. `RotateConsPubKey` - checks if `NewPubKey` is not duplicated on `ValidatorsByConsAddr` + - checks if the validator is does not exceed parameter `MaxConsPubKeyRotations` by iterating `ConsPubKeyRotationHistory` - overwrites `NewPubKey` in `validator.ConsPubKey` - deletes old `ValidatorByConsAddr` - `SetValidatorByConsAddr` for `NewPubKey` From a92ce8ba14864921196efd568b184057c72c917d Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Wed, 30 Oct 2019 18:15:04 +0900 Subject: [PATCH 14/24] clearer statement regarding to HSM and KMS --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 2304e6b14c67..9c3d08377b06 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -18,7 +18,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - create and broadcast a transaction with a `MsgRotateConsPubKey` that states the new consensus key is now coupled with the validator operator with signature from the validator's operator key. - 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. -- validators using HSM and KMS should update the consensus key in HSM to use the new rotated key for signing votes. +- validators using HSM and KMS should update the consensus key in HSM to use the new rotated key after the height `h` when `MsgRotateConsPubKey` committed to the blockchain. ### Considerations From 3e9f2d544b26006f763f8738cf4f6f8272b628de Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Wed, 6 Nov 2019 20:22:03 +0900 Subject: [PATCH 15/24] Update docs/architecture/adr-016-validator-consensus-key-rotation.md Co-Authored-By: Anton Kaliaev --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 9c3d08377b06..9166b5eec7e1 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -99,7 +99,7 @@ Proposed ### Negative - Slash module needs more computation because it needs to lookup corresponding consensus key of validators for each height - +- frequent key rotations will make light client bisection less efficient ### Neutral ## References From 6992552aa5340ab3ee571eef0500b9ae3a81d58b Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Wed, 6 Nov 2019 20:41:38 +0900 Subject: [PATCH 16/24] Update docs/architecture/adr-016-validator-consensus-key-rotation.md Co-Authored-By: Anton Kaliaev --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 9166b5eec7e1..b16888cfada4 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -95,7 +95,7 @@ Proposed ### Positive - Validators can immediately or periodically rotate their consensus key to have better security policy - +- improved security against Long-Range attacks (https://nearprotocol.com/blog/long-range-attacks-and-a-new-fork-choice-rule) given a validator throws away the old consensus key(s) ### Negative - Slash module needs more computation because it needs to lookup corresponding consensus key of validators for each height From 92e8b7eb1051cf9f7884e23328ff88251025b4ba Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 28 Nov 2019 14:25:50 +0900 Subject: [PATCH 17/24] add key rotation fee --- .../adr-016-validator-consensus-key-rotation.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index b16888cfada4..098d4ae6398c 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -3,6 +3,7 @@ ## Changelog - 2019 Oct 23: Initial draft +- 2019 Nov 28: Add key rotation fee ## Context @@ -30,6 +31,9 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - limits - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spam. - parameters can be decided by governance and stored in genesis file. +- key rotation fee + - a validator should pay `KeyRotationFee` to rotate the consensus key which is calculated as below + - `KeyRotationFee` = `InitialKeyRotationFee` * 2^(number of rotations in `ConsPubKeyRotationHistory` in recent unbonding period) - 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. - abci.ValidatorUpdate @@ -54,6 +58,8 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. 4. `RotateConsPubKey` - checks if `NewPubKey` is not duplicated on `ValidatorsByConsAddr` - checks if the validator is does not exceed parameter `MaxConsPubKeyRotations` by iterating `ConsPubKeyRotationHistory` + - checks if the signing account has enough balance to pay `KeyRotationFee` + - pays `KeyRotationFee` to community fund - overwrites `NewPubKey` in `validator.ConsPubKey` - deletes old `ValidatorByConsAddr` - `SetValidatorByConsAddr` for `NewPubKey` @@ -96,10 +102,12 @@ Proposed - Validators can immediately or periodically rotate their consensus key to have better security policy - improved security against Long-Range attacks (https://nearprotocol.com/blog/long-range-attacks-and-a-new-fork-choice-rule) given a validator throws away the old consensus key(s) + ### Negative - Slash module needs more computation because it needs to lookup corresponding consensus key of validators for each height - frequent key rotations will make light client bisection less efficient + ### Neutral ## References From 69b58a1be477a167d21a662485e68b743e1c7f90 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 28 Nov 2019 14:41:33 +0900 Subject: [PATCH 18/24] add new genesis parameters in considerations --- .../architecture/adr-016-validator-consensus-key-rotation.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 098d4ae6398c..8b23cfd0de81 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -29,7 +29,7 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - the state machine does not need any historical mapping information which is past more than unbonding period. - limits - - a validator cannot rotate its consensus key more than N time for any unbonding period, to prevent spam. + - a validator cannot rotate its consensus key more than `MaxConsPubKeyRotations` time for any unbonding period, to prevent spam. - parameters can be decided by governance and stored in genesis file. - key rotation fee - a validator should pay `KeyRotationFee` to rotate the consensus key which is calculated as below @@ -40,6 +40,9 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - 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. +- new genesis parameters + - `MaxConsPubKeyRotations` : maximum number of rotation can be executed by a validator in recent unbonding period. + - `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in recent unbonding period. ### Workflow From ebdc907dcd1c89966564a05f889eaaa0526a08c0 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Tue, 10 Dec 2019 22:17:02 +0900 Subject: [PATCH 19/24] update several things - add reasoning why the feature does not need any update on Tendermint consensus logic - add clarification related to multiple consensus key concept - edit slash module part in considerations to evidence module to adjust to the recent sdk update --- .../adr-016-validator-consensus-key-rotation.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 8b23cfd0de81..44a6b731b6fc 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -7,9 +7,11 @@ ## Context -Validator consensus key rotation feature has been discussed and requested for a long time, for the sake of safer validator -key management policy (e.g. https://github.com/tendermint/tendermint/issues/1136). So, we suggest one of the simplest form of -validator consensus key rotation implementation mostly onto Cosmos-SDK. +Validator consensus key rotation feature has been discussed and requested for a long time, for the sake of safer validator key management policy (e.g. https://github.com/tendermint/tendermint/issues/1136). So, we suggest one of the simplest form of validator consensus key rotation implementation mostly onto Cosmos-SDK. + +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. + +Also, it should be noted that this ADR includes only the simplest form of consensus key rotation without considering multiple consensus keys concept. Such multple consensus keys concept shall be remained as a long term goal of Tendermint and Cosmos-SDK. ## Decision @@ -34,8 +36,8 @@ validator consensus key rotation implementation mostly onto Cosmos-SDK. - key rotation fee - a validator should pay `KeyRotationFee` to rotate the consensus key which is calculated as below - `KeyRotationFee` = `InitialKeyRotationFee` * 2^(number of rotations in `ConsPubKeyRotationHistory` in recent unbonding period) -- 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. +- 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. - abci.ValidatorUpdate - 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. From 0b65286cdc64787cf14db2684ede2b3f829d33ee Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Wed, 11 Dec 2019 13:08:36 +0900 Subject: [PATCH 20/24] add suggested default value for genesis parameters add suggested default value for genesis parameters --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 44a6b731b6fc..cfc76b882da7 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -43,8 +43,8 @@ Also, it should be noted that this ADR includes only the simplest form of consen - 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. - new genesis parameters - - `MaxConsPubKeyRotations` : maximum number of rotation can be executed by a validator in recent unbonding period. - - `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in recent unbonding period. + - `MaxConsPubKeyRotations` : maximum number of rotation can be executed by a validator in recent unbonding period. default value 10 is suggested(11th key rotation will be rejected) + - `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in recent unbonding period. default value 1atom is suggested(1atom fee for the first key rotation in recent unbonding period) ### Workflow From 897a2da12ab9189bf4afcd03da5489db1d6815a8 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Thu, 12 Dec 2019 17:04:14 +0900 Subject: [PATCH 21/24] all additional features will be implemented on `staking` module all additional features will be implemented on `staking` module --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index cfc76b882da7..c51594ddf262 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -42,7 +42,7 @@ Also, it should be noted that this ADR includes only the simplest form of consen - 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. -- new genesis parameters +- new genesis parameters in `staking` module - `MaxConsPubKeyRotations` : maximum number of rotation can be executed by a validator in recent unbonding period. default value 10 is suggested(11th key rotation will be rejected) - `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in recent unbonding period. default value 1atom is suggested(1atom fee for the first key rotation in recent unbonding period) @@ -95,7 +95,7 @@ Also, it should be noted that this ADR includes only the simplest form of consen 6. at `previousVotes` Iteration logic of `AllocateTokens`, `previousVote` using `OldConsPubKey` match up with `ConsPubKeyRotationHistory`, and replace validator for token allocation 7. Migrate `ValidatorSigningInfo` and `ValidatorMissedBlockBitArray` from `OldConsPubKey` to `NewConsPubKey` - +- Note : All above features shall be implemented in `staking` module. ## Status From f00e56523da72a800bb4f5f8f9798495fbaffc26 Mon Sep 17 00:00:00 2001 From: dongsamb Date: Mon, 16 Dec 2019 10:52:40 +0900 Subject: [PATCH 22/24] Update docs/architecture/adr-016-validator-consensus-key-rotation.md Co-Authored-By: Aditya --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index c51594ddf262..6b63f56fd80a 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -11,7 +11,7 @@ Validator consensus key rotation feature has been discussed and requested for a 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. -Also, it should be noted that this ADR includes only the simplest form of consensus key rotation without considering multiple consensus keys concept. Such multple consensus keys concept shall be remained as a long term goal of Tendermint and Cosmos-SDK. +Also, it should be noted that this ADR includes only the simplest form of consensus key rotation without considering multiple consensus keys concept. Such multiple consensus keys concept shall remain a long term goal of Tendermint and Cosmos-SDK. ## Decision From 4b81ab44b6dff5d05df38156ff5ca890182af97c Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Mon, 16 Dec 2019 17:00:24 +0900 Subject: [PATCH 23/24] add "key rotation costs related to LCD and IBC" in considerations and.,.. - add "key rotation costs related to LCD and IBC" in considerations - fix `KeyRotationFee` formula. --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 6b63f56fd80a..0b4fd59e7da7 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -30,6 +30,10 @@ Also, it should be noted that this ADR includes only the simplest form of consen - store history of each key mapping changes in the kvstore. - the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period. - the state machine does not need any historical mapping information which is past more than unbonding period. +- key rotation costs related to LCD and IBC + - LCD and IBC will have traffic/computation burden when there exists frequent power changes + - In current Tendermint design, consensus key rotations are seen as power changes from LCD or IBC perspective + - Therefore, to minimize unnecessary frequent key rotation behavior, we limited maximum number of rotation in recent unbonding period and also applied exponentially increasing rotation fee - limits - a validator cannot rotate its consensus key more than `MaxConsPubKeyRotations` time for any unbonding period, to prevent spam. - parameters can be decided by governance and stored in genesis file. From f190c9d5b36012af8663381551495c0b927d44c0 Mon Sep 17 00:00:00 2001 From: b-harvest <38277329+dlguddus@users.noreply.github.com> Date: Mon, 16 Dec 2019 17:01:16 +0900 Subject: [PATCH 24/24] fix `KeyRotationFee` formula --- docs/architecture/adr-016-validator-consensus-key-rotation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/adr-016-validator-consensus-key-rotation.md b/docs/architecture/adr-016-validator-consensus-key-rotation.md index 0b4fd59e7da7..c668b7d29daa 100644 --- a/docs/architecture/adr-016-validator-consensus-key-rotation.md +++ b/docs/architecture/adr-016-validator-consensus-key-rotation.md @@ -39,7 +39,7 @@ Also, it should be noted that this ADR includes only the simplest form of consen - parameters can be decided by governance and stored in genesis file. - key rotation fee - a validator should pay `KeyRotationFee` to rotate the consensus key which is calculated as below - - `KeyRotationFee` = `InitialKeyRotationFee` * 2^(number of rotations in `ConsPubKeyRotationHistory` in recent unbonding period) + - `KeyRotationFee` = (max(`VotingPowerPercentage` * 100, 1) * `InitialKeyRotationFee`) * 2^(number of rotations in `ConsPubKeyRotationHistory` in recent unbonding period) - 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. - abci.ValidatorUpdate